Skip to content

handle cyclic dependancies when inserting records #80

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

Closed
wants to merge 2 commits into from

Conversation

loganm
Copy link
Contributor

@loganm loganm commented Aug 27, 2015

To resolve #59

Currently fflib does not handle cyclic dependancies. If a new record has a lookup to another new record of the same SObjectType, the records are inserted, but the lookup field is left blank.

I solved this by keeping a track of reflective (or self-referencing) relationships in a separate collection when the record being referenced has a blank Id (i.e. hasn't been inserted yet).

When committing the uow, I let all the records of an SObjectType be inserted as per usual, but then if one or more records from a particular SObjectType collection has reflective relationships, I follow up by resolving those relationships and updating the records in a second pass.

This solution does have overhead, but it is predictable and reliable (I've been using it for a few months now).

If a collection of SObjectType in the UoW has reflective relationships to be resolved, there will be +1 dml and x2 dmlrows incurred for that SObjectType (not x2 across the entire UoW).

loganm added 2 commits July 2, 2015 18:13
To achieve this, I kept a track of unresolved reflective lookups (where
the record and relatedTo SObjectTypes are the same, the the relatedTo
doesn't yet have an Id). If any of the records being inserted have a
reflective reference to anoth
er record being inserted in the same commit, then I perform an update
immediatel
y after the insert to "Backfill" the lookups with the Ids of the newly
inserted
records.

The tradeoff is clear and calculable. SObjects with unresolved
relationships incur 2xDML rows. I like this better than trying to order
the inserting of SObjects and incur and undetermined number of DML
statements.
@afawcett
Copy link
Contributor

Wow thanks for this, it looks quite elegant and light weight which i like! 👍

Question though, what depth of cyclic dependency does it go to?

Can I insert Accounts that reference A > B > C > D etc...?

@afawcett
Copy link
Contributor

Also does this help with the use cases referenced here #23?

@loganm
Copy link
Contributor Author

loganm commented Aug 27, 2015

Technically, it's infinite. You could insert 5000 records that form a chain of records.

1x insert of 5000 records
no relationships resolved
1x update of 5000 records
all relationships resolved

This works because you can't make a self relationship required on the platform.

@afawcett
Copy link
Contributor

Ah yes i see, on the face of it i started to wonder if doing a load of inserts then updates (admittedly on a a subset of the records) is inefficient compared to a directly coded solution not using UOW (since UOW is really supposed to aid in being performant). But then in contrast would you in reality without UOW burn through more DML statement invocations, i think so? I've also ping'd a couple of other contributors on the other PR to review this as well, hope thats OK.

@afawcett
Copy link
Contributor

Playing devils advocate here, but yet wanting to be sure we understand the pros and cons over direct vs UOW approaches, i guess the direct approach would not do any update's and perhaps perform better as updates are more expensive than inserts. Hmmm... tough one this, its elegant in its simplicity.... but potentially has some hidden costs not apparent to the developer using UOW...

@afawcett
Copy link
Contributor

Also, while the platform does not insist on relationships being populated on insert, i guess this is dependent on Apex Trigger logic on objects not insisting on the relationship being present on insert.

BTW, have you seen some of the extensibility points thats been added to UOW recently? Maybe this feature can be added via subclassing the existing one?

@adtennant
Copy link
Contributor

This resolves the primary use-case that I had for my original pull request, i.e. objects that have relationships with themselves. The solution seems elegant enough to me and guarantees (as far as I can see) that you only have as many DML statements as you have levels in your hierarchy.

The main problem I have with it is that the relationships are not populated on insert. If you're architecting the whole solution this isn't necessarily a problem as you can work around it, but it's not the way people tend to work with the platform and has a potential knock-on impact with managed package, existing triggers, workflow, processes, etc. I'm also generally against anything that forces people to work in an unnatural way.

This also doesn't solve is more complex object hierarchies, for example, although these are rare (I only have one example where I've needed this):

Object 1 (Type 1)
       |
Object 2 (Type 2)
       |
Object 3 (Type 1)

@loganm
Copy link
Contributor Author

loganm commented Aug 27, 2015

I do have to remind myself that fflib is for use in managed packages as well. I'm using it for a new project, so I've got a bit of tunnel vision as to possible use cases. I'm using the UOW to simplify my test setups, and that's where I'm hammering it the hardest, with cyclic dependencies right in the middle of the object hierarchy.

I first solved this by going into a loop, inserting the records where all the relationships were resolvable and repeating until all were inserted. This worked great until I got two records of the same object type that reference each other. Needless to say, Infinite loop ensues and neither record gets inserted. So I tried this insert, then upsert method because I could guarantee it would work regardless of how complicated the dependencies got.

I'll go back to the drawing board and have another crack at it. I've just had an idea for how to deal with those pesky records that look up to each other, and even the objects that lookup up to each other.

@loganm
Copy link
Contributor Author

loganm commented Aug 27, 2015

If I were to write out code manually that had records/objects that reference each other anyone would do it like this

// Insert the first record
SObject one = new SObject();
insert one;

// Insert the second record, and reference the first
SObject two = new SObject();
two.Lookup_Left__c = one.Id;
insert two;

// Update the first record to reference the second
one.Lookup_Right__c = two.Id;
update one;

So there's definitely a dml update in the mix here. This also applies to objects of different types. One is inserted first, then you must come back and update it. The question that needs to be answered is, if you have cross referencing records like this, which gets inserted first? The logical answer is, whichever was registered with the UoW first.

@maximorojr
Copy link

In my scenario I've to relate an existing record with a new record that are being created, and it worked very well for me! I'm not seeing a problem to use that. It's far better than my direct coded solution. @afawcett

@afawcett
Copy link
Contributor

Great feedback gents, i do wonder if the new extensibility in UOW allows for subclassing to provide a choice of 'uow strategies' to be chosen by the developer based on needs? Perhaps this might help balance optimisation vs flexibilty, rather than trying to teach UOW to handle all cases? Just a thought. @loganm thanks so much for moving this along in the meantime, looking forward to your next thoughts! 👍

@dudunato
Copy link

Is this live already?

@thegogz
Copy link

thegogz commented Jan 30, 2017

@afawcett @loganm is there an issue with merging this into master? it looks like a solid approach and will meet most use cases that I can identify and overhead is relatively small. I've hit my first cyclic relationship since adopting this lib and I'm really reluctant to move away from using uow or implementing some hacky insert solution.

@afawcett
Copy link
Contributor

I am not seeing any further views on my last thought in terms of adding this behavior via the extensibility features of UOW as a superclass. In which case i think we should just make this a core feature. Meanwhile this alternative version (uses much the same approach) #195 (does add some logging and error handling). It is done conditionally, but tbh i do not feel the added overhead is such a big issue vs how to manage configurability of the UOW, given the factory based exposure model of UOW.

@afawcett
Copy link
Contributor

afawcett commented Nov 23, 2018

@loganm thanks so much for driving this discussion and others here. This is actually one of three PR's on this topic. I am have been reviewing and discussing with the authors on them all over the last few weeks. I think its best to go forward with this #195 as it covers more use cases and has some reporting aspects to it that will be useful. So please continue to contribute and ensure your needs and if needed impl approach is considered over in the discussion for this PR. Thus i am recommended we close this PR. cc @agarciaodeian @stephenwillcock

@loganm
Copy link
Contributor Author

loganm commented Nov 25, 2018

@afawcett agreed. #195 looks like a solid starter.

@loganm loganm closed this Nov 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What's the current state of reflective lookups in Unit of Work?
6 participants