V1.3.1 - Heap Size Improvements, Deferred Rollup Bugfix (#163)

* Doing some cleanup on the v1.3.0 release - making the pipeline more resilient by hardening the scripts/build-and-promote-package.ps1 ability to find the prior package version, removing some duplication between deploy.yml and package.json npm scripts, and removed this.syncRollups property from RollupAsyncProcessor to cut down on heap size, made runCalc logic easier to follow

* Less of a hack to get the key value for the last rollup package, thanks to @gdoenlen for the assist on this one

* Fix deferred rollup issue where a queueable with multiple targets turns into a batch job without regard for the batch job's lookup items not all matching the inner rollups lookup SObjectType (and field references, accordingly)

* Bumping package version from Github Action

Author: jamessimone

.github/workflows/deploy.yml

@@ -73,7 +73,7 @@ jobs:
       - name: Install & run SFDX Scanner
         run: |
           sfdx plugins:install @salesforce/sfdx-scanner
-          sfdx scanner:run --pmdconfig config/pmd-ruleset.xml --target . --engine pmd --severity-threshold 3
+          npm run scan
 
       # Store secret for dev hub
       - name: 'Populate auth file with DEVHUB_SFDX_URL secret'

extra-tests/classes/RollupIntegrationTests.cls

@@ -1403,7 +1403,7 @@ private class RollupIntegrationTests {
     Rollup.records = cpas;
     Rollup.shouldRun = true;
     Rollup.apexContext = TriggerOperation.AFTER_INSERT;
-    Rollup.specificControl = new RollupControl__mdt(ShouldRunAs__c = 'Synchronous Rollup');
+    Rollup.specificControl = new RollupControl__mdt(ShouldRunAs__c = RollupMetaPicklists.ShouldRunAs.Synchronous);
 
     // specifically do NOT wrap in Test.startTest() / Test.stopTest() - we need to ensure this happened synchronously
     Rollup.countFromApex(ContactPointAddress.PreferenceRank, ContactPointAddress.ParentId, Account.Id, Account.AnnualRevenue, Account.SObjectType).runCalc();
@@ -1414,7 +1414,6 @@ private class RollupIntegrationTests {
 
   @IsTest
   static void shouldPartiallyDeferRollupCalculationWhenOverLimits() {
-    Rollup.specificControl = new RollupControl__mdt(ShouldAbortRun__c = true);
     Account acc = [SELECT Id, OwnerId FROM Account];
     Account secondParent = new Account(Name = 'Second parent');
     insert secondParent;
@@ -1425,10 +1424,14 @@ private class RollupIntegrationTests {
     };
     insert cpas;
 
-    Rollup.defaultControl = new RollupControl__mdt(BatchChunkSize__c = 1, MaxRollupRetries__c = 1, MaxNumberOfQueries__c = 2, IsRollupLoggingEnabled__c = true);
     // start as synchronous rollup to allow for one deferral
-    Rollup.specificControl = new RollupControl__mdt(ShouldRunAs__c = 'Synchronous Rollup');
-
+    Rollup.defaultControl = new RollupControl__mdt(
+      BatchChunkSize__c = 1,
+      MaxRollupRetries__c = 1,
+      MaxNumberOfQueries__c = 2,
+      IsRollupLoggingEnabled__c = true,
+      ShouldRunAs__c = RollupMetaPicklists.ShouldRunAs.Synchronous
+    );
     Rollup.shouldRun = true;
     Rollup.records = cpas;
     Rollup.rollupMetadata = new List<Rollup__mdt>{
@@ -1460,6 +1463,71 @@ private class RollupIntegrationTests {
     }
   }
 
+  @IsTest
+  static void shouldNotBlowUpWhenDeferralLeadsToMultipleSObjectsBeingPresent() {
+    Account acc = [SELECT Id, OwnerId FROM Account];
+
+    Individual indy = new Individual(LastName = 'Indy');
+    insert indy;
+
+    List<ContactPointAddress> cpas = new List<ContactPointAddress>{
+      new ContactPointAddress(ParentId = acc.Id, Name = 'One', PreferenceRank = 1),
+      new ContactPointAddress(ParentId = indy.Id, Name = 'Two', PreferenceRank = 1)
+    };
+    insert cpas;
+
+    Rollup.defaultControl = new RollupControl__mdt(
+      BatchChunkSize__c = 2,
+      MaxRollupRetries__c = 1,
+      MaxNumberOfQueries__c = 2,
+      IsRollupLoggingEnabled__c = true,
+      ShouldRunAs__c = RollupMetaPicklists.ShouldRunAs.Synchronous
+    );
+    RollupAsyncProcessor.shouldRunAsBatch = true;
+    Rollup.shouldRun = true;
+    Rollup.records = cpas;
+    Rollup.rollupMetadata = new List<Rollup__mdt>{
+      new Rollup__mdt(
+        CalcItem__c = 'ContactPointAddress',
+        RollupFieldOnCalcItem__c = 'PreferenceRank',
+        LookupFieldOnCalcItem__c = 'ParentId',
+        LookupObject__c = 'Account',
+        LookupFieldOnLookupObject__c = 'Id',
+        RollupFieldOnLookupObject__c = 'AnnualRevenue',
+        RollupOperation__c = 'AVERAGE'
+      ),
+      new Rollup__mdt(
+        CalcItem__c = 'ContactPointAddress',
+        RollupFieldOnCalcItem__c = 'PreferenceRank',
+        LookupFieldOnCalcItem__c = 'ParentId',
+        LookupObject__c = 'Individual',
+        LookupFieldOnLookupObject__c = 'Id',
+        RollupFieldOnLookupObject__c = 'ChildrenCount',
+        RollupOperation__c = 'COUNT'
+      ),
+      new Rollup__mdt(
+        CalcItem__c = 'ContactPointAddress',
+        RollupFieldOnCalcItem__c = 'Name',
+        LookupFieldOnCalcItem__c = 'ParentId',
+        LookupObject__c = 'Individual',
+        LookupFieldOnLookupObject__c = 'Id',
+        RollupFieldOnLookupObject__c = 'LastName',
+        RollupOperation__c = 'CONCAT'
+      )
+    };
+    Rollup.apexContext = TriggerOperation.AFTER_INSERT;
+
+    Test.startTest();
+    Rollup.runFromTrigger();
+    Test.stopTest();
+
+    acc = [SELECT AnnualRevenue FROM Account];
+    indy = [SELECT LastName, ChildrenCount FROM Individual];
+    System.assertEquals(1, acc.AnnualRevenue, 'Annual revenue should have been averaged properly');
+    System.assertEquals(1, indy.ChildrenCount, 'Children count should have been counted correctly');
+    System.assertEquals('Indy, Two',  indy.LastName, 'LastName should have been concat properly');
+  }
+
   @IsTest
   static void shouldRunDirectlyFromApex() {
     Account acc = [SELECT Id FROM Account];

package.json

@@ -1,6 +1,6 @@
 {
   "name": "apex-rollup",
-  "version": "1.3.0",
+  "version": "1.3.1",
   "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",

rollup/core/classes/Rollup.cls

@@ -542,16 +542,15 @@ global without sharing virtual class Rollup {
       if (flowInput.recordsToRollup?.isEmpty() != false) {
         flowOutput.message = 'No records to rollup, returning early';
       } else {
-        SObject firstRecord = flowInput.recordsToRollup[0];
         // flow collections are not strongly typed, so we grab from the first record
-        SObjectType sObjectType = firstRecord.getSObjectType();
+        SObjectType sObjectType = flowInput.recordsToRollup[0].getSObjectType();
         // this will throw back up to the Flow engine if the inputs don't pass validation
         enforceValidationRules(flowInput);
         enforceFlowSpecificRules(flowInput, sObjectType);
         winnowOldFlowRecords(flowInput);
 
         Map<Id, SObject> oldFlowRecords = new Map<Id, SObject>(flowInput.oldRecordsToRollup);
-        String rollupContext = getFlowRollupContext(flowInput, firstRecord, oldFlowRecords);
+        String rollupContext = getFlowRollupContext(flowInput, flowInput.recordsToRollup[0], sObjectType, oldFlowRecords);
 
         Rollup__mdt meta = transformFlowInputToRollupMetadata(flowInput, rollupContext, sObjectType);
         if (metaToInput.containsKey(meta)) {
@@ -2048,7 +2047,7 @@ global without sharing virtual class Rollup {
     }
   }
 
-  private static String getFlowRollupContext(FlowInput firstInput, SObject firstRecord, Map<Id, SObject> oldFlowRecords) {
+  private static String getFlowRollupContext(FlowInput firstInput, SObject firstRecord, SObjectType sObjectType, Map<Id, SObject> oldFlowRecords) {
     String flowContext = firstInput.rollupContext.toUpperCase();
     if (String.isBlank(flowContext) || flowContext == 'UPSERT' && oldFlowRecords.containsKey(firstRecord.Id) == false) {
       flowContext = 'INSERT';
@@ -2068,7 +2067,7 @@ global without sharing virtual class Rollup {
         matchingOperation = TriggerOperation.BEFORE_DELETE;
       }
     }
-    populateCachedApexOperations(firstRecord.getSObjectType(), matchingOperation);
+    populateCachedApexOperations(sObjectType, matchingOperation);
 
     return flowContext == 'INSERT' ? '' : flowContext + '_';
   }

rollup/core/classes/RollupAsyncProcessor.cls

@@ -8,7 +8,6 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
   private final Op op;
 
   private final List<RollupAsyncProcessor> deferredRollups = new List<RollupAsyncProcessor>();
-  private final List<RollupAsyncProcessor> syncRollups = new List<RollupAsyncProcessor>();
 
   protected final SObjectType calcItemType;
   protected Boolean isProcessed = false;
@@ -21,7 +20,6 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
   private List<SObject> lookupItems;
   private Map<String, List<SObject>> cachedFullRecalcs;
 
-  private static final RollupSettings__c SETTINGS = RollupSettings__c.getInstance();
   private static Integer stackDepth = 0;
   private static Boolean isRunningAsync = false;
   @TestVisible
@@ -239,9 +237,16 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
   }
 
   public virtual void execute(Database.BatchableContext context, List<SObject> lookupItems) {
-    for (RollupAsyncProcessor rollup : this.rollups) {
-      this.initializeRollupFieldDefaults(lookupItems, rollup);
+    for (Integer index = this.rollups.size() - 1; index >= 0; index--) {
+      RollupAsyncProcessor roll = this.rollups[index];
+      if (lookupItems.getSObjectType() != roll.lookupObj) {
+        this.rollups.remove(index);
+        this.deferredRollups.add(roll);
+      } else {
+        this.initializeRollupFieldDefaults(lookupItems, roll);
+      }
     }
+
     this.lookupItems = lookupItems;
     this.process(this.rollups);
     RollupLogger.Instance.save();
@@ -253,23 +258,20 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
 
   public virtual override String runCalc() {
     // side effect in the below method - rollups can be removed from this.rollups if a control record ShouldAbortRun__c == true
-    // they also can be added to this.syncRollups if we're already async
-    this.ingestRollupControlData();
+    // they also can be added to syncRollups if we're already async
+    List<RollupAsyncProcessor> syncRollups = new List<RollupAsyncProcessor>();
+    this.ingestRollupControlData(syncRollups);
 
     if (this.isNoOp) {
-      this.isNoOp = this.rollups.isEmpty() && this.syncRollups.isEmpty() && this.calcItems?.isEmpty() == true;
+      this.isNoOp = this.rollups.isEmpty() && syncRollups.isEmpty() && this.calcItems?.isEmpty() == true;
     }
 
     String rollupProcessId = 'No process Id';
-    if (this.isNoOp || this.rollupControl.ShouldAbortRun__c || SETTINGS.IsEnabled__c == false) {
+    if (this.isNoOp || this.rollupControl.ShouldAbortRun__c || RollupSettings__c.getInstance().IsEnabled__c == false) {
       RollupLogger.Instance.log('no-op, exiting early to avoid burning async job', LoggingLevel.FINE);
-      RollupLogger.Instance.save();
-      return rollupProcessId;
-    }
-
-    if (this.syncRollups.isEmpty() == false) {
+    } else if (syncRollups.isEmpty() == false) {
       RollupLogger.Instance.log('about to process sync rollups', LoggingLevel.DEBUG);
-      this.process(this.syncRollups);
+      this.process(syncRollups);
       RollupLogger.Instance.log('finished running sync rollups', LoggingLevel.DEBUG);
       rollupProcessId = 'running rollups flagged to go synchronously';
     } else {
@@ -781,7 +783,7 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
     return isValid;
   }
 
-  private void ingestRollupControlData() {
+  private void ingestRollupControlData(List<RollupAsyncProcessor> syncRollups) {
     RollupControl__mdt orgDefaults = this.rollupControl;
     for (Integer index = this.rollups.size() - 1; index >= 0; index--) {
       RollupAsyncProcessor rollup = this.rollups[index];
@@ -799,7 +801,7 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
         this.rollups.remove(index);
       } else if (couldRunSync && shouldRunSyncDeferred == false) {
         this.rollups.remove(index);
-        this.syncRollups.add(rollup);
+        syncRollups.add(rollup);
       } else if (couldRunSync && shouldRunSyncDeferred) {
         this.rollups.remove(index);
         this.getCachedRollups().add(rollup);

scripts/build-and-promote-package.ps1

@@ -13,8 +13,9 @@ function Get-Apex-Rollup-Package-Alias {
   param (
     $packageVersion
   )
-  if ($packageVersion.EndsWith(".0")) {
-    $packageVersion = $packageVersion.Substring(0, $packageVersion.length - 2);
+  $matchString = ".0-0"
+  if ($packageVersion.EndsWith($matchString)) {
+    $packageVersion = $packageVersion.Substring(0, $packageVersion.Length - $matchString.Length);
   }
   return "apex-rollup@$packageVersion-0"
 }
@@ -71,12 +72,20 @@ try {
   $priorPackageVersionId = $sfdxProjectJson.packageAliases[0] | Select-Object -ExpandProperty (Get-Apex-Rollup-Package-Alias $priorPackageVersionNumber)
 } catch {
   # if there hasn't been a current version of the package, get the previous version and its associated package Id
-  $currentPackageNumber = ([int]($currentPackageVersion | Select-String -Pattern \S\S\.0).Matches.Value)
+  $currentPackageNumber = ([int]($currentPackageVersion | Select-String -Pattern '(\d|\d\d)\.0').Matches.Value)
   $currentPackageNumberString = $currentPackageNumber.ToString()
   $priorPackageVersionString = ($currentPackageNumber - 1).ToString()
 
   $priorPackageVersionNumber = (Update-Last-Substring $currentPackageVersion $currentPackageNumberString $priorPackageVersionString)
-  $priorPackageVersionId = $sfdxProjectJson.packageAliases[0] | Select-Object -ExpandProperty (Get-Apex-Rollup-Package-Alias $priorPackageVersionNumber)
+  try {
+    $priorPackageVersionId = $sfdxProjectJson.packageAliases[0] | Select-Object -ExpandProperty (Get-Apex-Rollup-Package-Alias $priorPackageVersionNumber)
+  } catch {
+    $allPackages = $sfdxProjectJson.packageAliases[0] | Select-Object -ExcludeProperty 'Nebula Logger*' | Get-Member -MemberType NoteProperty | Select-Object
+    $priorPackageVersionFull = $allPackages[$allPackages.Length - 1].Name
+    $apexRollupPosition = $priorPackageVersionFull.IndexOf("@")
+    $priorPackageVersionId = $priorPackageVersionFull.Substring($apexRollupPosition + 1).Split('-')[0]
+    $priorPackageVersionNumber = $priorPackageVersionId
+  }
 }
 
 Write-Output "Prior package version: $priorPackageVersionId"
@@ -120,7 +129,7 @@ if($currentBranch -eq "main") {
   if ($packageJson.version -ne $currentPackageVersion) {
     Write-Output "Bumping package.json version to: $currentPackageVersion"
 
-    $packageJson.version = $currentPackageVersion
+    $packageJson.version = Update-Last-Substring $currentPackageVersion ".0" ""
     $packagePath = "./package.json"
     ConvertTo-Json -InputObject $packageJson | Set-Content -Path $packagePath -NoNewline
 

sfdx-project.json

@@ -4,7 +4,7 @@
             "default": true,
             "package": "apex-rollup",
             "path": "rollup",
-            "versionNumber": "1.3.0.0",
+            "versionNumber": "1.3.1.0",
             "versionDescription": "Fixes parent-level filtering for Flow, deprecated RollupLoggerPlugin__mdt in favor of RollupPlugin__mdt, adds PMD analysis",
             "releaseNotesUrl": "https://github.com/jamessimone/apex-rollup/releases/latest",
             "unpackagedMetadata": {
@@ -86,6 +86,7 @@
         "[email protected]": "04t6g000008ShMNAA0",
         "[email protected]": "04t6g000008ShPMAA0",
         "[email protected]": "04t6g000008ShSaAAK",
-        "[email protected]": "04t6g000008ShUCAA0"
+        "[email protected]": "04t6g000008ShUCAA0",
+        "[email protected]": "04t6g000008ShZ8AAK"
     }
 }