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