Skip to content

Commit

Permalink
v1.5.64 - Grandparent Rollup Optimizations (#439)
Browse files Browse the repository at this point in the history
* Finishing up an optimization for grandparent rollups triggered by full recalc runs - now, when a full recalc rollup has already queried up the hierarchy, we can assume all children are already present in RollupRelationshipFieldFinder, instead of re-traversing the entire hierarchy.

* Fixed up FlowBulkProcessor issue from 1.5.62, added some additional optimizations for when RollupCalcItemReplacer actually runs

* Further calc item replacer optimizations

* Continued optimizations within RollupFlowBulkProcessor

* Fixes issue reported by Joseph Mason where non-polymorphic Type fields were not being queried properly for full recalcs

* Fixes an issue reported by @solo-1234 where rollup order by fields were not being queried properly for full recalcs created from Flow when a rollup was triggered by the parent
  • Loading branch information
jamessimone authored May 4, 2023
1 parent a46390d commit 5cb1693
Show file tree
Hide file tree
Showing 18 changed files with 344 additions and 105 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ 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=04t6g000008o0DaAAI">
<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SnX0AAK">
<img alt="Deploy to Salesforce"
src="./media/deploy-package-to-prod.png">
</a>

<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008o0DaAAI">
<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SnX0AAK">
<img alt="Deploy to Salesforce Sandbox"
src="./media/deploy-package-to-sandbox.png">
</a>
Expand Down
2 changes: 1 addition & 1 deletion extra-tests/classes/RollupCalcItemReplacerTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private class RollupCalcItemReplacerTests {
new RollupControl__mdt(IsRollupLoggingEnabled__c = true, ReplaceCalcItemsAsyncWhenOverCount__c = 1)
);
Account updatedAccount = (Account) replacer.replace(
new List<Account>{ new Account(Id = acc.Id) },
new List<Account>{ acc },
new List<Rollup__mdt>{ new Rollup__mdt(CalcItemWhereClause__c = 'CreatedDate != null', CalcItem__c = 'Account') }
)[0];

Expand Down
9 changes: 9 additions & 0 deletions extra-tests/classes/RollupEvaluatorTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,15 @@ private class RollupEvaluatorTests {
System.assertEquals('Campaign.Type', queryFields[0]);
}

@IsTest
static void supportsTypeFieldWhenOnlyPresentOnParent() {
String whereClause = 'Account.Type != \'Something\'';
RollupEvaluator.WhereFieldEvaluator eval = new RollupEvaluator.WhereFieldEvaluator(whereClause, Contact.SObjectType);
List<String> queryFields = eval.getQueryFields();
System.assertEquals(1, queryFields.size(), queryFields);
System.assertEquals('Account.Type', queryFields[0]);
}

@IsTest
static void gracefullyHandlesFieldNotExistingForDateLiteral() {
String whereClause = 'SomeFieldThatDoesNotExist = THIS_YEAR';
Expand Down
48 changes: 47 additions & 1 deletion extra-tests/classes/RollupFlowBulkProcessorTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -326,12 +326,13 @@ private class RollupFlowBulkProcessorTests {
};

Test.startTest();
RollupFlowBulkProcessor.addRollup(new List<RollupFlowBulkProcessor.FlowInput>{ input });
List<Rollup.FlowOutput> flowOutputs = RollupFlowBulkProcessor.addRollup(new List<RollupFlowBulkProcessor.FlowInput>{ input });
Test.stopTest();

acc = [SELECT AnnualRevenue, NumberOfEmployees FROM Account];
System.assertEquals(10, acc.AnnualRevenue, 'Account annual revenue should have summed properly');
System.assertEquals(2, acc.NumberOfEmployees, 'Account number of employees should have counted properly');
System.assertEquals(1, flowOutputs.size(), 'Output size should be the same as input size');
}

@IsTest
Expand Down Expand Up @@ -378,4 +379,49 @@ private class RollupFlowBulkProcessorTests {
Account acc = [SELECT AnnualRevenue, NumberOfEmployees FROM Account];
System.assertEquals(10, acc.AnnualRevenue, 'Account annual revenue should have summed properly');
}

@IsTest
static void rollupOrderBysAreHandledProperlyForParentRollups() {
Rollup.onlyUseMockMetadata = true;
List<Account> accs = [SELECT Id, AnnualRevenue FROM Account];
insert new List<SObject>{
new Opportunity(Amount = 5, AccountId = accs[0].Id, StageName = 'A', CloseDate = System.today().addDays(5), Name = 'Amount 1'),
new Opportunity(Amount = 10, AccountId = accs[0].Id, StageName = 'A', CloseDate = System.today(), Name = 'Amount 2')
};

RollupFlowBulkProcessor.FlowInput input = new RollupFlowBulkProcessor.FlowInput();
input.recordsToRollup = accs;
input.rollupContext = 'UPSERT';
input.oldRecordsToRollup = new List<SObject>{ null, null };
input.deferProcessing = false;

Rollup.rollupMetadata = new List<Rollup__mdt>{
Rollup.appendOrderByMetadata(
new Rollup__mdt(
RollupOperation__c = 'FIRST',
CalcItem__c = 'Opportunity',
LookupObject__c = 'Account',
RollupFieldOnCalcItem__c = 'Amount',
LookupFieldOnCalcItem__c = 'AccountId',
LookupFieldOnLookupObject__c = 'Id',
RollupFieldOnLookupObject__c = 'AnnualRevenue',
IsRollupStartedFromParent__c = true
),
new List<RollupOrderBy__mdt>{ new RollupOrderBy__mdt(FieldName__c = 'CloseDate', Ranking__c = 0) }
)
};

Exception ex;
Test.startTest();
try {
RollupFlowBulkProcessor.addRollup(new List<RollupFlowBulkProcessor.FlowInput>{ input });
} catch (Exception e) {
ex = e;
}
Test.stopTest();

System.assertEquals(null, ex, 'Exception should not be thrown when child object info can be inferred from CMDT');
Account acc = [SELECT AnnualRevenue, NumberOfEmployees FROM Account];
System.assertEquals(10, acc.AnnualRevenue, 'Account annual revenue should have ordered properly');
}
}
25 changes: 25 additions & 0 deletions extra-tests/classes/RollupRelationshipFieldFinderTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,31 @@ private class RollupRelationshipFieldFinderTests {
System.assertEquals(false, traversal.getIsFinished(), 'Should have bailed early!');
}

