Skip to content

Commit bbe569a

Browse files
johnthussstariy95
johnthuss
authored and
stariy95
committed
CAY-2904 Disjoint prefetch returns incorrect data
1 parent 8b4c46e commit bbe569a

4 files changed

Lines changed: 203 additions & 1 deletion

File tree

‎cayenne-server/src/main/java/org/apache/cayenne/access/HierarchicalObjectResolver.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,20 @@ public boolean startDisjointByIdPrefetch(PrefetchTreeNode node) {
110110
return true;
111111
}
112112

113+
PrefetchProcessorNode processorNode = (PrefetchProcessorNode) node;
114+
PrefetchProcessorNode parentProcessorNode = (PrefetchProcessorNode) processorNode.getParent();
115+
116+
// If parent is a joint node, defer processing until the joint node is processed
117+
if (parentProcessorNode.getSemantics() == PrefetchTreeNode.JOINT_PREFETCH_SEMANTICS) {
118+
// Mark that we need to process this later, but don't fetch data now
119+
return true;
120+
}
121+
122+
return processDisjointByIdNode(node);
123+
}
124+
125+
// Process a disjointById node without checking for deferral
126+
private boolean processDisjointByIdNode(PrefetchTreeNode node) {
113127
PrefetchProcessorNode processorNode = (PrefetchProcessorNode) node;
114128
PrefetchProcessorNode parentProcessorNode = (PrefetchProcessorNode) processorNode.getParent();
115129
ObjRelationship relationship = processorNode.getIncoming().getRelationship();
@@ -142,6 +156,12 @@ public boolean startDisjointByIdPrefetch(PrefetchTreeNode node) {
142156
parentDataRows = parentProcessorNode.getDataRows();
143157
}
144158

159+
// If parent data rows is null or empty, there's nothing to prefetch
160+
if (parentDataRows == null || parentDataRows.isEmpty()) {
161+
processorNode.setDataRows(new ArrayList<>());
162+
return true;
163+
}
164+
145165
int maxIdQualifierSize = context.getParentDataDomain().getMaxIdQualifierSize();
146166
List<DbJoin> joins = lastDbRelationship.getJoins();
147167

@@ -322,6 +342,17 @@ public boolean startJointPrefetch(PrefetchTreeNode node) {
322342
processorNode.getResolvedRows(),
323343
queryMetadata.isRefreshingObjects());
324344

345+
// Now process any deferred disjointById children
346+
DisjointByIdProcessor byIdProcessor = new DisjointByIdProcessor();
347+
for (PrefetchTreeNode child : node.getChildren()) {
348+
if (child.isDisjointByIdPrefetch()) {
349+
// Now that the joint parent has been processed, we can fetch the disjointById data
350+
byIdProcessor.processDisjointByIdNode(child);
351+
// And resolve the objects
352+
startDisjointPrefetch(child);
353+
}
354+
}
355+
325356
return true;
326357
}
327358

@@ -404,6 +435,10 @@ public boolean startJointPrefetch(PrefetchTreeNode node) {
404435
}
405436

406437
// linking by parent needed even if an object is already there (many-to-many case)
438+
// we need the row for parent attachment even if object was already resolved
439+
if (row == null) {
440+
row = processorNode.rowFromFlatRow(currentFlatRow);
441+
}
407442
processorNode.getParentAttachmentStrategy().linkToParent(row, object);
408443

409444
processorNode.setLastResolved(object);

