This is an automated email from the ASF dual-hosted git repository.

ntimofeev pushed a commit to branch STABLE-4.2
in repository https://gitbox.apache.org/repos/asf/cayenne.git


The following commit(s) were added to refs/heads/STABLE-4.2 by this push:
     new d6fd4e28a CAY-2867 Tweak GraphBasedDbRowOpSorter logic to allow 
related updates
d6fd4e28a is described below

commit d6fd4e28a55aa8394553d6e62ab67bdf30fd8777
Author: Nikita Timofeev <stari...@gmail.com>
AuthorDate: Thu Aug 15 16:54:03 2024 +0300

    CAY-2867 Tweak GraphBasedDbRowOpSorter logic to allow related updates
---
 RELEASE-NOTES.txt                                  |   3 +
 .../flush/operation/GraphBasedDbRowOpSorter.java   |  58 +++++--
 .../DataContextEntityWithMeaningfulPKIT.java       |  20 +++
 .../operation/GraphBasedDbRowOpSorterTest.java     | 181 +++++++++++++++++++++
 4 files changed, 251 insertions(+), 11 deletions(-)

diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index 36dd5af24..55c709cdb 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -11,6 +11,9 @@ https://issues.apache.org/jira/browse/CAY
 Release: 4.2.2
 Date:
 ----------------------------------
+Changes/New Features:
+
+CAY-2867 Tweak GraphBasedDbRowOpSorter logic to allow related updates
 
 Bug Fixes:
 
diff --git 
a/cayenne-server/src/main/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorter.java
 
b/cayenne-server/src/main/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorter.java
index 8ed961773..b4bcd4cec 100644
--- 
a/cayenne-server/src/main/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorter.java
+++ 
b/cayenne-server/src/main/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorter.java
@@ -35,7 +35,9 @@ import org.apache.cayenne.di.Inject;
 import org.apache.cayenne.di.Provider;
 import org.apache.cayenne.exp.parser.ASTDbPath;
 import org.apache.cayenne.graph.GraphManager;
+import org.apache.cayenne.map.DbAttribute;
 import org.apache.cayenne.map.DbEntity;
+import org.apache.cayenne.map.DbJoin;
 import org.apache.cayenne.map.DbRelationship;
 import org.apache.cayenne.map.EntityResolver;
 import org.apache.cayenne.query.ObjectIdQuery;
