From 3f2d31e9a7b9be7c7384e1886a82b9fa9fcbd6bc Mon Sep 17 00:00:00 2001 From: Jon Davis Date: Sat, 29 Aug 2015 20:08:34 -0700 Subject: [PATCH] Issue #240 - query based rollup not recalculated when order by field changes --- rolluptool/src/classes/LREngine.cls | 28 ++- rolluptool/src/classes/RollupService.cls | 55 +++-- rolluptool/src/classes/RollupServiceTest.cls | 222 +++++++++++++++++++ rolluptool/src/classes/TestLREngine.cls | 22 ++ 4 files changed, 303 insertions(+), 24 deletions(-) diff --git a/rolluptool/src/classes/LREngine.cls b/rolluptool/src/classes/LREngine.cls index 8a2d8248..d57df3dc 100644 --- a/rolluptool/src/classes/LREngine.cls +++ b/rolluptool/src/classes/LREngine.cls @@ -543,19 +543,11 @@ public class LREngine { } public boolean isAggregateBasedRollup() { - return operation == RollupOperation.Sum || - operation == RollupOperation.Min || - operation == RollupOperation.Max || - operation == RollupOperation.Avg || - operation == RollupOperation.Count || - operation == RollupOperation.Count_Distinct; + return isAggregateBasedRollup(operation); } public boolean isQueryBasedRollup() { - return operation == RollupOperation.Concatenate || - operation == RollupOperation.Concatenate_Distinct || - operation == RollupOperation.First || - operation == RollupOperation.Last; + return isQueryBasedRollup(operation); } } @@ -564,6 +556,22 @@ public class LREngine { System_x } + public static boolean isAggregateBasedRollup(RollupOperation operation) { + return operation == RollupOperation.Sum || + operation == RollupOperation.Min || + operation == RollupOperation.Max || + operation == RollupOperation.Avg || + operation == RollupOperation.Count || + operation == RollupOperation.Count_Distinct; + } + + public static boolean isQueryBasedRollup(RollupOperation operation) { + return operation == RollupOperation.Concatenate || + operation == RollupOperation.Concatenate_Distinct || + operation == RollupOperation.First || + operation == RollupOperation.Last; + } + /** Context having all the information about the rollup to be done. Please note : This class encapsulates many rollup summary fields with different operations. diff --git a/rolluptool/src/classes/RollupService.cls b/rolluptool/src/classes/RollupService.cls index a453b042..e4fb08cd 100644 --- a/rolluptool/src/classes/RollupService.cls +++ b/rolluptool/src/classes/RollupService.cls @@ -539,12 +539,41 @@ global with sharing class RollupService // Set of field names from the child used in the rollup to search for changes on Set fieldsToSearchForChanges = new Set(); Set relationshipFields = new Set(); + // keep track of fields that should trigger a rollup to be processed + // this avoids having to re-parse RelationshipCriteria & OrderBy fields during field change detection + Map> fieldsInvolvedInLookup = new Map>(); for(LookupRollupSummary__c lookup : lookups) { - fieldsToSearchForChanges.add(lookup.FieldToAggregate__c); - if(lookup.RelationshipCriteriaFields__c!=null) - for(String criteriaField : lookup.RelationshipCriteriaFields__c.split('\r\n')) - fieldsToSearchForChanges.add(criteriaField); + Set lookupFields = new Set(); + lookupFields.add(lookup.FieldToAggregate__c); + if(!String.isBlank(lookup.RelationshipCriteriaFields__c)) { + for(String criteriaField : lookup.RelationshipCriteriaFields__c.split('\r\n')) { + lookupFields.add(criteriaField); + } + } + // only include order by fields when query based rollup (concat, first, last, etc.) since changes to them + // will not impact the outcome of an aggregate based rollup (sum, count, etc.) + if(LREngine.isQueryBasedRollup(RollupSummaries.OPERATION_PICKLIST_TO_ENUMS.get(lookup.AggregateOperation__c)) && !String.isBlank(lookup.FieldToOrderBy__c)) { + List orderByFields = Utilities.parseOrderByClause(lookup.FieldToOrderBy__c, sObjectType.getDescribe().fields.getMap()); + if (orderByFields != null && !orderByFields.isEmpty()) { + for (LREngine.Ordering orderByField :orderByFields) { + lookupFields.add(orderByField.getField().getName()); + } + } + } + + // add all lookup fields to our master list of fields to search for + fieldsToSearchForChanges.addAll(lookupFields); + + // add relationshipfield to fields for this lookup + // this comes after adding to fieldsToSearchForChanges because we handle + // change detection separately for non-relationship fields and relationship fields + lookupFields.add(lookup.RelationShipField__c); + + // add to map for later use + fieldsInvolvedInLookup.put(lookup.Id, lookupFields); + + // add relationship field to master list of relationship fields relationshipFields.add(lookup.RelationShipField__c); } @@ -624,16 +653,14 @@ global with sharing class RollupService for(LookupRollupSummary__c lookup : lookups) { // Are any of the changed fields used by this lookup? - Boolean processLookup = false; - if(fieldsChanged.contains(lookup.FieldToAggregate__c) || - fieldsChanged.contains(lookup.RelationShipField__c)) - processLookup = true; - if(lookup.RelationshipCriteriaFields__c!=null) - for(String criteriaField : lookup.RelationshipCriteriaFields__c.split('\r\n')) - if(fieldsChanged.contains(criteriaField)) - processLookup = true; - if(processLookup) - lookupsToProcess.add(lookup); + Set lookupFields = fieldsInvolvedInLookup.get(lookup.Id); + for (String lookupField :lookupFields) { + if (fieldsChanged.contains(lookupField)) { + // add lookup to be processed and exit for loop since we have our answer + lookupsToProcess.add(lookup); + break; + } + } } lookups = lookupsToProcess; diff --git a/rolluptool/src/classes/RollupServiceTest.cls b/rolluptool/src/classes/RollupServiceTest.cls index cf3e9c19..1350ba96 100644 --- a/rolluptool/src/classes/RollupServiceTest.cls +++ b/rolluptool/src/classes/RollupServiceTest.cls @@ -1531,4 +1531,226 @@ private with sharing class RollupServiceTest System.assertEquals(null, (String) assertParents.get(parentC.id).get(aggregateResultField1)); System.assertEquals(20, (Decimal) assertParents.get(parentC.id).get(aggregateResultField2)); } + + private testmethod static void testSingleQueryBasedRollupUpdateOrderByFieldChanged() + { + // Test supported? + if(!TestContext.isSupported()) + return; + + Schema.SObjectType parentType = LookupParent__c.sObjectType; + Schema.SObjectType childType = LookupChild__c.sObjectType; + String parentObjectName = parentType.getDescribe().getName(); + String childObjectName = childType.getDescribe().getName(); + String relationshipField = LookupChild__c.LookupParent__c.getDescribe().getName(); + String aggregateField = LookupChild__c.Color__c.getDescribe().getName(); + String aggregateResultField = LookupParent__c.Colours__c.getDescribe().getName(); + String orderByField = LookupChild__c.Amount__c.getDescribe().getName(); + + // Configure rollups + LookupRollupSummary__c rollupSummary = new LookupRollupSummary__c(); + rollupSummary.Name = 'Test Rollup'; + rollupSummary.ParentObject__c = parentObjectName; + rollupSummary.ChildObject__c = childObjectName; + rollupSummary.RelationShipField__c = relationshipField; + rollupSummary.FieldToAggregate__c = aggregateField; + rollupSummary.FieldToOrderBy__c = orderByField; + rollupSummary.AggregateOperation__c = RollupSummaries.AggregateOperation.First.name(); + rollupSummary.AggregateResultField__c = aggregateResultField; + rollupSummary.ConcatenateDelimiter__c = ';'; + rollupSummary.Active__c = true; + rollupSummary.CalculationMode__c = RollupSummaries.CalculationMode.Realtime.name(); + + List rollups = new List { rollupSummary }; + insert rollups; + + // Insert parents + SObject parentA = parentType.newSObject(); + parentA.put('Name', 'ParentA'); + SObject parentB = parentType.newSObject(); + parentB.put('Name', 'ParentB'); + SObject parentC = parentType.newSObject(); + parentC.put('Name', 'ParentC'); + List parents = new List { parentA, parentB, parentC }; + insert parents; + + // Insert children + List children = new List(); + for(SObject parent : parents) + { + SObject child1 = childType.newSObject(); + child1.put(relationshipField, parent.Id); + child1.put(aggregateField, 'Red'); + child1.put(orderByField, 10); + children.add(child1); + SObject child2 = childType.newSObject(); + child2.put(relationshipField, parent.Id); + child2.put(aggregateField, 'Yellow'); + child2.put(orderByField, 20); + children.add(child2); + SObject child3 = childType.newSObject(); + child3.put(relationshipField, parent.Id); + child3.put(aggregateField, 'Blue'); + child3.put(orderByField, 30); + children.add(child3); + } + insert children; + + // Assert rollups + Map assertParents = new Map(Database.query(String.format('select id, {0} from {1}', new List{ aggregateResultField, parentObjectName }))); + System.assertEquals('Red', (String) assertParents.get(parentA.id).get(aggregateResultField)); + System.assertEquals('Red', (String) assertParents.get(parentB.id).get(aggregateResultField)); + System.assertEquals('Red', (String) assertParents.get(parentC.id).get(aggregateResultField)); + + // change Amount__c to effect order by result of rollup + // this change will result in rollup being processed because it is a query based rollup + // and order by influences rolled up value + List childrenToUpdate = new List(); + for (SObject child :children) + { + Decimal orderByFieldValue = (Decimal)child.get(orderByField); + if (orderByFieldValue == 10) { + child.put(orderByField, 40); + childrenToUpdate.add(child); + } + } + + // Sample various limits prior to an update + Integer beforeQueries = Limits.getQueries(); + Integer beforeRows = Limits.getQueryRows(); + Integer beforeDMLRows = Limits.getDMLRows(); + + // update children + update childrenToUpdate; + + // Assert limits + // + One query on Rollup object + // + One query on LookupChild__c for rollup + System.assertEquals(beforeQueries + 2, Limits.getQueries()); + + // + One row for Rollup object + // + Nine rows for LookupChild__c for rollup + System.assertEquals(beforeRows + 10, Limits.getQueryRows()); + + // + Three rows for LookupChild__c (from the update statement itself) + // + Three rows for LookupParent__c for the rollup + System.assertEquals(beforeDMLRows + 6, Limits.getDMLRows()); + + // Assert rollups + assertParents = new Map(Database.query(String.format('select id, {0} from {1}', new List{ aggregateResultField, parentObjectName }))); + System.assertEquals('Yellow', (String) assertParents.get(parentA.id).get(aggregateResultField)); + System.assertEquals('Yellow', (String) assertParents.get(parentB.id).get(aggregateResultField)); + System.assertEquals('Yellow', (String) assertParents.get(parentC.id).get(aggregateResultField)); + } + + private testmethod static void testSingleAggregateBasedRollupUpdateOrderByFieldChanged() + { + // Test supported? + if(!TestContext.isSupported()) + return; + + Schema.SObjectType parentType = LookupParent__c.sObjectType; + Schema.SObjectType childType = LookupChild__c.sObjectType; + String parentObjectName = parentType.getDescribe().getName(); + String childObjectName = childType.getDescribe().getName(); + String relationshipField = LookupChild__c.LookupParent__c.getDescribe().getName(); + String aggregateField = LookupChild__c.Amount__c.getDescribe().getName(); + String aggregateResultField = LookupParent__c.Total__c.getDescribe().getName(); + String orderByField = LookupChild__c.Color__c.getDescribe().getName(); + + // Configure rollups + LookupRollupSummary__c rollupSummary = new LookupRollupSummary__c(); + rollupSummary.Name = 'Test Rollup'; + rollupSummary.ParentObject__c = parentObjectName; + rollupSummary.ChildObject__c = childObjectName; + rollupSummary.RelationShipField__c = relationshipField; + rollupSummary.FieldToAggregate__c = aggregateField; + rollupSummary.FieldToOrderBy__c = orderByField; + rollupSummary.AggregateOperation__c = RollupSummaries.AggregateOperation.Sum.name(); + rollupSummary.AggregateResultField__c = aggregateResultField; + rollupSummary.ConcatenateDelimiter__c = ';'; + rollupSummary.Active__c = true; + rollupSummary.CalculationMode__c = RollupSummaries.CalculationMode.Realtime.name(); + + List rollups = new List { rollupSummary }; + insert rollups; + + // Insert parents + SObject parentA = parentType.newSObject(); + parentA.put('Name', 'ParentA'); + SObject parentB = parentType.newSObject(); + parentB.put('Name', 'ParentB'); + SObject parentC = parentType.newSObject(); + parentC.put('Name', 'ParentC'); + List parents = new List { parentA, parentB, parentC }; + insert parents; + + // Insert children + List children = new List(); + for(SObject parent : parents) + { + SObject child1 = childType.newSObject(); + child1.put(relationshipField, parent.Id); + child1.put(aggregateField, 10); + child1.put(orderByField, 'Red'); + children.add(child1); + SObject child2 = childType.newSObject(); + child2.put(relationshipField, parent.Id); + child2.put(aggregateField, 20); + child2.put(orderByField, 'Yellow'); + children.add(child2); + SObject child3 = childType.newSObject(); + child3.put(relationshipField, parent.Id); + child3.put(aggregateField, 12); + child3.put(orderByField, 'Blue'); + children.add(child3); + } + insert children; + + // Assert rollups + Map assertParents = new Map(Database.query(String.format('select id, {0} from {1}', new List{ aggregateResultField, parentObjectName }))); + System.assertEquals(42, (Decimal) assertParents.get(parentA.id).get(aggregateResultField)); + System.assertEquals(42, (Decimal) assertParents.get(parentB.id).get(aggregateResultField)); + System.assertEquals(42, (Decimal) assertParents.get(parentC.id).get(aggregateResultField)); + + // change Color__c to effect order by result of rollup + // this change will NOT result in rollup being processed because it is a aggregate based rollup + // and order by does NOT influence rolled up result + List childrenToUpdate = new List(); + for (SObject child :children) + { + String orderByFieldValue = (String)child.get(orderByField); + if (orderByFieldValue == 'Red') { + child.put(orderByField, 'Green'); + childrenToUpdate.add(child); + } + } + + // Sample various limits prior to an update + Integer beforeQueries = Limits.getQueries(); + Integer beforeRows = Limits.getQueryRows(); + Integer beforeDMLRows = Limits.getDMLRows(); + + // update children + update childrenToUpdate; + + // Assert limits + // + One query on Rollup object + // No query on LookupChild__c because the changed Color__c should not be considered a change that would trigger the rollup to be processed + System.assertEquals(beforeQueries + 1, Limits.getQueries()); + + // + One row for Rollup object + // No rows on LookupChild__c because the changed Color__c should not be considered a change that would trigger the rollup to be processed + System.assertEquals(beforeRows + 1, Limits.getQueryRows()); + + // + Three rows for LookupChild__c (from the update statement itself) + // No query on LookupParent__c because the changed Color__c should not be considered a change that would trigger the rollup to be processed + System.assertEquals(beforeDMLRows + 3, Limits.getDMLRows()); + + // Assert rollups + assertParents = new Map(Database.query(String.format('select id, {0} from {1}', new List{ aggregateResultField, parentObjectName }))); + System.assertEquals(42, (Decimal) assertParents.get(parentA.id).get(aggregateResultField)); + System.assertEquals(42, (Decimal) assertParents.get(parentB.id).get(aggregateResultField)); + System.assertEquals(42, (Decimal) assertParents.get(parentC.id).get(aggregateResultField)); + } } \ No newline at end of file diff --git a/rolluptool/src/classes/TestLREngine.cls b/rolluptool/src/classes/TestLREngine.cls index 14167ff0..b7b2c0dd 100644 --- a/rolluptool/src/classes/TestLREngine.cls +++ b/rolluptool/src/classes/TestLREngine.cls @@ -1170,6 +1170,28 @@ private class TestLREngine { System.assertEquals('Won', reloadedAcc2.get(rollupField2.master.getName())); } + static testMethod void testIsAggregateOrQueryBasedRollup() + { + // map of operations with flag indicating if it is an aggregate operation + Map operations = new Map { + LREngine.RollupOperation.Sum => true, + LREngine.RollupOperation.Min => true, + LREngine.RollupOperation.Max => true, + LREngine.RollupOperation.Avg => true, + LREngine.RollupOperation.Count => true, + LREngine.RollupOperation.Count_Distinct => true, + LREngine.RollupOperation.Concatenate => false, + LREngine.RollupOperation.Concatenate_Distinct => false, + LREngine.RollupOperation.First => false, + LREngine.RollupOperation.Last => false + }; + for (LREngine.RollupOperation op :operations.keySet()) { + Boolean isAggregate = operations.get(op); + System.assertEquals(isAggregate, LREngine.isAggregateBasedRollup(op)); + System.assertEquals(!isAggregate, LREngine.isQueryBasedRollup(op)); + } + } + static private void testRollup(LREngine.RollupSummaryField rollupField, String expected1, String expected2) { prepareData();