diff --git a/rolluptool/src/classes/LREngine.cls b/rolluptool/src/classes/LREngine.cls index 4df02349..8a2ba059 100644 --- a/rolluptool/src/classes/LREngine.cls +++ b/rolluptool/src/classes/LREngine.cls @@ -182,9 +182,7 @@ public class LREngine { // #4 Order by clause fields // i.e. Amount ASC NULLS FIRST, Name DESC NULL LAST - // in order to maintain backwards compat, if there is no orderby specified and if the context contains only one rollupsummaryfield - // add the detail field to the orderby. See https://github.com/afawcett/declarative-lookup-rollup-summaries/issues/239#issuecomment-136122959. - String orderByClause = ctx.lookupField.getName() + (String.isBlank(ctx.detailOrderByClause) ? (ctx.fieldsToRoll.size() == 1 ? (',' + ctx.fieldsToRoll[0].detail.getName()) : '') : (',' + ctx.detailOrderByClause)); + String orderByClause = ctx.lookupField.getName() + (String.isBlank(ctx.detailOrderByClause) ? '' : (',' + ctx.detailOrderByClause)); // build approprite soql for this rollup context String soql = diff --git a/rolluptool/src/classes/RollupService.cls b/rolluptool/src/classes/RollupService.cls index d71d8ef4..8edf3ce8 100644 --- a/rolluptool/src/classes/RollupService.cls +++ b/rolluptool/src/classes/RollupService.cls @@ -133,7 +133,7 @@ global with sharing class RollupService // Process each context (parent child relationship) and its associated rollups Map masterRecords = new Map(); - for(LREngine.Context ctx : createLREngineContexts(lookups).values()) + for(LREngine.Context ctx : createLREngineContexts(lookups)) { // Produce a set of master Id's applicable to this context (parent only) Set ctxMasterIds = new Set(); @@ -214,7 +214,7 @@ global with sharing class RollupService // Process each context (parent child relationship) and its associated rollups Map masterRecords = new Map(); - for(LREngine.Context ctx : createLREngineContexts(lookups).values()) + for(LREngine.Context ctx : createLREngineContexts(lookups)) { // Produce a set of master Id's applicable to this context (parent only) Set ctxMasterIds = new Set(); @@ -341,11 +341,11 @@ global with sharing class RollupService } // Group lookups by parent and relationship into LREngine ctx's - Map engineCtxByParentRelationship = createLREngineContexts(lookups.values()); + List engineCtxByParentRelationship = createLREngineContexts(lookups.values()); // Process each context (parent child relationship) and its associated rollups Map masterRecords = new Map(); - for(LREngine.Context ctx : engineCtxByParentRelationship.values()) + for(LREngine.Context ctx : engineCtxByParentRelationship) { Set masterIds = parentIdsByParentType.get(ctx.master.getDescribe().getName()); // This maybe null if the reconcilation check above to match master ID to lookup parent failed @@ -433,7 +433,7 @@ global with sharing class RollupService // Process rollup List lookups = new RollupSummariesSelector().selectById(lookupIds); Map masterRecords = new Map(); - for(LREngine.Context ctx : createLREngineContexts(lookups).values()) + for(LREngine.Context ctx : createLREngineContexts(lookups)) { // Produce a set of master Id's applicable to this context (parent only) Set ctxMasterIds = new Set(); @@ -730,7 +730,7 @@ global with sharing class RollupService // Process each context (parent child relationship) and its associated rollups Map masterRecords = new Map(); - for(LREngine.Context ctx : createLREngineContexts(runnowLookups).values()) + for(LREngine.Context ctx : createLREngineContexts(runnowLookups)) { // Produce a set of master Id's applicable to this context (parent only) Set ctxMasterIds = new Set(); @@ -778,76 +778,192 @@ global with sharing class RollupService /** * Method takes a list of Lookups and creates the most optimum list of LREngine.Context's to execute **/ - private static Map createLREngineContexts(List lookups) + private static List createLREngineContexts(List lookups) { - // Group lookups by parent and relationship into LREngine ctx's - Map gd = Schema.getGlobalDescribe(); - Map> gdFields = new Map>(); - Map engineCtxByParentRelationship = - new Map(); + // list of contexts generated + List engineContexts = new List(); + // map of context criteria (except for order by) to a map of orderby with context + Map> engineCtxByParentRelationship = + new Map>(); + // list of rollupsummaryfields that do not have a specific orderby + List noOrderByLookupWrappers = new List(); Map scheduledItems = new Map(); for(LookupRollupSummary__c lookup : lookups) { - // Resolve (and cache) SObjectType's and fields for Parent and Child objects - SObjectType parentObjectType = gd.get(lookup.ParentObject__c); - if(parentObjectType==null) - throw RollupServiceException.invalidRollup(lookup); - Map parentFields = gdFields.get(parentObjectType); - if(parentFields==null) - gdFields.put(parentObjectType, ((parentFields = parentObjectType.getDescribe().fields.getMap()))); - SObjectType childObjectType = gd.get(lookup.ChildObject__c); - if(childObjectType==null) - throw RollupServiceException.invalidRollup(lookup); - Map childFields = gdFields.get(childObjectType); - if(childFields==null) - gdFields.put(childObjectType, ((childFields = childObjectType.getDescribe().fields.getMap()))); - SObjectField fieldToAggregate = childFields.get(lookup.FieldToAggregate__c); - SObjectField relationshipField = childFields.get(lookup.RelationshipField__c); - SObjectField aggregateResultField = parentFields.get(lookup.AggregateResultField__c); - if(fieldToAggregate==null || relationshipField==null || aggregateResultField==null) - throw RollupServiceException.invalidRollup(lookup); - - // Summary field definition used by LREngine - LREngine.RollupSummaryField rsf = - new LREngine.RollupSummaryField( - aggregateResultField.getDescribe(), - fieldToAggregate.getDescribe(), - RollupSummaries.OPERATION_PICKLIST_TO_ENUMS.get(lookup.AggregateOperation__c), - lookup.ConcatenateDelimiter__c); - - // Apply sharing rules when calculating the rollup? - LREngine.SharingMode sharingMode = - lookup.CalculationSharingMode__c == null || lookup.CalculationSharingMode__c == 'User' ? - LREngine.SharingMode.User : LREngine.SharingMode.System_x; - - // Determine if an LREngine Context has been created for this parent child relationship, filter combination or underlying query type and sharing mode? - String rsfType = rsf.isAggregateBasedRollup() ? 'aggregate' : 'query'; - String orderBy = String.isBlank(Lookup.FieldToOrderBy__c) ? '' : Lookup.FieldToOrderBy__c; - // Lowering case on Describable fields is only required for Legacy purposes since LookupRollupSummary__c records - // will be updated with describe names on insert/update moving forward. - // Ideally this would not be needed to save CPU cycles but including to ensure context is properly re-used when possible for - // rollups that have not been updated/inserted after the insert/update enhancement is applied - // Unable to lower RelationShipCriteria__c because of field value case-(in)sensitivity configuration - String contextKey = lookup.ParentObject__c.toLowerCase() + '#' + lookup.RelationshipField__c.toLowerCase() + '#' + lookup.RelationShipCriteria__c + '#' + rsfType + '#' + sharingMode + '#' + orderBy.toLowerCase(); - - LREngine.Context lreContext = engineCtxByParentRelationship.get(contextKey); - if(lreContext==null) - { - // Construct LREngine.Context - lreContext = new LREngine.Context( - parentObjectType, // parent object - childObjectType, // child object - relationshipField.getDescribe(), // relationship field name - lookup.RelationShipCriteria__c, - sharingMode, - lookup.FieldToOrderBy__c); - engineCtxByParentRelationship.put(contextKey, lreContext); - } - // Add the lookup - lreContext.add(rsf); - } - return engineCtxByParentRelationship; + // if no order is specified, we'll defer context identification until after we build contexts for + // all the lookups that contain a specific order by + LookupRollupSummaryWrapper lookupWrapper = createLSFWrapper(lookup); + if (!lookupWrapper.OrderByRequired) { + // add to the list of lookups that do not have an order by specified + noOrderByLookupWrappers.add(lookupWrapper); + } else { + // Note that context key here will not include the order by + String contextKey = getContextKey(lookupWrapper); + // obtain the map of orderby to context based on contextKey + Map lreContextByOrderBy = engineCtxByParentRelationship.get(contextKey); + // if we don't have a map yet, create it + if (lreContextByOrderBy == null) { + lreContextByOrderBy = new Map(); + engineCtxByParentRelationship.put(contextKey, lreContextByOrderBy); + } + String orderBy = lookupWrapper.OrderByClause; // this will be the FieldToOrderBy__c from the LookupRollupSummary__c + // Lowering case on Describable fields is only required for Legacy purposes since LookupRollupSummary__c records + // will be updated with describe names on insert/update moving forward. + String orderByKey = orderBy.toLowerCase(); + // obtain the context for this orderby key + LREngine.Context lreContext = lreContextByOrderBy.get(orderByKey); + // if not context yet, create one + if(lreContext==null) + { + // Construct LREngine.Context + lreContext = createContext(lookupWrapper); + lreContextByOrderBy.put(orderByKey, lreContext); + engineContexts.add(lreContext); + } + // Add the rollup summary field to the context + lreContext.add(lookupWrapper.RollupSummaryField); + } + } + + // now that we have contexts built for all lookups that have a specific order by + // we iterate those that do not and pick the first one that matches all other criteria + // if there is not one that matches other criteria, we create a new context + if (!noOrderByLookupWrappers.isEmpty()) + { + // loop through our list of lookups that do not have an orderby specified + for (LookupRollupSummaryWrapper lookupWrapper :noOrderByLookupWrappers) + { + // Note that context key here will not include the order by + String contextKey = getContextKey(lookupWrapper); + // obtain the map of orderby to context based on contextKey + Map lreContextByOrderBy = engineCtxByParentRelationship.get(contextKey); + // if we don't have a map yet, create it + if (lreContextByOrderBy == null) { + lreContextByOrderBy = new Map(); + engineCtxByParentRelationship.put(contextKey, lreContextByOrderBy); + } + + LREngine.Context lreContext = null; + if (lreContextByOrderBy.isEmpty()) + { + // if we do not have any contexts yet then create one + // when created, the OrderBy on the context will be set null since no order by was specified + // Construct LREngine.Context + lreContext = createContext(lookupWrapper); + // ensure key is lowered + lreContextByOrderBy.put('####NOORDERBY####', lreContext); + engineContexts.add(lreContext); + } + else + { + // since no orderby was specified we can execute in a non-deterministic manner + // so for that reason we'll just grab the first one + // Note - In an effort to support some backwards compat here, we could try to find a context + // whos first orderby field (ASC NULLS FIRST) matches this lookups FieldToAggregate__c + // however for performance reasons, we'll forgo that and just pick the first + lreContext = lreContextByOrderBy.values()[0]; + } + + // Add the rollup summary field to the context + lreContext.add(lookupWrapper.RollupSummaryField); + } + } + + return engineContexts; + } + + private static LREngine.Context createContext(LookupRollupSummaryWrapper lookupWrapper) + { + return new LREngine.Context( + lookupWrapper.ParentObjectType, // parent object + lookupWrapper.ChildObjectType, // child object + lookupWrapper.RelationshipField.getDescribe(), // relationship field name + lookupWrapper.Lookup.RelationShipCriteria__c, + lookupWrapper.SharingMode, + lookupWrapper.OrderByClause); + } + + private static SObjectType getSObjectType(String sObjectName) + { + fflib_SObjectDescribe describe = fflib_SObjectDescribe.getDescribe(sObjectName); + return (describe == null) ? null : describe.getSObjectType(); + + } + + private static Map getSObjectTypeFields(SObjectType sObjectType) + { + return fflib_SObjectDescribe.getDescribe(sObjectType).getFieldsMap(); + } + + private class LookupRollupSummaryWrapper + { + public LookupRollupSummary__c Lookup; + public Schema.SObjectType ParentObjectType; + public Schema.SObjectType ChildObjectType; + public Schema.SObjectField FieldToAggregate; + public Schema.SObjectField RelationshipField; + public Schema.SObjectField AggregateResultField; + public LREngine.SharingMode SharingMode; + public Boolean OrderByRequired; // true if FieldToOrderBy__c is not blank + public String OrderByClause; // FieldToOrderBy__c if OrderByRequired is true; null otherwise + public LREngine.RollupSummaryField RollupSummaryField; + } + + private static LookupRollupSummaryWrapper createLSFWrapper(LookupRollupSummary__c lookup) + { + // Resolve (and cache) SObjectType's and fields for Parent and Child objects + SObjectType parentObjectType = getSObjectType(lookup.ParentObject__c); + if(parentObjectType==null) + throw RollupServiceException.invalidRollup(lookup); + Map parentFields = getSObjectTypeFields(parentObjectType); + SObjectType childObjectType = getSObjectType(lookup.ChildObject__c); + if(childObjectType==null) + throw RollupServiceException.invalidRollup(lookup); + Map childFields = getSObjectTypeFields(childObjectType); + SObjectField fieldToAggregate = childFields.get(lookup.FieldToAggregate__c); + SObjectField relationshipField = childFields.get(lookup.RelationshipField__c); + SObjectField aggregateResultField = parentFields.get(lookup.AggregateResultField__c); + if(fieldToAggregate==null || relationshipField==null || aggregateResultField==null) + throw RollupServiceException.invalidRollup(lookup); + + // Summary field definition used by LREngine + LREngine.RollupSummaryField rsf = + new LREngine.RollupSummaryField( + aggregateResultField.getDescribe(), + fieldToAggregate.getDescribe(), + RollupSummaries.OPERATION_PICKLIST_TO_ENUMS.get(lookup.AggregateOperation__c), + lookup.ConcatenateDelimiter__c); + + LookupRollupSummaryWrapper wrapper = new LookupRollupSummaryWrapper(); + wrapper.Lookup = lookup; + wrapper.ParentObjectType = parentObjectType; + wrapper.ChildObjectType = childObjectType; + wrapper.FieldToAggregate = fieldToAggregate; + wrapper.RelationshipField = relationshipField; + wrapper.AggregateResultField = aggregateResultField; + wrapper.SharingMode = + lookup.CalculationSharingMode__c == null || lookup.CalculationSharingMode__c == 'User' ? + LREngine.SharingMode.User : LREngine.SharingMode.System_x; + // if order by is not specified, orderby will be null else it will be FieldToOrderBy__c + wrapper.OrderByRequired = !String.isBlank(lookup.FieldToOrderBy__c); + wrapper.OrderByClause = wrapper.OrderByRequired ? lookup.FieldToOrderBy__c : null; + wrapper.RollupSummaryField = rsf; + + return wrapper; + } + + private static String getContextKey(LookupRollupSummaryWrapper lookupWrapper) + { + // Context Key Based On: ParentObject__c, RelationshipField__c, RelationShipCriteria__c, rsfType, SharingMode + // Note we do not include OrderBy here because orderby map is contained within the map of contextKeys + String rsfType = lookupWrapper.RollupSummaryField.isAggregateBasedRollup() ? 'aggregate' : 'query'; + // Lowering case on Describable fields is only required for Legacy purposes since LookupRollupSummary__c records + // will be updated with describe names on insert/update moving forward. + // Ideally this would not be needed to save CPU cycles but including to ensure context is properly re-used when possible for + // rollups that have not been updated/inserted after the insert/update enhancement is applied + // Unable to lower RelationShipCriteria__c because of field value case-(in)sensitivity configuration + return (lookupWrapper.Lookup.ParentObject__c.toLowerCase() + '#' + lookupWrapper.Lookup.RelationshipField__c.toLowerCase() + '#' + lookupWrapper.Lookup.RelationShipCriteria__c + '#' + rsfType + '#' + lookupWrapper.SharingMode); } /** diff --git a/rolluptool/src/classes/RollupServiceTest.cls b/rolluptool/src/classes/RollupServiceTest.cls index 9c9e9802..9a164c5e 100644 --- a/rolluptool/src/classes/RollupServiceTest.cls +++ b/rolluptool/src/classes/RollupServiceTest.cls @@ -1103,15 +1103,12 @@ private with sharing class RollupServiceTest } /** - * Current default behavior of LREngine is to build the order by clause based on the following - * 1) RelationshipField__c (context.lookupfield in LRE) then by - * 2) If FieldToOrderBy__c (context.detailOrderByClause in LRE) is blank: - * If Context RollupSummaryFields.size() == 1 (context.fieldsToRoll) add FieldToAggregate__c (context.fieldsToRoll[0].detail in LRE) - * If Context RollupSummaryFields.size() > 1 (context.fieldsToRoll) then do not add any additional fields to orderby - * 3) If FieldToOrderBy__c (context.detailOrderByClause in LRE) is not blank, add FieldToOrderBy__c to orderby + * Current default behavior of LREngine is to apply RelationshipField__c in the order by + * for all contexts. Beyond that, if no order by is specified, no orderby is applied. * * This results in all queries having an order by of at least RelationshipField__c even if no orderby - * is specified in FieldToOrderBy__c. + * is specified in FieldToOrderBy__c. Ordering by RelationshipField__c does not impact Query based rollup results + * and is only applied to assist in materializing the results on master records * * Current default behavior of DLRS is to build the context with all rollupsummaries * retrieving them ordered by ParentObject__c (Account) and then by RelationshipField__c (e.g. AccountId) @@ -1249,82 +1246,6 @@ private with sharing class RollupServiceTest System.assertEquals(expectedResultB, accountResult.Description); } - /** - * Test default behavior with no order by making sure order is based on fieldtoaggregate - * - * See comment above setupMultiRollupDifferentTypes for information on how orderby is established - * - * Note that there is no reliable method for testing the situation where multiple fields that do not have - * orderby specified and would share a context. The reason for this is that the order is - * non-deterministic and there is no way currently (without adding one) to evaluate the SOQL generated. - * Using debug statements, it's been verified that only RelationshipField__c is included in - * order by in this scenario. - */ - private testmethod static void testSingleQueryRollupNoOrderByVerifyDetailFieldIncludedInOrderBy() - { - // 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(); - - // Create a picklist rollup - 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 = null; - rollupSummary.AggregateOperation__c = RollupSummaries.AggregateOperation.Concatenate.name(); - rollupSummary.AggregateResultField__c = aggregateResultField; - rollupSummary.ConcatenateDelimiter__c = ';'; - rollupSummary.Active__c = true; - rollupSummary.CalculationMode__c = 'Realtime'; - insert rollupSummary; - - // 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, 'silver'); - children.add(child1); - SObject child2 = childType.newSObject(); - child2.put(relationshipField, parent.Id); - child2.put(aggregateField, 'orange'); - children.add(child2); - SObject child3 = childType.newSObject(); - child3.put(relationshipField, parent.Id); - child3.put(aggregateField, 'purple'); - 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('orange;purple;silver', (String) assertParents.get(parentA.id).get(aggregateResultField)); - System.assertEquals('orange;purple;silver', (String) assertParents.get(parentB.id).get(aggregateResultField)); - System.assertEquals('orange;purple;silver', (String) assertParents.get(parentC.id).get(aggregateResultField)); - } - private testmethod static void testPicklistRollup() { // Test supported? diff --git a/rolluptool/src/classes/RollupServiceTest4.cls b/rolluptool/src/classes/RollupServiceTest4.cls index 3cceaf12..62c60477 100644 --- a/rolluptool/src/classes/RollupServiceTest4.cls +++ b/rolluptool/src/classes/RollupServiceTest4.cls @@ -1028,8 +1028,11 @@ private class RollupServiceTest4 { * - Different FieldToAggregate__c * - Different Order By one that has no order by specified * - Effective values for all other fields same differing only by case used - * Should result in Two (2) contexts used, two SOQL for the rollup itself and 3 DML rows (1 for each + * Should result in One (1) context used, one SOQL for the rollup itself and 3 DML rows (1 for each * parent - DLRS combines updates to identical master record ids) + * + * The Context will be shared because the rollup that doesn't have an orderby (Rollup B) matches all other criteria + * on the other rollup (Rollup A) and therefore its added to Context for that rollup **/ private testmethod static void testLimitsAndContextsUsedMultipleQueryRollupsDifferByOperationAggResultFieldAggFieldCaseOrderByOneOrderByIsNull() { @@ -1126,23 +1129,21 @@ private class RollupServiceTest4 { // 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()); + // + One query on LookupChild__c for rollup A & B (single query because orderby matches even though one is not specified) + System.assertEquals(beforeQueries + 2, 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()); + System.assertEquals(beforeRows + 11, Limits.getQueryRows()); // + Nine rows for LookupChild__c (from the insert 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 - // Note that we are able to reliably assert rollup B because even though it does not - // have orderby specified because it will receive a default orderby of FieldToAggregate__c since it is the only - // rollup in the "no order by specified" context + // Note that we are able to reliably assert rollup B even though it does not + // have orderby specified because it will end up using the same context as Rollup A because Rollup B's FieldToAggregate__c + // matches the first field in the orderby of Rollup A 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)); @@ -1162,8 +1163,11 @@ private class RollupServiceTest4 { * - Different FieldToAggregate__c * - Different Order By two that have no order by specified * - Effective values for all other fields same differing only by case used - * Should result in Two (2) contexts used, two SOQL for the rollup itself and 3 DML rows (1 for each + * Should result in One (1) context used, one SOQL for the rollup itself and 3 DML rows (1 for each * parent - DLRS combines updates to identical master record ids) + * + * The Context will be shared because the two rollups that do not have an orderby (Rollup B and C) matches all other criteria + * on the other rollup (Rollup A) and therefore are added to Context for that rollup **/ private testmethod static void testLimitsAndContextsUsedMultipleQueryRollupsDifferByOperationAggResultFieldAggFieldCaseOrderByTwoOrderByIsNullSameFieldToAggregate() { @@ -1283,27 +1287,32 @@ private class RollupServiceTest4 { // Assert limits // + One query on Rollup object - // + One query on LookupChild__c for rollup A - // + One query on LookupChild__c for rollup B and for rollup C - System.assertEquals(beforeQueries + 3, Limits.getQueries()); + // + One query on LookupChild__c for rollup A, B & C + System.assertEquals(beforeQueries + 2, Limits.getQueries()); // + Three rows for Rollup object - // + Nine rows for LookupChild__c for rollup A - // + Nine rows for LookupChild__c for rollup B and for rollup C - System.assertEquals(beforeRows + 21, Limits.getQueryRows()); + // + Nine rows for LookupChild__c for rollup A, B & C + System.assertEquals(beforeRows + 12, Limits.getQueryRows()); // + Nine rows for LookupChild__c (from the insert statement itself) // + Three rows for LookupParent__c for rollup A & B & C (DLRS combined updates to identical master ids) System.assertEquals(beforeDMLRows + 12, Limits.getDMLRows()); // Assert rollups - // Note that we are not able to reliably assert rollups B & C because they do not have orderby specified - // and will share a context. Since more than one field will be involved in context no orderby will be applied beyond - // the RelationshipField__c - Map assertParents = new Map(Database.query(String.format('select id, {0}, {1} from {2}', new List{ aggregateResultField1, aggregateResultField2, parentObjectName }))); + // Note that we are able to reliably assert rollups B & C even though they do not have an order by specified + // because they same the same criteria as A and therefore will be added to the same context that was created for A + Map assertParents = new Map(Database.query(String.format('select id, {0}, {1}, {2} from {3}', new List{ aggregateResultField1, aggregateResultField2, aggregateResultField3, parentObjectName }))); System.assertEquals('Yellow', (String) assertParents.get(parentA.id).get(aggregateResultField1)); + System.assertEquals('lemon', (String) assertParents.get(parentA.id).get(aggregateResultField2)); + System.assertEquals('pear', (String) assertParents.get(parentA.id).get(aggregateResultField3)); + System.assertEquals('Yellow', (String) assertParents.get(parentB.id).get(aggregateResultField1)); + System.assertEquals('lemon', (String) assertParents.get(parentB.id).get(aggregateResultField2)); + System.assertEquals('pear', (String) assertParents.get(parentB.id).get(aggregateResultField3)); + System.assertEquals('Yellow', (String) assertParents.get(parentC.id).get(aggregateResultField1)); + System.assertEquals('lemon', (String) assertParents.get(parentC.id).get(aggregateResultField2)); + System.assertEquals('pear', (String) assertParents.get(parentC.id).get(aggregateResultField3)); } /** @@ -1313,8 +1322,11 @@ private class RollupServiceTest4 { * - Different AggregateResultField__c * - Different Order By two that have no order by specified * - Effective values for all other fields same differing only by case used - * Should result in Two (2) contexts used, two SOQL for the rollup itself and 3 DML rows (1 for each + * Should result in One (1) context used, one SOQL for the rollup itself and 3 DML rows (1 for each * parent - DLRS combines updates to identical master record ids) + * + * The Context will be shared because the two rollups that do not have an orderby (Rollup B and C) matches all other criteria + * on the other rollup (Rollup A) and therefore are added to Context for that rollup **/ private testmethod static void testLimitsAndContextsUsedMultipleQueryRollupsDifferByOperationFieldCaseOrderByTwoOrderByIsNullDifferentFieldToAggregate() { @@ -1431,27 +1443,32 @@ private class RollupServiceTest4 { // Assert limits // + One query on Rollup object - // + One query on LookupChild__c for rollup A - // + One query on LookupChild__c for rollup B and for rollup C - System.assertEquals(beforeQueries + 3, Limits.getQueries()); + // + One query on LookupChild__c for rollup A, B & C + System.assertEquals(beforeQueries + 2, Limits.getQueries()); // + Three rows for Rollup object - // + Nine rows for LookupChild__c for rollup A - // + Nine rows for LookupChild__c for rollup B and for rollup C - System.assertEquals(beforeRows + 21, Limits.getQueryRows()); + // + Nine rows for LookupChild__c for rollup A, B & C + System.assertEquals(beforeRows + 12, Limits.getQueryRows()); // + Nine rows for LookupChild__c (from the insert statement itself) // + Three rows for LookupParent__c for rollup A & B & C (DLRS combined updates to identical master ids) System.assertEquals(beforeDMLRows + 12, Limits.getDMLRows()); // Assert rollups - // Note that we are not able to reliably assert rollups B & C because they do not have orderby specified - // and will share a context. Since more than one field will be involved in context no orderby will be applied beyond - // the RelationshipField__c - Map assertParents = new Map(Database.query(String.format('select id, {0}, {1} from {2}', new List{ aggregateResultField1, aggregateResultField2, parentObjectName }))); + // Note that we are able to reliably assert rollups B & C even though they do not have an order by specified + // because they same the same criteria as A and therefore will be added to the same context that was created for A + Map assertParents = new Map(Database.query(String.format('select id, {0}, {1}, {2} from {3}', new List{ aggregateResultField1, aggregateResultField2, aggregateResultField3, parentObjectName }))); System.assertEquals('apple', (String) assertParents.get(parentA.id).get(aggregateResultField1)); + System.assertEquals('pear', (String) assertParents.get(parentA.id).get(aggregateResultField2)); + System.assertEquals('Blue', (String) assertParents.get(parentA.id).get(aggregateResultField3)); + System.assertEquals('apple', (String) assertParents.get(parentB.id).get(aggregateResultField1)); + System.assertEquals('pear', (String) assertParents.get(parentB.id).get(aggregateResultField2)); + System.assertEquals('Blue', (String) assertParents.get(parentB.id).get(aggregateResultField3)); + System.assertEquals('apple', (String) assertParents.get(parentC.id).get(aggregateResultField1)); + System.assertEquals('pear', (String) assertParents.get(parentC.id).get(aggregateResultField2)); + System.assertEquals('Blue', (String) assertParents.get(parentC.id).get(aggregateResultField3)); } /** @@ -1631,8 +1648,8 @@ private class RollupServiceTest4 { // Assert rollups // Note that we are not able to reliably assert rollups A, B, C, D & E because they do not have orderby specified - // and will share a context. Since more than one field will be involved in context no orderby will be applied beyond - // the RelationshipField__c + // and will share a context. Contexts generated for "no orderby specified" do not have an orderby + // and therefore we cannot assert values since order is non-deterministic Map assertParents = new Map(Database.query(String.format('select id, {0}, {1} from {2}', new List{ aggregateResultField1, aggregateResultField2, parentObjectName }))); System.assertEquals(parents.size(), assertParents.size()); } diff --git a/rolluptool/src/classes/TestLREngine.cls b/rolluptool/src/classes/TestLREngine.cls index 30c88c1b..f6b442da 100644 --- a/rolluptool/src/classes/TestLREngine.cls +++ b/rolluptool/src/classes/TestLREngine.cls @@ -786,104 +786,6 @@ private class TestLREngine { 'Won,Won,Lost'); } - /** - * Current default behavior of LREngine is to build the order by clause based on the following - * 1) LookupField then by - * 2) If detailOrderByClause is blank: - * If context.fieldsToRoll.size() == 1 add context.fieldsToRoll[0].detail - * If context.fieldsToRoll.size() > 1 then do not add any additional fields to orderby - * 3) If detailOrderByClause is not blank, add detailOrderByClause to orderby - * - * This results in all queries having an order by of at least lookupfield even if no orderby - * is specified on the context. - * - * The context.fieldsToRoll.size() == 1 scenario is included strictly to maintain as much backwards compatibility - * as possible from previous release. See https://github.com/afawcett/declarative-lookup-rollup-summaries/issues/239#issuecomment-136122959 - * - * Note that there is no reliable method for testing that when multiple fields are included in context - * and no order by is specified that the only field included in the orderby is the lookupfield. To do - * this the SOQL would have to be exposed from LREngine.rollup and then parsed/evaluated and this seems - * kind of klunky. Using debug statements, it's been verified that only lookupfield is included in - * order by in this scenario. - * - * Test that when no orderby is specified and only a single field is included in the context - * that the data is ordered by the fieldtoaggregate (StageName) - */ - static testMethod void testSingleRollupSummaryFieldWithNoOrderByConcatVerifyDetailFieldIncludedInOrderBy() { - // create seed data - prepareData2(); - - LREngine.Context ctx = new LREngine.Context(Account.SobjectType, - Opportunity.SobjectType, - Schema.SObjectType.Opportunity.fields.AccountId); - - LREngine.RollupSummaryField rollupField = - new LREngine.RollupSummaryField( - Schema.SObjectType.Account.fields.Description, - Schema.SObjectType.Opportunity.fields.StageName, - LREngine.RollupOperation.Concatenate, ',' - ); - 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('blue,red,yellow', reloadedAcc3.get(rollupField.master.getName())); - System.assertEquals('green,orange,purple', reloadedAcc4.get(rollupField.master.getName())); - } - - /** - * Current default behavior of LREngine is to build the order by clause based on the following - * 1) LookupField then by - * 2) If detailOrderByClause is blank: - * If context.fieldsToRoll.size() == 1 add context.fieldsToRoll[0].detail - * If context.fieldsToRoll.size() > 1 then do not add any additional fields to orderby - * 3) If detailOrderByClause is not blank, add detailOrderByClause to orderby - * - * This results in all queries having an order by of at least lookupfield even if no orderby - * is specified on the context. - * - * The context.fieldsToRoll.size() == 1 scenario is included strictly to maintain as much backwards compatibility - * as possible from previous release. See https://github.com/afawcett/declarative-lookup-rollup-summaries/issues/239#issuecomment-136122959 - * - * Note that there is no reliable method for testing that when multiple fields are included in context - * and no order by is specified that the only field included in the orderby is the lookupfield. To do - * this the SOQL would have to be exposed from LREngine.rollup and then parsed/evaluated and this seems - * kind of klunky. Using debug statements, it's been verified that only lookupfield is included in - * order by in this scenario. - * - * Test that when no orderby is specified and only a single field is included in the context - * that the data is ordered by the fieldtoaggregate (Name) - */ - static testMethod void testSingleRollupSummaryFieldWithNoOrderByLastVerifyDetailFieldIncludedInOrderBy() { - // create seed data - prepareData2(); - - LREngine.Context ctx = new LREngine.Context(Account.SobjectType, - Opportunity.SobjectType, - Schema.SObjectType.Opportunity.fields.AccountId); - - LREngine.RollupSummaryField rollupField = - new LREngine.RollupSummaryField( - Schema.SObjectType.Account.fields.Description, - Schema.SObjectType.Opportunity.fields.Name, - LREngine.RollupOperation.Last - ); - 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('o3Acc3', reloadedAcc3.get(rollupField.master.getName())); - System.assertEquals('o3Acc4', reloadedAcc4.get(rollupField.master.getName())); - } - static testMethod void testRollupConcatenateOrderByMultipleAscendingNullsFirst() { testRollup2( 'CloseDate, Type, Amount, Name',