Skip to content

Commit

Permalink
V1.5.20 - Optimizations All Around (#356)
Browse files Browse the repository at this point in the history
* Swapping from Type.forName to Schema.describeSObjects where appropriate to save on speed thanks to research done by @jkranz-rk

* Fixing end-time range for NEXT_90_DAYS date literal

* Optimizing CPU time for full recalcs

* Making order by field readonly on Rollup__mdt since it's been deprecated for a while now

* CPU time improvements, particularly for rollups with many operatios defined

* Continuing to optimize Rollup flow code paths to reduce synchronous CPU time usage

* Cleaning up some of the code surrounding how synchronous full recalc rollups get passed through the rollup composite framework

* Fixes #329 by ensuring CurrencyIsoCode is queried for on all ultimate parent records

* Refactor to make retrieveAdditionalCalcItems less chaotic. Introduced additional guard clause to prevent full recalc processors that have already been processed from being accidentally deferred

* Optimizing merge code path for SObject deletes

* CPU usage optimizations within RollupQueryBuilder - much better string handling in place (still some room for improvement, though)

* Optimizing reparenting logic for non-REFRESH based rollups

* Fixing hour edge case with NEXT_N_MONTHS date literal

* Finishing up retrieveAdditionalCalcItems optimization work in RollupAsyncProcessor

* Fixes a typo reported by @solo-1234 in the Nebula Logger adapter readme

* Correctly strips CurrencyIsoCode from aggregate queries in multi-currency orgs

* Updating RollupNebulaLoggerAdapter to Nebula Logger 4.8.0 Summer 22 release - thanks to @jongpie's hard work, this allows us to set Apex Rollup as the Log__c.LogScenario__c, as well as successfully repressing rollup logger classes from showing in stacktraces/LogEntry__c.OriginLocation__c fields

* Further optimizing rollup code paths by caching the default RollupControl__mdt record retrieved by Apex Rollup

* Optimizing how many times RollupLogger.Instance.save() is called in any given async processor route

* Full recalcs no longer try to re-queue when already in an async context

* Incorporating code review feedback from @jongpie

* Updating field & flow labels as per @solo-1234's suggestion to move from calc/lookup notation to child/parent
  • Loading branch information
jamessimone authored Sep 12, 2022
1 parent c276469 commit 963659c
Show file tree
Hide file tree
Showing 58 changed files with 906 additions and 570 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ PACKAGING_SFDX_URL.txt
tests/apex
**/main/default/
coverage/
plugins/ExtraCodeCoverage
plugins/ExtraCodeCoverage
.sf/
6 changes: 4 additions & 2 deletions Contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ When submitting a pull request, please follow these guidelines:

- there should be no errors when running `npm run scan` or `yarn scan`
- ensure your dependencies have been installed and that any/all file(s) changed have had prettier-apex run on them (usually as simple as enabling "Format On Save" in your editor's options); alternatively you can always format the document in Vs Code using `Shift + Ctrl + F` or `cmd + Shift + F` once you're done writing. Your mileage may vary as to the hotkey if you're using Illuminated Cloud or another text editor; you always have the option of invoking prettier on the command-line
- ensure that tests have been run against a scratch org.
- ensure that tests have been run against a scratch org with multi-currency enabled (you can look at `scripts/test.ps1` to see how the scratch org is created and the currency ISO codes are loaded).
- ensure that any change to production code comes with the addition of tests. It's possible that I will accept PRs where _only_ production-level code is changed, if an optimization is put in place -- but otherwise, try to write a failing test first!
- there is another directory, `extra-tests`, included to provide code coverage for the accessing of custom fields linked as Field Definitions within `Rollup__mdt`, as well as complicated DML operations and CMDT-specific tests. You should add `extra-tests` temporarily to your `sfdx-project.json`'s `packageAliases` property in order to deploy those test classes and their dependencies. The reason these are optional test(s) is because they rely on custom fields, which I do not want anybody to have to install in their own orgs beyond what is necessary. You can browse the powershell test script located in `scripts/test.ps1` to see how the CI system swaps out the existing `sfdx-project.json` on deploy with the one located in the `scripts/` directory to ensure that tests requiring custom fields pass on a scratch org prior to a build passing. The `README` also includes additional context under the "Contributing" section related to validating all `Rollup` tests pass locally prior to submitting a PR
- if you are testing on a scratch org, `sfdx force:source:push` will fail due to the Rollup Nebula Logger Adapter plugin. You can either:
- temporarily comment out that plugin within the `packageDirectories` list in `sfdx-project.json`
- install whichever `04t...` version of Nebula Logger the plugin relies upon by looking at the dependency listed within `sfdx-project.json`'s `packageAliases` object
76 changes: 38 additions & 38 deletions README.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion extra-tests/classes/CustomMetadataDrivenTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private class CustomMetadataDrivenTests {
childOne.DateField__c = childOne.DateField__c.addDays(-1);
update childOne;

// begin recursive update, but with a Calc Item Where Clause field change
// begin recursive update, but with a Child Object Where Clause field change
childOne.NumberField__c = 1;
update childOne;

Expand Down
4 changes: 2 additions & 2 deletions extra-tests/classes/InvocableDrivenTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ private class InvocableDrivenTests {

// Description and Subject both are referenced in the Flow
Case matchesFlow = new Case(Description = 'Name Match', AccountId = acc.Id, Subject = 'Refresh Test');
Case nonMatch = new Case(Description = 'Non match', AccountId = acc.Id, Subject = 'Calc Item Where Clause test');
Case nonMatch = new Case(Description = 'Non match', AccountId = acc.Id, Subject = 'Child Object Where Clause test');

Test.startTest();
insert new List<Case>{ matchesFlow, nonMatch };
Expand Down Expand Up @@ -121,7 +121,7 @@ private class InvocableDrivenTests {
Test.stopTest();

acc = [SELECT Phone FROM Account WHERE Id = :acc.Id];
System.assertEquals(null, acc.Phone, 'Phone should not have been updated since calc item where clause does not match');
System.assertEquals(null, acc.Phone, 'Phone should not have been updated since child object where clause does not match');
matchingAccount = [SELECT Phone FROM Account WHERE Id = :matchingAccount.Id];
System.assertEquals(childThree.Phone, matchingAccount.Phone, 'Phone should have been ordered by LastName!');
}
Expand Down
4 changes: 2 additions & 2 deletions extra-tests/classes/RollupCalculatorTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ private class RollupCalculatorTests {
Task.WhatId
);

// the important things here: the current date is greater than both the passed in date (the "current" value on the lookup object)
// the important things here: the current date is greater than both the passed in date (the "current" value on the parent object)
// AND that the "current" value matches what's on the old item
Task t = new Task(ActivityDate = today.addDays(1), Id = RollupTestUtils.createId(Task.SObjectType));

Expand Down Expand Up @@ -1224,7 +1224,7 @@ private class RollupCalculatorTests {
Task.WhatId
);

// the important things here: the current date is less than both the passed in date (the "current" value on the lookup object)
// the important things here: the current date is less than both the passed in date (the "current" value on the parent object)
// AND that the "current" value matches what's on the old item
Task t = new Task(ActivityDate = today.addDays(-1), Id = RollupTestUtils.createId(Task.SObjectType));

Expand Down
2 changes: 1 addition & 1 deletion extra-tests/classes/RollupCurrencyInfoTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private class RollupCurrencyInfoTests {

@IsTest
static void convertsMultipleFieldsCorrectly() {
if (RollupCurrencyInfo.IS_MULTICURRENCY == false) {
if (RollupCurrencyInfo.isMultiCurrency() == false) {
return;
}
RollupCurrencyInfo mockUsdInfo = new RollupCurrencyInfo();
Expand Down
2 changes: 1 addition & 1 deletion extra-tests/classes/RollupEvaluatorTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ private class RollupEvaluatorTests {
eval = RollupEvaluator.getEvaluator(new RollupEvaluator.AlwaysTrueEvaluator(), rollupMetadata, new Map<Id, SObject>(), Opportunity.SObjectType);

System.assertEquals(false, eval.matches(opp), 'Should not return true when recursive and all other conditions true');
System.assertEquals(false, eval.matches(nonMatchingOpp), 'Should not match to begin with based on calc item where clause');
System.assertEquals(false, eval.matches(nonMatchingOpp), 'Should not match to begin with based on child object where clause');
}

@IsTest
Expand Down
2 changes: 1 addition & 1 deletion extra-tests/classes/RollupFlowBulkProcessorTests.cls
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ private class RollupFlowBulkProcessorTests {
}
Test.stopTest();

System.assertEquals(null, ex, 'Exception should not be thrown when calc item info can be inferred from CMDT');
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 summed properly');
}
Expand Down
Loading

0 comments on commit 963659c

Please sign in to comment.