-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes, cleanup dead code, static save option and flexible unitOfWork injection #14
base: master
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
@IsTest | ||
public class Random { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class did not have almost coverage and is only used within @istest classes, hence adding it too |
||
|
||
public String string() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ public class Account_t extends DomainBuilder { | |
|
||
public Account_t() { | ||
super(Account.SObjectType); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. invisible whitespace removal |
||
name('Acme Corp'); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,30 @@ | ||
@IsTest | ||
public class DomainBuilder_Test { | ||
@IsTest | ||
private static void staticHappyPath() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test method to demostrate the possibility of calling the save() static method, similar to persist() but without requiring an instance, since it only actually operates on static variables it is logically meaningful. |
||
|
||
Contact_t joe = new Contact_t().first('Joe').last('Harris'); | ||
|
||
new Account_t() | ||
.name('Acme Corp') | ||
.add( new Contact_t() ) | ||
.add( new Opportunity_t() | ||
.amount(1000) | ||
.closes(2019, 12) | ||
.contact(joe)); | ||
|
||
DomainBuilder.save(); | ||
|
||
System.assertEquals(2, [SELECT Count() FROM Account]); | ||
System.assertEquals(1, [SELECT Count() FROM Opportunity]); | ||
System.assertEquals(2, [SELECT Count() FROM Contact]); | ||
System.assertEquals(1, [SELECT Count() FROM OpportunityContactRole]); | ||
} | ||
|
||
@IsTest | ||
private static void happyPath() { | ||
|
||
Contact_t joe = new Contact_t().first('Ron').last('Harris'); | ||
Contact_t joe = new Contact_t().first('Joe').last('Harris'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the variable name is joe, so firstName should not be "Ron" |
||
|
||
new Account_t() | ||
.name('Acme Corp') | ||
|
@@ -15,7 +35,7 @@ public class DomainBuilder_Test { | |
.contact(joe)) | ||
.persist(); | ||
|
||
System.assertEquals(2, [SELECT Count() FROM Account]); | ||
System.assertEquals(2, [SELECT Count() FROM Account]); | ||
System.assertEquals(1, [SELECT Count() FROM Opportunity]); | ||
System.assertEquals(2, [SELECT Count() FROM Contact]); | ||
System.assertEquals(1, [SELECT Count() FROM OpportunityContactRole]); | ||
|
@@ -72,7 +92,7 @@ public class DomainBuilder_Test { | |
|
||
.persist(); | ||
|
||
System.assertEquals(1, [SELECT Count() FROM Account]); | ||
System.assertEquals(2, [SELECT Count() FROM Account]); | ||
System.assertEquals(1, [SELECT Count() FROM Opportunity]); | ||
System.assertEquals(2, [SELECT Count() FROM Contact]); | ||
} | ||
|
@@ -192,14 +212,14 @@ public class DomainBuilder_Test { | |
Opportunity_t opp = new Opportunity_t(); // 1x insert | ||
|
||
new Account_t() // 1x insert | ||
.add(new Contact_t()) // 1x insert | ||
.add(new Contact_t()) // 0x insert together with "con" | ||
.add(con) // 0x insert | ||
.add(opp // 0x insert | ||
.add(con)) // 1x insert OpportunityContactRole | ||
.persist(); // 1x setSavepoint | ||
// Verify | ||
System.assertEquals(6, Limits.getDmlRows()); | ||
|
||
// Verify | ||
System.assertEquals(5, Limits.getDmlRows()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion was throwing an error because only 5 DML rows are consumed, not 6. SetSavePoint counts against DML Statements, but not against DML Rows. Added extra assert with comments for demostration |
||
System.assertEquals(1, [SELECT Count() FROM Account]); | ||
System.assertEquals(2, [SELECT Count() FROM Contact]); | ||
System.assertEquals(1, [SELECT Count() FROM Opportunity]); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,36 +3,31 @@ public abstract class DomainBuilder { | |
private static DirectedGraph graph = new DirectedGraph(); | ||
private static Set<DomainBuilder> objects = new Set<DomainBuilder>(); | ||
|
||
private Boolean isSetupObject; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove unused related dead code |
||
private Map<SObjectField, DomainBuilder> parentByRelationship = new Map<SObjectField, DomainBuilder>(); | ||
private Map<SObjectField, Map<SObjectField, List<DomainBuilder>>> relationshipsToSync | ||
= new Map<SObjectField, Map<SObjectField, List<DomainBuilder>>>(); | ||
public SObject record; | ||
public SObjectType type; | ||
public Id id { private set; get {return record.Id;} } | ||
|
||
|
||
// CONSTRUCTORS | ||
|
||
public DomainBuilder(SObjectType type, Boolean isSetupObject) { | ||
public DomainBuilder(SObjectType type) { | ||
this.type = type; | ||
this.record = type.newSObject(null, true); | ||
this.isSetupObject = isSetupObject; | ||
|
||
graph.node(type); | ||
objects.add(this); | ||
} | ||
|
||
|
||
public DomainBuilder(SObjectType type) { | ||
this(type, false); | ||
} | ||
|
||
|
||
// PUBLIC | ||
|
||
public SObject persist() { | ||
fflib_SObjectUnitOfWork uow = unitOfWork(); | ||
public static void save() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. provide possible static save() call |
||
save(new fflib_SObjectUnitOfWork.SimpleDML()); | ||
} | ||
|
||
public static void save(fflib_SObjectUnitOfWork.IDML dml) { | ||
fflib_SObjectUnitOfWork uow = unitOfWork(dml); | ||
|
||
for(DomainBuilder obj: objects) { | ||
if(obj.record.Id == null) { | ||
|
@@ -48,12 +43,20 @@ public abstract class DomainBuilder { | |
uow.commitWork(); | ||
|
||
objects.clear(); | ||
} | ||
|
||
return record; | ||
public SObject persist() { | ||
return persist(new fflib_SObjectUnitOfWork.SimpleDML()); | ||
} | ||
|
||
public SObject persist(fflib_SObjectUnitOfWork.IDML dml) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. allow injection of custom implementation of UnitOfWork interface |
||
save(dml); | ||
|
||
public DomainBuilder recordType(String developerName) { | ||
return record; | ||
} | ||
|
||
// Note: virtual so it can be extended by implementations in order to cast the return type to its specifyc type | ||
public virtual DomainBuilder recordType(String developerName) { | ||
Id rtId = type.getDescribe().getRecordTypeInfosByDeveloperName().get(developerName).getRecordTypeId(); | ||
return set('RecordTypeId', rtId); | ||
} | ||
|
@@ -78,31 +81,13 @@ public abstract class DomainBuilder { | |
parent.registerIncludingParents(); | ||
} | ||
|
||
if(relationshipsToSync.containsKey(relationship)) { | ||
synchronize(relationship); | ||
} | ||
|
||
graph.edge(this.type, parent.type); | ||
|
||
// Note: Return parent instead of this as we call this always from the parent | ||
return parent; | ||
} | ||
|
||
|
||
protected void syncOnChange(SObjectField sourceField, DomainBuilder targetObject, SObjectField targetField) { | ||
if( !relationshipsToSync.containsKey(sourceField)) { | ||
relationshipsToSync.put(sourceField, new Map<SObjectField, List<DomainBuilder>>()); | ||
} | ||
if( !relationshipsToSync.get(sourceField).containsKey(targetField)) { | ||
relationshipsToSync.get(sourceField).put(targetField, new List<DomainBuilder>()); | ||
} | ||
|
||
relationshipsToSync.get(sourceField).get(targetField).add(targetObject); | ||
|
||
synchronize(sourceField); | ||
} | ||
|
||
|
||
protected DomainBuilder set(String fieldName, Object value) { | ||
record.put(fieldName, value); | ||
return this; | ||
|
@@ -137,25 +122,14 @@ public abstract class DomainBuilder { | |
} | ||
|
||
|
||
private void synchronize(SObjectField sourceField) { | ||
for(SObjectField targetField: relationshipsToSync.get(sourceField).keySet()) { | ||
for(DomainBuilder obj : relationshipsToSync.get(sourceField).get(targetField)) { | ||
|
||
DomainBuilder parent = parentByRelationship.get(sourceField); | ||
obj.setParent(targetField, parent); | ||
} | ||
} | ||
} | ||
|
||
|
||
private static fflib_SObjectUnitOfWork unitOfWork() { | ||
private static fflib_SObjectUnitOfWork unitOfWork(fflib_SObjectUnitOfWork.IDML dml) { | ||
List<SObjectType> insertOrder = new List<SObjectType>(); | ||
List<SObjectType> sorted = graph.sortTopologically(); | ||
|
||
for(Integer i = sorted.size() - 1; i >= 0; i--){ | ||
insertOrder.add(sorted[i]); | ||
} | ||
return new fflib_SObjectUnitOfWork(insertOrder); | ||
return new fflib_SObjectUnitOfWork(insertOrder, dml); | ||
} | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this ref was wrong