@IsTest
static void skipsIntermediateRelationshipFetchesForFullRecalc() {
// same as above except this one should succeed since everything should be fetched in one go
Account acc = [SELECT Id, OwnerId, Owner.Name FROM Account];

Contact con = new Contact(AccountId = acc.Id, LastName = 'Child con');
insert con;
control.MaxNumberOfQueries__c = 1;

RollupRelationshipFieldFinder finder = new RollupRelationshipFieldFinder(
control,
new Rollup__mdt(GrandparentRelationshipFieldPath__c = 'Account.Owner.Name'),
uniqueFieldNames,
uniqueFieldNames,
User.SObjectType,
new Map<Id, SObject>()
);
finder.setIsFullRecalc(true);
RollupRelationshipFieldFinder.Traversal traversal = finder.getParents(new List<Contact>{ con });

System.assertEquals(true, traversal.getParentLookupToRecords().containsKey(acc.OwnerId));
System.assertEquals(acc.Owner.Name, traversal.retrieveParents(con.Id)[0].get('Name'));
System.assertEquals(true, traversal.getIsFinished());
}

@IsTest
static void shouldNotReportFalsePositiveIfUltimateParentStaysTheSame() {
Account intermediateOne = new Account(Name = 'Intermediate 1');
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "apex-rollup",
"version": "1.5.63",
"version": "1.5.64",
"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",
Expand Down
4 changes: 2 additions & 2 deletions rollup-namespaced/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ For more info, see the base `README`.

## Deployment & Setup

<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008o0DfAAI">
<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SnX5AAK">
<img alt="Deploy to Salesforce"
src="./media/deploy-package-to-prod.png">
</a>

<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008o0DfAAI">
<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008SnX5AAK">
<img alt="Deploy to Salesforce Sandbox"
src="./media/deploy-package-to-sandbox.png">
</a>
7 changes: 4 additions & 3 deletions rollup-namespaced/sfdx-project.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"default": true,
"package": "apex-rollup-namespaced",
"path": "rollup-namespaced/source/rollup",
"versionName": "Fixes The number of results does not match the number of interviews that were executed in a single bulk execution request issue in RollupFlowBulkProcessor",
"versionNumber": "1.0.36.0",
"versionName": "Grandparent full recalc rollup optimizations",
"versionNumber": "1.0.37.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": {
Expand All @@ -30,6 +30,7 @@
"apex-rollup-namespaced@1.0.33-0": "04t6g000008o01cAAA",
"apex-rollup-namespaced@1.0.34-0": "04t6g000008o021AAA",
"apex-rollup-namespaced@1.0.35-0": "04t6g000008o0DVAAY",
"apex-rollup-namespaced@1.0.36-0": "04t6g000008o0DfAAI"
"apex-rollup-namespaced@1.0.36-0": "04t6g000008o0DfAAI",
"apex-rollup-namespaced@1.0.37-0": "04t6g000008SnX5AAK"
}
}
86 changes: 48 additions & 38 deletions rollup/core/classes/Rollup.cls
Original file line number Diff line number Diff line change
Expand Up @@ -752,10 +752,11 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
// Flow bulkifies things, but poorly. It's possible to get multiple FlowInputs with the same metadata, but different calc items (for example)
// let's head that off at the pass by bulkifying here
Map<Rollup__mdt, FlowInputWrapper> metaToInputWrapper = new Map<Rollup__mdt, FlowInputWrapper>();
RollupSettings__c settings = RollupSettings__c.getInstance();
for (FlowInput flowInput : flowInputs) {
FlowOutput flowOutput = new FlowOutput();
flowOutputs.add(flowOutput);
if (RollupSettings__c.getInstance().IsEnabled__c == false) {
if (settings.IsEnabled__c == false) {
continue;
}

Expand Down Expand Up @@ -809,7 +810,10 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
List<Rollup__mdt> metas = new List<Rollup__mdt>(wrapper.rollupMetadata);
wrapper.flowInput.recordsToRollup = replaceFlowInputCalcItemsIfNecessary(hashCodeToCalcItems, wrapper.flowInput.recordsToRollup, metas);
wrapper.flowInput.oldRecordsToRollup = replaceFlowInputCalcItemsIfNecessary(hashCodeToCalcItems, wrapper.flowInput.oldRecordsToRollup, metas);
Map<Id, SObject> oldFlowRecords = new Map<Id, SObject>(wrapper.flowInput.oldRecordsToRollup);
Map<Id, SObject> oldFlowRecords = new Map<Id, SObject>();
for (SObject oldFlowRecord : wrapper.flowInput.oldRecordsToRollup) {
oldFlowRecords.put(oldFlowRecord.Id, oldFlowRecord);
}
processCustomMetadata(
localRollups,
metas,
Expand Down Expand Up @@ -1865,36 +1869,39 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
}

private static Rollup__mdt transformFlowInputToRollupMetadata(FlowInput flowInput, String rollupContext, SObjectType sObjectType) {
return new Rollup__mdt(
CalcItem__c = flowInput.isRollupStartedFromParent || String.isNotBlank(flowInput.calcItemTypeWhenRollupStartedFromParent)
? flowInput.calcItemTypeWhenRollupStartedFromParent
: String.valueOf(sObjectType),
CalcItemWhereClause__c = flowInput.calcItemWhereClause,
ChangedFieldsOnCalcItem__c = flowInput.calcItemChangedFields,
ConcatDelimiter__c = flowInput.concatDelimiter,
CurrencyFieldMapping__c = flowInput.currencyFieldMapping,
FullRecalculationDefaultNumberValue__c = flowInput.fullRecalculationDefaultNumberValue,
FullRecalculationDefaultStringValue__c = flowInput.fullRecalculationDefaultStringValue,
GrandparentRelationshipFieldPath__c = flowInput.grandparentRelationshipFieldPath,
GroupByFields__c = flowInput.groupByFields,
GroupByRowEndDelimiter__c = flowInput.groupByRowEndDelimiter,
GroupByRowStartDelimiter__c = flowInput.groupByRowStartDelimiter,
IsFullRecordSet__c = flowInput.isFullRecordSet,
IsRollupStartedFromParent__c = flowInput.isRollupStartedFromParent,
IsTableFormatted__c = flowInput.isTableFormatted,
LookupFieldOnCalcItem__c = flowInput.lookupFieldOnCalcItem,
LookupFieldOnLookupObject__c = flowInput.lookupFieldOnOpObject,
LookupObject__c = flowInput.rollupSObjectName,
OneToManyGrandparentFields__c = flowInput.oneToManyGrandparentFields,
OrderByFirstLast__c = flowInput.orderByFirstLast,
RollupControl__r = getSingleControlOrDefault(RollupControl__mdt.Id, flowInput.rollupControlId, defaultControl),
RollupFieldOnCalcItem__c = flowInput.rollupFieldOnCalcItem,
RollupFieldOnLookupObject__c = flowInput.rollupFieldOnOpObject,
RollupOperation__c = rollupContext + flowInput.rollupOperation,
RollupToUltimateParent__c = flowInput.rollupToUltimateParent,
SharingMode__c = flowInput.sharingMode,
SplitConcatDelimiterOnCalcItem__c = flowInput.splitConcatDelimiterOnCalcItem,
UltimateParentLookup__c = flowInput.ultimateParentLookup
return getFirstLastMetadata(
new Rollup__mdt(
CalcItem__c = flowInput.isRollupStartedFromParent || String.isNotBlank(flowInput.calcItemTypeWhenRollupStartedFromParent)
? flowInput.calcItemTypeWhenRollupStartedFromParent
: String.valueOf(sObjectType),
CalcItemWhereClause__c = flowInput.calcItemWhereClause,
ChangedFieldsOnCalcItem__c = flowInput.calcItemChangedFields,
ConcatDelimiter__c = flowInput.concatDelimiter,
CurrencyFieldMapping__c = flowInput.currencyFieldMapping,
FullRecalculationDefaultNumberValue__c = flowInput.fullRecalculationDefaultNumberValue,
FullRecalculationDefaultStringValue__c = flowInput.fullRecalculationDefaultStringValue,
GrandparentRelationshipFieldPath__c = flowInput.grandparentRelationshipFieldPath,
GroupByFields__c = flowInput.groupByFields,
GroupByRowEndDelimiter__c = flowInput.groupByRowEndDelimiter,
GroupByRowStartDelimiter__c = flowInput.groupByRowStartDelimiter,
IsFullRecordSet__c = flowInput.isFullRecordSet,
IsRollupStartedFromParent__c = flowInput.isRollupStartedFromParent,
IsTableFormatted__c = flowInput.isTableFormatted,
LookupFieldOnCalcItem__c = flowInput.lookupFieldOnCalcItem,
LookupFieldOnLookupObject__c = flowInput.lookupFieldOnOpObject,
LookupObject__c = flowInput.rollupSObjectName,
OneToManyGrandparentFields__c = flowInput.oneToManyGrandparentFields,
OrderByFirstLast__c = flowInput.orderByFirstLast,
RollupControl__r = getSingleControlOrDefault(RollupControl__mdt.Id, flowInput.rollupControlId, defaultControl),
RollupFieldOnCalcItem__c = flowInput.rollupFieldOnCalcItem,
RollupFieldOnLookupObject__c = flowInput.rollupFieldOnOpObject,
RollupOperation__c = rollupContext + flowInput.rollupOperation,
RollupToUltimateParent__c = flowInput.rollupToUltimateParent,
SharingMode__c = flowInput.sharingMode,
SplitConcatDelimiterOnCalcItem__c = flowInput.splitConcatDelimiterOnCalcItem,
UltimateParentLookup__c = flowInput.ultimateParentLookup
),
flowInput.rollupOperation
);
}

Expand Down Expand Up @@ -2183,7 +2190,7 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
}
}
for (RollupOrderBy__mdt orderByInfo : meta.RollupOrderBys__r) {
queryFields.add(orderByInfo.FieldName__c);
queryFields.add(orderByInfo.FieldName__c.trim());
}
return queryFields;
}
Expand Down Expand Up @@ -2212,6 +2219,9 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
wrappedMeta.metadata.add(innerMeta);
RollupEvaluator.WhereFieldEvaluator eval = String.isBlank(whereClause) ? null : RollupEvaluator.getWhereEval(whereClause, childType);
wrappedMeta.queryFields.addAll(getQueryFieldsFromMetadata(innerMeta, eval));
if (innerMeta.IsRollupStartedFromParent__c && innerMeta.GrandparentRelationshipFieldPath__c != null) {
wrappedMeta.queryFields.add(innerMeta.GrandparentRelationshipFieldPath__c);
}
return wrappedMeta;
}

