v1.7.4 - Allow Cursor Usage From Batch Apex (#648)

* Fixes issue reported by internal colleague with RollupCalcItemReplacer when two rollups with parent-level fields use mutually exclusive where clauses on the same parent-level field * Fixes #646 by providing escape hatch for cursor usage when batch apex is the originating point for Apex Rollup
jamessimone · Jan 10, 2025 · 98f7b13 · 98f7b13
1 parent 1a1134b
commit 98f7b13
Show file tree

Hide file tree

Showing 11 changed files with 145 additions and 27 deletions.
diff --git a/README.md b/README.md
@@ -24,11 +24,11 @@ As well, don't miss [the Wiki](../../wiki), which includes even more info for co
 
 ## Deployment & Setup
 
-<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfdwAAC">
+<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfezAAC">
   <img alt="Deploy to Salesforce" src="./media/deploy-package-to-prod.png">
 </a>
 
-<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfdwAAC">
+<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfezAAC">
   <img alt="Deploy to Salesforce Sandbox" src="./media/deploy-package-to-sandbox.png">
 </a>
 <br/>

diff --git a/extra-tests/classes/RollupCalcItemReplacerTests.cls b/extra-tests/classes/RollupCalcItemReplacerTests.cls
@@ -196,6 +196,25 @@ private class RollupCalcItemReplacerTests {
     Assert.areEqual(0, records.size());
   }
 
+  @IsTest
+  static void doesNotUseAndForMultipleParentWhereClauses() {
+    Account acc = [SELECT Id FROM Account];
+    RollupCalcItemReplacer replacer = new RollupCalcItemReplacer(
+      new RollupControl__mdt(IsRollupLoggingEnabled__c = true, ReplaceCalcItemsAsyncWhenOverCount__c = 3)
+    );
+
+    List<SObject> records = replacer.replace(
+      new List<Account>{ acc, acc },
+      new List<Rollup__mdt>{
+        new Rollup__mdt(CalcItemWhereClause__c = 'Id != \'' + acc.Id + '\' AND Owner.Name = null', CalcItem__c = 'Account'),
+        new Rollup__mdt(CalcItemWhereClause__c = 'Owner.Name != null AND Type != null', CalcItem__c = 'Account')
+      }
+    );
+
+    Assert.areEqual(1, records.size());
+    Assert.areEqual(acc.Id, records[0].Id);
+  }
+
   @IsTest
   static void doesNotReplaceForCustomTypeFields() {
     List<Account> accounts = [SELECT Id FROM Account];

diff --git a/extra-tests/classes/RollupIntegrationTests.cls b/extra-tests/classes/RollupIntegrationTests.cls
@@ -1,5 +1,5 @@
 @IsTest
-private class RollupIntegrationTests {
+private class RollupIntegrationTests implements Database.Batchable<SObject> {
   @TestSetup
   static void setup() {
     Rollup.onlyUseMockMetadata = true;
@@ -2038,4 +2038,47 @@ private class RollupIntegrationTests {
     acc = [SELECT AnnualRevenue FROM Account WHERE Id = :acc.Id LIMIT 1];
     Assert.areEqual(2, acc.AnnualRevenue);
   }
+
+  @IsTest
+  static void startingFromBatchDoesNotBlowUpCursorBasedFullRecalc() {
+    Test.startTest();
+    Database.executeBatch(new RollupIntegrationTests());
+    Test.stopTest();
+
+    Assert.areEqual(1, [SELECT AnnualRevenue FROM Account].AnnualRevenue);
+  }
+
+  public List<SObject> start(Database.BatchableContext bc) {
+    return [SELECT Id FROM Organization LIMIT 1];
+  }
+
+  public void execute(Database.BatchableContext bc, List<SObject> records) {
+    Rollup.defaultControl = new RollupControl__mdt(
+      BatchChunkSize__c = 1,
+      MaxLookupRowsBeforeBatching__c = 0,
+      IsRollupLoggingEnabled__c = true,
+      MaxRollupRetries__c = 1
+    );
+    Rollup.onlyUseMockMetadata = true;
+    Rollup.rollupMetadata = new List<Rollup__mdt>{
+      new Rollup__mdt(
+        RollupFieldOnCalcItem__c = 'Id',
+        LookupObject__c = 'Account',
+        LookupFieldOnCalcItem__c = 'ParentId',
+        LookupFieldOnLookupObject__c = 'Id',
+        RollupFieldOnLookupObject__c = 'AnnualRevenue',
+        RollupOperation__c = 'REFRESH_COUNT',
+        CalcItem__c = 'ContactPointAddress'
+      )
+    };
+    ContactPointAddress con = new ContactPointAddress(ParentId = [SELECT Id FROM Account LIMIT 1].Id, Name = 'Child');
+    insert con;
+    Rollup.apexContext = TriggerOperation.AFTER_INSERT;
+    rollup.records = new List<ContactPointAddress>{ con };
+    rollup.shouldRun = true;
+    Rollup.runFromTrigger();
+  }
+
+  public void finish(Database.BatchableContext bc) {
+  }
 }
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "apex-rollup",
-  "version": "1.7.3",
+  "version": "1.7.4",
   "description": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
   "repository": {
     "type": "git",

diff --git a/rollup-namespaced/README.md b/rollup-namespaced/README.md
@@ -18,12 +18,12 @@ For more info, see the base `README`.
 
 ## Deployment & Setup
 
-<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfXYAA0">
+<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Off4AAC">
   <img alt="Deploy to Salesforce"
        src="./media/deploy-package-to-prod.png">
 </a>
 
-<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfXYAA0">
+<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Off4AAC">
   <img alt="Deploy to Salesforce Sandbox"
        src="./media/deploy-package-to-sandbox.png">
 </a>
diff --git a/rollup-namespaced/sfdx-project.json b/rollup-namespaced/sfdx-project.json
@@ -4,7 +4,7 @@
             "default": true,
             "package": "apex-rollup-namespaced",
             "path": "rollup-namespaced/source/rollup",
-            "versionName": "Fixes namespace shadowing issue in RollupFieldInitializer",
+            "versionName": "Fixes RollupCalcItemReplacer usage with mutually exclusive where clauses, and enables Cursor usage from batch apex jobs",
             "versionNumber": "1.2.3.0",
             "versionDescription": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
             "releaseNotesUrl": "https://github.com/jamessimone/apex-rollup/releases/latest",
@@ -27,6 +27,7 @@
         "[email protected]": "04t6g000008OfSJAA0",
         "[email protected]": "04t6g000008OfU0AAK",
         "[email protected]": "04t6g000008OfWVAA0",
-        "[email protected]": "04t6g000008OfXYAA0"
+        "[email protected]": "04t6g000008OfXYAA0",
+        "[email protected]": "04t6g000008Off4AAC"
     }
 }
diff --git a/rollup/core/classes/RollupAsyncProcessor.cls b/rollup/core/classes/RollupAsyncProcessor.cls
@@ -266,9 +266,9 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
     if (this.isNoOp) {
       logMessage = 'no-op, exiting early to avoid burning async job';
     } else if (this.rollupControl.ShouldAbortRun__c) {
-      logMessage = String.valueOf(RollupControl__mdt.SObjectType) + '.' + String.valueOf(RollupControl__mdt.ShouldAbortRun__c) + ' set to true, exiting early';
+      logMessage = RollupControl__mdt.SObjectType.toString() + '.' + RollupControl__mdt.ShouldAbortRun__c + ' set to true, exiting early';
     } else if (RollupSettings__c.getInstance().IsEnabled__c == false && shouldRunWithoutCustomSetting == false) {
-      logMessage = String.valueOf(RollupSettings__c.SObjectType) + '.' + String.valueOf(RollupSettings__c.IsEnabled__c) + ' is false, exiting early';
+      logMessage = RollupSettings__c.SObjectType.toString() + '.' + RollupSettings__c.IsEnabled__c + ' is false, exiting early';
     } else {
       if (syncRollups.isEmpty() == false) {
         rollupProcessId = this.getNoProcessId();
@@ -506,10 +506,18 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
   }
 
   protected String startBatchProcessor() {
+    String processId;
     if (this.isFromBatchExecute) {
-      return new QueueableProcessor(this).startAsyncWork();
+      processId = new QueueableProcessor(this).startAsyncWork();
+    } else {
+      try {
+        processId = Database.executeBatch(this, this.rollupControl.BatchChunkSize__c.intValue());
+      } catch (Exception ex) {
+        this.logger.log('Could not start batch, trying again as Queueable', ex, System.LoggingLevel.WARN);
+        processId = new QueueableProcessor(this).startAsyncWork();
+      }
     }
-    return Database.executeBatch(this, this.rollupControl.BatchChunkSize__c.intValue());
+    return processId;
   }
 
   protected List<SObject> getExistingLookupItems(Set<String> lookupKeys, RollupAsyncProcessor roll, Set<String> uniqueQueryFieldNames) {

diff --git a/rollup/core/classes/RollupCalcItemReplacer.cls b/rollup/core/classes/RollupCalcItemReplacer.cls
@@ -239,7 +239,7 @@ public without sharing class RollupCalcItemReplacer {
       new List<String>(additionalQueryFields),
       'Id',
       '=',
-      String.join(optionalWhereClauses, ' AND ')
+      String.join(optionalWhereClauses, ' OR ')
     );
 
     calcItems = this.repo.setQuery(queryString).setArg(calcItems).get();

diff --git a/rollup/core/classes/RollupFullBatchRecalculator.cls b/rollup/core/classes/RollupFullBatchRecalculator.cls
@@ -1,7 +1,6 @@
 public without sharing virtual class RollupFullBatchRecalculator extends RollupFullRecalcProcessor {
   private final RollupState state = new RollupState();
-  private Database.Cursor cursor;
-  private Integer currentPosition = 0;
+  private CalcItemRetriever retriever;
 
   private static final Integer DEFAULT_CHUNK_SIZE = 500;
 
@@ -56,29 +55,76 @@ public without sharing virtual class RollupFullBatchRecalculator extends RollupF
         this.finalizer.addCaboose(this.cabooses.remove(0));
       }
     }
-    this.cursor = this.cursor ?? this.preStart().getCursor();
-
-    Integer countOfRecordsToReturn = this.rollupControl.BatchChunkSize__c.intValue();
-    if (countOfRecordsToReturn + this.currentPosition > this.cursor.getNumRecords()) {
-      countOfRecordsToReturn = this.cursor.getNumRecords() - this.currentPosition;
+    if (this.retriever == null) {
+      try {
+        this.retriever = new CursorBasedRetriever(this.preStart(), this.rollupControl);
+      } catch (Exception ex) {
+        this.logger.log('cursor use disallowed', ex, System.LoggingLevel.WARN);
+        return System.enqueueJob(new FullBatchQueueableFailsafe(this));
+      }
     }
-    this.calcItems = this.cursor.fetch(this.currentPosition, countOfRecordsToReturn);
-    this.currentPosition += countOfRecordsToReturn;
+
+    this.calcItems = this.retriever.fetch();
     return super.startAsyncWork();
   }
 
+  private String runBatch() {
+    return this.startBatchProcessor();
+  }
+
   protected override RollupFinalizer getFinalizer() {
     return new FullRecalcFinalizer(this);
   }
 
+  private interface CalcItemRetriever {
+    List<SObject> fetch();
+    Boolean shouldContinue();
+  }
+
+  private class CursorBasedRetriever implements CalcItemRetriever {
+    private final Database.Cursor cursor;
+    private Integer countOfRecordsToReturn;
+    private Integer currentPosition = 0;
+
+    public CursorBasedRetriever(RollupRepository repo, RollupControl__mdt control) {
+      this.cursor = repo.getCursor();
+      this.countOfRecordsToReturn = control.BatchChunkSize__c.intValue();
+    }
+
+    public List<SObject> fetch() {
+      if (this.countOfRecordsToReturn + this.currentPosition > this.cursor.getNumRecords()) {
+        this.countOfRecordsToReturn = this.cursor.getNumRecords() - this.currentPosition;
+      }
+      List<SObject> fetchedRecords = this.cursor.fetch(this.currentPosition, this.countOfRecordsToReturn);
+      this.currentPosition += this.countOfRecordsToReturn;
+      return fetchedRecords;
+    }
+
+    public Boolean shouldContinue() {
+      return this.currentPosition < this.cursor.getNumRecords();
+    }
+  }
+
+  private class FullBatchQueueableFailsafe implements System.Queueable {
+    private final RollupFullBatchRecalculator roll;
+
+    public FullBatchQueueableFailsafe(RollupFullBatchRecalculator roll) {
+      this.roll = roll;
+    }
+
+    public void execute(QueueableContext qc) {
+      this.roll.runBatch();
+    }
+  }
+
   private class FullRecalcFinalizer extends RollupFinalizer {
     private final RollupFullBatchRecalculator conductor;
     public FullRecalcFinalizer(RollupFullBatchRecalculator conductor) {
       this.conductor = conductor;
     }
 
     public override void handleSuccess() {
-      if (this.conductor.currentPosition < this.conductor.cursor.getNumRecords()) {
+      if (this.conductor.retriever?.shouldContinue() ?? false) {
         this.conductor.startAsyncWork();
       } else {
         this.conductor.finish();

diff --git a/rollup/core/classes/RollupLogger.cls b/rollup/core/classes/RollupLogger.cls
@@ -1,7 +1,7 @@
 global without sharing virtual class RollupLogger implements ILogger {
   @TestVisible
   // this gets updated via the pipeline as the version number gets incremented
-  private static final String CURRENT_VERSION_NUMBER = 'v1.7.3';
+  private static final String CURRENT_VERSION_NUMBER = 'v1.7.4';
   private static final System.LoggingLevel FALLBACK_LOGGING_LEVEL = System.LoggingLevel.DEBUG;
   private static final RollupPlugin PLUGIN = new RollupPlugin();
 

diff --git a/sfdx-project.json b/sfdx-project.json
@@ -5,8 +5,8 @@
             "package": "apex-rollup",
             "path": "rollup",
             "scopeProfiles": true,
-            "versionName": "Fixes namespace shadowing issue in RollupFieldInitializer",
-            "versionNumber": "1.7.3.0",
+            "versionName": "Fixes RollupCalcItemReplacer usage with mutually exclusive where clauses, and enables Cursor usage from batch apex jobs",
+            "versionNumber": "1.7.4.0",
             "versionDescription": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
             "releaseNotesUrl": "https://github.com/jamessimone/apex-rollup/releases/latest",
             "unpackagedMetadata": {
@@ -109,6 +109,7 @@
         "[email protected]": "04t6g000008OfTvAAK",
         "[email protected]": "04t6g000008OfWQAA0",
         "[email protected]": "04t6g000008OfXTAA0",
-        "[email protected]": "04t6g000008OfdwAAC"
+        "[email protected]": "04t6g000008OfdwAAC",
+        "[email protected]": "04t6g000008OfezAAC"
     }
 }