stariy95 commented on code in PR #614:
URL: https://github.com/apache/cayenne/pull/614#discussion_r1595029409
##########
cayenne/src/test/java/org/apache/cayenne/access/DataContextFlattenedAttributesIT.java:
##########
@@ -457,28 +457,42 @@ public void testDelete() throws Exception {
a.setArtistName("AX");
context.commitChanges();
- CompoundPainting o1 = context.newObject(CompoundPainting.class);
- o1.setArtistName("A1");
- o1.setEstimatedPrice(new BigDecimal(1.0d));
- o1.setGalleryName("G1");
- o1.setPaintingTitle("P1");
- o1.setTextReview("T1");
+ createTestDataSet();
- context.commitChanges();
+ List<CompoundPainting> objects =
ObjectSelect.query(CompoundPainting.class)
+ .where(CompoundPainting.ARTIST_NAME.eq("artist2"))
+ .select(context);
- context.deleteObjects(o1);
- context.commitChanges();
+ // Should have two paintings by the same artist
+ assertEquals("Paintings", 2, objects.size());
- Number artistCount = (Number) Cayenne.objectForQuery(context, new
EJBQLQuery(
- "select count(a) from Artist a"));
- assertEquals(1, artistCount.intValue());
- Number paintingCount = (Number) Cayenne.objectForQuery(context, new
EJBQLQuery(
- "select count(a) from Painting a"));
- assertEquals(0, paintingCount.intValue());
+ CompoundPainting cp0 = objects.get(0);
+ CompoundPainting cp1 = objects.get(1);
- Number galleryCount = (Number) Cayenne.objectForQuery(context, new
EJBQLQuery(
- "select count(a) from Gallery a"));
- assertEquals(0, galleryCount.intValue());
+ // Both paintings are at the same gallery
+ assertEquals("Gallery", cp0.getGalleryName(), cp1.getGalleryName());
+
+ context.invalidateObjects(cp0);
+ context.deleteObjects(cp1);
+ context.commitChanges();
+
+ // Delete should only have deleted the painting and its info,
+ // the painting's artist and gallery should not be deleted.
+
+ objects = ObjectSelect.query(CompoundPainting.class)
+ .where(CompoundPainting.ARTIST_NAME.eq("artist2"))
+ .select(runtime.newContext());
+
+ // Should now only have one painting by artist2
+ assertEquals("Painting", 1, objects.size());
+ // and that painting should have a valid gallery
+ assertNotNull("Gallery is null", objects.get(0).getToGallery());
+ assertNotNull("GalleryName is null",
objects.get(0).getToGallery().getGalleryName());
+
+ // There should be one less painting info now
+ Number infoCount = (Number) Cayenne.objectForQuery(context, new
EJBQLQuery(
Review Comment:
A minor feature advertisement, you could query it just like this: `long
count = ObjectSelect.query(PaintingInfo.class).selectCount(context);`
##########
cayenne/src/main/java/org/apache/cayenne/access/flush/RootRowOpProcessor.java:
##########
@@ -74,14 +79,37 @@ public Void visitUpdate(UpdateDbRowOp dbRow) {
@Override
public Void visitDelete(DeleteDbRowOp dbRow) {
- if (dbRowOpFactory.getDescriptor().getEntity().isReadOnly()) {
+ ObjEntity entity = dbRowOpFactory.getDescriptor().getEntity();
+ if (entity.isReadOnly()) {
throw new CayenneRuntimeException("Attempt to modify object(s)
mapped to a read-only entity: '%s'. " +
- "Can't commit changes.",
dbRowOpFactory.getDescriptor().getEntity().getName());
+ "Can't commit changes.", entity.getName());
}
diff.apply(deleteHandler);
- Collection<ObjectId> flattenedIds =
dbRowOpFactory.getStore().getFlattenedIds(dbRow.getChangeId());
- flattenedIds.forEach(id ->
dbRowOpFactory.getOrCreate(dbRowOpFactory.getDbEntity(id), id,
DbRowOpType.DELETE));
- if (dbRowOpFactory.getDescriptor().getEntity().getDeclaredLockType()
== ObjEntity.LOCK_TYPE_OPTIMISTIC) {
+
+ DbEntity dbSource = entity.getDbEntity();
+ Set<Entry<CayennePath,ObjectId>> flattenedEntries =
dbRowOpFactory.getStore().getFlattenedEntries(dbRow.getChangeId());
+
+ flattenedEntries.forEach(entry -> {
+ DbRelationship dbRel =
dbSource.getRelationship(entry.getKey().first().toString());
+
+ // Don't delete if the target entity has a toMany relationship
with the source entity,
+ // as there may be other records in the source entity with
references to it.
+ if (!dbRel.getReverseRelationship().isToMany()) {
+
+ // Check if there's an ObjRelationship matching the flattened
attributes DbRelationship
+ boolean hasObjRelationship = entity.getRelationships().stream()
Review Comment:
This looks heavy, though not many flattened attributes are usually mapped.
Ultimately this could be precalculated and moved to the model
(`ClassDescriptor.additionalDbEntities` maybe?), but that is definitely 5.0
task only, while this fix could be easily backported to 4.2
##########
cayenne/src/main/java/org/apache/cayenne/access/ObjectStore.java:
##########
@@ -1011,6 +1013,18 @@ public Collection<ObjectId> getFlattenedIds(ObjectId
objectId) {
.getOrDefault(objectId, Collections.emptyMap()).values();
}
+ /**
+ * @since 5.0
+ */
+ public Set<Entry<CayennePath,ObjectId>> getFlattenedEntries(ObjectId
objectId) {
Review Comment:
I think `Map<CayennePath, ObjectId>` would be a bit cleaner to return here.
##########
cayenne/src/test/java/org/apache/cayenne/access/DataContextFlattenedAttributesIT.java:
##########
@@ -457,28 +457,42 @@ public void testDelete() throws Exception {
a.setArtistName("AX");
context.commitChanges();
- CompoundPainting o1 = context.newObject(CompoundPainting.class);
- o1.setArtistName("A1");
- o1.setEstimatedPrice(new BigDecimal(1.0d));
- o1.setGalleryName("G1");
- o1.setPaintingTitle("P1");
- o1.setTextReview("T1");
+ createTestDataSet();
Review Comment:
I think it's better to keep an old test as is and just add a new one for the
task
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
