From 1d522b45bc53bfe0b72684caeaae7a5f42c88782 Mon Sep 17 00:00:00 2001 From: James Simone Date: Fri, 3 May 2024 14:03:51 -0400 Subject: [PATCH] Tidies up #587 --- .../classes/RollupCalcItemSorterTests.cls | 105 +++++------ extra-tests/classes/RollupCalculatorTests.cls | 22 ++- rollup/core/classes/RollupCalcItemSorter.cls | 8 +- rollup/core/classes/RollupCalculator.cls | 178 +++++++++--------- sfdx-project.json | 4 +- 5 files changed, 152 insertions(+), 165 deletions(-) diff --git a/extra-tests/classes/RollupCalcItemSorterTests.cls b/extra-tests/classes/RollupCalcItemSorterTests.cls index 1e41766f..44037d26 100644 --- a/extra-tests/classes/RollupCalcItemSorterTests.cls +++ b/extra-tests/classes/RollupCalcItemSorterTests.cls @@ -12,18 +12,18 @@ private class RollupCalcItemSorterTests { Date severalDaysAgo = System.today().addDays(-2); Opportunity expectedFirstItem = new Opportunity(Amount = null, CloseDate = severalDaysAgo); Opportunity expectedSecondItem = new Opportunity(Amount = 1, CloseDate = severalDaysAgo); - List oppsToSort = new List{ - new RollupCalculator.WinnowResult(new Opportunity(Amount = 1, CloseDate = System.today())), + List oppsToSort = new List{ + new Opportunity(Amount = 1, CloseDate = System.today()), // this record should essentially be thrown out of sorting since it "loses" on the first ordering, // which is on Amount - new RollupCalculator.WinnowResult(new Opportunity(Amount = 3, CloseDate = severalDaysAgo.addDays(-1))), - new RollupCalculator.WinnowResult(expectedSecondItem), - new RollupCalculator.WinnowResult(expectedFirstItem) + new Opportunity(Amount = 3, CloseDate = severalDaysAgo.addDays(-1)), + expectedSecondItem, + expectedFirstItem }; oppsToSort.sort(sorter); - System.assertEquals(expectedFirstItem, oppsToSort[0].item); - System.assertEquals(expectedSecondItem, oppsToSort[1].item); + System.assertEquals(expectedFirstItem, oppsToSort[0]); + System.assertEquals(expectedSecondItem, oppsToSort[1]); } @IsTest @@ -41,18 +41,13 @@ private class RollupCalcItemSorterTests { Opportunity expectedThirdItem = new Opportunity(Amount = 2, CloseDate = today, Name = 'a'); Opportunity expectedSecondItem = new Opportunity(Amount = 1, CloseDate = today, Name = 'c'); Opportunity expectedFourthItem = new Opportunity(Amount = 2, CloseDate = today.addDays(1), Name = 'a'); - List oppsToSort = new List{ - new RollupCalculator.WinnowResult(expectedSecondItem), - new RollupCalculator.WinnowResult(expectedFourthItem), - new RollupCalculator.WinnowResult(expectedThirdItem), - new RollupCalculator.WinnowResult(expectedFirstItem) - }; + List oppsToSort = new List{ expectedSecondItem, expectedFourthItem, expectedThirdItem, expectedFirstItem }; oppsToSort.sort(sorter); - System.assertEquals(expectedFirstItem, oppsToSort[0].item); - System.assertEquals(expectedSecondItem, oppsToSort[1].item); - System.assertEquals(expectedThirdItem, oppsToSort[2].item); - System.assertEquals(expectedFourthItem, oppsToSort[3].item); + System.assertEquals(expectedFirstItem, oppsToSort[0]); + System.assertEquals(expectedSecondItem, oppsToSort[1]); + System.assertEquals(expectedThirdItem, oppsToSort[2]); + System.assertEquals(expectedFourthItem, oppsToSort[3]); } @IsTest @@ -67,15 +62,15 @@ private class RollupCalcItemSorterTests { Date severalDaysAgo = System.today().addDays(-2); Opportunity expectedFirstItem = new Opportunity(Amount = 1, CloseDate = System.today()); Opportunity expectedSecondItem = new Opportunity(Amount = 3, CloseDate = severalDaysAgo.addDays(-1)); - List oppsToSort = new List{ - new RollupCalculator.WinnowResult(new Opportunity(Amount = 3, CloseDate = severalDaysAgo.addDays(-1))), - new RollupCalculator.WinnowResult(expectedSecondItem), - new RollupCalculator.WinnowResult(expectedFirstItem) + List oppsToSort = new List{ + new Opportunity(Amount = 3, CloseDate = severalDaysAgo.addDays(-1)), + expectedSecondItem, + expectedFirstItem }; oppsToSort.sort(sorter); - System.assertEquals(expectedFirstItem, oppsToSort[0].item); - System.assertEquals(expectedSecondItem, oppsToSort[1].item); + System.assertEquals(expectedFirstItem, oppsToSort[0]); + System.assertEquals(expectedSecondItem, oppsToSort[1]); } @IsTest @@ -93,16 +88,12 @@ private class RollupCalcItemSorterTests { Opportunity expectedSecondItem = new Opportunity(Amount = 5, CloseDate = today); Opportunity expectedThirdItem = new Opportunity(Amount = 1, CloseDate = today); - List oppsToSort = new List{ - new RollupCalculator.WinnowResult(expectedThirdItem), - new RollupCalculator.WinnowResult(expectedFirstItem), - new RollupCalculator.WinnowResult(expectedSecondItem) - }; + List oppsToSort = new List{ expectedThirdItem, expectedFirstItem, expectedSecondItem }; oppsToSort.sort(sorter); - System.assertEquals(expectedFirstItem, oppsToSort[0].item); - System.assertEquals(expectedSecondItem, oppsToSort[1].item); - System.assertEquals(expectedThirdItem, oppsToSort[2].item); + System.assertEquals(expectedFirstItem, oppsToSort[0]); + System.assertEquals(expectedSecondItem, oppsToSort[1]); + System.assertEquals(expectedThirdItem, oppsToSort[2]); } @IsTest @@ -110,16 +101,16 @@ private class RollupCalcItemSorterTests { RollupCalcItemSorter sorter = new RollupCalcItemSorter(new List{ new RollupOrderBy__mdt(Ranking__c = 0, FieldName__c = 'Industry') }); List picklistEntries = Account.Industry.getDescribe().getPicklistValues(); - List accs = new List(); + List accs = new List(); for (Integer reverseIndex = picklistEntries.size() - 1; reverseIndex >= 0; reverseIndex--) { Schema.PicklistEntry entry = picklistEntries[reverseIndex]; - accs.add(new RollupCalculator.WinnowResult(new Account(Name = entry.getValue(), Industry = entry.getValue()))); + accs.add(new Account(Name = entry.getValue(), Industry = entry.getValue())); } accs.sort(sorter); for (Integer index = 0; index < accs.size(); index++) { - System.assertEquals(picklistEntries[index].getValue(), accs[index].item.get(Account.Industry), 'Account at index: ' + index + ' should have matched'); + System.assertEquals(picklistEntries[index].getValue(), accs[index].Industry, 'Account at index: ' + index + ' should have matched'); } } @@ -138,14 +129,11 @@ private class RollupCalcItemSorterTests { } ); - List opps = new List{ - new RollupCalculator.WinnowResult(second), - new RollupCalculator.WinnowResult(opp) - }; + List opps = new List{ second, opp }; opps.sort(sorter); - System.assertEquals(opp, opps[0].item); - System.assertEquals(second, opps[1].item); + System.assertEquals(opp, opps[0]); + System.assertEquals(second, opps[1]); } @IsTest @@ -156,12 +144,7 @@ private class RollupCalcItemSorterTests { Opportunity expectedFirstItem = new Opportunity(Amount = null); Opportunity expectedSecondItem = new Opportunity(Amount = 1); - List oppsToSort = new List{ - new RollupCalculator.WinnowResult(new Opportunity(Amount = 1)), - new RollupCalculator.WinnowResult(new Opportunity(Amount = 3)), - new RollupCalculator.WinnowResult(expectedSecondItem), - new RollupCalculator.WinnowResult(expectedFirstItem) - }; + List oppsToSort = new List{ new Opportunity(Amount = 1), new Opportunity(Amount = 3), expectedSecondItem, expectedFirstItem }; oppsToSort.sort(sorter); System.assert(true, 'Should make it here'); @@ -169,24 +152,24 @@ private class RollupCalcItemSorterTests { @IsTest static void sortsFieldNames() { - List itemsToSort = new List{ - new RollupCalculator.WinnowResult(new Opportunity(StageName = 'Two')), - new RollupCalculator.WinnowResult(new Opportunity(StageName = 'Uno Reverse Card')), - new RollupCalculator.WinnowResult(new Opportunity(StageName = 'Two')), - new RollupCalculator.WinnowResult(new Opportunity()), - new RollupCalculator.WinnowResult(new Opportunity(StageName = 'Z')), - new RollupCalculator.WinnowResult(new Opportunity(StageName = 'One')), - new RollupCalculator.WinnowResult(new Opportunity(StageName = 'One')) + List itemsToSort = new List{ + new Opportunity(StageName = 'Two'), + new Opportunity(StageName = 'Uno Reverse Card'), + new Opportunity(StageName = 'Two'), + new Opportunity(), + new Opportunity(StageName = 'Z'), + new Opportunity(StageName = 'One'), + new Opportunity(StageName = 'One') }; itemsToSort.sort(new RollupCalcItemSorter(new List{ Opportunity.Name.getDescribe().getName(), Opportunity.StageName.getDescribe().getName() })); - System.assertEquals(null, itemsToSort.get(0).item.get(Opportunity.StageName)); - System.assertEquals('One', itemsToSort.get(1).item.get(Opportunity.StageName)); - System.assertEquals('One', itemsToSort.get(2).item.get(Opportunity.StageName)); - System.assertEquals('Two', itemsToSort.get(3).item.get(Opportunity.StageName)); - System.assertEquals('Two', itemsToSort.get(4).item.get(Opportunity.StageName)); - System.assertEquals('Uno Reverse Card', itemsToSort.get(5).item.get(Opportunity.StageName)); - System.assertEquals('Z', itemsToSort.get(6).item.get(Opportunity.StageName)); + System.assertEquals(null, itemsToSort.get(0).StageName); + System.assertEquals('One', itemsToSort.get(1).StageName); + System.assertEquals('One', itemsToSort.get(2).StageName); + System.assertEquals('Two', itemsToSort.get(3).StageName); + System.assertEquals('Two', itemsToSort.get(4).StageName); + System.assertEquals('Uno Reverse Card', itemsToSort.get(5).StageName); + System.assertEquals('Z', itemsToSort.get(6).StageName); } } diff --git a/extra-tests/classes/RollupCalculatorTests.cls b/extra-tests/classes/RollupCalculatorTests.cls index f8098391..bb7dc7a1 100644 --- a/extra-tests/classes/RollupCalculatorTests.cls +++ b/extra-tests/classes/RollupCalculatorTests.cls @@ -1,3 +1,4 @@ +@SuppressWarnings('PMD.NcssTypeCount') @IsTest private class RollupCalculatorTests { // use these tests when DML is not required, or only *light* DML is necessary @@ -793,18 +794,23 @@ private class RollupCalculatorTests { @IsTest static void sumsDistinctWhenFlagged() { + Id stubParentId = RollupTestUtils.createId(Schema.Account.SObjectType); + Object parentStartingValue = 0; RollupCalculator calc = getCalculator( - 0, + parentStartingValue, Rollup.Op.SUM, Opportunity.Amount, Account.AnnualRevenue, new Rollup__mdt(IsDistinct__c = true), - '0011g00003VDGbF002', + stubParentId, Opportunity.AccountId ); calc.performRollup( - new List{ new Opportunity(Id = '0066g00003VDGbF001', Amount = 2), new Opportunity(Id = '0066g00003VDGbF002', Amount = 2) }, + new List{ + new Opportunity(Id = RollupTestUtils.createId(Schema.Opportunity.SObjectType), Amount = 2, AccountId = stubParentId), + new Opportunity(Id = RollupTestUtils.createId(Schema.Opportunity.SObjectType), Amount = 2, AccountId = stubParentId) + }, new Map() ); @@ -927,20 +933,22 @@ private class RollupCalculatorTests { @IsTest static void shouldDistinctByFlagForConcat() { String distinct = 'distinct'; + Object parentStartingValue = ''; + Id stubParentId = RollupTestUtils.createId(Schema.Account.SObjectType); RollupCalculator calc = getCalculator( - '', + parentStartingValue, Rollup.Op.CONCAT, Opportunity.Name, Account.Name, new Rollup__mdt(IsDistinct__c = true), - '0011g00003VDGbF002', + stubParentId, Opportunity.AccountId ); calc.performRollup( new List{ - new Opportunity(Id = '0066g00003VDGbF001', Name = distinct, AccountId = '0016g00003VDGbF001'), - new Opportunity(Id = '0066g00003VDGbF002', Name = distinct, AccountId = '0016g00003VDGbF001') + new Opportunity(Id = RollupTestUtils.createId(Schema.Opportunity.SObjectType), Name = distinct, AccountId = stubParentId), + new Opportunity(Id = RollupTestUtils.createId(Schema.Opportunity.SObjectType), Name = distinct, AccountId = stubParentId) }, new Map() ); diff --git a/rollup/core/classes/RollupCalcItemSorter.cls b/rollup/core/classes/RollupCalcItemSorter.cls index 16ad6a8a..c360eb82 100644 --- a/rollup/core/classes/RollupCalcItemSorter.cls +++ b/rollup/core/classes/RollupCalcItemSorter.cls @@ -1,4 +1,4 @@ -public without sharing class RollupCalcItemSorter implements System.Comparator { +public without sharing class RollupCalcItemSorter implements System.Comparator { private final List orderByOptions; private final Map fieldToPicklistController; @@ -11,10 +11,10 @@ public without sharing class RollupCalcItemSorter implements System.Comparator fieldTokens = objOne.item.getSObjectType().getDescribe().fields.getMap(); + Map fieldTokens = objOne.getSObjectType().getDescribe().fields.getMap(); for (RollupOrderBy__mdt orderByOption : this.orderByOptions) { Schema.DescribeFieldResult fieldDescribe = fieldTokens.get(orderByOption.FieldName__c)?.getDescribe(); RollupFieldInitializer.PicklistController picklistController = this.fieldToPicklistController.get(fieldDescribe); @@ -22,7 +22,7 @@ public without sharing class RollupCalcItemSorter implements System.Comparator distinctValues = new Set(); protected List childrenIds = new List(); + protected Set distinctValues = new Set(); protected Rollup.Op op; protected String lookupKeyQuery; protected String lookupRecordKey; @@ -135,7 +135,6 @@ public without sharing abstract class RollupCalculator { '\'' + (String.isBlank(metadata.CalcItemWhereClause__c) ? '' : ' AND (' + metadata.CalcItemWhereClause__c + ')'); this.childrenIds = new List(); - this.distinctValues = new Set(); } public virtual Object getReturnValue() { @@ -167,12 +166,11 @@ public without sharing abstract class RollupCalculator { public virtual void performRollup(List calcItems, Map oldCalcItems) { // if we're already in a full recalc, we've got all the items we need already this.shouldTriggerFullRecalc = this.isFullRecalc == false; + List results = this.winnowItems(calcItems, oldCalcItems); - List winnowResults = this.winnowItems(calcItems, oldCalcItems); - - for (Integer index = 0; index < winnowResults.size(); index++) { - this.isLastItem = index == winnowResults.size() - 1; - WinnowResult result = winnowResults[index]; + for (Integer index = 0; index < results.size(); index++) { + this.isLastItem = index == results.size() - 1; + WinnowResult result = results[index]; Boolean doesNotMatch = result.matchesCurrent == false; if (doesNotMatch && result.hasOldItem) { doesNotMatch = doesNotMatch && result.matchesOld == false; @@ -184,10 +182,8 @@ public without sharing abstract class RollupCalculator { this.shouldTriggerFullRecalc = doesNotMatch; } - Object currentVal = result.currentValue; - if (this.shouldShortCircuit) { - this.handleShortCircuit(currentVal); + this.handleShortCircuit(result); continue; } else if (this.isCDCUpdate) { // here we don't exclude items because the calc items have already been updated @@ -255,7 +251,7 @@ public without sharing abstract class RollupCalculator { this.returnVal = this.calculateNewAggregateValue(this.op, this.childrenIds); // bail on calling setReturnValue below return; - } else if (this.shouldTriggerFullRecalc == false && winnowResults.isEmpty() && this.isFullRecalc == false) { + } else if (this.shouldTriggerFullRecalc == false && results.isEmpty() && this.isFullRecalc == false) { this.returnVal = this.defaultVal; return; } @@ -285,19 +281,20 @@ public without sharing abstract class RollupCalculator { } public virtual void handleDeleteConcat(WinnowResult result) { } - protected virtual void handleShortCircuit(Object currentVal) { + protected virtual void handleShortCircuit(WinnowResult result) { } protected virtual void setReturnValue() { } - public class WinnowResult { - Boolean hasOldItem = false; - Boolean matchesCurrent = true; - Boolean matchesOld = false; - Object currentValue; - Object priorValue; - Boolean isReparented = false; - public SObject item; + @TestVisible + private class WinnowResult { + private Object currentValue; + private Object priorValue; + private Boolean hasOldItem = false; + private Boolean matchesCurrent = false; + private Boolean matchesOld = true; + private Boolean isReparented = false; + private final SObject item; public WinnowResult(SObject item) { this.item = item; @@ -306,10 +303,17 @@ public without sharing abstract class RollupCalculator { public WinnowResult(SObject item, Schema.SObjectField token) { this(item); this.currentValue = item.get(token); + this.matchesCurrent = true; } } protected List winnowItems(List items, Map oldCalcItems) { + List orderBys = this.metadata?.LimitAmount__c != null && this.metadata.RollupOrderBys__r.isEmpty() + ? new List{ new RollupOrderBy__mdt(FieldName__c = 'Id', Ranking__c = 0) } + : this.metadata.RollupOrderBys__r; + if (orderBys.isEmpty() == false) { + items.sort(new RollupCalcItemSorter(orderBys)); + } List winnowedItems = new List(); this.transformForMultiCurrencyOrgs(items); if (oldCalcItems.isEmpty() == false && this.isMultiCurrencyRollup) { @@ -323,52 +327,44 @@ public without sharing abstract class RollupCalculator { if (item.Id != null) { this.childrenIds.add(item.Id); } - Boolean currentItemMatches = this.eval?.matches(item) != false; - Boolean shouldAddToWinnowedItems = currentItemMatches; - WinnowResult result = new WinnowResult(item); - - SObject potentialOldItem = oldCalcItems.get(item.Id); - if (potentialOldItem != null) { + SObject transformedItem = this.getTransformedCalcItem(item); + WinnowResult result = new WinnowResult(transformedItem); + Boolean shouldAddToResults = this.eval?.matches(transformedItem) != false; + Boolean currentItemMatches = shouldAddToResults; + SObject potentialPriorItem = oldCalcItems.get(transformedItem.Id); + if (potentialPriorItem != null) { result.hasOldItem = true; - result.priorValue = this.getTransformedCalcItem(potentialOldItem).get(this.opFieldOnCalcItem); - result.matchesOld = this.eval?.matches(potentialOldItem) != false; - result.isReparented = this.isReparented(item, potentialOldItem); + result.isReparented = this.isReparented(transformedItem, potentialPriorItem); + result.matchesOld = this.eval?.matches(potentialPriorItem) != false; + result.priorValue = potentialPriorItem.get(this.opFieldOnCalcItem); } - if (currentItemMatches == false) { switch on this.op { // not all downstream updates for old item matching when new item doesn't have been defined // and this switch statement is how other operations opt-in when UPDATE_COUNT, UPDATE_CONCAT_DISTINCT, UPDATE_CONCAT, UPDATE_SUM { - if (this.isChangedFieldCalc == false && result.matchesOld) { - shouldAddToWinnowedItems = true; + if (this.isChangedFieldCalc == false && result.hasOldItem && result.matchesOld) { + shouldAddToResults = true; } } } } - if (shouldAddToWinnowedItems) { - result.matchesCurrent = currentItemMatches; - Object currentVal = this.getTransformedCalcItem(item).get(this.opFieldOnCalcItem); - result.currentValue = currentVal; + if (shouldAddToResults) { + result.currentValue = transformedItem.get(this.opFieldOnCalcItem); if (this.isDistinct) { - if (this.distinctValues.contains(currentVal)) { + if (this.distinctValues.contains(result.currentValue)) { continue; } - this.distinctValues.add(currentVal); + this.distinctValues.add(result.currentValue); } + result.matchesCurrent = currentItemMatches; winnowedItems.add(result); if (this.op == Rollup.Op.SOME) { break; } } } - List orderBys = this.metadata.LimitAmount__c != null && this.metadata.RollupOrderBys__r.isEmpty() - ? new List{ new RollupOrderBy__mdt(FieldName__c = 'Id', Ranking__c = 0) } - : this.metadata.RollupOrderBys__r; - if (orderBys.isEmpty() == false) { - winnowedItems.sort(new RollupCalcItemSorter(orderBys)); - } - if (this.metadata.LimitAmount__c != null) { + if (this.metadata?.LimitAmount__c != null) { // we can only safely remove the items after sorting while (winnowedItems.size() > this.metadata.LimitAmount__c && winnowedItems.isEmpty() == false) { winnowedItems.remove(winnowedItems.size() - 1); @@ -385,7 +381,7 @@ public without sharing abstract class RollupCalculator { protected virtual Object calculateNewAggregateValue(Rollup.Op op, List objIds) { String operationName = Rollup.getBaseOperationName(op.name()); String alias = operationName.toLowerCase() + 'Field'; - List aggregate = this.tryQuery(new Set{ operationName + '(' + this.opFieldOnCalcItem + ')' + alias }, objIds); + List aggregate = this.tryQuery(this.calcItemSObjectType, new Set{ operationName + '(' + this.opFieldOnCalcItem + ')' + alias }, objIds); return aggregate.isEmpty() == false ? aggregate[0].get(alias) : this.defaultVal; } @@ -400,7 +396,7 @@ public without sharing abstract class RollupCalculator { this.isRecursiveRecalc = true; // now we're cooking with gas Set queryFields = this.getQueryFields(); - List allOtherItems = this.tryQuery(queryFields, objIds); + List allOtherItems = this.tryQuery(this.calcItemSObjectType, queryFields, objIds); if (allOtherItems.isEmpty()) { // break out of recursion if there's nothing to calculate this.returnVal = this.defaultVal; @@ -427,16 +423,14 @@ public without sharing abstract class RollupCalculator { return queryFields; } - protected List tryQuery(Set queryFields, Object bindVar) { + protected List tryQuery(Schema.SObjectType sObjectType, Set queryFields, Object bindVar) { List results; try { - results = this.repo.setQuery( - RollupQueryBuilder.Current.getQuery(this.calcItemSObjectType, new List(queryFields), 'Id', '!=', this.lookupKeyQuery) - ) + results = this.repo.setQuery(RollupQueryBuilder.Current.getQuery(sObjectType, new List(queryFields), 'Id', '!=', this.lookupKeyQuery)) .setArg(bindVar) .get(); } catch (System.QueryException qex) { - results = this.repo.setQuery(RollupQueryBuilder.Current.getQuery(this.calcItemSObjectType, new List(queryFields), 'Id', '!=')).get(); + results = this.repo.setQuery(RollupQueryBuilder.Current.getQuery(sObjectType, new List(queryFields), 'Id', '!=')).get(); } return results; } @@ -479,6 +473,7 @@ public without sharing abstract class RollupCalculator { public override void setDefaultValues(String lookupRecordKey, Object priorVal) { super.setDefaultValues(lookupRecordKey, priorVal); + this.distinctValues = new Set(); this.isIdCount = this.opFieldOnCalcItem.getDescribe().getName() == 'Id'; Object defaultVal = RollupFieldInitializer.Current.getDefaultValue(opFieldOnLookupObject); if (defaultVal != 0 && this.returnVal != defaultVal && this.isIdCount == false) { @@ -490,9 +485,9 @@ public without sharing abstract class RollupCalculator { this.returnVal = this.distinctValues.size() == 0 ? this.defaultVal : Decimal.valueOf(this.distinctValues.size()); } - protected override void handleShortCircuit(Object currentVal) { - if (currentVal != null && this.op != Rollup.Op.DELETE_COUNT_DISTINCT) { - this.distinctValues.add(currentVal); + protected override void handleShortCircuit(WinnowResult result) { + if (result.currentValue != null && this.op != Rollup.Op.DELETE_COUNT_DISTINCT) { + this.distinctValues.add(result.currentValue); } } @@ -509,7 +504,7 @@ public without sharing abstract class RollupCalculator { public override void handleUpdateCountDistinct(WinnowResult result) { this.shouldShortCircuit = true; - this.handleShortCircuit(result.currentValue); + this.handleShortCircuit(result); } protected override Object calculateNewAggregateValue(Rollup.Op op, List objIds) { @@ -579,13 +574,13 @@ public without sharing abstract class RollupCalculator { } } - protected virtual Decimal getNumericValue(Object currentVal) { - return this.getDecimalOrDefault(currentVal); + protected virtual Decimal getNumericValue(Object currentValue) { + return this.getDecimalOrDefault(currentValue); } protected virtual Decimal getNumericChangedValue(WinnowResult result) { Decimal newVal = this.getNumericValue(result.currentValue); - Decimal oldVal = this.getNumericValue(result.priorValue); + Decimal oldVal = result.hasOldItem ? this.getNumericValue(result.priorValue) : 0; if (this.isFullRecalc && result.matchesCurrent) { return newVal; @@ -613,19 +608,19 @@ public without sharing abstract class RollupCalculator { return this.returnVal; } - protected override void handleShortCircuit(Object currentVal) { + protected override void handleShortCircuit(WinnowResult result) { switch on this.op { when UPDATE_MAX { // re-maxing by way of query has occurred, but is it **correct**? - // if one of the other updated calcItems is numerically superior, assign the new max - Decimal newVal = this.getNumericValue(currentVal); + // if one of the other updated children is numerically superior, assign the new max + Decimal newVal = this.getNumericValue(result.currentValue); if (newVal > returnDecimal) { this.returnDecimal = newVal; } } when UPDATE_MIN { - // re-"min"-ing has occurred by way of query, but is an in-memory calcItem even less? - Decimal newVal = this.getNumericValue(currentVal); + // re-"min"-ing has occurred by way of query, but is an in-memory child even less? + Decimal newVal = this.getNumericValue(result.currentValue); if (newVal < returnDecimal) { this.returnDecimal = newVal; } @@ -661,7 +656,7 @@ public without sharing abstract class RollupCalculator { public override void handleUpdateMinOrMax(WinnowResult result) { Decimal newVal = this.getNumericValue(result.currentValue); - Decimal thisPriorVal = this.getNumericValue(result.priorValue); + Decimal thisPriorVal = this.getNumericValue(result.hasOldItem ? result.priorValue : result.currentValue); if ( thisPriorVal != 0 && thisPriorVal == this.returnDecimal && @@ -842,8 +837,8 @@ public without sharing abstract class RollupCalculator { return potentialReturnVal < 0 ? 0.00 : potentialReturnVal; } - protected override Decimal getNumericValue(Object currentVal) { - return currentVal != null ? 1.00 : 0.00; + protected override Decimal getNumericValue(Object value) { + return value != null ? 1.00 : 0.00; } protected override Decimal getNumericChangedValue(WinnowResult result) { @@ -872,6 +867,10 @@ public without sharing abstract class RollupCalculator { } } + private static Boolean isConcatDistinct(Rollup.Op op) { + return op.name().contains(Rollup.Op.CONCAT_DISTINCT.name()); + } + private without sharing abstract class DelimiterCalculator extends RollupCalculator { protected final String concatDelimiter; @@ -906,11 +905,11 @@ public without sharing abstract class RollupCalculator { Schema.SObjectField lookupKeyField ) { super(op, opFieldOnCalcItem, opFieldOnLookupObject, metadata, lookupKeyField); - this.isConcatDistinct = this.op.name().contains(Rollup.Op.CONCAT_DISTINCT.name()); + this.isConcatDistinct = isConcatDistinct(this.op); } public virtual override void setDefaultValues(String lookupRecordKey, Object priorVal) { - super.setDefaultValues(lookupRecordKey, this.isConcatDistinct ? '' : priorVal); + super.setDefaultValues(lookupRecordKey, isConcatDistinct(this.op) ? '' : priorVal); if (this.returnVal instanceof String) { this.stringVal = (String) this.returnVal; } @@ -933,16 +932,16 @@ public without sharing abstract class RollupCalculator { this.returnVal = this.stringVal; } - protected override void handleShortCircuit(Object currentVal) { + protected override void handleShortCircuit(WinnowResult result) { switch on this.op { when UPDATE_MAX, UPDATE_MIN { - String newVal = String.valueOf(currentVal); + String newVal = String.valueOf(result.currentValue); if (this.isTrueFor(newVal, this.stringVal)) { this.stringVal = newVal; } } when DELETE_CONCAT_DISTINCT { - this.handleConcatDistinctDelete(currentVal); + this.handleConcatDistinctDelete(result); } } } @@ -970,7 +969,7 @@ public without sharing abstract class RollupCalculator { return; } else if (this.isConcatDistinct) { this.shouldShortCircuit = this.op.name().contains('DELETE'); - this.handleConcatDistinctDelete(existingVal); + this.handleConcatDistinctDelete(result); } else { this.stringVal = this.replaceWithDelimiter(this.stringVal, existingVal, ''); } @@ -989,7 +988,7 @@ public without sharing abstract class RollupCalculator { public override void handleUpdateMinOrMax(WinnowResult result) { String newVal = String.valueOf(result.currentValue); - String priorString = String.valueOf(result.hasOldItem ? result.priorValue : newVal); + String priorString = String.valueOf(result.hasOldItem ? result.priorValue : result.currentValue); if ( (this.op.name().contains(Rollup.Op.MAX.name()) && priorString == this.stringVal && newVal <= this.stringVal) || @@ -999,7 +998,7 @@ public without sharing abstract class RollupCalculator { ) { this.shouldShortCircuit = true; Object potentialReturnValue = this.calculateNewAggregateValue(this.op, this.childrenIds); - this.stringVal = potentialReturnValue == null ? '' : String.valueOf(potentialReturnValue); + this.stringVal = String.valueOf(potentialReturnValue ?? ''); } else if (this.isTrueFor(newVal, this.stringVal)) { this.stringVal = newVal; } @@ -1065,10 +1064,11 @@ public without sharing abstract class RollupCalculator { return existingVal += this.concatDelimiter + replacementVal; } - private void handleConcatDistinctDelete(Object currentVal) { + private void handleConcatDistinctDelete(WinnowResult result) { // we do a replace first, in case this is a reparenting operation - this.stringVal = this.replaceWithDelimiter(this.stringVal, (String) currentVal, ''); - // we have to wait till it's the last iteration; this is what ensures that all of the deleted items are excluded + this.stringVal = this.replaceWithDelimiter(this.stringVal, (String) result.currentValue, ''); + // we have to wait till it's the last iteration; this is what ensures that all of the deleted + // items are accounted for in this.childrenIds (for proper exclusion) if (this.isLastItem) { List relatedItems = this.repo.setArg(this.childrenIds) .setQuery( @@ -1156,9 +1156,9 @@ public without sharing abstract class RollupCalculator { } public override void performRollup(List calcItems, Map oldCalcItems) { List applicableCalcItems = this.op == Rollup.Op.DELETE_AVERAGE ? new List() : calcItems; - List winnowResults = this.winnowItems(applicableCalcItems, oldCalcItems); + List results = this.winnowItems(applicableCalcItems, oldCalcItems); Decimal oldSum; - Decimal denominator = Decimal.valueOf(winnowResults.size()); + Decimal denominator = Decimal.valueOf(results.size()); if (this.isFullRecalc == false || this.hasAlreadyBeenFullRecalcUpdated(oldCalcItems)) { denominator += this.repo.setQuery(RollupQueryBuilder.Current.getQuery(this.calcItemSObjectType, new List(), 'Id', '!=', this.lookupKeyQuery)) .setArg(calcItems) @@ -1167,9 +1167,8 @@ public without sharing abstract class RollupCalculator { } Decimal newSum = 0.00; - for (WinnowResult result : winnowResults) { - Object potentialDecimal = result.currentValue; - newSum += (Decimal) potentialDecimal ?? 0.00; + for (WinnowResult result : results) { + newSum += (Decimal) result.currentValue ?? 0.00; } Decimal average = (Decimal) this.defaultVal; @@ -1255,7 +1254,7 @@ public without sharing abstract class RollupCalculator { this.occurrenceToCount = new Map(); if (this.hasAlreadyBeenFullRecalcUpdated(oldCalcItems)) { - calcItems.addAll(this.tryQuery(this.getQueryFields(), calcItems)); + calcItems.addAll(this.tryQuery(this.calcItemSObjectType, this.getQueryFields(), calcItems)); } this.largestCountPointer = -1; this.returnVal = this.defaultVal; @@ -1263,10 +1262,7 @@ public without sharing abstract class RollupCalculator { for (WinnowResult result : results) { Object value = result.currentValue; - Integer localCount = 0; - if (this.occurrenceToCount.containsKey(value)) { - localCount = this.occurrenceToCount.get(value); - } + Integer localCount = this.occurrenceToCount.get(value) ?? 0; this.occurrenceToCount.put(value, ++localCount); if (this.largestCountPointer < localCount) { this.largestCountPointer = localCount; @@ -1329,7 +1325,7 @@ public without sharing abstract class RollupCalculator { private without sharing class GroupByCalculator extends DelimiterCalculator { private final GroupingFormatter formatter; - private final System.Comparator sorter; + private final System.Comparator sorter; private final RollupCalculator innerCalculator; private final List fieldNames = new List(); private GroupByCalculator( @@ -1360,8 +1356,8 @@ public without sharing abstract class RollupCalculator { this.innerCalculator.parentIsoCode = this.parentIsoCode; this.innerCalculator.isMultiCurrencyRollup = this.isMultiCurrencyRollup; + calcItems.sort(this.sorter); List results = this.winnowItems(calcItems, oldCalcItems); - results.sort(this.sorter); Map> groupingStringToItems = new Map>(); for (WinnowResult result : results) { @@ -1495,7 +1491,7 @@ public without sharing abstract class RollupCalculator { private static Object getDefaultRecalculationValue(Rollup__mdt meta) { // some operations could possibly use either default value // we also have to cast to Object to avoid the compilation error: - // "Incompatible types in null coalescing operator: Decimal, String" + // "Incompatible types in null coalesce operator: Decimal, String" return (Object) meta.FullRecalculationDefaultNumberValue__c ?? (Object) meta.FullRecalculationDefaultStringValue__c; } } diff --git a/sfdx-project.json b/sfdx-project.json index e3bfef0d..a09ed612 100644 --- a/sfdx-project.json +++ b/sfdx-project.json @@ -5,8 +5,8 @@ "package": "apex-rollup", "path": "rollup", "scopeProfiles": true, - "versionName": "Adds the ability to flag different rollup calculations as being distinct", - "versionNumber": "1.6.25.0", + "versionName": "Tidies up newly released Rollup__mdt.IsDistinct__c functionality", + "versionNumber": "1.6.26.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": {