@@ -125,7 +127,7 @@ public class GraphBasedDbRowOpSorter implements 
DbRowOpSorter {
         // get graph edges for reflexive relationships
         DbRowOpType opType = op.accept(rowOpTypeVisitor);
         relationships.getOrDefault(op.getEntity(), 
Collections.emptyList()).forEach(relationship ->
-            getParentsOpId(op, relationship).forEach(parentOpId ->
+            getParentsOpId(op, relationship).forEach((parentOpId, snapshot) ->
                 indexByDbId.getOrDefault(parentOpId, 
Collections.emptyList()).forEach(parentOp -> {
                     if(op == parentOp) {
                         return;
@@ -141,12 +143,26 @@ public class GraphBasedDbRowOpSorter implements 
DbRowOpSorter {
                             }
                             break;
                         case UPDATE:
-                            if(parentOpType != DbRowOpType.DELETE) {
-                                graph.add(op, parentOp);
-                            } else {
-                                graph.add(parentOp, op);
+                            switch (parentOpType) {
+                                case INSERT:
+                                    graph.add(op, parentOp);
+                                    break;
+                                case DELETE:
+                                    graph.add(parentOp, op);
+                                    break;
+                                case UPDATE:
+                                    // Update will only depend on the 
operations where FK could be affected
+                                    Boolean fkReferenceUpdated = 
parentOp.accept(new FkUpdateChecker(relationship));
+                                    if (fkReferenceUpdated == Boolean.TRUE) {
+                                        // TODO: this sorting logic works only 
for the setting relationship to null case
+                                        //       if the meaningful PK is 
updated we got more places to cover, before fixing this logic
+                                        //       namely:
+                                        //          - no update would be 
generated for any dependent entities for the master PK update
+                                        //          - update would fail with 
constraint violation even if it is generated
+                                        graph.add(parentOp, op);
+                                    }
+                                    break;
                             }
-                            break;
                         case DELETE:
                             if(parentOpType == DbRowOpType.DELETE) {
                                 graph.add(parentOp, op);
@@ -170,21 +186,21 @@ public class GraphBasedDbRowOpSorter implements 
DbRowOpSorter {
         });
     }
 
-    private List<EffectiveOpId> getParentsOpId(DbRowOp op, DbRelationship 
relationship) {
+    private Map<EffectiveOpId, Map<String, Object>> getParentsOpId(DbRowOp op, 
DbRelationship relationship) {
         List<Map<String, Object>> parentIdSnapshots = op.accept(new 
DbRowOpSnapshotVisitor(relationship));
         if(parentIdSnapshots.size() == 1) {
             EffectiveOpId id = effectiveIdFor(relationship, 
parentIdSnapshots.get(0));
             if(id != null) {
-                return Collections.singletonList(id);
+                return Collections.singletonMap(id, parentIdSnapshots.get(0));
             } else {
-                return Collections.emptyList();
+                return Collections.emptyMap();
             }
         } else {
-            List<EffectiveOpId> effectiveOpIds = new 
ArrayList<>(parentIdSnapshots.size());
+            Map<EffectiveOpId, Map<String, Object>> effectiveOpIds = new 
HashMap<>(parentIdSnapshots.size());
             parentIdSnapshots.forEach(snapshot -> {
                 EffectiveOpId id = this.effectiveIdFor(relationship, snapshot);
                 if(id != null) {
-                    effectiveOpIds.add(id);
+                    effectiveOpIds.put(id, snapshot);
                 }
             });
             return effectiveOpIds;
@@ -311,4 +327,24 @@ public class GraphBasedDbRowOpSorter implements 
DbRowOpSorter {
             return DbRowOpType.UPDATE;
         }
     }
+
+    private static class FkUpdateChecker implements DbRowOpVisitor<Boolean> {
+        private final DbRelationship relationship;
+
+        public FkUpdateChecker(DbRelationship relationship) {
+            this.relationship = relationship;
+        }
+
+        @Override
+        public Boolean visitUpdate(UpdateDbRowOp dbRow) {
+            for(DbJoin join : relationship.getJoins()) {
+                for (DbAttribute attribute : 
dbRow.getValues().getUpdatedAttributes()) {
+                    if (attribute.getName().equals(join.getTargetName())) {
+                        return true;
+                    }
+                }
+            }
+            return false;
+        }
+    }
 }
diff --git 
a/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKIT.java
 
b/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKIT.java
index e465da3ec..770072f7c 100644
--- 
a/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKIT.java
+++ 
b/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKIT.java
@@ -20,6 +20,7 @@
 package org.apache.cayenne.access;
 
 import org.apache.cayenne.Cayenne;
+import org.apache.cayenne.CayenneRuntimeException;
 import org.apache.cayenne.DataRow;
 import org.apache.cayenne.ObjectContext;
 import org.apache.cayenne.ObjectId;
@@ -250,4 +251,23 @@ public class DataContextEntityWithMeaningfulPKIT extends 
ServerCase {
         assertNotNull(depChild.getMeaningfulPk());
         assertNull(depChild.getMeaningfulPk().getPk());
     }
+
+    @Test(expected = CayenneRuntimeException.class)
+    public void test_MeaningfulPkWithFkUpdate() {
+        // setup
+        MeaningfulPKTest1 obj = context.newObject(MeaningfulPKTest1.class);
+        obj.setPkAttribute(1001);
+        obj.setDescr("aaa");
+        context.commitChanges();
+
+        MeaningfulPKDep dep = context.newObject(MeaningfulPKDep.class);
+        dep.setToMeaningfulPK(obj);
+        dep.setPk(10);
+        context.commitChanges();
+
+        // this would fail as the DefaultDbRowOpSorter unable to deal with the 
meaningful PK update
+        dep.setToMeaningfulPK(null);
+        obj.setPkAttribute(1002);
+        context.commitChanges();
+    }
 }
diff --git 
a/cayenne-server/src/test/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorterTest.java
 
b/cayenne-server/src/test/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorterTest.java
new file mode 100644
index 000000000..3ce3e69be
--- /dev/null
+++ 
b/cayenne-server/src/test/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorterTest.java
@@ -0,0 +1,181 @@
+/*****************************************************************
+ *   Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ ****************************************************************/
+
+package org.apache.cayenne.access.flush.operation;
+
+import org.apache.cayenne.ObjectId;
+import org.apache.cayenne.PersistenceState;
+import org.apache.cayenne.Persistent;
+import org.apache.cayenne.access.DataDomain;
+import org.apache.cayenne.map.DbAttribute;
+import org.apache.cayenne.map.DbEntity;
+import org.apache.cayenne.map.EntityResolver;
+import org.apache.cayenne.map.EntitySorter;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.when;
+
+public class GraphBasedDbRowOpSorterTest {
+    private EntitySorter entitySorter;
+    private DbRowOpSorter sorter;
+
+    @Before
+    public void createSorter() {
+        entitySorter = mock(EntitySorter.class);
+        EntityResolver entityResolver = mock(EntityResolver.class);
+
+        when(entitySorter.getDbEntityComparator())
+                .thenReturn(Comparator.comparing(DbEntity::getName));
+        when(entitySorter.isReflexive(argThat(ent -> 
ent.getName().equals("reflexive"))))
+                .thenReturn(true);
+
+        DataDomain dataDomain = mock(DataDomain.class);
+        when(dataDomain.getEntitySorter()).thenReturn(entitySorter);
+        when(dataDomain.getEntityResolver()).thenReturn(entityResolver);
+
+        sorter = new GraphBasedDbRowOpSorter(() -> dataDomain);
+    }
+
+    @Test
+    public void sortEmptyList() {
+        List<DbRowOp> rows = new ArrayList<>();
+        List<DbRowOp> sorted = sorter.sort(rows);
+        assertTrue(sorted.isEmpty());
+    }
+
+    @Test
+    public void sortByOpEntity() {
+        ObjectId id1 = ObjectId.of("test4", "id", 1);
+        ObjectId id2 = ObjectId.of("test2", "id", 2);
+        ObjectId id3 = ObjectId.of("test3", "id", 3);
+        ObjectId id4 = ObjectId.of("test1", "id", 4);
+
+        DbRowOp op1 = new InsertDbRowOp(mockObject(id1), mockEntity("test4"), 
id1);
+        DbRowOp op2 = new InsertDbRowOp(mockObject(id2), mockEntity("test2"), 
id2);
+        DbRowOp op3 = new InsertDbRowOp(mockObject(id3), mockEntity("test3"), 
id3);
+        DbRowOp op4 = new InsertDbRowOp(mockObject(id4), mockEntity("test1"), 
id4);
+
+        List<DbRowOp> rows = Arrays.asList(op1, op2, op3, op4);
+        List<DbRowOp> expected = Arrays.asList(op1, op2, op3, op4);
+
+        List<DbRowOp> sorted = sorter.sort(rows);
+        assertEquals(expected, sorted);
+    }
+
+    @Test
+    public void sortById() {
+        ObjectId id1 = ObjectId.of("test", "id", 1);
+        ObjectId id2 = ObjectId.of("test", "id", 2);
+        ObjectId id3 = ObjectId.of("test", "id", 2);
+        ObjectId id4 = ObjectId.of("test", "id", 3);
+
+        DbEntity test = mockEntity("test");
+        InsertDbRowOp op1 = new InsertDbRowOp(mockObject(id1), test, id1);
+        InsertDbRowOp op2 = new InsertDbRowOp(mockObject(id2), test, id2);
+        DeleteDbRowOp op3 = new DeleteDbRowOp(mockObject(id3), test, id3);
+        DeleteDbRowOp op4 = new DeleteDbRowOp(mockObject(id4), test, id4);
+
+        List<DbRowOp> rows = Arrays.asList(op1, op2, op3, op4);
+        List<DbRowOp> expected = Arrays.asList(op3, op1, op2, op4);
+
+        List<DbRowOp> sorted = sorter.sort(rows);
+        assertEquals(expected, sorted);
+    }
+
+    @Test
+    public void sortByIdDifferentEntities() {
+        ObjectId id1  = ObjectId.of("test2", "id", 1);
+        ObjectId id2  = ObjectId.of("test",  "id", 2);
+        ObjectId id3  = ObjectId.of("test",  "id", 2);
+        ObjectId id4  = ObjectId.of("test",  "id", 3);
+        ObjectId id5  = ObjectId.of("test2", "id", 4);
+        ObjectId id6  = ObjectId.of("test",  "id", 5);
+        ObjectId id7  = ObjectId.of("test",  "id", 8);
+        ObjectId id8  = ObjectId.of("test2", "id", 7);
+        ObjectId id9  = ObjectId.of("test2", "id", 4);
+        ObjectId id10 = ObjectId.of("test",  "id", 8);
+
+        DbEntity test = mockEntity("test");
+        DbEntity test2 = mockEntity("test2");
+        BaseDbRowOp[] op = new BaseDbRowOp[10];
+        op[0] = new InsertDbRowOp(mockObject(id1),  test2, id1);
+        op[1] = new InsertDbRowOp(mockObject(id2),  test,  id2);
+        op[2] = new DeleteDbRowOp(mockObject(id3),  test,  id3);
+        op[3] = new UpdateDbRowOp(mockObject(id4),  test,  id4);
+        op[4] = new InsertDbRowOp(mockObject(id5),  test2, id5);
+        op[5] = new DeleteDbRowOp(mockObject(id6),  test,  id6);
+        op[6] = new InsertDbRowOp(mockObject(id7),  test,  id7);
+        op[7] = new UpdateDbRowOp(mockObject(id8),  test2, id8);
+        op[8] = new DeleteDbRowOp(mockObject(id9),  test2, id9);
+        op[9] = new DeleteDbRowOp(mockObject(id10), test,  id10);
+
+        List<DbRowOp> expected = Arrays.asList(op[2], op[8], op[9], op[0], 
op[1], op[3], op[4], op[5], op[6], op[7]);
+        List<DbRowOp> sorted = sorter.sort(Arrays.asList(op));
+
+        assertEquals(expected, sorted);
+    }
+
+    @Test
+    public void sortReflexive() {
+        ObjectId id1 = ObjectId.of("reflexive", "id", 1);
+        ObjectId id2 = ObjectId.of("reflexive", "id", 2);
+        ObjectId id3 = ObjectId.of("reflexive", "id", 3);
+        ObjectId id4 = ObjectId.of("reflexive", "id", 4);
+
+        DbEntity reflexive = mockEntity("reflexive");
+        DbRowOp op1 = new InsertDbRowOp(mockObject(id1), reflexive, id1);
+        DbRowOp op2 = new InsertDbRowOp(mockObject(id2), reflexive, id2);
+        DbRowOp op3 = new InsertDbRowOp(mockObject(id3), reflexive, id3);
+        DbRowOp op4 = new InsertDbRowOp(mockObject(id4), reflexive, id4);
+
+        List<DbRowOp> rows = Arrays.asList(op1, op2, op3, op4);
+        List<DbRowOp> expected = Arrays.asList(op1, op2, op3, op4);
+
+        List<DbRowOp> sorted = sorter.sort(rows);
+        assertEquals(expected, sorted); // no actual sorting is done
+        verifyZeroInteractions(entitySorter);
+    }
+
+    private Persistent mockObject(ObjectId id) {
+        Persistent persistent = mock(Persistent.class);
+        when(persistent.getObjectId()).thenReturn(id);
+        
when(persistent.getPersistenceState()).thenReturn(PersistenceState.MODIFIED);
+        return persistent;
+    }
+
+    private DbEntity mockEntity(String name) {
+        DbAttribute attribute1 = new DbAttribute("id");
+        attribute1.setPrimaryKey(true);
+        DbAttribute attribute2 = new DbAttribute("attr");
+        DbEntity testEntity = new DbEntity(name);
+        testEntity.addAttribute(attribute1);
+        testEntity.addAttribute(attribute2);
+        return testEntity;
+    }
+}
\ No newline at end of file

Reply via email to