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: commits-unsubscr...@cayenne.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to