Add Identity Map design pattern

[wimvelzeboer]

I noticed that recently the UnitOfWork registerDirty method got an additional parameter 'dirtyFields'. That is a very good improvement, but what if you are not so fortunate to know the exact list of changed fields then the Identity map design pattern that I implemented might be helpful.
With the added registerClean method the UnitOfWork can recognise the dirty fields automatically.

When you like this change I can also add an extra option to the Selector functionality to automatically store queried records as clean records in the UnitOfWork. Then we really can make use of the full potential of the Identity map design pattern, because then we have enabled a type of caching. A record queried for the second time would then be fetched from the Identity map, instead of queried in the database.

Please let me know your thoughts about his change

---

This change is 

[afawcett]

@wimvelzeboer i am curious... what use cases are you seeing being of improved by this type change?

[afawcett]

@wimvelzeboer so i just later noticed this was a PR! Nice work. So i looked at this test and learnt a little more on the use cases at a detailed level. It kinda looks cool, but... tell me more about the motivations and general code use cases and challenges that lead you to this approach?

[wimvelzeboer]

@afawcett When application grow it could happen that two methods query the same record. Once from the original service class method and then later on by a domain class as part of its trigger logic.
To avoid that the same records need to be queried multiple times I wanted to introduce the Identity Map design pattern. That is using the UnitOfWork to store the clean map.

This pull-request is only for the UnitOfWork part. Where we can have two methods querying and changing a record independently from each other, registering them as dirty with the UnitOfWork and do not overwrite each other work. Particular useful on dynamic method that do not really know which fields are dirty.

Scenario 1

Given a single account record:
myAccount = new Account( Name = 'Test', AccountNumber = '123', BillingCountry = 'Holland');

Method A
This method is called from just a random Service class

• queries myAccount
• register myAccount as Clean in the UnitOfWork
• updates the AccountNumber
• register myAccount as Dirty in the UnitOfWork
• UnitOfWork.commitWork()

Method B
Called from inside a Contact trigger, because the Account trigger created a default contact record that is required by the business for all Accounts.

• queries myAccount
• register myAccount as Clean in the UnitOfWork
• updates the BillingCountry
• register myAccount as Dirty in the UnitOfWork
• UnitOfWork.commitWork()

In this scenario the myAccount record is queried twice during one execution context. If the selector method would use the clean version list in the UnitOfWork then it doesn't require a second query to the Database. I did not add this logic for the selector class yet, because it is dependent on having the cleanMap in the UnitOfWork.

Scenario 2

Is similar to scenario 1, but both methods are called from the same Service class logic. The service class is calling two other methods passing a instance of the UnitOfWork as a parameter to the methods.
Ideally you would want to pass a List to both methods so that they do not have to do a query. But as applications grow this is hard to avoid.

To not overwrite the changed fields, the UnitOfWork can look to the clean version to find out which field is different and only register those fields as dirty, and not overwriting the first dirty registered version.

Scenario 3

This scenario has the same justification as the reason why the new method was added with a list of dirty fields by Jonny Power in its commit on Sept 14th, 2017.
public void registerDirty(SObject record, List dirtyFields)

That method is very handy if you know exactly which fields are dirty. But when writing scripts that dynamically update records, you do not know which fields are dirty, until you compare them with the original queried record. That is a job that the UnitOfWork can do for you.

Dynamically updating records are usually done when there is a separate configuration (editable by admins) that define which fields should be updated/cloned/etc.

I'll hope this explains a bit further, but am happy to answer any other questions if you have them.

[afawcett]

@wimvelzeboer thanks for this write-up and sorry for the delay.

I actually did work with the identity map pattern on force.com in the early years. There are sime hidden challanges and mindset side effects with it when used in a bulkified world. The challenge is that we are encouraged to do bulk reads to the database and thus if criteria are used, the query still needs to be made (so really this only then applies to id queries). Once the result set is returned it is correct the identity map can be used to return the execution context copy of the record not that from the database. However, this starts to get its own side effects (what if we wanted new formula field values from the db). Furthermore, we found that it was getting hard to manage heap when a request scope variable held the references and it also nullified any approaches the calling code could take to scope the references and manage heap (sobject list for loops).

I feel providing a framework that hides the need for optimizing e.g. in Scenario 2 is borderline useful vs dangerous. I say this having written frameworks in the past that attempt to optimize and it's not ended well in terms of developers not really thinking about the implications of the code they write or learning the platform properly. I appreciate this is quite a subjective opinion this one though.

All this said I think it "might" be worth still having a go at this pattern but in a more explicit way that sits on top of the default usage and is very clear when it's being used. Where an IdentityMap is created by the calling code and thus the behavior is clear rather than indirect.

fflib_IdentityMap idMap = new fflib_IdentityMap();
fflib_SObjectUnitOfWork uow = new fflib_SObjectUnitOfWork();
AccountsSelector acctSelector = new AccountsSelector();
List<Account> accounts = acctSelector.selectById(accountIds, idMap);
Account account = accounts[0];
account.Name = 'Fred';
uow.registerDirty(account, idMap);
List<Account> accounts2 = acctSelector.selectById(accountIds, idMap);
Account account2 = accounts2[0];
account2.Website = 'http://fred.com';
uow.registerDirty(account2, idMap);

Having written the above up, I do also wonder if the complexity of the merge on the second registerDIrty will get confusing, though I guess the code could use the SObject method to only the dirty fields?

