From 0a6567a3cf3b0a83b897a78b378422db8312f35b Mon Sep 17 00:00:00 2001 From: James Simone <16430727+jamessimone@users.noreply.github.com> Date: Mon, 25 Sep 2023 09:37:00 -0600 Subject: [PATCH] v1.5.86 - OR clause bugfix and full recalc update (#503) * Fixes #494 by updating where clause query parsing to prevent batch apex from running unexpectedly for nested OR clauses * Making some notes on WEEK_IN_YEAR date literal to come back to later * Optimizations, added a test on full recalc path to ensure parent reset processor only runs when it's supposed to * Fixes #500 by properly handling the stateful maintenance of previously reset parents when children span across multiple batches --- README.md | 4 +- .../classes/RollupDateLiteralTests.cls | 1 + extra-tests/classes/RollupFullRecalcTests.cls | 49 ++++++-- .../classes/RollupIntegrationTests.cls | 31 +++++ .../RollupParentResetProcessorTests.cls | 57 ++++++++- .../classes/RollupQueryBuilderTests.cls | 13 ++ .../classes/RollupSObjectUpdaterTests.cls | 6 +- package.json | 2 +- rollup-namespaced/README.md | 4 +- rollup-namespaced/sfdx-project.json | 7 +- rollup/core/classes/RollupAsyncProcessor.cls | 116 ++++++++---------- rollup/core/classes/RollupDateLiteral.cls | 3 +- .../classes/RollupFullBatchRecalculator.cls | 12 -- .../classes/RollupFullRecalcProcessor.cls | 5 +- rollup/core/classes/RollupLogger.cls | 2 +- .../classes/RollupParentResetProcessor.cls | 9 +- rollup/core/classes/RollupQueryBuilder.cls | 4 +- rollup/core/classes/RollupSObjectUpdater.cls | 2 +- sfdx-project.json | 7 +- 19 files changed, 222 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index b8df4c71..bbf69c99 100644 --- a/README.md +++ b/README.md @@ -24,12 +24,12 @@ As well, don't miss [the Wiki](../../wiki), which includes even more info for co ## Deployment & Setup - + Deploy to Salesforce - + Deploy to Salesforce Sandbox diff --git a/extra-tests/classes/RollupDateLiteralTests.cls b/extra-tests/classes/RollupDateLiteralTests.cls index 078b346d..3ae91a20 100644 --- a/extra-tests/classes/RollupDateLiteralTests.cls +++ b/extra-tests/classes/RollupDateLiteralTests.cls @@ -566,6 +566,7 @@ private class RollupDateLiteralTests { comparisonDate = Datetime.newInstance(2020, 12, 22); oneWeekBefore = comparisonDate.addDays(-7); oneWeekAfter = comparisonDate.addDays(7); + // TODO according to SOQL, the below should actually be 51 weekInYear = RollupDateLiteral.getFunction('WEEK_IN_YEAR(CreatedDate)', '52'); System.assertEquals(true, weekInYear.matches(comparisonDate, '='), weekInYear); diff --git a/extra-tests/classes/RollupFullRecalcTests.cls b/extra-tests/classes/RollupFullRecalcTests.cls index e05f59ec..eb0531c0 100644 --- a/extra-tests/classes/RollupFullRecalcTests.cls +++ b/extra-tests/classes/RollupFullRecalcTests.cls @@ -1663,12 +1663,21 @@ private class RollupFullRecalcTests { IsRollupLoggingEnabled__c = true, BatchChunkSize__c = 100 ); - Account acc = [SELECT Id FROM Account]; + Account parentA = [SELECT Id FROM Account]; + Account parentB = new Account(Name = 'Parent B'); + Account parentC = new Account(Name = 'Parent C'); + insert new List{ parentB, parentC }; List cpas = new List{ - new ContactPointAddress(ParentId = acc.Id, PreferenceRank = 1, Name = 'One'), - new ContactPointAddress(ParentId = acc.Id, PreferenceRank = 2, Name = 'Two'), - new ContactPointAddress(ParentId = acc.Id, PreferenceRank = 3, Name = 'Three') + new ContactPointAddress(ParentId = parentA.Id, PreferenceRank = 1, Name = 'One A', UsageType = '1'), + new ContactPointAddress(ParentId = parentB.Id, PreferenceRank = 1, Name = 'One B', UsageType = '1'), + new ContactPointAddress(ParentId = parentC.Id, PreferenceRank = 1, Name = 'One C', UsageType = '1'), + new ContactPointAddress(ParentId = parentA.Id, PreferenceRank = 2, Name = 'Two A', UsageType = '1'), + new ContactPointAddress(ParentId = parentB.Id, PreferenceRank = 2, Name = 'Two B', UsageType = '1'), + new ContactPointAddress(ParentId = parentC.Id, PreferenceRank = 2, Name = 'Two C', UsageType = '1'), + new ContactPointAddress(ParentId = parentA.Id, PreferenceRank = 3, Name = 'Three A', UsageType = '2'), + new ContactPointAddress(ParentId = parentB.Id, PreferenceRank = 3, Name = 'Three B', UsageType = '2'), + new ContactPointAddress(ParentId = parentC.Id, PreferenceRank = 3, Name = 'Three C', UsageType = '2') }; insert cpas; @@ -1684,6 +1693,24 @@ private class RollupFullRecalcTests { LookupFieldOnLookupObject__c = 'Id', RollupFieldOnLookupObject__c = 'AnnualRevenue', RollupOperation__c = 'SUM' + ), + new Rollup__mdt( + CalcItem__c = 'ContactPointAddress', + RollupFieldOnCalcItem__c = 'PreferenceRank', + LookupFieldOnCalcItem__c = 'ParentId', + LookupObject__c = 'Account', + LookupFieldOnLookupObject__c = 'Id', + RollupFieldOnLookupObject__c = 'NumberOfEmployees', + RollupOperation__c = 'FIRST' + ), + new Rollup__mdt( + CalcItem__c = 'ContactPointAddress', + RollupFieldOnCalcItem__c = 'UsageType', + LookupFieldOnCalcItem__c = 'ParentId', + LookupObject__c = 'Account', + LookupFieldOnLookupObject__c = 'Id', + RollupFieldOnLookupObject__c = 'Description', + RollupOperation__c = 'MOST' ) }, ContactPointAddress.SObjectType, @@ -1692,13 +1719,17 @@ private class RollupFullRecalcTests { ); Test.startTest(); - fullRecalc.execute(null, new List{ cpas[0] }); - fullRecalc.execute(null, new List{ cpas[1] }); - fullRecalc.execute(null, new List{ cpas[2] }); + fullRecalc.execute(null, new List{ cpas[0], cpas[1], cpas[2] }); + fullRecalc.execute(null, new List{ cpas[3], cpas[4], cpas[5] }); + fullRecalc.execute(null, new List{ cpas[6], cpas[7], cpas[8] }); Test.stopTest(); - acc = [SELECT AnnualRevenue FROM Account]; - System.assertEquals(6, acc.AnnualRevenue, 'Should have included all calc items without double-counting or incorrectly summing'); + for (Account parent : [SELECT AnnualRevenue, Description, NumberOfEmployees, Name FROM Account]) { + System.assertEquals(6, parent.AnnualRevenue, parent); + // TODO - add functionality for FIRST/LAST/MOST etc rollups + // System.assertEquals(1, parent.NumberOfEmployees, parent); + // System.assertEquals('1', parent.Description, parent); + } System.assertEquals(true, Rollup.CACHED_ROLLUPS.isEmpty(), 'all deferrals should have been run'); } diff --git a/extra-tests/classes/RollupIntegrationTests.cls b/extra-tests/classes/RollupIntegrationTests.cls index 61b41f3b..dc504ea0 100644 --- a/extra-tests/classes/RollupIntegrationTests.cls +++ b/extra-tests/classes/RollupIntegrationTests.cls @@ -1769,6 +1769,37 @@ private class RollupIntegrationTests { System.assert(true, 'Should make it here'); } + @IsTest + static void shouldWorkMultipleOrInWhereClause() { + Account acc = [SELECT Id, Name FROM Account]; + + insert new List{ + new Opportunity(StageName = 'b', CloseDate = System.today(), AccountId = acc.Id, Name = 'a', Amount = 2), + new Opportunity(StageName = 'a', CloseDate = System.today(), AccountId = acc.Id, Name = 'b', Amount = 2), + new Opportunity(StageName = 'c', CloseDate = System.today().addDays(-2), AccountId = acc.Id, Name = 'c', Amount = 2), + new Opportunity(StageName = 'd', CloseDate = System.today().addDays(2), AccountId = acc.Id, Name = 'd', Amount = 2) + }; + + Test.startTest(); + Rollup__mdt mdt = new Rollup__mdt( + CalcItem__c = 'Opportunity', + LookupObject__c = 'Account', + RollupFieldOnCalcItem__c = 'StageName', + LookupFieldOnCalcItem__c = 'AccountId', + LookupFieldOnLookupObject__c = 'Id', + RollupFieldOnLookupObject__c = 'SicDesc', + RollupOperation__c = 'LAST', + CalcItemWhereClause__c = '(StageName IN (\'a\', \'b\') AND CloseDate = TODAY) OR (StageName = \'d\' AND CloseDate = TODAY)' + ); + Rollup.performFullRecalculation(mdt); + Test.stopTest(); + + acc = [SELECT SicDesc FROM Account]; + System.assertEquals('b', acc.SicDesc); + // Validate that job ran as queueable - that the where clause was formatted correctly, in other words + System.assertEquals('Completed', [SELECT Status FROM AsyncApexJob WHERE JobType = 'Queueable' LIMIT 1]?.Status); + } + /** Schedulable tests */ @IsTest static void shouldThrowExceptionForBadQuery() { diff --git a/extra-tests/classes/RollupParentResetProcessorTests.cls b/extra-tests/classes/RollupParentResetProcessorTests.cls index ff92740e..d6874936 100644 --- a/extra-tests/classes/RollupParentResetProcessorTests.cls +++ b/extra-tests/classes/RollupParentResetProcessorTests.cls @@ -1,5 +1,10 @@ @IsTest private class RollupParentResetProcessorTests { + @TestSetup + static void setup() { + upsert new RollupSettings__c(IsEnabled__c = true); + } + @IsTest static void shouldNotFailWhenRollupFieldNotFilterable() { RollupParentResetProcessor processor = new RollupParentResetProcessor( @@ -117,8 +122,18 @@ private class RollupParentResetProcessorTests { insert new List{ new Account(Name = 'Account With Null'), new Contact(LastName = 'Contact With Null') }; RollupParentResetProcessor processor = new RollupParentResetProcessor( new List{ - new Rollup__mdt(RollupFieldOnLookupObject__c = 'AnnualRevenue', LookupObject__c = 'Account', FullRecalculationDefaultNumberValue__c = 0), - new Rollup__mdt(RollupFieldOnLookupObject__c = 'AccountNumber', LookupObject__c = 'Account', FullRecalculationDefaultStringValue__c = 'a') + new Rollup__mdt( + RollupFieldOnLookupObject__c = 'AnnualRevenue', + LookupObject__c = 'Account', + LookupFieldOnLookupObject__c = 'Id', + FullRecalculationDefaultNumberValue__c = 0 + ), + new Rollup__mdt( + RollupFieldOnLookupObject__c = 'AccountNumber', + LookupObject__c = 'Account', + LookupFieldOnLookupObject__c = 'Id', + FullRecalculationDefaultStringValue__c = 'a' + ) }, Account.SObjectType, 'SELECT Id\nFROM Account WHERE Id != null', @@ -135,7 +150,6 @@ private class RollupParentResetProcessorTests { @IsTest static void skipsTransformWhenDisabled() { - upsert new RollupSettings__c(IsEnabled__c = true); Rollup.defaultControl = Rollup.getDefaultControl(); Rollup.defaultControl.ShouldSkipResettingParentFields__c = true; Rollup.defaultControl.ShouldRunAs__c = RollupMetaPicklists.ShouldRunAs.Synchronous; @@ -154,4 +168,41 @@ private class RollupParentResetProcessorTests { System.assertEquals(0, [SELECT COUNT() FROM AsyncApexJob WHERE ApexClass.Name = :RollupParentResetProcessor.class.getName()]); } + + @IsTest + static void shouldNotResetWhenParentAlreadyMatchesDuringFullRecalc() { + Decimal originalValue = 3; + Account acc = new Account(AnnualRevenue = originalValue, Name = 'Should Not Reset'); + insert acc; + + Opportunity opp = new Opportunity(AccountId = acc.Id, Amount = 100, Name = 'Will Not Match', CloseDate = System.today().addDays(2), StageName = 'A'); + insert new List{ + new Opportunity(AccountId = acc.Id, Amount = originalValue - 1, Name = 'First Match', CloseDate = System.today().addDays(-2), StageName = 'A'), + new Opportunity(AccountId = acc.Id, Amount = originalValue - 2, Name = 'Second Match', CloseDate = System.today().addDays(-2), StageName = 'A'), + opp + }; + + Test.startTest(); + Rollup.performBulkFullRecalc( + new List{ + new Rollup__mdt( + RollupFieldOnCalcItem__c = 'Amount', + LookupObject__c = 'Account', + LookupFieldOnCalcItem__c = 'AccountId', + LookupFieldOnLookupObject__c = 'Id', + RollupFieldOnLookupObject__c = 'AnnualRevenue', + RollupOperation__c = 'SUM', + CalcItem__c = 'Opportunity', + IsFullRecordSet__c = true, + CalcItemWhereClause__c = 'CloseDate <= TODAY' + ) + }, + Rollup.InvocationPoint.FROM_FULL_RECALC_LWC.name() + ); + + Test.stopTest(); + + Account updatedAcc = [SELECT AnnualRevenue FROM Account WHERE Id = :acc.Id]; + System.assertEquals(originalValue, updatedAcc.AnnualRevenue); + } } diff --git a/extra-tests/classes/RollupQueryBuilderTests.cls b/extra-tests/classes/RollupQueryBuilderTests.cls index 4541f84e..ea5ca498 100644 --- a/extra-tests/classes/RollupQueryBuilderTests.cls +++ b/extra-tests/classes/RollupQueryBuilderTests.cls @@ -160,4 +160,17 @@ private class RollupQueryBuilderTests { System.assertEquals(true, tasks.isEmpty(), 'Should have made it here and query should have been run'); } + + @IsTest + static void properlyFormatsOrWithAndClause() { + String whereClause = '(StageName = \'one\' AND CloseDate = TODAY) OR (StageName = \'four\' AND CloseDate = TODAY)'; + + String actualQuery = RollupQueryBuilder.Current.getQuery(Opportunity.SObjectType, new List{ 'COUNT()' }, 'Id', '!=', whereClause); + + System.assertEquals(0, Database.countQuery(actualQuery), 'Validation that query is syntactically valid'); + System.assertEquals( + 'SELECT count()\nFROM Opportunity\nWHERE Id != :objIds\nAND ((StageName = \'one\' AND CloseDate = TODAY) OR (StageName = \'four\' AND CloseDate = TODAY))', + actualQuery + ); + } } diff --git a/extra-tests/classes/RollupSObjectUpdaterTests.cls b/extra-tests/classes/RollupSObjectUpdaterTests.cls index 28ffdd33..59726de6 100644 --- a/extra-tests/classes/RollupSObjectUpdaterTests.cls +++ b/extra-tests/classes/RollupSObjectUpdaterTests.cls @@ -97,7 +97,6 @@ public class RollupSObjectUpdaterTests { System.assertEquals(String.valueOf(blobValue), opp.Description); } - @SuppressWarnings('PMD.UnusedLocalVariable') @IsTest static void onlyUpdatesRecordsWithRollupChanges() { Account acc = new Account(Name = 'Should Not Be Updated'); @@ -105,10 +104,7 @@ public class RollupSObjectUpdaterTests { acc = [SELECT Id, LastModifiedDate FROM Account]; waitSeconds(1); - List fieldNames = new List{ 'Id' }; - - Id accId = acc.Id; - new RollupSObjectUpdater().doUpdate(Database.query('SELECT ' + String.join(fieldNames, ',') + ' FROM Account WHERE Id = :accId')); + new RollupSObjectUpdater().doUpdate([SELECT Id FROM Account WHERE Id = :acc.Id]); Account updatedAccount = [SELECT Id, LastModifiedDate FROM Account]; System.assertEquals(acc, updatedAccount, 'Last modified date should not have updated if only Id was passed'); diff --git a/package.json b/package.json index e4d6ea5e..51ea0e4d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "apex-rollup", - "version": "1.5.85", + "version": "1.5.86", "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 index 31d0de71..979341c8 100644 --- a/rollup-namespaced/README.md +++ b/rollup-namespaced/README.md @@ -18,12 +18,12 @@ For more info, see the base `README`. ## Deployment & Setup - + Deploy to Salesforce - + Deploy to Salesforce Sandbox diff --git a/rollup-namespaced/sfdx-project.json b/rollup-namespaced/sfdx-project.json index 5c9e064a..f3dc2175 100644 --- a/rollup-namespaced/sfdx-project.json +++ b/rollup-namespaced/sfdx-project.json @@ -4,8 +4,8 @@ "default": true, "package": "apex-rollup-namespaced", "path": "rollup-namespaced/source/rollup", - "versionName": "COUNT-based rollup bugfixes", - "versionNumber": "1.0.50.0", + "versionName": "Fixes where clause parsing for nested OR statements", + "versionNumber": "1.0.51.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": { @@ -30,6 +30,7 @@ "apex-rollup-namespaced@1.0.47-0": "04t6g000008C6lcAAC", "apex-rollup-namespaced@1.0.48-0": "04t6g000008C6rMAAS", "apex-rollup-namespaced@1.0.49-0": "04t6g000008C6stAAC", - "apex-rollup-namespaced@1.0.50-0": "04t6g000008C6zbAAC" + "apex-rollup-namespaced@1.0.50-0": "04t6g000008C6zbAAC", + "apex-rollup-namespaced@1.0.51-0": "04t6g000008C71cAAC" } } \ No newline at end of file diff --git a/rollup/core/classes/RollupAsyncProcessor.cls b/rollup/core/classes/RollupAsyncProcessor.cls index 87f8bc3b..1de037bc 100644 --- a/rollup/core/classes/RollupAsyncProcessor.cls +++ b/rollup/core/classes/RollupAsyncProcessor.cls @@ -41,16 +41,6 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme REPARENTED } - private final Set previouslyResetParents { - get { - if (previouslyResetParents == null) { - previouslyResetParents = new Set(); - } - return previouslyResetParents; - } - set; - } - private Map childToUnexpectedFullRecalc { get { if (childToUnexpectedFullRecalc == null) { @@ -81,6 +71,16 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme set; } + protected final Set previouslyResetParents { + get { + if (previouslyResetParents == null) { + previouslyResetParents = new Set(); + } + return previouslyResetParents; + } + set; + } + protected final Set uniqueParentFields { get { if (uniqueParentFields == null) { @@ -153,7 +153,7 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme this.stackDepth = innerRollup.stackDepth; this.calcItemReplacer = innerRollup.calcItemReplacer; this.lookupItems = innerRollup.lookupItems; - this.previouslyResetParents.addAll(innerRollup.getPreviouslyResetParents()); + this.previouslyResetParents.addAll(innerRollup.previouslyResetParents); } // Inner rollup constructor - a rollup solely concerned with calculations @@ -267,11 +267,6 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme this.logger.save(); } - public virtual void storeParentResetField(RollupAsyncProcessor processor, SObject parent) { - this.addToPreviouslyResetParents(processor, parent); - this.fullRecalcProcessor?.storeParentResetField(processor, parent); - } - 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 syncRollups if we're already async @@ -520,7 +515,14 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme } List recordsToReset = new List(); for (SObject lookupItem : localLookupItems) { - if (this.parentRollupFieldHasBeenReset(rollup, lookupItem) == false) { + if ( + this.parentRollupFieldHasBeenReset( + rollup.lookupFieldOnLookupObject.getDescribe().getName(), + lookupItem, + '' + rollup.lookupObj, + '' + rollup.opFieldOnLookupObject + ) == false + ) { recordsToReset.add(lookupItem); } } @@ -611,10 +613,12 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme if (shouldLimitCalcItemQuery && remainingQueryRowsLeft < outstandingItemCount) { rollup.wasConvertedToFullRecalculation = true; rollup.metadata.RollupOperation__c = getBaseOperationName(rollup.metadata.RollupOperation__c); + // TODO - instead of calling storeParentResetField here, do something else to prevent the parent reset processor from running + // based off of wasConvertedToFullRecalculation? for (String lookupKey : recordIds) { SObject parentResetRecord = rollup.lookupObj.newSObject(); parentResetRecord.put(rollup.lookupFieldOnLookupObject, lookupKey); - this.storeParentResetField(rollup, parentResetRecord); + this.storeParentResetField('' + rollup.lookupFieldOnLookupObject, parentResetRecord); } if (this.childToUnexpectedFullRecalc.containsKey(rollup.calcItemType)) { this.childToUnexpectedFullRecalc.get(rollup.calcItemType).addMetadata(rollup.metadata); @@ -694,6 +698,16 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme updatedLookupRecords.putAll(this.getUpdatedLookupItemsByRollup(roll, calcItemsByLookupField, localLookupItems)); } + for (SObject lookupRecord : updatedLookupRecords.values()) { + for (RollupAsyncProcessor roll : rollups) { + if (lookupRecord.getSObjectType() == roll.lookupObj) { + // TODO use Id + rollup field? + this.previouslyResetParents.add(String.valueOf(lookupRecord.get(roll.lookupFieldOnLookupObject))); + this.fullRecalcProcessor?.storeParentResetField('' + roll.lookupFieldOnLookupObject, lookupRecord); + } + } + } + if (this.isTimingOut) { this.getDML().forceSyncUpdate(); } @@ -735,49 +749,30 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme } } - protected Boolean parentRollupFieldHasBeenReset(Rollup__mdt meta, SObject parent) { - return this.hasParentRollupFieldBeenReset(meta.RollupFieldOnLookupObject__c, parent); + protected virtual void storeParentResetField(String lookupField, SObject parent) { + String resetKey = String.valueOf(parent.get(lookupField)); + if (resetKey != null) { + this.previouslyResetParents.add(resetKey); + } } - protected Boolean parentRollupFieldHasBeenReset(RollupAsyncProcessor processor, SObject parent) { - return this.hasParentRollupFieldBeenReset(String.valueOf(processor.opFieldOnLookupObject), parent) || - this.uniqueParentFields.contains(this.getUniqueParentKey(processor.metadata)); + protected Boolean parentRollupFieldHasBeenReset(String parentKeyField, SObject parent, String parentTypeName, String parentRollupField) { + return this.previouslyResetParents.contains(String.valueOf(parent.get(parentKeyField))) || + this.uniqueParentFields.contains('' + parentTypeName + parentRollupField); } protected void storeUniqueParentFields(Rollup__mdt meta) { if (meta != null) { - String uniqueParentKey = this.getUniqueParentKey(meta); + String uniqueParentKey = meta.LookupObject__c + meta.RollupFieldOnLookupObject__c; this.uniqueParentFields.add(uniqueParentKey); this.fullRecalcProcessor?.uniqueParentFields.add(uniqueParentKey); } } - protected virtual Set getPreviouslyResetParents() { - return this.previouslyResetParents; - } - - protected void addToPreviouslyResetParents(RollupAsyncProcessor processor, SObject parent) { - this.getPreviouslyResetParents().add(this.getParentResetKey(String.valueOf(processor.opFieldOnLookupObject), parent)); - } - protected virtual Boolean getCanRollupWithoutCustomSetting() { return this.metadata?.ShouldRunWithoutCustomSettingEnabled__c == true; } - private String getUniqueParentKey(Rollup__mdt meta) { - return meta.RollupOperation__c + meta.LookupObject__c + meta.RollupFieldOnLookupObject__c; - } - - private void clearPreviouslyResetParents(RollupAsyncProcessor processor, SObject parent) { - String key = this.getParentResetKey(String.valueOf(processor.opFieldOnLookupObject), parent); - this.getPreviouslyResetParents().remove(key); - } - - private Boolean hasParentRollupFieldBeenReset(String rollupFieldName, SObject parent) { - String resetKey = this.getParentResetKey(rollupFieldName, parent); - return this.getPreviouslyResetParents().contains(resetKey); - } - protected Boolean getCanEnqueue() { return Limits.getLimitQueueableJobs() > Limits.getQueueableJobs(); } @@ -787,10 +782,6 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme this.deferredRollups.add(processor); } - private String getParentResetKey(String fieldKey, SObject parent) { - return fieldKey + parent.Id; - } - private Boolean isValidAdditionalCalcItemRetrieval(RollupAsyncProcessor roll) { if (roll.fullRecalcProcessor != null) { return false; @@ -923,6 +914,12 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme private List getLookupItems(Map calcItemsByLookupField, Map updatedLookupRecords, RollupAsyncProcessor roll) { List localLookupItems = new List(); Set lookupItemKeys = new Set(calcItemsByLookupField.keySet()); + for (String resetKey : this.previouslyResetParents) { + if (lookupItemKeys.contains(resetKey) == false) { + this.previouslyResetParents.remove(resetKey); + this.fullRecalcProcessor?.previouslyResetParents.remove(resetKey); + } + } for (String lookupId : calcItemsByLookupField.keySet()) { if (updatedLookupRecords.containsKey(lookupId)) { lookupItemKeys.remove(lookupId); @@ -1222,7 +1219,6 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme Map> oldLookupItems = new Map>(); Map unprocessedCalcItems = new Map(); RollupCalculator calc; - String priorKey; for (Integer index = lookupItems.size() - 1; index >= 0; index--) { SObject lookupRecord = lookupItems[index]; if (lookupRecord.getSObjectType() != roll.lookupObj) { @@ -1246,18 +1242,13 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme // has already been setup to adjust the lookup item's rollup field if (localCalcItems.isEmpty() == false || (localCalcItems.isEmpty() && oldLookupItems.isEmpty())) { Object priorVal = lookupRecord.get(roll.opFieldOnLookupObject); - Object priorValToUse = roll.isFullRecalc && this.parentRollupFieldHasBeenReset(roll, lookupRecord) == false ? null : priorVal; + Object priorValToUse = roll.isFullRecalc && + this.parentRollupFieldHasBeenReset('' + roll.lookupFieldOnLookupObject, lookupRecord, '' + roll.lookupObj, '' + roll.opFieldOnLookupObject) == false + ? null + : priorVal; calc = this.getCalculator(calc, roll, localCalcItems, lookupRecord, priorValToUse, key, roll.lookupFieldOnCalcItem); this.conditionallyPerformUpdate(priorVal, calc, lookupRecord, roll, recordsToUpdate, ParentUpdateType.LOOKUP, localCalcItems); } - if (priorKey != null && key != priorKey) { - // once we've fully processed a parent (in separate batches) - // it's safe to clear the stateful map - this.clearPreviouslyResetParents(roll, lookupRecord); - priorKey = key; - } else if (priorKey == null) { - priorKey = key; - } } } @@ -1409,12 +1400,11 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme if (this.rollupControl.ShouldSkipResettingParentFields__c == true && ((localCalcItems.isEmpty() == false && newVal == null) || localCalcItems.isEmpty())) { newVal = priorVal; } + String key = (String) lookupRecord.get(roll.lookupFieldOnLookupObject); if (priorVal != newVal) { - String key = (String) lookupRecord.get(roll.lookupFieldOnLookupObject); this.getDML().updateField(roll.opFieldOnLookupObject, lookupRecord, newVal); - recordsToUpdate.put(key, lookupRecord); } - this.storeParentResetField(roll, lookupRecord); + recordsToUpdate.put(key, lookupRecord); this.logger.log(logKey + ' record after rolling up:', lookupRecord, LoggingLevel.DEBUG); } diff --git a/rollup/core/classes/RollupDateLiteral.cls b/rollup/core/classes/RollupDateLiteral.cls index de056284..d5c02bc7 100644 --- a/rollup/core/classes/RollupDateLiteral.cls +++ b/rollup/core/classes/RollupDateLiteral.cls @@ -997,8 +997,7 @@ public without sharing abstract class RollupDateLiteral { } /** - * Week 1 is the first week that has at least 4 days in the current year - yikes! - * There can also be a week 53. The sadness. + * The first week is from January 1 through January 7. */ private class WeekInYearFunctionLiteral extends FunctionLiteral { protected override Integer getComparisonNumber(Datetime val) { diff --git a/rollup/core/classes/RollupFullBatchRecalculator.cls b/rollup/core/classes/RollupFullBatchRecalculator.cls index 15a950f8..697f53c1 100644 --- a/rollup/core/classes/RollupFullBatchRecalculator.cls +++ b/rollup/core/classes/RollupFullBatchRecalculator.cls @@ -1,6 +1,4 @@ public without sharing virtual class RollupFullBatchRecalculator extends RollupFullRecalcProcessor implements Database.Stateful, Database.RaisesPlatformEvents { - protected final Set statefulPreviouslyResetParents = new Set(); - public RollupFullBatchRecalculator( String queryString, InvocationPoint invokePoint, @@ -16,12 +14,6 @@ public without sharing virtual class RollupFullBatchRecalculator extends RollupF this.process(this.getDelegatedFullRecalcRollups(this.calcItems)); } - protected override Map customizeToStringEntries(Map props) { - super.customizeToStringEntries(props); - this.addToMap(props, 'Stateful Previously Reset Parents', this.statefulPreviouslyResetParents); - return props; - } - protected override Boolean isBatch() { return true; } @@ -40,8 +32,4 @@ public without sharing virtual class RollupFullBatchRecalculator extends RollupF protected virtual override String startAsyncWork() { return this.startBatchProcessor(); } - - protected override Set getPreviouslyResetParents() { - return this.statefulPreviouslyResetParents; - } } diff --git a/rollup/core/classes/RollupFullRecalcProcessor.cls b/rollup/core/classes/RollupFullRecalcProcessor.cls index d8520a18..86d6183c 100644 --- a/rollup/core/classes/RollupFullRecalcProcessor.cls +++ b/rollup/core/classes/RollupFullRecalcProcessor.cls @@ -89,8 +89,8 @@ global abstract without sharing class RollupFullRecalcProcessor extends RollupAs } } - public override void storeParentResetField(RollupAsyncProcessor processor, SObject parent) { - this.addToPreviouslyResetParents(processor, parent); + public override void storeParentResetField(String lookupField, SObject parent) { + this.previouslyResetParents.add(String.valueOf(parent.get(lookupField))); this.postProcessor?.recordIds.add(parent.Id); } @@ -142,7 +142,6 @@ global abstract without sharing class RollupFullRecalcProcessor extends RollupAs this.addToMap(props, 'Rollup Metadata', this.rollupMetas); this.addToMap(props, 'Query String', this.queryString); this.addToMap(props, 'Caboose Count', this.cabooses.size()); - return props; } diff --git a/rollup/core/classes/RollupLogger.cls b/rollup/core/classes/RollupLogger.cls index 360ceb45..7d8e018b 100644 --- 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.5.85'; + private static final String CURRENT_VERSION_NUMBER = 'v1.5.86'; private static final LoggingLevel FALLBACK_LOGGING_LEVEL = LoggingLevel.DEBUG; private static final RollupPlugin PLUGIN = new RollupPlugin(); diff --git a/rollup/core/classes/RollupParentResetProcessor.cls b/rollup/core/classes/RollupParentResetProcessor.cls index 086e02a8..ef6d8ee6 100644 --- a/rollup/core/classes/RollupParentResetProcessor.cls +++ b/rollup/core/classes/RollupParentResetProcessor.cls @@ -67,7 +67,14 @@ public without sharing class RollupParentResetProcessor extends RollupFullBatchR Map parentFields = parentItems.get(0).getSObjectType().getDescribe().fields.getMap(); for (SObject parentItem : parentItems) { for (Rollup__mdt rollupMeta : this.rollupMetas) { - if (this.parentRollupFieldHasBeenReset(rollupMeta, parentItem) == false && parentFields.containsKey(rollupMeta.RollupFieldOnLookupObject__c)) { + if ( + this.parentRollupFieldHasBeenReset( + rollupMeta.LookupFieldOnLookupObject__c, + parentItem, + rollupMeta.LookupObject__c, + rollupMeta.RollupFieldOnLookupObject__c + ) == false && parentFields.containsKey(rollupMeta.RollupFieldOnLookupObject__c) + ) { Object resetVal = getDefaultValue(rollupMeta); parentItem.put(rollupMeta.RollupFieldOnLookupObject__c, resetVal); } diff --git a/rollup/core/classes/RollupQueryBuilder.cls b/rollup/core/classes/RollupQueryBuilder.cls index a8f9014c..61942af9 100644 --- a/rollup/core/classes/RollupQueryBuilder.cls +++ b/rollup/core/classes/RollupQueryBuilder.cls @@ -165,7 +165,9 @@ public without sharing class RollupQueryBuilder { whereClause = whereClause.replace('()', ''); whereClause = RollupEvaluator.getWhereEval(whereClause, sObjectType).getSafeQuery(); - if (whereClause.containsIgnoreCase(' or ') && whereClause.containsIgnoreCase(' and ') == false) { + if ( + whereClause.containsIgnoreCase(' or ') && (whereClause.containsIgnoreCase(' and ') == false || whereClause.startsWith('(') && whereClause.endsWith(')')) + ) { whereClause = '(' + whereClause + ')'; } } diff --git a/rollup/core/classes/RollupSObjectUpdater.cls b/rollup/core/classes/RollupSObjectUpdater.cls index 7377c829..15bcfac3 100644 --- a/rollup/core/classes/RollupSObjectUpdater.cls +++ b/rollup/core/classes/RollupSObjectUpdater.cls @@ -55,6 +55,7 @@ global without sharing virtual class RollupSObjectUpdater { this.performAsyncUpdate(recordsToUpdate); return; } + this.winnowRecords(recordsToUpdate); this.splitUpdates(recordsToUpdate); // typically I wouldn't advocate for the use of a guard clause here since an empty list // getting updated is a no-op, but the addition of the logging item is annoying ... @@ -66,7 +67,6 @@ global without sharing virtual class RollupSObjectUpdater { if (this.rollupControl.ShouldDuplicateRulesBeIgnored__c != false) { dmlOptions.DuplicateRuleHeader.AllowSave = true; } - this.winnowRecords(recordsToUpdate); this.updateRecords(recordsToUpdate, dmlOptions); this.dispatch(recordsToUpdate); } diff --git a/sfdx-project.json b/sfdx-project.json index 5dc4098b..6ddb0c7d 100644 --- a/sfdx-project.json +++ b/sfdx-project.json @@ -4,8 +4,8 @@ "default": true, "package": "apex-rollup", "path": "rollup", - "versionName": "COUNT-based rollup bugfixes", - "versionNumber": "1.5.85.0", + "versionName": "Fixes where clause parsing for nested OR statements", + "versionNumber": "1.5.86.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": { @@ -103,6 +103,7 @@ "apex-rollup@1.5.82-0": "04t6g000008C6lXAAS", "apex-rollup@1.5.83-0": "04t6g000008C6rHAAS", "apex-rollup@1.5.84-0": "04t6g000008C6soAAC", - "apex-rollup@1.5.85-0": "04t6g000008C6zWAAS" + "apex-rollup@1.5.85-0": "04t6g000008C6zWAAS", + "apex-rollup@1.5.86-0": "04t6g000008C71XAAS" } } \ No newline at end of file