From 3c5e5d64f7b7bd21112116d535239ed374ac8a72 Mon Sep 17 00:00:00 2001
From: James Simone <16430727+jamessimone@users.noreply.github.com>
Date: Fri, 17 Mar 2023 09:38:15 -0600
Subject: [PATCH] v0.1.2 - Bulkification updates for flow (#10)
* Bulkification updates for flow
---
README.md | 4 +-
core/classes/FlowRoundRobinAssigner.cls | 31 ++++-----
core/classes/FlowRoundRobinAssignerTests.cls | 1 -
core/classes/QueryAssigner.cls | 10 ++-
core/classes/RoundRobinRepository.cls | 64 ++++++++++++-------
.../RoundRobinFlowIntegrationTests.cls | 14 ++--
package.json | 2 +-
sfdx-project.json | 7 +-
8 files changed, 82 insertions(+), 51 deletions(-)
diff --git a/README.md b/README.md
index fa479de..c84c910 100644
--- a/README.md
+++ b/README.md
@@ -5,12 +5,12 @@
## Deployment
-<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008fjhFAAQ">
+<a href="https://login.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008fjhyAAA">
<img alt="Deploy to Salesforce"
src="./media/deploy-package-to-prod.png">
</a>
-<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008fjhFAAQ">
+<a href="https://test.salesforce.com/packaging/installPackage.apexp?p0=04t6g000008fjhyAAA">
<img alt="Deploy to Salesforce Sandbox"
src="./media/deploy-package-to-sandbox.png">
</a>
diff --git a/core/classes/FlowRoundRobinAssigner.cls b/core/classes/FlowRoundRobinAssigner.cls
index 90c29b4..3e3d953 100644
--- a/core/classes/FlowRoundRobinAssigner.cls
+++ b/core/classes/FlowRoundRobinAssigner.cls
@@ -1,8 +1,6 @@
global without sharing class FlowRoundRobinAssigner {
@TestVisible
private static RoundRobinAssigner.IAssignmentRepo stubAssignmentRepo;
- @TestVisible
- private static Boolean hasBeenUpdated = false;
private static final Set<Id> PROCESSED_RECORD_IDS = new Set<Id>();
private static final FlowRoundRobinAssigner SELF = new FlowRoundRobinAssigner();
@@ -26,16 +24,23 @@ global without sharing class FlowRoundRobinAssigner {
@InvocableMethod(category='Round Robin' label='Round robin records')
global static void assign(List<FlowInput> flowInputs) {
- if (hasBeenUpdated == false) {
- for (FlowInput input : flowInputs) {
- if (input.recordsToRoundRobin?.isEmpty() != false && input.recordToRoundRobin != null) {
- input.recordsToRoundRobin = new List<SObject>{ input.recordToRoundRobin };
- }
- if (input.recordsToRoundRobin.isEmpty() == false) {
- SELF.trackAssignedIds(input);
- SELF.roundRobin(input);
- }
+ FlowInput bulkifiedInput;
+ for (FlowInput input : flowInputs) {
+ if (bulkifiedInput == null) {
+ bulkifiedInput = input;
+ }
+ if (input.recordsToRoundRobin?.isEmpty() != false && input.recordToRoundRobin != null) {
+ input.recordsToRoundRobin = new List<SObject>{ input.recordToRoundRobin };
}
+ bulkifiedInput.recordsToRoundRobin.addAll(input.recordsToRoundRobin);
+ }
+
+ if (bulkifiedInput?.recordsToRoundRobin.isEmpty() == false) {
+ SELF.trackAssignedIds(bulkifiedInput);
+ SELF.roundRobin(bulkifiedInput);
+ }
+ if (bulkifiedInput?.updateRecords == true) {
+ update bulkifiedInput.recordsToRoundRobin;
}
}
@@ -44,10 +49,6 @@ global without sharing class FlowRoundRobinAssigner {
RoundRobinAssigner.IAssignmentRepo assignmentRepo = this.getAssignmentRepo(input);
RoundRobinAssigner.Details assignmentDetails = this.getAssignmentDetails(input);
new RoundRobinAssigner(assignmentRepo, assignmentDetails).assignOwners(input.recordsToRoundRobin);
- if (input.updateRecords) {
- update input.recordsToRoundRobin;
- hasBeenUpdated = true;
- }
}
private void validateInput(FlowInput input) {
diff --git a/core/classes/FlowRoundRobinAssignerTests.cls b/core/classes/FlowRoundRobinAssignerTests.cls
index 3c41b53..b33cbad 100644
--- a/core/classes/FlowRoundRobinAssignerTests.cls
+++ b/core/classes/FlowRoundRobinAssignerTests.cls
@@ -60,7 +60,6 @@ private class FlowRoundRobinAssignerTests {
FlowRoundRobinAssigner.assign(new List<FlowRoundRobinAssigner.FlowInput>{ input });
- System.assertEquals(true, FlowRoundRobinAssigner.hasBeenUpdated);
System.assertEquals(UserInfo.getUserId(), cpa.OwnerId);
}
diff --git a/core/classes/QueryAssigner.cls b/core/classes/QueryAssigner.cls
index 925150c..9b348f3 100644
--- a/core/classes/QueryAssigner.cls
+++ b/core/classes/QueryAssigner.cls
@@ -1,9 +1,17 @@
public without sharing class QueryAssigner implements RoundRobinAssigner.IAssignmentRepo {
+ private static final Map<String, List<SObject>> QUERY_TO_RECORDS = new Map<String, List<SObject>>();
private final List<Id> validAssignmentIds;
public QueryAssigner(String query, String assignmentFieldName) {
Set<Id> assignmentIds = new Set<Id>();
- List<SObject> matchingRecords = Database.query(query);
+ List<SObject> matchingRecords;
+ if (QUERY_TO_RECORDS.containsKey(query)) {
+ matchingRecords = QUERY_TO_RECORDS.get(query);
+ } else {
+ matchingRecords = Database.query(query);
+ QUERY_TO_RECORDS.put(query, matchingRecords);
+ }
+
for (SObject matchingRecord : matchingRecords) {
assignmentIds.add((Id) matchingRecord.get(assignmentFieldName));
}
diff --git a/core/classes/RoundRobinRepository.cls b/core/classes/RoundRobinRepository.cls
index 75d54e2..72b801e 100644
--- a/core/classes/RoundRobinRepository.cls
+++ b/core/classes/RoundRobinRepository.cls
@@ -1,7 +1,6 @@
public without sharing class RoundRobinRepository extends AbstractCacheRepo {
private static Map<String, RoundRobin__c> CACHED_ASSIGNMENTS;
- @SuppressWarnings('PMD.ApexCRUDViolation')
public void accept(IThreadSafeCacheVisitor visitor, List<SObject> records) {
RoundRobin__c currentAssignment = this.getCurrentAssignment(visitor.getVisitKey());
visitor.visitRecords(records, currentAssignment);
@@ -47,39 +46,56 @@ public without sharing class RoundRobinRepository extends AbstractCacheRepo {
@SuppressWarnings('PMD.ApexCRUDViolation')
private Boolean commitUpdatedAssignment(RoundRobin__c assignment) {
- Boolean wasCommitSuccessful = true;
Map<String, RoundRobin__c> currentCache = this.getCachedAssignments();
if (
currentCache.containsKey(assignment.Name) &&
currentCache.get(assignment.Name).LastUpdated__c > CACHED_ASSIGNMENTS.get(assignment.Name).LastUpdated__c
) {
- assignment = currentCache.get(assignment.Name);
- wasCommitSuccessful = false;
- } else {
- assignment.LastUpdated__c = System.now();
- /**
- * integration tests with after save Flows have shown something unfortunate:
- * though the second (recursive) call to the assigner is spawned in a second transaction
- * the RoundRobin__c.getAll() still doesn't contain the Id of the inserted record (for the times where the assignment
- * is being run for the first time).
- * That means that we can't just call "upsert", and instead have to do this goofy
- * song and dance to ensure the Id is appended correctly
- */
- if (assignment.Id == null) {
- List<RoundRobin__c> existingAssignments = [SELECT Id FROM RoundRobin__c WHERE Name = :assignment.Name];
- if (existingAssignments.isEmpty() == false) {
- assignment.Id = existingAssignments[0].Id;
- }
+ return false;
+ }
+ assignment.LastUpdated__c = System.now();
+ /**
+ * integration tests with after save Flows have shown something unfortunate:
+ * though the second (recursive) call to the assigner is spawned in a second transaction
+ * the RoundRobin__c.getAll() call still doesn't contain the Id of the inserted record (for the times where the assignment
+ * is being run for the first time).
+ * That means that we can't just call "upsert", and instead have to do this goofy
+ * song and dance to ensure the Id is appended correctly
+ */
+ if (assignment.Id == null) {
+ List<RoundRobin__c> existingAssignments = [SELECT Id FROM RoundRobin__c WHERE Name = :assignment.Name];
+ if (existingAssignments.isEmpty() == false) {
+ assignment.Id = existingAssignments[0].Id;
}
- if (assignment.Id != null) {
- update assignment;
- } else {
- insert assignment;
+ }
+ if (assignment.Id != null) {
+ try {
+ /**
+ * if two separate threads are trying to round robin at the same time, the LastUpdated__c check above
+ * isn't enough to ensure write-safety, and unfortunately FOR UPDATE is the only mutex Apex offers
+ * as a write-safe guarantee. One downside (among many) is that FOR UPDATE frequently throws; another is
+ * that another locking thread can release early - let's protect against both those eventualities
+ */
+ RoundRobin__c lockedAssignment = [SELECT Id, Name, LastUpdated__c FROM RoundRobin__c WHERE Id = :assignment.Id FOR UPDATE];
+ if (lockedAssignment.LastUpdated__c >= assignment.LastUpdated__c) {
+ // lock was released early, but the existing Index__c now almost certainly has stale values in it
+ // re-round robin to get the now-correct values
+ return false;
+ }
+ lockedAssignment.Index__c = assignment.Index__c;
+ lockedAssignment.LastUpdated__c = assignment.LastUpdated__c;
+ update lockedAssignment;
+ // purely for the map assignment, below
+ assignment = lockedAssignment;
+ } catch (DmlException ex) {
+ return false;
}
+ } else {
+ insert assignment;
}
CACHED_ASSIGNMENTS.put(assignment.Name, assignment);
- return wasCommitSuccessful;
+ return true;
}
private Map<String, RoundRobin__c> getCachedAssignments() {
diff --git a/integration-tests/classes/RoundRobinFlowIntegrationTests.cls b/integration-tests/classes/RoundRobinFlowIntegrationTests.cls
index 1d4f83b..3702687 100644
--- a/integration-tests/classes/RoundRobinFlowIntegrationTests.cls
+++ b/integration-tests/classes/RoundRobinFlowIntegrationTests.cls
@@ -14,10 +14,16 @@ private class RoundRobinFlowIntegrationTests {
roundRobinUser.LastName = 'roundRobinUser';
insert roundRobinUser;
- Lead lead = new Lead(LastName = 'Test Assignment', Company = 'Test');
- insert lead;
+ List<Lead> leads = new List<Lead>();
+ // stress test!
+ for (Integer index = 0; index < 200; index++) {
+ leads.add(new Lead(LastName = 'Assignment ' + index, Company = 'Round Robin'));
+ }
+ insert leads;
- lead = [SELECT Id, OwnerId FROM Lead];
- System.assertEquals(roundRobinUser.Id, lead.OwnerId);
+ leads = [SELECT Id, OwnerId FROM Lead];
+ for (Lead lead : leads) {
+ System.assertEquals(roundRobinUser.Id, lead.OwnerId);
+ }
}
}
diff --git a/package.json b/package.json
index e7182bc..3d9af25 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "salesforce-round-robin",
- "version": "0.1.1",
+ "version": "0.1.2",
"description": "Round robin records in Salesforce (SFDC) using Flow or Apex. Performant, fair, fast assignment with configurable user pools",
"repository": {
"type": "git",
diff --git a/sfdx-project.json b/sfdx-project.json
index 0195c0f..6e1962f 100644
--- a/sfdx-project.json
+++ b/sfdx-project.json
@@ -5,8 +5,8 @@
"definitionFile": "config/project-scratch-def.json",
"package": "salesforce-round-robin",
"path": "core",
- "versionName": "Reverts mutex due to bulkification issues with Flow, adds more safety for recursive Flow transactions",
- "versionNumber": "0.1.1.0",
+ "versionName": "Better flow bulkification for updates, reinstated mutex",
+ "versionNumber": "0.1.2.0",
"versionDescription": "Invocable & Apex-ready Round Robin Assigner for Salesforce Flow, Process Builder, Apex and more",
"releaseNotesUrl": "https://github.com/jamessimone/salesforce-round-robin/releases/latest"
},
@@ -21,6 +21,7 @@
"salesforce-round-robin": "0Ho6g000000GnClCAK",
"salesforce-round-robin@0.0.4-0": "04t6g000008SjZEAA0",
"salesforce-round-robin@0.1.0-0": "04t6g000008SjpyAAC",
- "salesforce-round-robin@0.1.1-0": "04t6g000008fjhFAAQ"
+ "salesforce-round-robin@0.1.1-0": "04t6g000008fjhFAAQ",
+ "salesforce-round-robin@0.1.2-0": "04t6g000008fjhyAAA"
}
}
\ No newline at end of file