This is an automated email from the ASF dual-hosted git repository.
ntimofeev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cayenne.git
The following commit(s) were added to refs/heads/master by this push:
new 3c2512600 CAY-2867 Tweak GraphBasedDbRowOpSorter logic to allow
related updates
3c2512600 is described below
commit 3c25126008ee01f01fccbcb1c29b5f93cb3e527d
Author: Nikita Timofeev <[email protected]>
AuthorDate: Thu Aug 15 16:54:03 2024 +0400
CAY-2867 Tweak GraphBasedDbRowOpSorter logic to allow related updates
---
RELEASE-NOTES.txt | 1 +
.../flush/operation/GraphBasedDbRowOpSorter.java | 59 +++++--
...tyWithMeaningfulPKAndCustomDbRowOpSorterIT.java | 24 +++
.../DataContextEntityWithMeaningfulPKIT.java | 23 ++-
.../operation/GraphBasedDbRowOpSorterTest.java | 181 +++++++++++++++++++++
5 files changed, 275 insertions(+), 13 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index b7f985f28..3a5031e99 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -78,6 +78,7 @@ CAY-2857 Java 22 support
CAY-2858 Redesign Collection and Map Property API
CAY-2862 Cleanup and upgrade Maven plugins dependencies
CAY-2865 Upgrade test dependencies
+CAY-2867 Tweak GraphBasedDbRowOpSorter logic to allow related updates
Bug Fixes:
diff --git
a/cayenne/src/main/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorter.java
b/cayenne/src/main/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorter.java
index 8ed961773..7818afd1e 100644
---
a/cayenne/src/main/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorter.java
+++
b/cayenne/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,10 +143,25 @@ 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:
@@ -170,21 +187,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;
@@ -258,7 +275,7 @@ public class GraphBasedDbRowOpSorter implements
DbRowOpSorter {
QueryResponse response =
object.getObjectContext().getChannel().onQuery(null, query);
@SuppressWarnings("unchecked")
List<DataRow> result = (List<DataRow>) response.firstList();
- if (result == null || result.size() == 0) {
+ if (result == null || result.isEmpty()) {
return Collections.emptyMap();
}
@@ -311,4 +328,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/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKAndCustomDbRowOpSorterIT.java
b/cayenne/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKAndCustomDbRowOpSorterIT.java
index 09bfeaf34..fbc17e995 100644
---
a/cayenne/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKAndCustomDbRowOpSorterIT.java
+++
b/cayenne/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKAndCustomDbRowOpSorterIT.java
@@ -76,4 +76,28 @@ public class
DataContextEntityWithMeaningfulPKAndCustomDbRowOpSorterIT extends R
context.commitChanges();
}
+ @Test
+ 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();
+
+ // check that operations are sorted correctly
+ dep.setToMeaningfulPK(null);
+ obj.setPkAttribute(1002);
+ context.commitChanges();
+
+ // set relationship with a new PK
+ dep.setDescr("test");
+ dep.setToMeaningfulPK(obj);
+ context.commitChanges();
+ }
+
}
diff --git
a/cayenne/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKIT.java
b/cayenne/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKIT.java
index 0bffbbe29..76f149fc9 100644
---
a/cayenne/src/test/java/org/apache/cayenne/access/DataContextEntityWithMeaningfulPKIT.java
+++
b/cayenne/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;
@@ -39,7 +40,6 @@ import org.junit.Ignore;
import org.junit.Test;
import java.util.List;
-import java.util.Map;
import static org.junit.Assert.*;
@@ -78,7 +78,7 @@ public class DataContextEntityWithMeaningfulPKIT extends
RuntimeCase {
int id = Cayenne.intPKForObject(obj);
- Map snapshot =
context.getObjectStore().getDataRowCache().getCachedSnapshot(obj.getObjectId());
+ DataRow snapshot =
context.getObjectStore().getDataRowCache().getCachedSnapshot(obj.getObjectId());
assertNotNull(snapshot);
assertTrue(snapshot.containsKey(MeaningfulPKTest1.PK_ATTRIBUTE_PK_COLUMN));
assertEquals(id,
snapshot.get(MeaningfulPKTest1.PK_ATTRIBUTE_PK_COLUMN));
@@ -250,4 +250,23 @@ public class DataContextEntityWithMeaningfulPKIT extends
RuntimeCase {
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/src/test/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorterTest.java
b/cayenne/src/test/java/org/apache/cayenne/access/flush/operation/GraphBasedDbRowOpSorterTest.java
new file mode 100644
index 000000000..7b1a0f192
--- /dev/null
+++
b/cayenne/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
+ verifyNoInteractions(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
