From 6b52f53cd47814328fbc51d157ad1bc0aab34a0d Mon Sep 17 00:00:00 2001 From: Jon Davis Date: Fri, 28 Aug 2015 22:36:01 -0700 Subject: [PATCH 1/4] Issue #216 - add order by support of multiple fields, ASC/DESC and NULLS FIRST/LAST --- rolluptool/src/classes/LREngine.cls | 92 ++++- rolluptool/src/classes/RollupService.cls | 4 +- rolluptool/src/classes/RollupServiceTest.cls | 64 +++- rolluptool/src/classes/RollupServiceTest4.cls | 122 ++++++- rolluptool/src/classes/RollupServiceTest5.cls | 269 ++++++++++++++ rolluptool/src/classes/RollupSummaries.cls | 33 +- .../src/classes/RollupSummariesTest.cls | 27 +- rolluptool/src/classes/TestLREngine.cls | 339 +++++++++++++++++- rolluptool/src/classes/Utilities.cls | 54 +++ .../src/objects/LookupRollupSummary__c.object | 8 +- 10 files changed, 966 insertions(+), 46 deletions(-) diff --git a/rolluptool/src/classes/LREngine.cls b/rolluptool/src/classes/LREngine.cls index 67ed4e81..8a2d8248 100644 --- a/rolluptool/src/classes/LREngine.cls +++ b/rolluptool/src/classes/LREngine.cls @@ -128,7 +128,7 @@ public class LREngine { // #0 token : SOQL projection String soqlProjection = ctx.lookupField.getName(); List orderByFields = new List(); - orderByFields.add(ctx.lookupField.getName()); // ensure details records are ordered by parent record + orderByFields.add(new Ordering(ctx.lookupField).toString()); // ensure details records are ordered by parent record // k: detail field name, v: master field name Integer exprIdx = 0; @@ -163,11 +163,12 @@ public class LREngine { } // create order by projections // i.e. Amount ASC NULLS FIRST - String orderByField = - rsf.detailOrderBy!=null ? rsf.detailOrderBy.getName() : rsf.detail.getName(); - if(!orderByFieldsSet.contains(orderByField)) { - orderByFields.add(orderByField); - orderByFieldsSet.add(orderByField); + for (Ordering orderByField :(rsf.detailOrderBy == null || rsf.detailOrderBy.isEmpty()) ? new List { new Ordering(rsf.detail) } : rsf.detailOrderBy) { + String fieldName = orderByField.getField().getName(); + if(!orderByFieldsSet.contains(fieldName)) { + orderByFields.add(orderByField.toString()); + orderByFieldsSet.add(fieldName); + } } } } @@ -344,7 +345,68 @@ public class LREngine { return String.join(listOfString, delimiter == null ? '' : delimiter); } } - + + /** + Sort Order + */ + public enum SortOrder {ASCENDING, DESCENDING} + + /** + Represents a single portion of the Order By clause for SOQL statement + */ + public class Ordering{ + private SortOrder direction; + private Boolean nullsLast; + private Schema.DescribeFieldResult field; + private Boolean directionSpecified; // if direction was specified during construction + private Boolean nullsLastSpecified; // if nullsLast was specified during construction + + /** + * Construct a new ordering instance + **/ + public Ordering(Schema.DescribeFieldResult field) { + this(field, null); + } + public Ordering(Schema.DescribeFieldResult field, SortOrder direction) { + this(field, direction, null); + } + public Ordering(Schema.DescribeFieldResult field, SortOrder direction, Boolean nullsLast) { + // field must be specified + if (field == null) { + throw new BadOrderingStateException('field cannot be null.'); + } + + this.field = field; + this.directionSpecified = direction != null; + this.nullsLastSpecified = nullsLast != null; + this.direction = this.directionSpecified ? direction : SortOrder.ASCENDING; //SOQL docs ASC is default behavior + this.nullsLast = this.nullsLastSpecified ? nullsLast : false; //SOQL docs state NULLS FIRST is default behavior + } + public Schema.DescribeFieldResult getField(){ + return field; + } + public SortOrder getDirection() { + return direction; + } + public Boolean getNullsLast() { + return nullsLast; + } + public override String toString() { + return field.getName() + ' ' + (direction == SortOrder.ASCENDING ? 'ASC' : 'DESC') + ' ' + (nullsLast ? 'NULLS LAST' : 'NULLS FIRST'); + } + public String toAsSpecifiedString() { + // emit order by using describe info with the direction and nullsLast + // that was provided during construction. This allows to regurgitate + // the proper SOQL order by using exactly what was passed in + return field.getName() + (directionSpecified ? (direction == SortOrder.ASCENDING ? ' ASC' : ' DESC') : '') + (nullsLastSpecified ? (nullsLast ? ' NULLS LAST' : ' NULLS FIRST') : ''); + } + } + + /** + Exception thrown if Ordering is in bad state + */ + public class BadOrderingStateException extends Exception {} + /** Exception throwed if Rollup Summary field is in bad state */ @@ -367,7 +429,7 @@ public class LREngine { public class RollupSummaryField { public Schema.Describefieldresult master; public Schema.Describefieldresult detail; - public Schema.Describefieldresult detailOrderBy; + public List detailOrderBy; public RollupOperation operation; public String concatenateDelimiter; @@ -383,7 +445,7 @@ public class LREngine { public RollupSummaryField(Schema.Describefieldresult m, Schema.Describefieldresult d, RollupOperation op) { - this(m, d, null, op, null); + this(m, d, (Schema.Describefieldresult)null, op, null); } public RollupSummaryField(Schema.Describefieldresult m, @@ -391,6 +453,13 @@ public class LREngine { Schema.Describefieldresult detailOrderBy, RollupOperation op, String concatenateDelimiter) { + this(m, d, (detailOrderBy == null ? null : new List { new Ordering(detailOrderBy) }), op, concatenateDelimiter); + } + public RollupSummaryField(Schema.Describefieldresult m, + Schema.Describefieldresult d, + List detailOrderBy, + RollupOperation op, + String concatenateDelimiter) { this.master = m; this.detail = d; this.detailOrderBy = detailOrderBy; @@ -442,6 +511,11 @@ public class LREngine { if (isMasterTypeDateOrTime && (RollupOperation.Sum == operation || RollupOperation.Avg == operation)) { throw new BadRollUpSummaryStateException('Sum/Avg doesnt looks like valid for dates ! Still want, then implement the IRollerCoaster yourself and change this class as required.'); } + + // If we had the SObjectType of Detail field here we could + // iterate the detailOrderBy SObjectFields and ensure they belong to the + // detail SObject but there is no way to determine SObjectType from DescribeFieldResult + // therefore we cannot validate the order by in any way } boolean isText (Schema.Displaytype dt) { diff --git a/rolluptool/src/classes/RollupService.cls b/rolluptool/src/classes/RollupService.cls index 80980f1f..a453b042 100644 --- a/rolluptool/src/classes/RollupService.cls +++ b/rolluptool/src/classes/RollupService.cls @@ -776,7 +776,7 @@ global with sharing class RollupService if(childFields==null) gdFields.put(childObjectType, ((childFields = childObjectType.getDescribe().fields.getMap()))); SObjectField fieldToAggregate = childFields.get(lookup.FieldToAggregate__c); - SObjectField fieldToOrderBy = lookup.FieldToOrderBy__c!=null ? childFields.get(lookup.FieldToOrderBy__c) : null; + List fieldsToOrderBy = Utilities.parseOrderByClause(lookup.FieldToOrderBy__c, childFields); SObjectField relationshipField = childFields.get(lookup.RelationshipField__c); SObjectField aggregateResultField = parentFields.get(lookup.AggregateResultField__c); if(fieldToAggregate==null || relationshipField==null || aggregateResultField==null) @@ -787,7 +787,7 @@ global with sharing class RollupService new LREngine.RollupSummaryField( aggregateResultField.getDescribe(), fieldToAggregate.getDescribe(), - fieldToOrderBy !=null ? fieldToOrderBy.getDescribe() : null, // field to order by on child + fieldsToOrderBy, // field to order by on child RollupSummaries.OPERATION_PICKLIST_TO_ENUMS.get(lookup.AggregateOperation__c), lookup.ConcatenateDelimiter__c); diff --git a/rolluptool/src/classes/RollupServiceTest.cls b/rolluptool/src/classes/RollupServiceTest.cls index 8a312cc2..cf3e9c19 100644 --- a/rolluptool/src/classes/RollupServiceTest.cls +++ b/rolluptool/src/classes/RollupServiceTest.cls @@ -1114,16 +1114,8 @@ private with sharing class RollupServiceTest * which results in non-deterministic result so a test cannot reliabily be written against * multiple rollups on same parent/child relationship when no order by is specified. */ - private static Id setupMultiRollupDifferentTypes(RollupSummaries.AggregateOperation operationA, Schema.DescribeFieldResult orderByFieldA, RollupSummaries.AggregateOperation operationB, Schema.DescribeFieldResult orderByFieldB) + private static Id setupMultiRollupDifferentTypes(Map opportunityData, RollupSummaries.AggregateOperation operationA, String orderByFieldA, RollupSummaries.AggregateOperation operationB, String orderByFieldB) { - // Test data - // OpportunityName => Amount;CloseDateAddMonthsToToday;StageName - Map opportunityData = new Map { - 'Joe' => '250;0;Open', - 'Steve' => '50;1;Prospecting', - 'Kim' => '100;-2;Closed Won', - 'Charlie' => '225;-1;Needs Analysis'}; - // Configure rollup A LookupRollupSummary__c rollupSummaryA = new LookupRollupSummary__c(); rollupSummaryA.Name = 'First Opportunity Name into Sic on Account'; @@ -1132,7 +1124,7 @@ private with sharing class RollupServiceTest rollupSummaryA.RelationShipField__c = 'AccountId'; rollupSummaryA.RelationShipCriteria__c = null; rollupSummaryA.FieldToAggregate__c = 'StageName'; - rollupSummaryA.FieldToOrderBy__c = orderByFieldA != null ? orderByFieldA.getName() : null; + rollupSummaryA.FieldToOrderBy__c = orderByFieldA; rollupSummaryA.AggregateOperation__c = operationA.name(); rollupSummaryA.AggregateResultField__c = 'Sic'; rollupSummaryA.Active__c = true; @@ -1146,7 +1138,7 @@ private with sharing class RollupServiceTest rollupSummaryB.RelationShipField__c = 'AccountId'; rollupSummaryB.RelationShipCriteria__c = null; rollupSummaryB.FieldToAggregate__c = 'Name'; - rollupSummaryB.FieldToOrderBy__c = orderByFieldB != null ? orderByFieldB.getName() : null; + rollupSummaryB.FieldToOrderBy__c = orderByFieldB; rollupSummaryB.AggregateOperation__c = operationB.name(); rollupSummaryB.AggregateResultField__c = 'Description'; rollupSummaryB.ConcatenateDelimiter__c = ','; @@ -1189,18 +1181,26 @@ private with sharing class RollupServiceTest if(!TestContext.isSupported()) return; + // Test data + // OpportunityName => Amount;CloseDateAddMonthsToToday;StageName + Map opportunityData = new Map { + 'Joe' => '250;0;Open', + 'Steve' => '50;1;Prospecting', + 'Kim' => '100;-2;Closed Won', + 'Charlie' => '225;-1;Needs Analysis'}; + // Test data for rollup A String expectedResultA = 'Closed Won'; RollupSummaries.AggregateOperation operationA = RollupSummaries.AggregateOperation.First; - Schema.DescribeFieldResult orderByFieldA = Schema.SObjectType.Opportunity.fields.CloseDate; + String orderByClauseA = Schema.SObjectType.Opportunity.fields.CloseDate.getName(); // Test data for rollup B String expectedResultB = 'Steve,Kim,Charlie,Joe'; RollupSummaries.AggregateOperation operationB = RollupSummaries.AggregateOperation.Concatenate; - Schema.DescribeFieldResult orderByFieldB = Schema.SObjectType.Opportunity.fields.Amount; + String orderByClauseB = Schema.SObjectType.Opportunity.fields.Amount.getName(); // generate rollups and data - Id accountId = setupMultiRollupDifferentTypes(operationA, orderByFieldA, operationB, orderByFieldB); + Id accountId = setupMultiRollupDifferentTypes(opportunityData, operationA, orderByClauseA, operationB, orderByClauseB); // Assert rollup Account accountResult = Database.query('select Sic, Description from Account where Id = :accountId'); @@ -1208,6 +1208,42 @@ private with sharing class RollupServiceTest System.assertEquals(expectedResultB, accountResult.Description); } + /** + * Test default behavior with different order by containing multiple fields on each rollup + */ + private testmethod static void testMultiRollupOfDifferentTypesDifferentMultipleFieldsOrderBy() + { + // Test supported? + if(!TestContext.isSupported()) + return; + + // Test data + // OpportunityName => Amount;CloseDateAddMonthsToToday;StageName + Map opportunityData = new Map { + 'Joe' => '100;0;Open', + 'Steve' => '100;-2;Prospecting', + 'Kim' => '100;1;Closed Won', + 'Charlie' => '100;-1;Needs Analysis'}; + + // Test data for rollup A + String expectedResultA = 'Prospecting'; + RollupSummaries.AggregateOperation operationA = RollupSummaries.AggregateOperation.First; + String orderByClauseA = 'Amount ASC NULLS FIRST, CloseDate ASC NULLS FIRST, Name'; + + // Test data for rollup B + String expectedResultB = 'Kim,Joe,Charlie,Steve'; + RollupSummaries.AggregateOperation operationB = RollupSummaries.AggregateOperation.Concatenate; + String orderByClauseB = 'Amount ASC NULLS FIRST, CloseDate DESC NULLS FIRST, Name'; + + // generate rollups and data + Id accountId = setupMultiRollupDifferentTypes(opportunityData, operationA, orderByClauseA, operationB, orderByClauseB); + + // Assert rollup + Account accountResult = Database.query('select Sic, Description from Account where Id = :accountId'); + System.assertEquals(expectedResultA, accountResult.Sic); + System.assertEquals(expectedResultB, accountResult.Description); + } + private testmethod static void testPicklistRollup() { // Test supported? diff --git a/rolluptool/src/classes/RollupServiceTest4.cls b/rolluptool/src/classes/RollupServiceTest4.cls index 59b6c539..56694f37 100644 --- a/rolluptool/src/classes/RollupServiceTest4.cls +++ b/rolluptool/src/classes/RollupServiceTest4.cls @@ -1016,6 +1016,126 @@ private class RollupServiceTest4 { System.assertEquals(42, (Decimal) assertParents.get(parentC.id).get(aggregateResultField2)); } + private testmethod static void testLimitsAndContextsUsedMultipleQueryRollupsDifferByOperationFieldCaseMultipleFieldsOrderBy() + { + // 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 aggregateField1 = LookupChild__c.Color__c.getDescribe().getName(); + String aggregateField2 = LookupChild__c.Amount__c.getDescribe().getName(); + String aggregateResultField1 = LookupParent__c.Colours__c.getDescribe().getName(); + String aggregateResultField2 = LookupParent__c.Total2__c.getDescribe().getName(); + String condition = 'Amount__c > 1'; + String relationshipCriteriaFields = 'Amount__c'; + String sharingMode = LREngine.SharingMode.User.name(); + String orderBy1 = 'Amount__c ASC NULLS FIRST, Color__c ASC NULLS FIRST'; + String orderBy2 = 'Amount__c ASC NULLS FIRST, Color__c DESC NULLS FIRST'; + + // Configure rollups + LookupRollupSummary__c rollupSummaryA = new LookupRollupSummary__c(); + rollupSummaryA.Name = 'Test Rollup A'; + rollupSummaryA.ParentObject__c = parentObjectName.toLowerCase(); + rollupSummaryA.ChildObject__c = childObjectName.toLowerCase(); + rollupSummaryA.RelationShipField__c = relationshipField.toLowerCase(); + rollupSummaryA.RelationShipCriteria__c = condition; + rollupSummaryA.RelationShipCriteriaFields__c = relationshipCriteriaFields; + rollupSummaryA.FieldToAggregate__c = aggregateField1.toLowerCase(); + rollupSummaryA.FieldToOrderBy__c = orderBy1; + rollupSummaryA.AggregateOperation__c = RollupSummaries.AggregateOperation.First.name(); + rollupSummaryA.AggregateResultField__c = aggregateResultField1.toLowerCase(); + rollupSummaryA.Active__c = true; + rollupSummaryA.CalculationMode__c = RollupSummaries.CalculationMode.Realtime.name(); + rollupSummaryA.CalculationSharingMode__c = sharingMode.toLowerCase(); + + LookupRollupSummary__c rollupSummaryB = new LookupRollupSummary__c(); + rollupSummaryB.Name = 'Test Rollup B'; + rollupSummaryB.ParentObject__c = parentObjectName; + rollupSummaryB.ChildObject__c = childObjectName; + rollupSummaryB.RelationShipField__c = relationshipField; + rollupSummaryB.RelationShipCriteria__c = condition; + rollupSummaryB.RelationShipCriteriaFields__c = relationshipCriteriaFields; + rollupSummaryB.FieldToAggregate__c = aggregateField2; + rollupSummaryB.FieldToOrderBy__c = orderBy2; + rollupSummaryB.AggregateOperation__c = RollupSummaries.AggregateOperation.Last.name(); + rollupSummaryB.AggregateResultField__c = aggregateResultField2; + rollupSummaryB.Active__c = true; + rollupSummaryB.CalculationMode__c = RollupSummaries.CalculationMode.Realtime.name(); + rollupSummaryB.CalculationSharingMode__c = sharingMode; + + List rollups = new List { rollupSummaryA, rollupSummaryB }; + 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(aggregateField1, 'Red'); + child1.put(aggregateField2, 42); + children.add(child1); + SObject child2 = childType.newSObject(); + child2.put(relationshipField, parent.Id); + child2.put(aggregateField1, 'Yellow'); + child2.put(aggregateField2, 15); + children.add(child2); + SObject child3 = childType.newSObject(); + child3.put(relationshipField, parent.Id); + child3.put(aggregateField1, 'Blue'); + child3.put(aggregateField2, 10); + children.add(child3); + } + + // Sample various limits prior to an update + Integer beforeQueries = Limits.getQueries(); + Integer beforeRows = Limits.getQueryRows(); + Integer beforeDMLRows = Limits.getDMLRows(); + + insert children; + + // Assert limits + // + One query on Rollup object + // + One query on LookupChild__c for rollup A + // + One query on LookupChild__c for rollup B + System.assertEquals(beforeQueries + 3, Limits.getQueries()); + + // + Two rows for Rollup object + // + Nine rows for LookupChild__c for rollup A + // + Nine rows for LookupChild__c for rollup B + System.assertEquals(beforeRows + 20, Limits.getQueryRows()); + + // + Nine rows for LookupChild__c (from the update statement itself) + // + Three rows for LookupParent__c for rollup A & B (DLRS combined updates to identical master ids) + System.assertEquals(beforeDMLRows + 12, Limits.getDMLRows()); + + // Assert rollups + Map assertParents = new Map(Database.query(String.format('select id, {0}, {1} from {2}', new List{ aggregateResultField1, aggregateResultField2, parentObjectName }))); + System.assertEquals('Blue', (String) assertParents.get(parentA.id).get(aggregateResultField1)); + System.assertEquals(42, (Decimal) assertParents.get(parentA.id).get(aggregateResultField2)); + + System.assertEquals('Blue', (String) assertParents.get(parentB.id).get(aggregateResultField1)); + System.assertEquals(42, (Decimal) assertParents.get(parentB.id).get(aggregateResultField2)); + + System.assertEquals('Blue', (String) assertParents.get(parentC.id).get(aggregateResultField1)); + System.assertEquals(42, (Decimal) assertParents.get(parentC.id).get(aggregateResultField2)); + } + /** * Test for issue https://github.com/afawcett/declarative-lookup-rollup-summaries/issues/229 * Ensure that any field on LookupRollupSummary__c that is describable is updated with describe info @@ -1037,7 +1157,7 @@ private class RollupServiceTest4 { String condition = 'Amount__c > 1'; List relationshipCriteriaFields = new List { 'Amount__c', 'Name', 'Id', 'IsDeleted' }; String sharingMode = LREngine.SharingMode.User.name(); - String fieldToOrderBy = LookupChild__c.Amount__c.getDescribe().getName(); + String fieldToOrderBy = 'Amount__c,Color__c ASC,Name NULLS LAST,Id DESC NULLS FIRST'; // Configure rollups LookupRollupSummary__c rollupSummary = new LookupRollupSummary__c(); diff --git a/rolluptool/src/classes/RollupServiceTest5.cls b/rolluptool/src/classes/RollupServiceTest5.cls index c396f86f..08564d00 100644 --- a/rolluptool/src/classes/RollupServiceTest5.cls +++ b/rolluptool/src/classes/RollupServiceTest5.cls @@ -219,4 +219,273 @@ private class RollupServiceTest5 { System.assertEquals(1, [select AnnualRevenue from Account where id = :accountParent.Id][0].AnnualRevenue); System.assertEquals(null, [select TotalOpportunityQuantity from Opportunity where id = :oppParent.Id][0].TotalOpportunityQuantity); } + + private static void assertOrdering(List order, Integer numFields, List fields, List directions, List nullsLast) + { + System.assertNotEquals(null, order); + System.assertEquals(numFields, order.size()); + for (Integer i = 0; i < numFields; i++) + { + assertOrdering(order[i], fields[i], directions[i], nullsLast[i]); + } + } + + private static void assertOrdering(LREngine.Ordering o, Schema.DescribeFieldResult field, LREngine.SortOrder direction, Boolean nullsLast) + { + System.assertEquals(field, o.getField()); + System.assertEquals(direction, o.getDirection()); + System.assertEquals(nullsLast, o.getNullsLast()); + } + + @IsTest + private static void testParseOrderByFieldOnly() { + List order = Utilities.parseOrderByClause('Amount__c', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + + @IsTest + private static void testParseOrderByFieldAndASCDirection() { + List order = Utilities.parseOrderByClause('Amount__c ASC', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + + @IsTest + private static void testParseOrderByFieldAndDESCDirection() { + List order = Utilities.parseOrderByClause('Amount__c DESC', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.DESCENDING }, + new List { false }); + } + + @IsTest + private static void testParseOrderByFieldAndNullsFirst() { + List order = Utilities.parseOrderByClause('Amount__c NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + + @IsTest + private static void testParseOrderByFieldAndNullsLast() { + List order = Utilities.parseOrderByClause('Amount__c NULLS LAST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { true }); + } + + @IsTest + private static void testParseOrderByFieldAndASCDirectionAndNullsFirst() { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + + @IsTest + private static void testParseOrderByFieldAndASCDirectionAndNullsLast() { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS LAST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { true }); + } + + @IsTest + private static void testParseOrderByFieldAndDESCDirectionAndNullsFirst() { + List order = Utilities.parseOrderByClause('Amount__c DESC NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.DESCENDING }, + new List { false }); + } + + @IsTest + private static void testParseOrderByFieldAndDESCDirectionAndNullsLast() { + List order = Utilities.parseOrderByClause('Amount__c DESC NULLS LAST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.DESCENDING }, + new List { true }); + } + + @IsTest + private static void testParseOrderByMultipleFieldOnly() { + List order = Utilities.parseOrderByClause('Amount__c, Color__c, Name', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 3, + new List { LookupChild__c.Amount__c.getDescribe(), LookupChild__c.Color__c.getDescribe(), LookupChild__c.Name.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING, LREngine.SortOrder.ASCENDING, LREngine.SortOrder.ASCENDING }, + new List { false, false, false }); + } + + @IsTest + private static void testParseOrderByMultipleFieldAndMixedDirection() { + List order = Utilities.parseOrderByClause('Amount__c ASC, Color__c DESC, Name ASC', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 3, + new List { LookupChild__c.Amount__c.getDescribe(), LookupChild__c.Color__c.getDescribe(), LookupChild__c.Name.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING, LREngine.SortOrder.DESCENDING, LREngine.SortOrder.ASCENDING }, + new List { false, false, false }); + } + + @IsTest + private static void testParseOrderByMultipleFieldAndMixedDirectionAndNulls() { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS LAST, Color__c DESC NULLS FIRST, Name ASC NULLS LAST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 3, + new List { LookupChild__c.Amount__c.getDescribe(), LookupChild__c.Color__c.getDescribe(), LookupChild__c.Name.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING, LREngine.SortOrder.DESCENDING, LREngine.SortOrder.ASCENDING }, + new List { true, false, true }); + } + + @IsTest + private static void testParseOrderByBadField() { + try { + List order = Utilities.parseOrderByClause('BadField__c', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Field does not exist.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByBadDirection() { + try { + List order = Utilities.parseOrderByClause('Amount__c BAD', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByBadNulls() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC BAD', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByMissingNulls() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC LAST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByBadNullsType() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS BAD', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByMissingNullsType() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByInvalidStart() { + try { + List order = Utilities.parseOrderByClause('BAD Amount__c ASC NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByInvalidMiddle() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC BAD NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByInvalidEnd() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS FIRST BAD', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByMultipleSecondFieldInvalid() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS FIRST, Color__c ASC NULLS BAD, Name ASC NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByMultipleThirdFieldInvalid() { + try { + List order = Utilities.parseOrderByClause('Amount__c ASC NULLS FIRST, Color__c ASC NULLS LAST, Name ASC FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assert(false, 'Expected exception'); + } catch(Utilities.OrderByInvalidException e) { + System.assertEquals('Invalid order by clause.', e.getMessage()); + } + } + + @IsTest + private static void testParseOrderByMultipleFieldWhitespaceEverywhere() { + List order = Utilities.parseOrderByClause(' Amount__c ASC NULLS LAST , Color__c DESC NULLS FIRST ', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 2, + new List { LookupChild__c.Amount__c.getDescribe(), LookupChild__c.Color__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING, LREngine.SortOrder.DESCENDING }, + new List { true, false}); + } + + @IsTest + private static void testParseOrderByBlankClause() { + List order = Utilities.parseOrderByClause(null, LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assertEquals(null, order); + + order = Utilities.parseOrderByClause('', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assertEquals(null, order); + + order = Utilities.parseOrderByClause(' ', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + System.assertEquals(null, order); + } } \ No newline at end of file diff --git a/rolluptool/src/classes/RollupSummaries.cls b/rolluptool/src/classes/RollupSummaries.cls index 1c6ca149..3517e8a2 100644 --- a/rolluptool/src/classes/RollupSummaries.cls +++ b/rolluptool/src/classes/RollupSummaries.cls @@ -131,7 +131,6 @@ public class RollupSummaries extends fflib_SObjectDomain // Child Object fields SObjectField relationshipField = null; SObjectField fieldToAggregate = null; - SObjectField fieldToOrderBy = null; Map childObjectFields = gdFields.get(childObjectType); if(childObjectFields!=null) { @@ -145,9 +144,23 @@ public class RollupSummaries extends fflib_SObjectDomain lookupRollupSummary.FieldToAggregate__c = fieldToAggregate.getDescribe().getName(); // Field to Order By if(lookupRollupSummary.FieldToOrderBy__c!=null) { - fieldToOrderBy = childObjectFields.get(lookupRollupSummary.FieldToOrderBy__c); - if(fieldToOrderBy!=null) - lookupRollupSummary.FieldToOrderBy__c = fieldToOrderBy.getDescribe().getName(); + try { + List fieldsToOrderBy = Utilities.parseOrderByClause(lookupRollupSummary.FieldToOrderBy__c, childObjectFields); + if (fieldsToOrderBy != null && !fieldsToOrderBy.isEmpty()) { + String orderByClause = ''; + for (LREngine.Ordering orderByField :fieldsToOrderBy) { + // using toAsSpecifiedString so that we update the field name to proper describe info + // but leave the rest of what was input unchanged. If we called toString() we would + // add fully qualified Order By Clause and we don't want to add in portions of the clause + // that the user didn't provide in the first place. + orderByClause += (String.isBlank(orderByClause) ? '' : ',') + orderByField.toAsSpecifiedString(); + } + lookupRollupSummary.FieldToOrderBy__c = orderByClause; + } + } catch(Utilities.OrderByInvalidException e) { + // there is a problem with order by so we ignore it intentionally here since we're just trying + // to update field names with describe info. The error will be caught during validation phase. + } } } // Parent Object fields @@ -212,7 +225,7 @@ public class RollupSummaries extends fflib_SObjectDomain // Child Object fields valid? SObjectField relationshipField = null; SObjectField fieldToAggregate = null; - SObjectField fieldToOrderBy = null; + List fieldsToOrderBy = null; Map childObjectFields = gdFields.get(childObjectType); if(childObjectFields!=null) { @@ -226,9 +239,11 @@ public class RollupSummaries extends fflib_SObjectDomain lookupRollupSummary.FieldToAggregate__c.addError(error('Field does not exist.', lookupRollupSummary, LookupRollupSummary__c.FieldToAggregate__c)); // Field to Order By valid? if(lookupRollupSummary.FieldToOrderBy__c!=null) { - fieldToOrderBy = childObjectFields.get(lookupRollupSummary.FieldToOrderBy__c); - if(fieldToOrderBy==null) - lookupRollupSummary.FieldToOrderBy__c.addError(error('Field does not exist.', lookupRollupSummary, LookupRollupSummary__c.FieldToOrderBy__c)); + try { + fieldsToOrderBy = Utilities.parseOrderByClause(lookupRollupSummary.FieldToOrderBy__c, childObjectFields); + } catch(Utilities.OrderByInvalidException e) { + lookupRollupSummary.FieldToOrderBy__c.addError(error(e.getMessage(), lookupRollupSummary, LookupRollupSummary__c.FieldToOrderBy__c)); + } } // TODO: Validate relationship field is a lookup to the parent // ... @@ -285,7 +300,7 @@ public class RollupSummaries extends fflib_SObjectDomain new LREngine.RollupSummaryField( aggregateResultField.getDescribe(), fieldToAggregate.getDescribe(), - fieldToOrderBy!=null ? fieldToOrderBy.getDescribe() : null, // optional field to order by + fieldsToOrderBy, // optional field to order by OPERATION_PICKLIST_TO_ENUMS.get(lookupRollupSummary.AggregateOperation__c), lookupRollupSummary.ConcatenateDelimiter__c)); // Validate the SOQL diff --git a/rolluptool/src/classes/RollupSummariesTest.cls b/rolluptool/src/classes/RollupSummariesTest.cls index 3f5bb6ca..cce7a3fc 100644 --- a/rolluptool/src/classes/RollupSummariesTest.cls +++ b/rolluptool/src/classes/RollupSummariesTest.cls @@ -294,7 +294,32 @@ private class RollupSummariesTest System.assertEquals(1, fflib_SObjectDomain.Errors.getAll().size()); System.assertEquals('Field does not exist.', fflib_SObjectDomain.Errors.getAll()[0].message); System.assertEquals(LookupRollupSummary__c.FieldToOrderBy__c, ((fflib_SObjectDomain.FieldError)fflib_SObjectDomain.Errors.getAll()[0]).field); - } + } + + private testmethod static void testInsertFieldToOrderByInvalidClauseValidation() + { + // Test supported? + if(!TestContext.isSupported()) + return; + + LookupRollupSummary__c rollupSummary = new LookupRollupSummary__c(); + rollupSummary.Name = 'Total Opportunities into Annual Revenue on Account'; + rollupSummary.ParentObject__c = 'Account'; + rollupSummary.ChildObject__c = 'Opportunity'; + rollupSummary.RelationShipField__c = 'AccountId'; + rollupSummary.RelationShipCriteria__c = null; + rollupSummary.FieldToAggregate__c = 'Amount'; + rollupSummary.FieldToOrderBy__c = 'Amount ASC NULLS BAD'; + rollupSummary.AggregateOperation__c = 'Sum'; + rollupSummary.AggregateResultField__c = 'AnnualRevenue'; + rollupSummary.Active__c = true; + rollupSummary.CalculationMode__c = 'Realtime'; + fflib_SObjectDomain.Test.Database.onInsert(new LookupRollupSummary__c[] { rollupSummary } ); + fflib_SObjectDomain.triggerHandler(RollupSummaries.class); + System.assertEquals(1, fflib_SObjectDomain.Errors.getAll().size()); + System.assertEquals('Invalid order by clause.', fflib_SObjectDomain.Errors.getAll()[0].message); + System.assertEquals(LookupRollupSummary__c.FieldToOrderBy__c, ((fflib_SObjectDomain.FieldError)fflib_SObjectDomain.Errors.getAll()[0]).field); + } private testmethod static void testInsertAggregateResultFieldValidation() { diff --git a/rolluptool/src/classes/TestLREngine.cls b/rolluptool/src/classes/TestLREngine.cls index 0e2d80ba..14167ff0 100644 --- a/rolluptool/src/classes/TestLREngine.cls +++ b/rolluptool/src/classes/TestLREngine.cls @@ -33,10 +33,12 @@ THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. @isTest private class TestLREngine { // common master records for the test case - static Account acc1, acc2; + static Account acc1, acc2, acc3, acc4; // common bunch of detail records for the test case static Opportunity[] detailRecords; + static Opportunity[] detailRecords2; static Opportunity[] detailRecordsAcc1; + static Opportunity[] detailRecordsAcc3; // dynamic reference to this field to avoid it being included in the package static Schema.SObjectField ACCOUNT_SLA_EXPIRATION_DATE; static Schema.SObjectField ACCOUNT_NUMBER_OF_EMPLOYEES; @@ -131,7 +133,76 @@ private class TestLREngine { detailRecord.put(ANNUALIZED_RECCURING_REVENUE, 1000); detailRecordsAcc1 = new Opportunity[] {o1Acc1, o2Acc1, o3Acc1}; insert detailRecords; - } + } + + /* + creates the common seed data using Opportunity and Account objects. + */ + static void prepareData2() { + acc3 = new Account(Name = 'Acc3'); + acc4 = new Account(Name = 'Acc4'); + insert new Account[] {acc3, acc4}; + + Date today = System.today(); + Opportunity o1Acc3 = new Opportunity( + Name = 'o1Acc3', + AccountId = acc3.Id, + Amount = 100.00, + CloseDate = today, + Type = 'New Customer', + StageName = 'red' + ); + Opportunity o2Acc3 = new Opportunity( + Name = 'o2Acc3', + AccountId = acc3.Id, + Amount = 100.00, + CloseDate = today, + Type = 'New Customer', + StageName = 'yellow' + ); + + Opportunity o3Acc3 = new Opportunity( + Name = 'o3Acc3', + AccountId = acc3.Id, + Amount = null, + CloseDate = today, + Type = 'New Customer', + StageName = 'blue' + ); + + Opportunity o1Acc4 = new Opportunity( + Name = 'o1Acc4', + AccountId = acc4.Id, + Amount = 100.00, + CloseDate = today, + Type = 'New Customer', + StageName = 'orange' + ); + + Opportunity o2Acc4 = new Opportunity( + Name = 'o2Acc4', + AccountId = acc4.Id, + Amount = 100.00, + CloseDate = today, + Type = 'New Customer', + StageName = 'green' + ); + + Opportunity o3Acc4 = new Opportunity( + Name = 'o3Acc4', + AccountId = acc4.Id, + Amount = 100.00, + CloseDate = today, + Type = 'New Customer', + StageName = 'purple' + ); + detailRecords2 = new Opportunity[] {o1Acc3, o2Acc3, o3Acc3, o1Acc4, o2Acc4, o3Acc4}; + if(ANNUALIZED_RECCURING_REVENUE!=null) + for(Opportunity detailRecord : detailRecords2) + detailRecord.put(ANNUALIZED_RECCURING_REVENUE, 1000); + detailRecordsAcc3 = new Opportunity[] {o1Acc3, o2Acc3, o3Acc3}; + insert detailRecords2; + } /* @@ -675,7 +746,7 @@ private class TestLREngine { new LREngine.RollupSummaryField( Schema.SObjectType.Account.fields.AccountNumber, Schema.SObjectType.Opportunity.fields.StageName, - null, LREngine.RollupOperation.Concatenate, '01234567890123456789,'), + (Schema.DescribeFieldResult)null, LREngine.RollupOperation.Concatenate, '01234567890123456789,'), 'test01234567890123456789,test01234567...', 'Lost01234567890123456789,Won012345678...'); } @@ -685,7 +756,7 @@ private class TestLREngine { new LREngine.RollupSummaryField( Schema.SObjectType.Account.fields.Description, Schema.SObjectType.Opportunity.fields.StageName, - null, LREngine.RollupOperation.Concatenate, ','), + (Schema.DescribeFieldResult)null, LREngine.RollupOperation.Concatenate, ','), 'test,test,test', 'Lost,Won,Won'); } @@ -696,7 +767,7 @@ private class TestLREngine { new LREngine.RollupSummaryField( Schema.SObjectType.Account.fields.Description, Schema.SObjectType.Opportunity.fields.StageName, - null, LREngine.RollupOperation.Concatenate, 'BR()'), + (Schema.DescribeFieldResult)null, LREngine.RollupOperation.Concatenate, 'BR()'), 'test\ntest\ntest', 'Lost\nWon\nWon'); } @@ -712,6 +783,150 @@ private class TestLREngine { 'Won,Won,Lost'); } + static testMethod void testRollupConcatenateOrderByMultipleAscendingNullsFirst() { + List order = new List(); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Type, LREngine.SortOrder.ASCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Amount, LREngine.SortOrder.ASCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Name, LREngine.SortOrder.ASCENDING, false)); + + testRollup2( + new LREngine.RollupSummaryField( + Schema.SObjectType.Account.fields.Description, + Schema.SObjectType.Opportunity.fields.StageName, + order, + LREngine.RollupOperation.Concatenate, ','), + 'blue,red,yellow', + 'orange,green,purple'); + } + + static testMethod void testRollupConcatenateOrderByMultipleAscendingNullsLast() { + List order = new List(); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, true)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Type, LREngine.SortOrder.ASCENDING, true)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Amount, LREngine.SortOrder.ASCENDING, true)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Name, LREngine.SortOrder.ASCENDING, true)); + + testRollup2( + new LREngine.RollupSummaryField( + Schema.SObjectType.Account.fields.Description, + Schema.SObjectType.Opportunity.fields.StageName, + order, + LREngine.RollupOperation.Concatenate, ','), + 'red,yellow,blue', + 'orange,green,purple'); + } + + static testMethod void testRollupConcatenateOrderByMultipleDescendingNullsFirst() { + List order = new List(); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Type, LREngine.SortOrder.DESCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Amount, LREngine.SortOrder.DESCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Name, LREngine.SortOrder.DESCENDING, false)); + + testRollup2( + new LREngine.RollupSummaryField( + Schema.SObjectType.Account.fields.Description, + Schema.SObjectType.Opportunity.fields.StageName, + order, + LREngine.RollupOperation.Concatenate, ','), + 'blue,yellow,red', + 'purple,green,orange'); + } + + static testMethod void testRollupConcatenateOrderByMultipleDescendingNullsLast() { + List order = new List(); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, true)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Type, LREngine.SortOrder.DESCENDING, true)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Amount, LREngine.SortOrder.DESCENDING, true)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Name, LREngine.SortOrder.DESCENDING, true)); + + testRollup2( + new LREngine.RollupSummaryField( + Schema.SObjectType.Account.fields.Description, + Schema.SObjectType.Opportunity.fields.StageName, + order, + LREngine.RollupOperation.Concatenate, ','), + 'yellow,red,blue', + 'purple,green,orange'); + } + + private static void assertOrdering(LREngine.Ordering o, String orderBy, Boolean useAsSpecifiedString, Schema.DescribeFieldResult field, LREngine.SortOrder direction, Boolean nullsLast) + { + System.assertEquals(orderBy, useAsSpecifiedString ? o.toAsSpecifiedString() : o.toString()); + System.assertEquals(field, o.getField()); + System.assertEquals(direction, o.getDirection()); + System.assertEquals(nullsLast, o.getNullsLast()); + } + + static testMethod void testOrderingStringification() + { + LREngine.Ordering o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate); + assertOrdering(o, 'CloseDate ASC NULLS FIRST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING); + assertOrdering(o, 'CloseDate ASC NULLS FIRST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING); + assertOrdering(o, 'CloseDate DESC NULLS FIRST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, null, false); + assertOrdering(o, 'CloseDate ASC NULLS FIRST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, null, true); + assertOrdering(o, 'CloseDate ASC NULLS LAST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, true); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + assertOrdering(o, 'CloseDate ASC NULLS FIRST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, true); + assertOrdering(o, 'CloseDate ASC NULLS LAST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, true); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false); + assertOrdering(o, 'CloseDate DESC NULLS FIRST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, true); + assertOrdering(o, 'CloseDate DESC NULLS LAST', false, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, true); + + try { + o = new LREngine.Ordering(null); + System.assert(false, 'Expecting an exception'); + } + catch (LREngine.BadOrderingStateException e) { + System.assertEquals('field cannot be null.', e.getMessage()); + } + } + + static testMethod void testOrderingAsSpecifiedStringification() + { + LREngine.Ordering o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate); + assertOrdering(o, 'CloseDate', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING); + assertOrdering(o, 'CloseDate ASC', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING); + assertOrdering(o, 'CloseDate DESC', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, null, false); + assertOrdering(o, 'CloseDate NULLS FIRST', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, null, true); + assertOrdering(o, 'CloseDate NULLS LAST', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, true); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + assertOrdering(o, 'CloseDate ASC NULLS FIRST', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, true); + assertOrdering(o, 'CloseDate ASC NULLS LAST', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, true); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false); + assertOrdering(o, 'CloseDate DESC NULLS FIRST', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false); + + o = new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, true); + assertOrdering(o, 'CloseDate DESC NULLS LAST', true, Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, true); + } + /** * Current default behavior of LREngine is to build the order by clause based on the following * 1) LookupField then by @@ -780,12 +995,82 @@ private class TestLREngine { System.assertEquals('Won', reloadedAcc2.get(rollupField2.master.getName())); } + /** + * Current default behavior of LREngine is to build the order by clause based on the following + * 1) LookupField then by + * 2) For each RollupSummaryField in context (in the order specified in the context) + * a) if detailOrderBy is specified use detailOrderBy.getName() + * b) else use detail.getName() + * + * This results in all queries having an order by even if one is not specified. + * Also, results in summary fields after the first having their order by influenced by previous summary fields + * + * For example, if two rollup summary fields are in the context as follows: + * 1) Order By Amount + * 2) Order By CloseDate + * + * A single SOQL will be executed with an order by of AccountId, Amount, CloseDate + */ + static testMethod void testMultipleRollupsDifferentFieldWithDifferentMultipleFieldsOrderBy() { + // create seed data + prepareData(); + + // force the 'Lost' Opportunity to be the oldest to demonstrate that + // even when ordering by CloseDate, order by will be based on AccountId, Amount, CloseDate + // since summaries are in the same context and context applies order by fields + // using "Then By" approach + Opportunity makeOldest = [SELECT Id, CloseDate FROM Opportunity WHERE AccountId = :acc2.Id AND Name = 'o2Acc2' LIMIT 1]; + makeOldest.CloseDate = System.today().addMonths(-24); + update makeOldest; + + // assert that the oldest opportunity is the 400 one that we just changed and that its + // stage name is lost + Opportunity assertOldest = [SELECT Id, Amount, StageName FROM Opportunity WHERE AccountId = :acc2.Id ORDER BY AccountId,CloseDate LIMIT 1]; + System.assertEquals(400, assertOldest.Amount); + System.assertEquals('Lost', assertOldest.StageName); + + LREngine.Context ctx = new LREngine.Context(Account.SobjectType, + Opportunity.SobjectType, + Schema.SObjectType.Opportunity.fields.AccountId); + + List order1 = new List { new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Amount, LREngine.SortOrder.DESCENDING, false), new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.ASCENDING, false) }; + LREngine.RollupSummaryField rollupField1 = + new LREngine.RollupSummaryField( + Schema.SObjectType.Account.fields.Description, + Schema.SObjectType.Opportunity.fields.StageName, + order1, + LREngine.RollupOperation.Concatenate, ',' + ); + ctx.add(rollupField1); + List order2 = new List { new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false), new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Amount, LREngine.SortOrder.ASCENDING, false) }; + LREngine.RollupSummaryField rollupField2 = + new LREngine.RollupSummaryField( + Schema.SObjectType.Account.fields.Sic, + Schema.SObjectType.Opportunity.fields.StageName, + order2, + LREngine.RollupOperation.First, null + ); + ctx.add(rollupField2); + + SObject[] masters = LREngine.rollUp(ctx, detailRecords); + + Map mastersById = new Map(masters); + Account reloadedAcc1 = (Account)mastersById.get(acc1.Id); + Account reloadedAcc2 = (Account)mastersById.get(acc2.Id); + System.assertEquals(2, masters.size()); + System.assertEquals('test,test,test', reloadedAcc1.get(rollupField1.master.getName())); + System.assertEquals('test', reloadedAcc1.get(rollupField2.master.getName())); + System.assertEquals('Lost,Won,Won', reloadedAcc2.get(rollupField1.master.getName())); + // the oldest is 'Lost' but due to Then By approach, the oldest should be 'Won' + System.assertEquals('Lost', reloadedAcc2.get(rollupField2.master.getName())); + } + static testMethod void testRollupConcatenateNoDelimiter() { testRollup( new LREngine.RollupSummaryField( Schema.SObjectType.Account.fields.Description, Schema.SObjectType.Opportunity.fields.StageName, - null, LREngine.RollupOperation.Concatenate, null), + (Schema.DescribeFieldResult)null, LREngine.RollupOperation.Concatenate, null), 'testtesttest', 'LostWonWon'); } @@ -795,7 +1080,7 @@ private class TestLREngine { new LREngine.RollupSummaryField( Schema.SObjectType.Account.fields.Description, Schema.SObjectType.Opportunity.fields.StageName, - null, LREngine.RollupOperation.Concatenate_Distinct, ','), + (Schema.DescribeFieldResult)null, LREngine.RollupOperation.Concatenate_Distinct, ','), 'test', 'Lost,Won'); } @@ -811,6 +1096,23 @@ private class TestLREngine { 'Won,Lost'); } + static testMethod void testRollupConcatenateDistinctWithMultipleFieldsOrderBy() { + List order = new List(); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Amount, LREngine.SortOrder.DESCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.CloseDate, LREngine.SortOrder.DESCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Type, LREngine.SortOrder.DESCENDING, false)); + order.add(new LREngine.Ordering(Schema.SObjectType.Opportunity.fields.Name, LREngine.SortOrder.DESCENDING, false)); + + testRollup( + new LREngine.RollupSummaryField( + Schema.SObjectType.Account.fields.Description, + Schema.SObjectType.Opportunity.fields.StageName, + order, + LREngine.RollupOperation.Concatenate_Distinct, ','), + 'test', + 'Lost,Won'); + } + static testMethod void testRollupFirst() { testRollup( new LREngine.RollupSummaryField( @@ -887,5 +1189,26 @@ private class TestLREngine { System.assertEquals(2, masters.size()); System.assertEquals(expected1, reloadedAcc1.get(rollupField.master.getName())); System.assertEquals(expected2, reloadedAcc2.get(rollupField.master.getName())); - } + } + + static private void testRollup2(LREngine.RollupSummaryField rollupField, String expected1, String expected2) { + + prepareData2(); + + LREngine.Context ctx = new LREngine.Context( + Account.SobjectType, + Opportunity.SobjectType, + Schema.SObjectType.Opportunity.fields.AccountId); + + ctx.add(rollupField); + + SObject[] masters = LREngine.rollUp(ctx, detailRecords2); + + Map mastersById = new Map(masters); + Account reloadedAcc3 = (Account)mastersById.get(acc3.Id); + Account reloadedAcc4 = (Account)mastersById.get(acc4.Id); + System.assertEquals(2, masters.size()); + System.assertEquals(expected1, reloadedAcc3.get(rollupField.master.getName())); + System.assertEquals(expected2, reloadedAcc4.get(rollupField.master.getName())); + } } \ No newline at end of file diff --git a/rolluptool/src/classes/Utilities.cls b/rolluptool/src/classes/Utilities.cls index cf74e4b3..29908737 100644 --- a/rolluptool/src/classes/Utilities.cls +++ b/rolluptool/src/classes/Utilities.cls @@ -63,4 +63,58 @@ public class Utilities { String namespace = namespace(); return String.isEmpty(namespace) ? '' : (namespace + '__'); } + + public static List parseOrderByClause(String orderByClause, Map fields) + { + if (String.isBlank(orderByClause)) { + return null; + } + + List orderByFields = new List(); + List orderByClauseFields = orderByClause.split(','); + for(String field :orderByClauseFields) { + orderByFields.add(parseOrderByField(field, fields)); + } + + return orderByFields; + } + + // Regular expression for Order By Clause + // Case-Insensitive pattern + // Group 1 - Field Name (required) + // Group 2 - ASC/DESC (optional) + // Group 3 - NULLS FIRST (optional) + // Group 4 - NULLS (required if Group 3 != null) + // Group 5 - FIRST (required if Group 3 != null) + private static Pattern orderByPattern = Pattern.compile('^(?i)[\\s]*([\\w]*)[\\s]*(ASC|DESC)?[\\s]*((NULLS)[\\s]*(FIRST|LAST))?[\\s]*$'); + private static LREngine.Ordering parseOrderByField(String orderByField, Map fields) + { + Matcher matcher = orderByPattern.matcher(orderByField); + if (!matcher.matches() || matcher.groupCount() != 5) { + throw new OrderByInvalidException('Invalid order by clause.'); + } + + // need to validate the field against Describe Information + String fieldName = matcher.group(1); + if (String.isBlank(fieldName) || !fields.containsKey(fieldName)) { + // this error message should likely be something more like 'Invalid field in order by clause' + // since we support one or more fields. However, to ensure backwards compat and avoid breaking + // tests that might exist out in the wild, making this error message the same as what it + // would have been when only a single field was supported. + throw new OrderByInvalidException('Field does not exist.'); + } + SObjectField sobjField = fields.get(fieldName); + + // regex enforces that ordering be null, ASC or DESC + String ordering = matcher.group(2); + LREngine.SortOrder sortOrder = (ordering == null) ? null : (ordering == 'DESC' ? LREngine.SortOrder.DESCENDING : LREngine.SortOrder.ASCENDING); + + // regex enforces that firstLast be null, FIRST or LAST + String firstLast = matcher.group(5); + Boolean nullsLast = (firstLast == null) ? null : (firstLast == 'LAST'); + + return new LREngine.Ordering(sobjField.getDescribe(), sortOrder, nullsLast); + } + + public class OrderByInvalidException extends Exception {} } diff --git a/rolluptool/src/objects/LookupRollupSummary__c.object b/rolluptool/src/objects/LookupRollupSummary__c.object index 9a5c6646..229d2c56 100644 --- a/rolluptool/src/objects/LookupRollupSummary__c.object +++ b/rolluptool/src/objects/LookupRollupSummary__c.object @@ -237,10 +237,14 @@ FieldToOrderBy__c false + Examples: +1) Amount__c +2) Amount__c, Color__c ASC +3) Amount__c NULLS LAST, Color__c DESC NULLS LAST false - Only applicable when using the Concatenate, Concatenate Distinct, Last and First aggregate operations. Defaults to the field given in Field to Aggregate. + Only applicable when using the Concatenate, Concatenate Distinct, Last and First aggregate operations. Defaults to the field given in Field to Aggregate. Supports multiple fields (comma separated) with optional ASC/DESC and/or NULLS FIRST/LAST. - 80 + 255 false false Text From 06a7a4c74409554374e195fb0a56878d50c13ebb Mon Sep 17 00:00:00 2001 From: Jon Davis Date: Sat, 29 Aug 2015 09:34:34 -0700 Subject: [PATCH 2/4] issue #216 - add tests to ensure coverage for case (in)sensitivity --- rolluptool/src/classes/RollupServiceTest5.cls | 40 +++++++++++++++++++ rolluptool/src/classes/Utilities.cls | 2 + 2 files changed, 42 insertions(+) diff --git a/rolluptool/src/classes/RollupServiceTest5.cls b/rolluptool/src/classes/RollupServiceTest5.cls index 08564d00..a82d93ca 100644 --- a/rolluptool/src/classes/RollupServiceTest5.cls +++ b/rolluptool/src/classes/RollupServiceTest5.cls @@ -257,6 +257,16 @@ private class RollupServiceTest5 { new List { false }); } + @IsTest + private static void testParseOrderByFieldAndASCDirectionLowered() { + List order = Utilities.parseOrderByClause('Amount__c asc', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + @IsTest private static void testParseOrderByFieldAndDESCDirection() { List order = Utilities.parseOrderByClause('Amount__c DESC', LookupChild__c.sObjectType.getDescribe().fields.getMap()); @@ -267,6 +277,16 @@ private class RollupServiceTest5 { new List { false }); } + @IsTest + private static void testParseOrderByFieldAndDESCDirectionLowered() { + List order = Utilities.parseOrderByClause('Amount__c desc', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.DESCENDING }, + new List { false }); + } + @IsTest private static void testParseOrderByFieldAndNullsFirst() { List order = Utilities.parseOrderByClause('Amount__c NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); @@ -277,6 +297,16 @@ private class RollupServiceTest5 { new List { false }); } + @IsTest + private static void testParseOrderByFieldAndNullsFirstLowered() { + List order = Utilities.parseOrderByClause('Amount__c nulls first', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + @IsTest private static void testParseOrderByFieldAndNullsLast() { List order = Utilities.parseOrderByClause('Amount__c NULLS LAST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); @@ -287,6 +317,16 @@ private class RollupServiceTest5 { new List { true }); } + @IsTest + private static void testParseOrderByFieldAndNullsLastLowered() { + List order = Utilities.parseOrderByClause('Amount__c nulls last', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { true }); + } + @IsTest private static void testParseOrderByFieldAndASCDirectionAndNullsFirst() { List order = Utilities.parseOrderByClause('Amount__c ASC NULLS FIRST', LookupChild__c.sObjectType.getDescribe().fields.getMap()); diff --git a/rolluptool/src/classes/Utilities.cls b/rolluptool/src/classes/Utilities.cls index 29908737..e8bd0539 100644 --- a/rolluptool/src/classes/Utilities.cls +++ b/rolluptool/src/classes/Utilities.cls @@ -106,10 +106,12 @@ public class Utilities { SObjectField sobjField = fields.get(fieldName); // regex enforces that ordering be null, ASC or DESC + // == operator is case-insensitive String ordering = matcher.group(2); LREngine.SortOrder sortOrder = (ordering == null) ? null : (ordering == 'DESC' ? LREngine.SortOrder.DESCENDING : LREngine.SortOrder.ASCENDING); // regex enforces that firstLast be null, FIRST or LAST + // == operator is case-insensitive String firstLast = matcher.group(5); Boolean nullsLast = (firstLast == null) ? null : (firstLast == 'LAST'); From 3f2d31e9a7b9be7c7384e1886a82b9fa9fcbd6bc Mon Sep 17 00:00:00 2001 From: Jon Davis Date: Sat, 29 Aug 2015 20:08:34 -0700 Subject: [PATCH 3/4] 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(); From 26c4b3acd9e6ec508672c38e5ddbf5751103059a Mon Sep 17 00:00:00 2001 From: Jon Davis Date: Sat, 29 Aug 2015 21:10:39 -0700 Subject: [PATCH 4/4] Issue #216 - additional tests for order by parsing field name case-(in)sensitivity --- rolluptool/src/classes/RollupServiceTest5.cls | 20 +++++++++++++++++++ rolluptool/src/classes/Utilities.cls | 14 +++++++++++++ 2 files changed, 34 insertions(+) diff --git a/rolluptool/src/classes/RollupServiceTest5.cls b/rolluptool/src/classes/RollupServiceTest5.cls index a82d93ca..06b53280 100644 --- a/rolluptool/src/classes/RollupServiceTest5.cls +++ b/rolluptool/src/classes/RollupServiceTest5.cls @@ -247,6 +247,26 @@ private class RollupServiceTest5 { new List { false }); } + @IsTest + private static void testParseOrderByFieldOnlyLowered() { + List order = Utilities.parseOrderByClause('amount__c', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + + @IsTest + private static void testParseOrderByFieldOnlyMixedCase() { + List order = Utilities.parseOrderByClause('aMoUnT__c', LookupChild__c.sObjectType.getDescribe().fields.getMap()); + assertOrdering( order, + 1, + new List { LookupChild__c.Amount__c.getDescribe() }, + new List { LREngine.SortOrder.ASCENDING }, + new List { false }); + } + @IsTest private static void testParseOrderByFieldAndASCDirection() { List order = Utilities.parseOrderByClause('Amount__c ASC', LookupChild__c.sObjectType.getDescribe().fields.getMap()); diff --git a/rolluptool/src/classes/Utilities.cls b/rolluptool/src/classes/Utilities.cls index e8bd0539..fd458e03 100644 --- a/rolluptool/src/classes/Utilities.cls +++ b/rolluptool/src/classes/Utilities.cls @@ -64,6 +64,20 @@ public class Utilities { return String.isEmpty(namespace) ? '' : (namespace + '__'); } + /** + * Parse a string that follows the SOQL Order By standard + * + * @param orderByClause - order by clause (not including ORDER BY keywords) following standard at https://developer.salesforce.com/docs/atlas.en-us.soql_sosl.meta/soql_sosl/sforce_api_calls_soql_select_orderby.htm + * @param fields - Describe Fields obtained from getDescribe().fields.getMap() + * + * @return list containing one LREngine.Ordering element for each field in the order by clause + * + * @remark In order to ensure proper case-insensitive identification of fields in the clause, the fields passed + * in MUST be generated from Describe information. If the map is generated elsewhere or even using describe keySet() + * field identification will be case sensitive and could result in unexpected failure to parse clause + * + * @throw OrderByInvalidException when order by is not in proper format or field is not found in fields + **/ public static List parseOrderByClause(String orderByClause, Map fields) { if (String.isBlank(orderByClause)) {