[
https://issues.apache.org/jira/browse/CAY-2792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17684980#comment-17684980
]
Matt Watson commented on CAY-2792:
----------------------------------
I did confirm that the {{DefaultDbRowOpSorter.sortReflexive}} is not moving any
records, when it comes in with this order [Insert-ChildReflexive,
Insert=ParentReflexive, Update-Other].
The way that for loop is written it, by the time it recognizes that
{{row.getEntity() != lastEntity}} AND {{lastEntity != null &&
sorter.isReflexive(lastEntity)}}, we are already on the 3rd/last {{DbRowOp}}.
Which means we can't sort the previous 2 that were out of order, because we are
inside a {{for}} loop and would get {{ConcurrentModificationException}}. And I
also see that the call to {{sorter.sortObjectsForEntity(...)}} is never going
to do any moving under any circumstance, because the {{reflexiveSublist}} past
into it is always going to be EMPTY. The line above always sets {{start =
idx}}, so {{sortedDbRows.subList(start, idx)}} will always be empty.
After the for loop there is a chance to {{sort last chunk}}, but now the
{{lastEntity}} (Other) is not "Reflexive", so it still does nothing.
Thinking of doing a couple things:
# rewrite the {{sortReflexive}} method to capture the indices of each chunk of
the {{sortedDbRows}} that needs to be sorted (by DbEntity), then sort those
chunks while not in the middle of an iteration loop.
# The Update-Other ends up being a No-Op-Query, which gets eliminated in the
following line of the {{DefaultDataDomainFlushAction.flush}} method for
{{createQueries}}. This method recognizes that the Update-Other ,
{{dbRow.getValues().isEmpty()}} and therefore does not add a query. Wondering
if this process should happen before handling the {{sortReflexive}}?
# maybe even a third option to update {{DbRowComparator.compare}} to handle the
Reflexive sorting after we already know that {{left}} and {{right}} are the
same {{DbRowOpType}} and the same {{DbEntity}}. It may be more efficient
handling in the first iteration?
I am leaning towards starting on option #1 above, because it seems like a less
intrusive code change. However, I wanted to you consider the other 2 as
possible performance improvements.
> Fix Insertion Order For Reflexive DataObjects
> ---------------------------------------------
>
> Key: CAY-2792
> URL: https://issues.apache.org/jira/browse/CAY-2792
> Project: Cayenne
> Issue Type: Bug
> Components: Core Library
> Affects Versions: 4.2.RC2, 4.2.RC1
> Environment: Java 17
> Reporter: Matt Watson
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> For terminology's sake, I assume _Reflexive_ refers to when you have 2
> objects of the same Model, that are related, in this case, parent/child
> (ToOne/HasMany). If this assumption is incorrect we can rename the title.
> For a little history on this, we never encountered this bug until we upgraded
> to {{4.2.RC1}}, and I recently tested on {{4.2.RC2}} and the bug still exists.
> For a while I thought this was only a bug in our environment, thinking that
> our code was causing the issue because Cayenne has a passing test in
> {{CayenneDataObjectRelationshipsIT.testReflexiveRelationshipInsertOrder1}}.
> But as I got to digging, the error occurs when the parent also has a
> relationship to some "Other" object that is not of the same model.
> The [pull request|https://github.com/apache/cayenne/pull/563] has a breaking
> test. I have not worked on a fix yet, but will begin investigating. I suspect
> it has to do with the work done in CAY-2571, but I am not certain yet.
> The issue is that Cayenne randomly tries to commit the child record in front
> of the parent record, therefore the database throws a Foreign Key error,
> since the parent does not yet exist when the child is inserted. Because of
> this randomness, my breaking test attempts this process 100 times. You will
> also see another test that passes with the same Models but without setting
> the "ToOne" Other object.
> here is a look at the code that "randomly" fails:
> {code:java}
> public void addReflexiveParentAndChildWithOtherRelationshipOnParent() {
> // can add Reflexive Parent (that belongsTo Other) and Child,
> // we will do this 100 times, because it randomly does it
> correctly/incorrectly
> // given some "other" Object
> final Other other = context.newObject(Other.class);
> other.setName("OtherB");
> context.commitChanges();
> final int attempts = 100;
> int errors = 0;
> for (int i = 0; i < attempts; i++) {
> // when parent is created and associated to "Other"
> final Reflexive parent = context.newObject(Reflexive.class);
> parent.setName("parentB"+i);
> parent.setToOther(other);
> // and child is created and associated to "Parent"
> final Reflexive child = context.newObject(Reflexive.class);
> child.setName("childB"+i);
> child.setToParent(parent);
> try {
> context.commitChanges();
> } catch (final Exception e) {
> errors++;
> e.printStackTrace();
> context.rollbackChanges();
> }
> }
> // then no error occurred
> assertEquals(String.format("Failed on %s of %s attempts.", errors,
> attempts), 0, errors);
> }
> {code}
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