Thoughts?

[afawcett]

@ImJohnMDaniel thoughts?

[afawcett]

Throwing this out to the community for further discussion. 👍 https://twitter.com/andyinthecloud/status/1066021111010213890

[ImJohnMDaniel]

@afawcett -- in your example code above, why would you not include the IdentityMap directly in the UOW? Why a separate class?

@wimvelzeboer you mentioned that you would add some functionality to the selectors to automatically load clean records to the UOW. Can you provide a basically example of how that would be implemented?

[wimvelzeboer]

Hi @ImJohnMDaniel,

In my latest commit I updated the fflib_SObjectUnitOfWork a little bit and added functionality in the fflib_SObjectSelector to automatically return the clean records if they were previously registered.

You can use the identity pattern in the following manner. Let's assume we already have an AccountsSelector class which is extended from the fflib_SObjectSelector class, and a list of accountIds;

AccountsSelector mySelector = new AccountsSelector();
List<Accounts> myRecords = mySelector.selectById(idSet);
fflib_ISObjectUnitOfWork uow = 
    new fflib_SObjectUnitOfWork(new List<Schema.SObjectType>{Account.SObjectType});
mySelector.addUnitOfWork(uow);
List<Accounts> myRecordsFromClean = mySelector.selectById(idSet);

The second call to the selectById method will now query return the same results but without a SOQL query to the database. I check the performance with 200 records and it showed 20 milliseconds for the fist execution and 6 for the second.

The performance isn't the only reason for doing this. My primary concern is that there can be two service methods accepting the same set of Ids, each doing the same query separately.

AccountsService.cls
public void someServiceMethod(Set<Id> accountIds)
{
    firstServiceMethod(accountIds);
    secondServiceMethod(accountIds);
}

We might want to have a closer look on how you can enable the pattern to a selector class, so that it can be enabled directly when doing something like:
List<Accounts> myAccounts = (List<Account>) Application.Selector.selectById(idSet);
What are your thoughts on this one? We could just pass in the UnitOfWork as an extra parameter.

[wimvelzeboer]

@ImJohnMDaniel I have my previous commit some thought and looked at the comments from @afawcett again, and changes the way of invoking the selectById with the UnitOfWork. Now it represents more the idea of @afawcett.

Now you can also call the selector via:
List<Accounts> myAccounts = (List<Account>) Application.Selector.selectById(idSet, unitOfWork);

For now the identity pattern is limited for just only fetching a list of records via a set of record Ids. I am also thinking of including other IDs fields (master-detail/lookup). I don't think we should extend it to all other custom queries for now, because of the other challenges that @afawcett also mentioned.

Please let me know what you think of this.

[afawcett]

@ImJohnMDaniel my motivation for not embedding into UOW is to keep IDMAP as a separate "caching" concern to the default operation of the UOW and Selectors. Its been my experience having implemented this as an embedded concern of these patterns on Force.com that it adds to much-hidden behavior that results in to many corner cases that are hard to debug. I really like the idea of having it as a kind of opt-in augmenting service/facility to uow and selectors that is visible to the developer and the scope of the IDMAP is clearly owned by the controlling code.

@wimvelzeboer as you can imagine, I like your latest iteration, are you also passing in the IDMAP into the UOW methods? I must admit I am wondering if its a UOW scope association thing, so should be a contructor arg.... That said... I have seen a couple of cases now where UOW is being configured via the constructor so we may want to start a fluent style UnitOfWork factory pattern here.... this would also avoid having to keep adding overloads to the Application.UnitOfWork factory....

Thoughts on Fluent Style UOW Config Syntax

So have the UOW fluent class in fflib_SObjectUnitOfWork as an inner class, e.g. Config with a static final instance called Factory. For classic (non-Application factory uses) folks can move to using...

fflib_SObjectUnitOfwork uow = 
    fflib_SObjectUnitOfwork.Factory
       .forObjects(mySobjects)
       .overrideDML(myCustomDML)
       .withIdentityMap(idMap)
       .newInstance();

For those using Application factory pattern we could add Factory a instance final member to the existing Application.UnitOfWorkFactory inner class. Thus the constructor of Application.UnitOfWorkFactory would call forObjects with those given as part of thre factory initialization. Developers could continue to call Application.UnitOfWork.newInstance or use Application.UnitOfWork.Factory as above just without having to worry about using forObjects.

fflib_ISObjectUnitOfwork uow = 
   Application.UnitOfWork.Factory
       .overrideDML(myCustomDML)
       .withIdentityMap(idMap)
       .newInstance();

NOTE: Those that want to default this sort of config can extend the Application.UnitOfWorkFactory contructor to add default application scope configres.

I appreciate this is a bit of scope creep to this PR btw so apologies. @ImJohnMDaniel what do you think about this you know Application pattern quite well.... Meanwhile i am going to cc a few others from the other PR where we are talking about features being added to UOW that require opt-in config.

[afawcett]

@ImJohnMDaniel this is an old thread, any more recent thoughts?

@wimvelzeboer Does relatively new advent of SObject. getPopulatedFieldsAsMap help in anyway here?

In general I do like the way the PR is taking to a more visible opt-in behavior here. That said it really only effects the standard select by id base class method and needs a retro fit into custom selectors (which may not be at first apparent). Also i think overloading the UOW class for the cache is perhaps not the best place for it, hence why i suggested fflib_IdentityMap in my prior post.