Skip to content

Commit

Permalink
Issue SFDO-Community#240 - query based rollup not recalculated when o…
Browse files Browse the repository at this point in the history 
…rder by field changes
  • Loading branch information
@jondavis9898
jondavis9898 committed Aug 30, 2015
1 parent 574302d commit e24f7a1
Show file tree
Hide file tree
Showing 4 changed files with 303 additions and 24 deletions.
28 changes: 18 additions & 10 deletions rolluptool/src/classes/LREngine.cls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,19 +459,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);
}
}

Expand All @@ -480,6 +472,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.
Expand Down
55 changes: 41 additions & 14 deletions rolluptool/src/classes/RollupService.cls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> fieldsToSearchForChanges = new Set<String>();
Set<String> relationshipFields = new Set<String>();
// 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<Id, Set<String>> fieldsInvolvedInLookup = new Map<Id, Set<String>>();
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<String> lookupFields = new Set<String>();
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<Utilities.Ordering> orderByFields = Utilities.parseOrderByClause(lookup.FieldToOrderBy__c);
if (orderByFields != null && !orderByFields.isEmpty()) {
for (Utilities.Ordering orderByField :orderByFields) {
lookupFields.add(orderByField.getField());
}
}
}

// 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);
}

Expand Down Expand Up @@ -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<String> 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;

Expand Down
222 changes: 222 additions & 0 deletions rolluptool/src/classes/RollupServiceTest.cls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1618,4 +1618,226 @@ private with sharing class RollupServiceTest
o = new Utilities.Ordering('CloseDate', Utilities.SortOrder.DESCENDING, true);
assertOrdering(o, 'CloseDate DESC NULLS LAST', true, 'CloseDate', Utilities.SortOrder.DESCENDING, true);
}

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<LookupRollupSummary__c> rollups = new List<LookupRollupSummary__c> { 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<SObject> parents = new List<SObject> { parentA, parentB, parentC };
insert parents;

// Insert children
List<SObject> children = new List<SObject>();
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<Id, SObject> assertParents = new Map<Id, SObject>(Database.query(String.format('select id, {0} from {1}', new List<String>{ 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<SObject> childrenToUpdate = new List<SObject>();
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<Id, SObject>(Database.query(String.format('select id, {0} from {1}', new List<String>{ 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<LookupRollupSummary__c> rollups = new List<LookupRollupSummary__c> { 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<SObject> parents = new List<SObject> { parentA, parentB, parentC };
insert parents;

// Insert children
List<SObject> children = new List<SObject>();
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<Id, SObject> assertParents = new Map<Id, SObject>(Database.query(String.format('select id, {0} from {1}', new List<String>{ 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<SObject> childrenToUpdate = new List<SObject>();
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<Id, SObject>(Database.query(String.format('select id, {0} from {1}', new List<String>{ 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));
}
}
22 changes: 22 additions & 0 deletions rolluptool/src/classes/TestLREngine.cls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,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<LREngine.RollupOperation, Boolean> operations = new Map<LREngine.RollupOperation, Boolean> {
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(String detailOrderByClause, LREngine.RollupSummaryField rollupField, String expected1, String expected2) {
prepareData();

Expand Down

0 comments on commit e24f7a1

Please sign in to comment.