fflib-apex-common icon indicating copy to clipboard operation
fflib-apex-common copied to clipboard

fflib_SObjectUnitOfWork order dependency

Open cropredy opened this issue 8 years ago • 12 comments

Andy (as you can imagine, I'm been retrofitting existing orgs to the SoC pattern and running into interesting use cases..appreciate the forum)

Use Case: Sync Quote + Quote Items to Opportunity + OLIs

The preferred pattern for me is to

  1. Delete the OLIs in the target
  2. Update the Opportunity
  3. Insert new OLIs from the Quote Items

But the UoW framework won't support this if you have SFDC Duplicate Management in place to prevent duplicate SKUs (ProductCode). Why?

The fflib_SObjectUnitOfWork.commitWork() method first does inserts, then does updates, then does deletes, and finally registered workers.

Hence, the inserts are likely to have the same ProductCodes as in the OLIs to be deleted and the insert fails. Order of Execution says that Duplicate Checks run immediately after VRs.

IDoWork doesn't directly help because it gets called long after the inserts. The only workaround is to use an IDoWork handler for all three steps above - but then the uow pattern isn't helping much (only a savepoint-rollback wrapper).

One possible enhancement to the framework could simply be an argument to commitWork(Boolean deletesFirst) .

cropredy avatar Jul 18 '16 17:07 cropredy

I also ran into a similar issue when my custom Child object record was using updated field value from the parent record, but because of IUD order of operations in UoW it was failing as all inserts are done before updates. I agree with the 'preferred pattern' of DUI (Delete, Update, Insert) you provided as in this case all operation are done in the order from the least dependant to the most

o-lexi avatar Sep 07 '16 21:09 o-lexi

@o-lexi - Funny, I ran into your use case myself today. Here it is:

Use Case: Construct in one transaction an updated Opportunity (new pricebook2Id) + insert OLI

Let's say that the Opportunity starts with Pricebook2Id = IdA and we want to change the pricebook to IdB and insert some OLI using PBEs for pricebook IdB -- all in the same Unit of Work.

Unless the Opportunity is updated with the new Pricebook2Id IdB first, the insert of the OLI using PricebookEntries that reference IdB will fail with a Field Integrity Exception PricebookEntryId (pricebook entry is in a different pricebook than the one assigned to the opportunity

But, the code in onCommitWork() blindly does the inserts of the OLIs first before ever processing the registered-as-dirty Opportunity. Hence the Field Integrity Exception.

A step in the right direction would be to register the DML order as either I - U - D (default) or U - I - D or U - D - I, or D - I - U or D - U - I or I - D - U. This won't solve every issue but would mean less resorting to IDoWork.

cropredy avatar Sep 30 '16 18:09 cropredy

@afawcett Any idea if there will be movement on this issue or should I plan on figuring out an IDoWork pattern?

@cropredy Any change of sharing a general pattern on how your IDoWork is setup? Might be helpful for me and others that come across this issue :)

Thanks! Tyler

tmowbrey avatar May 03 '17 03:05 tmowbrey

well, what I should do is actually code up an alternative and do a PR but I haven't had a chance to do so yet :-(

On Tue, May 2, 2017 at 8:22 PM, Tyler Mowbrey [email protected] wrote:

@afawcett https://github.com/afawcett Any idea if there will be movement on this issue or should I plan on figuring out an IDoWork pattern?

@cropredy https://github.com/cropredy Any change of sharing a general pattern on how your IDoWork is setup? Might be helpful for me and others that come across this issue :)

Thanks! Tyler

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/financialforcedev/fflib-apex-common/issues/129#issuecomment-298815189, or mute the thread https://github.com/notifications/unsubscribe-auth/ACNrcqpoEYtZlE0iAkaLcQ7pyMGccxSEks5r1_MTgaJpZM4JO-No .

cropredy avatar May 05 '17 15:05 cropredy

I'm working on an apex sharing project for community/partner access and trying to delete before inserting, as this is the recommended approach:

"For a sharing recalculation, we recommend that the execute method delete and then re-create all Apex managed sharing for the records in the batch. This process ensures that sharing is accurate and complete."

https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_batch_interface.htm#apex_batch_best_practices

Sorry, no PR to contribute, but wanted to add more rationale behind configurable order of dml.

daveerickson avatar Jun 12 '20 16:06 daveerickson

@cropredy @ImJohnMDaniel there is some good evidence to support this enhancement here - agree?

afawcett avatar Jul 20 '20 19:07 afawcett

Yes, I agree - IDoWork should be reserved for unsupported UoW use cases like partial success DML; the delete-before-insert use case is a common-enough pattern; especially when dealing with objects that can't have certain fields updated like PricebookEntryId on OpportunityLineItem, QuoteItem, OrderItem, ServiceContractLineItem - or the sharing rules as @daveerickson states

cropredy avatar Jul 21 '20 00:07 cropredy

Hi All,

It is great we can tailor the unit of work by adding peaces of work. I would need to first send emails, then update related objects. I see that the class SendEmailWork, which is located here, is set to private, I would like to add an instance of SendEmailWork using the method registerWork(IDoWork work). Can you set the class SendEmailWork to public?

marco10507 avatar Jul 22 '20 11:07 marco10507

@marco10507 If you would like to open a separate PR for that change please feel free to do so. One request that I will be asking is of you is to document why you would want to make that method public and what scenario/use case it supports. Cheers!

ImJohnMDaniel avatar Jul 22 '20 14:07 ImJohnMDaniel

Hi, Can I get some guidance on how to achieve partial success in DML as I am trying to extend FFLib framework for batches and other asynchronous processes.

sruthikattula-okta avatar Oct 15 '20 22:10 sruthikattula-okta

You'll have to delegate the DML to a class that implements IDoWork and avoid using any of the registerXXX methods.

cropredyHelix avatar Oct 18 '20 21:10 cropredyHelix

Would a PR be accepted that added support for a delete-before-insert usecase?

hansds avatar Sep 16 '21 07:09 hansds