CAY-2904 disjoint prefetch returns incorrect data

cayenne/src/main/java/org/apache/cayenne/access/HierarchicalObjectResolver.java

@@ -114,6 +114,20 @@ public boolean startDisjointByIdPrefetch(PrefetchTreeNode node) {
                 return true;
             }
 
+            PrefetchProcessorNode processorNode = (PrefetchProcessorNode) node;
+            PrefetchProcessorNode parentProcessorNode = (PrefetchProcessorNode) processorNode.getParent();
+
+            // If parent is a joint node, defer processing until the joint node is processed
+            if (parentProcessorNode.getSemantics() == PrefetchTreeNode.JOINT_PREFETCH_SEMANTICS) {
+                // Mark that we need to process this later, but don't fetch data now
+                return true;
+            }
+
+            return processDisjointByIdNode(node);
+        }
+
+        // Process a disjointById node without checking for deferral
+        private boolean processDisjointByIdNode(PrefetchTreeNode node) {
             PrefetchProcessorNode processorNode = (PrefetchProcessorNode) node;
             PrefetchProcessorNode parentProcessorNode = (PrefetchProcessorNode) processorNode.getParent();
             ObjRelationship relationship = processorNode.getIncoming().getRelationship();
@@ -137,6 +151,12 @@ public boolean startDisjointByIdPrefetch(PrefetchTreeNode node) {
                 parentDataRows = parentProcessorNode.getDataRows();
             }
 
+            // If parent data rows is null or empty, there's nothing to prefetch
+            if (parentDataRows == null || parentDataRows.isEmpty()) {
+                processorNode.setDataRows(new ArrayList<>());
+                return true;
+            }
+
             int maxIdQualifierSize = context.getParentDataDomain().getMaxIdQualifierSize();
             List<DbJoin> joins = getDbJoins(relationship);
             Map<DbJoin, String> joinToDataRowKey = getDataRowKeys(joins, pathPrefix);
@@ -362,6 +382,17 @@ public boolean startJointPrefetch(PrefetchTreeNode node) {
                     processorNode.getResolvedRows(),
                     queryMetadata.isRefreshingObjects());
 
+            // Now process any deferred disjointById children
+            DisjointByIdProcessor byIdProcessor = new DisjointByIdProcessor();
+            for (PrefetchTreeNode child : node.getChildren()) {
+                if (child.isDisjointByIdPrefetch()) {
+                    // Now that the joint parent has been processed, we can fetch the disjointById data
+                    byIdProcessor.processDisjointByIdNode(child);
+                    // And resolve the objects
+                    startDisjointPrefetch(child);
+                }
+            }
+
             return true;
         }
 
@@ -444,6 +475,10 @@ public boolean startJointPrefetch(PrefetchTreeNode node) {
             }
 
             // linking by parent needed even if an object is already there (many-to-many case)
+            // we need the row for parent attachment even if object was already resolved
+            if (row == null) {
+                row = processorNode.rowFromFlatRow(currentFlatRow);
+            }
             processorNode.getParentAttachmentStrategy().linkToParent(row, object);
 
             processorNode.setLastResolved(object);

cayenne/src/main/java/org/apache/cayenne/access/ResultScanParentAttachmentStrategy.java

@@ -101,7 +101,12 @@ private void indexParents() {
 
         List<DataRow> rows = parentNode.getDataRows();
         if(rows == null) {
-            return;
+            if(parentNode instanceof PrefetchProcessorJointNode) {
+                rows = ((PrefetchProcessorJointNode) parentNode).getResolvedRows();
+            }
+            if(rows == null) {
+                return;
+            }
         }
 
         int size = objects.size();

cayenne/src/main/java/org/apache/cayenne/util/concurrentlinkedhashmap/ConcurrentLinkedHashMap.java

@@ -788,6 +788,7 @@ else if (onlyIfAbsent) {
 
     @Override
     public V remove(Object key) {
+    	if (key == null) return null; // this class does allow null to be used as a key or value (returning here prevents an NPE).
         final Node node = data.remove(key);
         if (node == null) {
             return null;

cayenne/src/test/java/org/apache/cayenne/access/DataContextPrefetchMultistepIT.java

@@ -20,14 +20,17 @@
 package org.apache.cayenne.access;
 
 import org.apache.cayenne.Fault;
+import org.apache.cayenne.ObjectContext;
 import org.apache.cayenne.ObjectId;
 import org.apache.cayenne.PersistenceState;
 import org.apache.cayenne.Persistent;
 import org.apache.cayenne.ValueHolder;
 import org.apache.cayenne.di.Inject;
 import org.apache.cayenne.query.ObjectSelect;
+import org.apache.cayenne.runtime.CayenneRuntime;
 import org.apache.cayenne.test.jdbc.DBHelper;
 import org.apache.cayenne.test.jdbc.TableHelper;
+import org.apache.cayenne.testdo.testmap.Artist;
 import org.apache.cayenne.testdo.testmap.ArtistExhibit;
 import org.apache.cayenne.testdo.testmap.Exhibit;
 import org.apache.cayenne.testdo.testmap.Gallery;
@@ -51,6 +54,9 @@ public class DataContextPrefetchMultistepIT extends RuntimeCase {
     @Inject
     protected DataContext context;
 
+    @Inject
+    protected CayenneRuntime runtime;
+
     @Inject
     protected DBHelper dbHelper;
 
@@ -286,4 +292,159 @@ public void testToManyToOne_EmptyToMany_NoRootQualifier() throws Exception {
         assertFalse(((ValueHolder) exhibits).isFault());
         assertEquals(0, exhibits.size());
     }
+
+
+    private Gallery createArtistWithPaintingInGallery() {
+        Artist artist = context.newObject(Artist.class);
+        artist.setArtistName("Picasso");
+
+        Painting painting = context.newObject(Painting.class);
+        painting.setPaintingTitle("Guernica");
+        artist.addToPaintingArray(painting);
+
+        Gallery gallery = context.newObject(Gallery.class);
+        gallery.setGalleryName("MOMA");
+        painting.setToGallery(gallery);
+
+        context.commitChanges();
+        return gallery;
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_JointAndJoint() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.joint())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).joint())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_JointAndDisjoint() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        ObjectContext objectContext = runtime.newContext();
+
+        // Prefetch the artist through the two relationships
+        Gallery gallery2 = ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.joint())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjoint())
+                .selectOne(objectContext);
+
+        assertNotNull(gallery2.getPaintingArray().get(0).getToArtist());
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_JointAndDisjointById() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.joint())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjointById())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointAndJoint() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.disjoint())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).joint())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointAndDisjoint() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.disjoint())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjoint())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointAndDisjointById() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.disjoint())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjointById())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointByIdAndJoint() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.disjointById())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).joint())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointByIdAndDisjoint() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.disjointById())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjoint())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
+
+    @Test
+    public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointByIdAndDisjointById() {
+        Gallery gallery = createArtistWithPaintingInGallery();
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+
+        // Prefetch the artist through the two relationships
+        ObjectSelect.query(Gallery.class)
+                .prefetch(Gallery.PAINTING_ARRAY.disjointById())
+                .prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjointById())
+                .select(context);
+
+        assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
+    }
 }