v1.7.8 - RollupCalcItemReplacer bugfix (#661)

* Fixes #660 by ensuring calls to split don't accidentally greedily split when detecting whether or not RollupCalcItemReplacer needs to run

Author: jamessimone

README.md

@@ -24,11 +24,11 @@ As well, don't miss [the Wiki](../../wiki), which includes even more info for co
 
 ## Deployment & Setup
 
-<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfrlAAC">
+<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfrvAAC">
   <img alt="Deploy to Salesforce" src="./media/deploy-package-to-prod.png">
 </a>
 
-<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfrlAAC">
+<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfrvAAC">
   <img alt="Deploy to Salesforce Sandbox" src="./media/deploy-package-to-sandbox.png">
 </a>
 <br/>

extra-tests/classes/RollupCalcItemReplacerTests.cls

@@ -218,6 +218,7 @@ private class RollupCalcItemReplacerTests {
   @IsTest
   static void doesNotReplaceForCustomTypeFields() {
     List<Account> accounts = [SELECT Id FROM Account];
+    Integer currentQueryCount = Limits.getQueries();
     RollupCalcItemReplacer replacer = new RollupCalcItemReplacer(
       new RollupControl__mdt(IsRollupLoggingEnabled__c = true, ReplaceCalcItemsAsyncWhenOverCount__c = 3)
     );
@@ -227,5 +228,22 @@ private class RollupCalcItemReplacerTests {
 
     Assert.areEqual(1, records.size());
     Assert.isTrue(replacer.hasProcessedMetadata(metas, accounts));
+    Assert.areEqual(currentQueryCount, Limits.getQueries());
+  }
+
+  @IsTest
+  static void doesNotReplaceForCustomTypeFieldsWithPrecedingCharacters() {
+    List<Account> accounts = [SELECT Id, AType__c FROM Account];
+    Integer currentQueryCount = Limits.getQueries();
+    RollupCalcItemReplacer replacer = new RollupCalcItemReplacer(
+      new RollupControl__mdt(IsRollupLoggingEnabled__c = true, ReplaceCalcItemsAsyncWhenOverCount__c = 3)
+    );
+    List<Rollup__mdt> metas = new List<Rollup__mdt>{ new Rollup__mdt(CalcItemWhereClause__c = 'AType__c = \'hello\'', CalcItem__c = 'Account') };
+
+    List<SObject> records = replacer.replace(accounts, metas);
+
+    Assert.areEqual(1, records.size());
+    Assert.isTrue(replacer.hasProcessedMetadata(metas, accounts));
+    Assert.areEqual(currentQueryCount, Limits.getQueries());
   }
 }

extra-tests/objects/Account/fields/AType__c.field-meta.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<CustomField xmlns="http://soap.sforce.com/2006/04/metadata">
+    <fullName>AType__c</fullName>
+    <externalId>false</externalId>
+    <label>Type With Another Character Before It</label>
+    <length>255</length>
+    <required>false</required>
+    <trackTrending>false</trackTrending>
+    <type>Text</type>
+    <unique>false</unique>
+</CustomField>

package.json

@@ -1,6 +1,6 @@
 {
   "name": "apex-rollup",
-  "version": "1.7.7",
+  "version": "1.7.8",
   "description": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
   "repository": {
     "type": "git",

rollup-namespaced/README.md

@@ -18,12 +18,12 @@ For more info, see the base `README`.
 
 ## Deployment & Setup
 
-<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfrqAAC">
+<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Ofs0AAC">
   <img alt="Deploy to Salesforce"
        src="./media/deploy-package-to-prod.png">
 </a>
 
-<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008OfrqAAC">
+<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008Ofs0AAC">
   <img alt="Deploy to Salesforce Sandbox"
        src="./media/deploy-package-to-sandbox.png">
 </a>

rollup-namespaced/sfdx-project.json

@@ -4,8 +4,8 @@
             "default": true,
             "package": "apex-rollup-namespaced",
             "path": "rollup-namespaced/source/rollup",
-            "versionName": "More multi-currency bugfixes",
-            "versionNumber": "1.2.6.0",
+            "versionName": "Properly handle namespaced and other Type__c fields in RollupCalcItemReplacer",
+            "versionNumber": "1.2.7.0",
             "versionDescription": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
             "releaseNotesUrl": "https://github.com/jamessimone/apex-rollup/releases/latest",
             "unpackagedMetadata": {
@@ -31,6 +31,7 @@
         "apex-rollup-namespaced@1.2.3": "04t6g000008Off4AAC",
         "apex-rollup-namespaced@1.2.4": "04t6g000008OfqYAAS",
         "apex-rollup-namespaced@1.2.5": "04t6g000008OfrCAAS",
-        "apex-rollup-namespaced@1.2.6": "04t6g000008OfrqAAC"
+        "apex-rollup-namespaced@1.2.6": "04t6g000008OfrqAAC",
+        "apex-rollup-namespaced@1.2.7": "04t6g000008Ofs0AAC"
     }
 }

rollup/core/classes/RollupCalcItemReplacer.cls

@@ -150,20 +150,21 @@ public without sharing class RollupCalcItemReplacer {
     if (calcItems.isEmpty()) {
       return calcItems;
     }
-    Map<Id, SObject> idToCalcItem = RollupFieldInitializer.Current.createSafeMap(calcItems);
     Set<String> baseFields = this.baseQueryFields.get('' + calcItems[0].getSObjectType());
-    if (baseFields?.isEmpty() == false) {
-      SObject firstItem = calcItems[0];
-      String queryString = RollupQueryBuilder.Current.getQuery(firstItem.getSObjectType(), new List<String>(baseFields), 'Id', '=');
-      List<SObject> calcItemsWithReplacement = this.repo.setQuery(queryString).setArg(calcItems).get();
-      Map<String, Schema.SObjectField> fieldNameToDescribe = firstItem.getSObjectType().getDescribe().fields.getMap();
-      for (SObject calcItemWithReplacement : calcItemsWithReplacement) {
-        if (idToCalcItem.containsKey(calcItemWithReplacement.Id)) {
-          SObject calcItem = idToCalcItem.get(calcItemWithReplacement.Id);
-          for (String baseField : baseFields) {
-            Schema.SObjectfield baseFieldToken = fieldNameToDescribe.get(baseField);
-            idToCalcItem.put(calcItem.Id, replaceField(calcItem, baseFieldToken, calcItemWithReplacement.get(baseField)));
-          }
+    if (baseFields?.isEmpty() != false) {
+      return calcItems;
+    }
+    Map<Id, SObject> idToCalcItem = RollupFieldInitializer.Current.createSafeMap(calcItems);
+    SObject firstItem = calcItems[0];
+    String queryString = RollupQueryBuilder.Current.getQuery(firstItem.getSObjectType(), new List<String>(baseFields), 'Id', '=');
+    List<SObject> calcItemsWithReplacement = this.repo.setQuery(queryString).setArg(calcItems).get();
+    Map<String, Schema.SObjectField> fieldNameToDescribe = firstItem.getSObjectType().getDescribe().fields.getMap();
+    for (SObject calcItemWithReplacement : calcItemsWithReplacement) {
+      if (idToCalcItem.containsKey(calcItemWithReplacement.Id)) {
+        SObject calcItem = idToCalcItem.get(calcItemWithReplacement.Id);
+        for (String baseField : baseFields) {
+          Schema.SObjectfield baseFieldToken = fieldNameToDescribe.get(baseField);
+          idToCalcItem.put(calcItem.Id, replaceField(calcItem, baseFieldToken, calcItemWithReplacement.get(baseField)));
         }
       }
     }
@@ -249,7 +250,8 @@ public without sharing class RollupCalcItemReplacer {
 
   private void processWhereClauseForDownstreamEvals(Schema.SObjectType sObjectType, Rollup__mdt metadata, RollupEvaluator.WhereFieldEvaluator whereEval) {
     for (String whereClause : whereEval.getWhereClauses()) {
-      Boolean hasTypeField = whereClause.split(TYPE_FIELD + '(?!__r)').size() > 1;
+      // the period needs to be escaped when splitting, otherwise this can match namespaced fields accidentally
+      Boolean hasTypeField = whereClause.split('\\' + TYPE_FIELD + '(?!__r)').size() > 1;
       Boolean hasOwnerField = whereClause.contains(OWNER);
       if (hasTypeField == false && hasOwnerField == false) {
         continue;

rollup/core/classes/RollupCurrencyInfo.cls

@@ -1,18 +1,36 @@
 @SuppressWarnings('PMD.ExcessiveParameterList,PMD.PropertyNamingConventions')
 public without sharing virtual class RollupCurrencyInfo implements RollupLogger.ToStringObject {
   public static final String CURRENCY_ISO_CODE_FIELD_NAME = 'CurrencyIsoCode';
-  @TestVisible
-  private static final RollupRepository REPOSITORY = new RollupRepository(RollupRepository.RunAsMode.SYSTEM_LEVEL);
-  private static final Map<String, SObject> TRANSFORMED_MULTICURRENCY_CALC_ITEMS = new Map<String, SObject>();
-  private static final Set<String> HASHED_ITEM_VALUES = new Set<String>();
 
+  @TestVisible
+  private static Boolean hasLoadedDatedCurrencyInfo = false;
   @TestVisible
   private static List<RollupCurrencyInfo> mockBasicCurrencies;
   @TestVisible
   private static List<RollupCurrencyInfo> mockDatedCurrencies;
 
   @TestVisible
-  private static Boolean hasLoadedDatedCurrencyInfo = false;
+  private static RollupRepository REPOSITORY {
+    get {
+      REPOSITORY = REPOSITORY ?? new RollupRepository(RollupRepository.RunAsMode.SYSTEM_LEVEL);
+      return REPOSITORY;
+    }
+    set;
+  }
+  private static Map<String, SObject> TRANSFORMED_MULTICURRENCY_CALC_ITEMS {
+    get {
+      TRANSFORMED_MULTICURRENCY_CALC_ITEMS = TRANSFORMED_MULTICURRENCY_CALC_ITEMS ?? new Map<String, SObject>();
+      return TRANSFORMED_MULTICURRENCY_CALC_ITEMS;
+    }
+    set;
+  }
+  private static Set<String> HASHED_ITEM_VALUES {
+    get {
+      HASHED_ITEM_VALUES = HASHED_ITEM_VALUES ?? new Set<String>();
+      return HASHED_ITEM_VALUES;
+    }
+    set;
+  }
 
   private static Date minDatedCurrencyLookup {
     get {
@@ -52,7 +70,7 @@ public without sharing virtual class RollupCurrencyInfo implements RollupLogger.
   // Can't be Schema.SObjectType => Schema.SObjectField because not all orgs have OppLineItems/Splits
   // technically there's a hierarchy for OpportunityLineItem that goes Opportunity.CloseDate > ServiceDate > (ProductDate || ScheduleDate)
   // but because this can be configured on a per-rollup basis, it's fine to leave Opportunity.CloseDate as the default since it can be overridden
-  private static final Map<String, List<String>> DATED_MULTICURRENCY_SUPPORTED_OBJECTS {
+  private static Map<String, List<String>> DATED_MULTICURRENCY_SUPPORTED_OBJECTS {
     get {
       DATED_MULTICURRENCY_SUPPORTED_OBJECTS = DATED_MULTICURRENCY_SUPPORTED_OBJECTS ??
         new Map<String, List<String>>{
@@ -66,7 +84,7 @@ public without sharing virtual class RollupCurrencyInfo implements RollupLogger.
     set;
   }
 
-  private static final Set<String> DATED_CURRENCY_QUERIES {
+  private static Set<String> DATED_CURRENCY_QUERIES {
     get {
       DATED_CURRENCY_QUERIES = DATED_CURRENCY_QUERIES ?? new Set<String>();
       return DATED_CURRENCY_QUERIES;
@@ -83,23 +101,23 @@ public without sharing virtual class RollupCurrencyInfo implements RollupLogger.
     set;
   }
 
-  private static final RollupCurrencyInfo FALLBACK_INFO {
+  private static RollupCurrencyInfo FALLBACK_INFO {
     get {
       FALLBACK_INFO = FALLBACK_INFO ?? new RollupCurrencyInfo().setDefaults(1);
       return FALLBACK_INFO;
     }
     set;
   }
 
-  private static final Map<String, RollupCurrencyInfo> CURRENCY_ISO_CODE_TO_CURRENCY {
+  private static Map<String, RollupCurrencyInfo> CURRENCY_ISO_CODE_TO_CURRENCY {
     get {
       CURRENCY_ISO_CODE_TO_CURRENCY = CURRENCY_ISO_CODE_TO_CURRENCY ?? getCurrencyMap();
       return CURRENCY_ISO_CODE_TO_CURRENCY;
     }
     set;
   }
 
-  private static final Map<Schema.SObjectType, Map<String, Schema.SObjectField>> TYPE_TO_FIELDS {
+  private static Map<Schema.SObjectType, Map<String, Schema.SObjectField>> TYPE_TO_FIELDS {
     get {
       TYPE_TO_FIELDS = TYPE_TO_FIELDS ?? new Map<Schema.SObjectType, Map<String, Schema.SObjectField>>();
       return TYPE_TO_FIELDS;

rollup/core/classes/RollupLogger.cls

@@ -2,7 +2,7 @@
 global without sharing virtual class RollupLogger implements ILogger {
   @TestVisible
   // this gets updated via the pipeline as the version number gets incremented
-  private static final String CURRENT_VERSION_NUMBER = 'v1.7.7';
+  private static final String CURRENT_VERSION_NUMBER = 'v1.7.8';
   private static final System.LoggingLevel FALLBACK_LOGGING_LEVEL = System.LoggingLevel.DEBUG;
   private static final RollupPlugin PLUGIN = new RollupPlugin();
 

sfdx-project.json

@@ -5,8 +5,8 @@
             "package": "apex-rollup",
             "path": "rollup",
             "scopeProfiles": true,
-            "versionName": "More multi-currency bugfixes",
-            "versionNumber": "1.7.7.0",
+            "versionName": "Properly handle namespaced and other Type__c fields in RollupCalcItemReplacer",
+            "versionNumber": "1.7.8.0",
             "versionDescription": "Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.",
             "releaseNotesUrl": "https://github.com/jamessimone/apex-rollup/releases/latest",
             "unpackagedMetadata": {
@@ -104,6 +104,7 @@
         "apex-rollup": "0Ho6g000000TNcOCAW",
         "apex-rollup@1.7.5": "04t6g000008OfqTAAS",
         "apex-rollup@1.7.6": "04t6g000008Ofr7AAC",
-        "apex-rollup@1.7.7": "04t6g000008OfrlAAC"
+        "apex-rollup@1.7.7": "04t6g000008OfrlAAC",
+        "apex-rollup@1.7.8": "04t6g000008OfrvAAC"
     }
 }