‎cayenne-server/src/main/java/org/apache/cayenne/access/ResultScanParentAttachmentStrategy.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,12 @@ private void indexParents() {
101101

102102
List<DataRow> rows = parentNode.getDataRows();
103103
if(rows == null) {
104-
return;
104+
if(parentNode instanceof PrefetchProcessorJointNode) {
105+
rows = ((PrefetchProcessorJointNode) parentNode).getResolvedRows();
106+
}
107+
if(rows == null) {
108+
return;
109+
}
105110
}
106111

107112
int size = objects.size();

‎cayenne-server/src/main/java/org/apache/cayenne/util/concurrentlinkedhashmap/ConcurrentLinkedHashMap.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,7 @@ else if (onlyIfAbsent) {
788788

789789
@Override
790790
public V remove(Object key) {
791+
if (key == null) return null; // this class does allow null to be used as a key or value (returning here prevents an NPE).
791792
final Node node = data.remove(key);
792793
if (node == null) {
793794
return null;

‎cayenne-server/src/test/java/org/apache/cayenne/access/DataContextPrefetchMultistepIT.java‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@
2020
package org.apache.cayenne.access;
2121

2222
import org.apache.cayenne.Fault;
23+
import org.apache.cayenne.ObjectContext;
2324
import org.apache.cayenne.ObjectId;
2425
import org.apache.cayenne.PersistenceState;
2526
import org.apache.cayenne.Persistent;
2627
import org.apache.cayenne.ValueHolder;
2728
import org.apache.cayenne.di.Inject;
2829
import org.apache.cayenne.query.ObjectSelect;
30+
import org.apache.cayenne.runtime.CayenneRuntime;
2931
import org.apache.cayenne.test.jdbc.DBHelper;
3032
import org.apache.cayenne.test.jdbc.TableHelper;
33+
import org.apache.cayenne.testdo.testmap.Artist;
3134
import org.apache.cayenne.testdo.testmap.ArtistExhibit;
3235
import org.apache.cayenne.testdo.testmap.Exhibit;
3336
import org.apache.cayenne.testdo.testmap.Gallery;
@@ -51,6 +54,9 @@ public class DataContextPrefetchMultistepIT extends ServerCase {
5154
@Inject
5255
protected DataContext context;
5356

57+
@Inject
58+
protected CayenneRuntime runtime;
59+
5460
@Inject
5561
protected DBHelper dbHelper;
5662

@@ -286,4 +292,159 @@ public void testToManyToOne_EmptyToMany_NoRootQualifier() throws Exception {
286292
assertFalse(((ValueHolder) exhibits).isFault());
287293
assertEquals(0, exhibits.size());
288294
}
295+
296+
297+
private Gallery createArtistWithPaintingInGallery() {
298+
Artist artist = context.newObject(Artist.class);
299+
artist.setArtistName("Picasso");
300+
301+
Painting painting = context.newObject(Painting.class);
302+
painting.setPaintingTitle("Guernica");
303+
artist.addToPaintingArray(painting);
304+
305+
Gallery gallery = context.newObject(Gallery.class);
306+
gallery.setGalleryName("MOMA");
307+
painting.setToGallery(gallery);
308+
309+
context.commitChanges();
310+
return gallery;
311+
}
312+
313+
@Test
314+
public void testPrefetchAcross2RelationshipsKeepingBoth_JointAndJoint() {
315+
Gallery gallery = createArtistWithPaintingInGallery();
316+
317+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
318+
319+
// Prefetch the artist through the two relationships
320+
ObjectSelect.query(Gallery.class)
321+
.prefetch(Gallery.PAINTING_ARRAY.joint())
322+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).joint())
323+
.select(context);
324+
325+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
326+
}
327+
328+
@Test
329+
public void testPrefetchAcross2RelationshipsKeepingBoth_JointAndDisjoint() {
330+
Gallery gallery = createArtistWithPaintingInGallery();
331+
332+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
333+
334+
ObjectContext objectContext = runtime.newContext();
335+
336+
// Prefetch the artist through the two relationships
337+
Gallery gallery2 = ObjectSelect.query(Gallery.class)
338+
.prefetch(Gallery.PAINTING_ARRAY.joint())
339+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjoint())
340+
.selectOne(objectContext);
341+
342+
assertNotNull(gallery2.getPaintingArray().get(0).getToArtist());
343+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
344+
}
345+
346+
@Test
347+
public void testPrefetchAcross2RelationshipsKeepingBoth_JointAndDisjointById() {
348+
Gallery gallery = createArtistWithPaintingInGallery();
349+
350+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
351+
352+
// Prefetch the artist through the two relationships
353+
ObjectSelect.query(Gallery.class)
354+
.prefetch(Gallery.PAINTING_ARRAY.joint())
355+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjointById())
356+
.select(context);
357+
358+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
359+
}
360+
361+
@Test
362+
public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointAndJoint() {
363+
Gallery gallery = createArtistWithPaintingInGallery();
364+
365+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
366+
367+
// Prefetch the artist through the two relationships
368+
ObjectSelect.query(Gallery.class)
369+
.prefetch(Gallery.PAINTING_ARRAY.disjoint())
370+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).joint())
371+
.select(context);
372+
373+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
374+
}
375+
376+
@Test
377+
public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointAndDisjoint() {
378+
Gallery gallery = createArtistWithPaintingInGallery();
379+
380+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
381+
382+
// Prefetch the artist through the two relationships
383+
ObjectSelect.query(Gallery.class)
384+
.prefetch(Gallery.PAINTING_ARRAY.disjoint())
385+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjoint())
386+
.select(context);
387+
388+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
389+
}
390+
391+
@Test
392+
public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointAndDisjointById() {
393+
Gallery gallery = createArtistWithPaintingInGallery();
394+
395+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
396+
397+
// Prefetch the artist through the two relationships
398+
ObjectSelect.query(Gallery.class)
399+
.prefetch(Gallery.PAINTING_ARRAY.disjoint())
400+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjointById())
401+
.select(context);
402+
403+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
404+
}
405+
406+
@Test
407+
public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointByIdAndJoint() {
408+
Gallery gallery = createArtistWithPaintingInGallery();
409+
410+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
411+
412+
// Prefetch the artist through the two relationships
413+
ObjectSelect.query(Gallery.class)
414+
.prefetch(Gallery.PAINTING_ARRAY.disjointById())
415+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).joint())
416+
.select(context);
417+
418+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
419+
}
420+
421+
@Test
422+
public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointByIdAndDisjoint() {
423+
Gallery gallery = createArtistWithPaintingInGallery();
424+
425+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
426+
427+
// Prefetch the artist through the two relationships
428+
ObjectSelect.query(Gallery.class)
429+
.prefetch(Gallery.PAINTING_ARRAY.disjointById())
430+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjoint())
431+
.select(context);
432+
433+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
434+
}
435+
436+
@Test
437+
public void testPrefetchAcross2RelationshipsKeepingBoth_DisjointByIdAndDisjointById() {
438+
Gallery gallery = createArtistWithPaintingInGallery();
439+
440+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
441+
442+
// Prefetch the artist through the two relationships
443+
ObjectSelect.query(Gallery.class)
444+
.prefetch(Gallery.PAINTING_ARRAY.disjointById())
445+
.prefetch(Gallery.PAINTING_ARRAY.dot(Painting.TO_ARTIST).disjointById())
446+
.select(context);
447+
448+
assertNotNull(gallery.getPaintingArray().get(0).getToArtist());
449+
}
289450
}

0 commit comments

Comments
 (0)
⚡