Expand Down Expand Up @@ -2702,9 +2712,7 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
if (isFullRecalcOp(getBaseOperationName(rollupMetadata.RollupOperation__c)) || rollupMetadata.GroupByFields__c != null) {
rollupMetadata.IsFullRecordSet__c = true;
}
if (rollupOp.name().contains(Op.FIRST.name()) || rollupOp.name().contains(Op.LAST.name())) {
rollupMetadata = getFirstLastMetadata(rollupMetadata);
}
rollupMetadata = getFirstLastMetadata(rollupMetadata, rollupOp.name());

RollupControl__mdt localControl;
if (rollupMetadata.RollupControl__c != null) {
Expand Down Expand Up @@ -2740,7 +2748,7 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
return rollupConductor;
}

private static Rollup__mdt getFirstLastMetadata(Rollup__mdt meta) {
private static Rollup__mdt getFirstLastMetadata(Rollup__mdt meta, String rollupOpName) {
List<RollupOrderBy__mdt> replacementOrderBys;
if (meta.RollupOrderBys__r.isEmpty() && String.isNotBlank(meta.OrderByFirstLast__c)) {
List<String> unparsedOrderBys = meta.OrderByFirstLast__c.split(',');
Expand All @@ -2759,7 +2767,9 @@ global without sharing virtual class Rollup implements RollupLogger.ToStringObje
}
replacementOrderBys.add(orderBy);
}
} else if (meta.RollupOrderBys__r.isEmpty()) {
} else if (
meta.RollupOrderBys__r.isEmpty() && (rollupOpName.contains(Op.FIRST.name()) || rollupOpName.contains(Op.LAST.name())) || meta.LimitAmount__c != null
) {
replacementOrderBys = new List<RollupOrderBy__mdt>{ new RollupOrderBy__mdt(Ranking__c = 0, FieldName__c = meta.RollupFieldOnCalcItem__c) };
}
return appendOrderByMetadata(meta, replacementOrderBys);
Expand Down
1 change: 1 addition & 0 deletions rollup/core/classes/RollupAsyncProcessor.cls
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,7 @@ global virtual without sharing class RollupAsyncProcessor extends Rollup impleme
roll.lookupObj,
roll.oldCalcItems
)
.setIsFullRecalc(this.isFullRecalc)
.getParents(roll.calcItems);
} else if (roll.traversal?.getIsFinished() == false) {
roll.traversal.recommence();
Expand Down
Loading

0 comments on commit 5cb1693

Please sign in to comment.