From e1f687dfd54d8d67948a6b4abc217abc43f4d069 Mon Sep 17 00:00:00 2001 From: Sean Cuevo Date: Fri, 15 Mar 2019 11:50:27 -0600 Subject: [PATCH 1/4] adding 10,000,000 record chunking to Opportunity Naming batch job --- src/classes/OPP_OpportunityNaming_BATCH.cls | 69 +++-- .../OPP_OpportunityNaming_BATCH_TEST.cls | 247 ++++++++++++++++++ ..._OpportunityNaming_BATCH_TEST.cls-meta.xml | 5 + 3 files changed, 304 insertions(+), 17 deletions(-) create mode 100644 src/classes/OPP_OpportunityNaming_BATCH_TEST.cls create mode 100644 src/classes/OPP_OpportunityNaming_BATCH_TEST.cls-meta.xml diff --git a/src/classes/OPP_OpportunityNaming_BATCH.cls b/src/classes/OPP_OpportunityNaming_BATCH.cls index a593f193502..6fd02096c28 100644 --- a/src/classes/OPP_OpportunityNaming_BATCH.cls +++ b/src/classes/OPP_OpportunityNaming_BATCH.cls @@ -34,14 +34,32 @@ * @group-content ../../ApexDocContent/Opportunity.htm * @description Batch class creates names all Opportunities per the naming spec. */ -public class OPP_OpportunityNaming_BATCH implements Database.Batchable, Schedulable { - +public class OPP_OpportunityNaming_BATCH implements Database.Batchable, Schedulable, Database.Stateful { /** @description The query for the batch process to run on.*/ - String query; - + private String query; + + @TestVisible + private static Integer defaultQueryLimit= 10000000; + + @TestVisible + private Id lastOppIdProcessed; + /** @description The batch process constructor; creates opportunity query for all opportunities.*/ public OPP_OpportunityNaming_BATCH() { - query = 'SELECT Id, Name FROM Opportunity'; + query = 'SELECT Id, Name FROM Opportunity ORDER BY Id LIMIT ' + defaultQueryLimit; + } + + /** @description Constructor that accepts Id offset*/ + public OPP_OpportunityNaming_BATCH(Id idToOffset) { + query = buildOffsetQuery(idToOffset, defaultQueryLimit); + } + + /** @description Builds query string with given Id to offset and query limit*/ + private String buildOffsetQuery(Id idToOffset, Integer queryLimit) { + return String.format( + 'SELECT Id, Name FROM Opportunity WHERE Id > \'\'{0}\'\' ORDER BY Id LIMIT {1}', + new List{ idToOffset, String.valueOf(queryLimit) } + ); } /** @description Batch process start method.*/ @@ -49,7 +67,6 @@ public class OPP_OpportunityNaming_BATCH implements Database.Batchable, return Database.getQueryLocator(query); } - /** @description Schedulable execute method.*/ public void execute(SchedulableContext context) { Database.executeBatch(new OPP_OpportunityNaming_BATCH(), 200); @@ -59,27 +76,45 @@ public class OPP_OpportunityNaming_BATCH implements Database.Batchable, * @description Batch process execute method. Names and updates all opportunities in the current batch. */ public void execute(Database.BatchableContext BC, List scope) { + List oppsToProcess = scope; + lastOppIdProcessed = oppsToProcess[oppsToProcess.size() - 1].Id; + //save old opp names to see if we need an update - list oppsForUpdate = new list(); - map oppNames = new map(); - for (Opportunity opp : (list)scope) - oppNames.put(opp.id, opp.Name); + Map originalOppNamesById = new Map(); + for (Opportunity opp : oppsToProcess) { + originalOppNamesById.put(opp.id, opp.Name); + } //refresh names - OPP_OpportunityNaming.refreshOppNames((list)scope); + OPP_OpportunityNaming.refreshOppNames(oppsToProcess); //find which names have been updated, add to list - for (Opportunity opp : (list)scope) { - if (opp.Name != oppNames.get(opp.id)) + List oppsForUpdate = new List(); + for (Opportunity opp : oppsToProcess) { + if (opp.Name != originalOppNamesById.get(opp.id)) { oppsForUpdate.add(opp); + } } if (!oppsForUpdate.isEmpty()) { - database.update(oppsForUpdate, false); + Database.update(oppsForUpdate, false); } } - /** @description Batch process finish method, does nothing.*/ - public void finish(Database.BatchableContext BC) {} - + /** @description Batch process finish method, chains another batch if there are more opportunities to process.*/ + public void finish(Database.BatchableContext BC) { + if (shouldChainNextBatch()) { + Database.executeBatch(new OPP_OpportunityNaming_BATCH(lastOppIdProcessed), 200); + } + } + + /** @description Returns whether or not another batch should be chained*/ + private Boolean shouldChainNextBatch() { + if (lastOppIdProcessed == null) { + return false; + } + + String hasMoreQuery = buildOffsetQuery(lastOppIdProcessed, 1); + return !Database.query(hasMoreQuery).isEmpty(); + } } \ No newline at end of file diff --git a/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls b/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls new file mode 100644 index 00000000000..13713115bd4 --- /dev/null +++ b/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls @@ -0,0 +1,247 @@ +/* + Copyright (c) 2019, Salesforce.org + All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of Salesforce.org nor the names of + its contributors may be used to endorse or promote products derived + from this software without specific prior written permission. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN + ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + POSSIBILITY OF SUCH DAMAGE. +*/ +/** +* @author Salesforce.org +* @date 2019 +* @group Opportunity +* @group-content +* @description Unit Tests for the Opportunity Naming batch job +*/ + +@isTest +private with sharing class OPP_OpportunityNaming_BATCH_TEST { + private static final Integer NUM_OPPS = 10; + + @TestSetup + static void setup() { + List testOpps = createOpportunities(NUM_OPPS); + createOpportunityNamingSetting(); + } + + /** + * @description Confirms that Opportunities are renamed in chained batch chunks + */ + @isTest + private static void shouldRenameOpportunitiesInChunks() { + OPP_OpportunityNaming_BATCH.defaultQueryLimit = NUM_OPPS - 1; + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(); + + Test.startTest(); + Database.executeBatch(batch); + Test.stopTest(); + + List jobsApexBatch = queryOppNamingBatchJobs(); + System.assertEquals(2, jobsApexBatch.size(), 'Batch should run for each chunk of opportunities'); + + for (Opportunity opp : [SELECT Name FROM Opportunity]) { + System.assertEquals(false, opp.Name.startsWith('Test Opp'), + 'The opportunity should have been renamed: ' + opp.Name); + } + } + + /** + * @description Confirms that batch query locator retrieves Opportunitie in order of Id + */ + @isTest + private static void shouldQueryAllOpportunitiesInIdOrder() { + List testOpps = [SELECT Id FROM Opportunity ORDER BY Id]; + + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(testOpps[0].Id); + + String query = batch.start(null).getQuery(); + List queriedOpps = Database.query(query); + System.assertEquals(NUM_OPPS - 1, queriedOpps.size(), 'Only the offset opportunities should have been returned'); + + testOpps.remove(0); + for (Integer i = 0; i < NUM_OPPS - 1; i++) { + System.assertEquals(testOpps[i].Id, queriedOpps[i].Id, + 'The opportunities should have been queried in order of Id'); + } + } + + /** + * @description Confirms that batch query locator uses an Id offset when constructed with an Id parameter + */ + @isTest + private static void shouldQueryOffsetOpportunitiesWhenGivenId() { + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(); + List testOpps = [SELECT Id FROM Opportunity ORDER BY Id]; + + String query = batch.start(null).getQuery(); + List queriedOpps = Database.query(query); + System.assertEquals(NUM_OPPS, queriedOpps.size(), 'All of the opportunities should have been returned'); + + for (Integer i = 0; i < NUM_OPPS; i++) { + System.assertEquals(testOpps[i].Id, queriedOpps[i].Id, + 'The opportunities should have been queried in order of Id'); + } + } + + /** + * @description Confirms that batch query locator is limited + */ + @isTest + private static void shouldLimitQuery() { + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(); + String query = batch.start(null).getQuery(); + System.assert(query.endsWith('LIMIT 10000000'), 'The query should have the correct limit'); + } + + /** + * @description Confirms that the execute method tracks the Id of the last Opportunity processed + */ + @isTest + private static void shouldTrackLastOpportunityIdProcessed() { + List testOpps = [SELECT Id, Name FROM Opportunity ORDER BY Id]; + + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(); + + Test.startTest(); + batch.execute(null,testOpps); + Test.stopTest(); + + Id expectedId = testOpps[NUM_OPPS - 1].Id; + System.assertEquals(expectedId, batch.lastOppIdProcessed, + 'The execute method should track the Id of the last opportunity processed'); + } + + /** + * @description Confirms that the finish method chains the next batch with an offset + * of the last Opportunity Id processed + */ + + @isTest + private static void shouldChainNextBatchOffsetByLastRecordProcessed() { + List testOpps = [SELECT Id FROM Opportunity ORDER BY Id]; + + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(); + batch.lastOppIdProcessed = testOpps[0].Id; + + Test.startTest(); + batch.finish(null); + Test.stopTest(); + + List jobsApexBatch = queryOppNamingBatchJobs(); + System.assertEquals(1, jobsApexBatch.size(), 'The naming batch should be started again'); + + Opportunity offsetOpportunity = [SELECT Name FROM Opportunity WHERE Id = :testOpps[0].Id]; + System.assert(offsetOpportunity.Name.startsWith('Test Opp'), + 'The offset opportunity should not have been processed in the chained batch'); + + for (Opportunity opp : [SELECT Name FROM Opportunity WHERE Id != :testOpps[0].Id]) { + System.assertEquals(false, opp.Name.startsWith('Test Opp'), + 'The remaining opportunities should have been renamed: ' + opp.Name); + } + } + + /** + * @description Confirms that the finish method does not chain the next batch + * if it fails to capture the last Opportunity Id processed + */ + @isTest + private static void shouldNotChainNextBatchIfLastOppIdProcessedIsNull() { + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(); + + Test.startTest(); + batch.finish(null); + Test.stopTest(); + + List jobsApexBatch = [ + SELECT Id FROM AsyncApexJob + WHERE JobType = 'BatchApex' + AND ApexClass.Name = 'OPP_OpportunityNaming_BATCH' + ]; + + System.assert(jobsApexBatch.isEmpty(), 'The naming batch should not be started again'); + } + + /** + * @description Confirms that the finish method does not chain the next batch + * if there are no more records to process + */ + @isTest + private static void shouldNotChainNextBatchIfThereAreNoMoreRecordsToProcess() { + List testOpps = [SELECT Id FROM Opportunity ORDER BY Id]; + + OPP_OpportunityNaming_BATCH batch = new OPP_OpportunityNaming_BATCH(); + batch.lastOppIdProcessed = testOpps[testOpps.size()-1].Id; + + Test.startTest(); + batch.finish(null); + Test.stopTest(); + + List jobsApexBatch = queryOppNamingBatchJobs(); + System.assert(jobsApexBatch.isEmpty(), 'The naming batch should not be started again'); + } + + /** + * @description Creates a given number of test opportunities + */ + private static List createOpportunities(Integer numOpps) { + Account testAccount = new Account(Name = 'Test Account'); + insert testAccount; + List opps = new List(); + for (Integer i = 0; i < numOpps; i++) { + opps.add( + new Opportunity( + AccountId = testAccount.Id, + Name = 'Test Opp ' + i, + StageName = 'Closed Won', + CloseDate = Date.today() + ) + ); + } + insert opps; + return opps; + } + + /** + * @description Creates a Opportunity_Naming_Settings__c record + */ + private static void createOpportunityNamingSetting() { + Opportunity_Naming_Settings__c oppNamingSettings = new Opportunity_Naming_Settings__c( + Name = 'foo', + Opportunity_Name_Format__c = '{!Account.Name} {!CloseDate}', + Attribution__c = Label.oppNamingBoth + ); + insert oppNamingSettings; + } + + /** + * @description Retrieves OPP_OpportunityNaming_BATCH batch jobs + */ + private static List queryOppNamingBatchJobs() { + return [ + SELECT Id FROM AsyncApexJob + WHERE JobType = 'BatchApex' + AND ApexClass.Name = 'OPP_OpportunityNaming_BATCH' + ]; + } +} diff --git a/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls-meta.xml b/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls-meta.xml new file mode 100644 index 00000000000..f3f6d1fde5f --- /dev/null +++ b/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls-meta.xml @@ -0,0 +1,5 @@ + + + 45.0 + Active + From 69d530adb5a28ef82355cd89830010bd5c80f433 Mon Sep 17 00:00:00 2001 From: Sean Cuevo Date: Fri, 15 Mar 2019 14:36:14 -0600 Subject: [PATCH 2/4] adding missing file to package, continuing polling even on complete to capture chain opp naming batches --- src/classes/OPP_OpportunityNaming_BATCH.cls | 2 +- src/package.xml | 1 + src/pages/STG_PanelOppNamingBatch.page | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/classes/OPP_OpportunityNaming_BATCH.cls b/src/classes/OPP_OpportunityNaming_BATCH.cls index 6fd02096c28..e0e97ae277c 100644 --- a/src/classes/OPP_OpportunityNaming_BATCH.cls +++ b/src/classes/OPP_OpportunityNaming_BATCH.cls @@ -39,7 +39,7 @@ public class OPP_OpportunityNaming_BATCH implements Database.Batchable, private String query; @TestVisible - private static Integer defaultQueryLimit= 10000000; + private static Integer defaultQueryLimit = 10000000; @TestVisible private Id lastOppIdProcessed; diff --git a/src/package.xml b/src/package.xml index 50c9d4ec39c..4b53431df85 100644 --- a/src/package.xml +++ b/src/package.xml @@ -265,6 +265,7 @@ OPP_OpportunityContactRoles_TEST OPP_OpportunityNaming OPP_OpportunityNamingBTN_CTRL + OPP_OpportunityNaming_BATCH_TEST OPP_OpportunityNaming_BATCH OPP_OpportunityNaming_TEST OPP_PrimaryContactRoleMerge diff --git a/src/pages/STG_PanelOppNamingBatch.page b/src/pages/STG_PanelOppNamingBatch.page index 8be4c874e81..a3ab1dd8f0b 100644 --- a/src/pages/STG_PanelOppNamingBatch.page +++ b/src/pages/STG_PanelOppNamingBatch.page @@ -12,7 +12,7 @@ strBatchComponentLabel="{!$Label.stgLabelOppNamingRefreshTitle}" cNumberOfJobs="1" startPolling="True" - stopPollingOnComplete="True" + stopPollingOnComplete="False" pollingDelay="1000"/> From ec22362baabb77e38c4b9ddca0bce59a21bf2e92 Mon Sep 17 00:00:00 2001 From: Sean Cuevo Date: Fri, 15 Mar 2019 15:27:36 -0600 Subject: [PATCH 3/4] stopping polling on complete to prevent fetching of the wrong batch job --- src/pages/STG_PanelOppNamingBatch.page | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/STG_PanelOppNamingBatch.page b/src/pages/STG_PanelOppNamingBatch.page index a3ab1dd8f0b..8be4c874e81 100644 --- a/src/pages/STG_PanelOppNamingBatch.page +++ b/src/pages/STG_PanelOppNamingBatch.page @@ -12,7 +12,7 @@ strBatchComponentLabel="{!$Label.stgLabelOppNamingRefreshTitle}" cNumberOfJobs="1" startPolling="True" - stopPollingOnComplete="False" + stopPollingOnComplete="True" pollingDelay="1000"/> From 64c000b4ff0e20bb03027a4bfa5f1dbc9b8735e2 Mon Sep 17 00:00:00 2001 From: Sean Cuevo Date: Fri, 15 Mar 2019 15:58:21 -0600 Subject: [PATCH 4/4] adding chunking pattern to batch description, using wrapper method for dml, formatting --- src/classes/OPP_OpportunityNaming_BATCH.cls | 8 ++++---- src/classes/OPP_OpportunityNaming_BATCH_TEST.cls | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/classes/OPP_OpportunityNaming_BATCH.cls b/src/classes/OPP_OpportunityNaming_BATCH.cls index e0e97ae277c..da94badf718 100644 --- a/src/classes/OPP_OpportunityNaming_BATCH.cls +++ b/src/classes/OPP_OpportunityNaming_BATCH.cls @@ -32,7 +32,8 @@ * @date 2015 * @group Opportunity * @group-content ../../ApexDocContent/Opportunity.htm -* @description Batch class creates names all Opportunities per the naming spec. +* @description Batch class creates names all Opportunities per the naming spec. Batch job chunks Opportunities +* in groups of 10,000,000 records ordered by Id and chains itself to process additional records to avoid query limits */ public class OPP_OpportunityNaming_BATCH implements Database.Batchable, Schedulable, Database.Stateful { /** @description The query for the batch process to run on.*/ @@ -75,8 +76,7 @@ public class OPP_OpportunityNaming_BATCH implements Database.Batchable, /********************************************************************************************************* * @description Batch process execute method. Names and updates all opportunities in the current batch. */ - public void execute(Database.BatchableContext BC, List scope) { - List oppsToProcess = scope; + public void execute(Database.BatchableContext BC, List oppsToProcess) { lastOppIdProcessed = oppsToProcess[oppsToProcess.size() - 1].Id; //save old opp names to see if we need an update @@ -97,7 +97,7 @@ public class OPP_OpportunityNaming_BATCH implements Database.Batchable, } if (!oppsForUpdate.isEmpty()) { - Database.update(oppsForUpdate, false); + UTIL_DMLService.updateRecords(oppsForUpdate, false); } } diff --git a/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls b/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls index 13713115bd4..5f615dba121 100644 --- a/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls +++ b/src/classes/OPP_OpportunityNaming_BATCH_TEST.cls @@ -136,7 +136,6 @@ private with sharing class OPP_OpportunityNaming_BATCH_TEST { * @description Confirms that the finish method chains the next batch with an offset * of the last Opportunity Id processed */ - @isTest private static void shouldChainNextBatchOffsetByLastRecordProcessed() { List testOpps = [SELECT Id FROM Opportunity ORDER BY Id];