Skip to content

Commit ef7f245

Browse files
stariy95stariy95
authored
Merge pull request #624 from johnthuss/prefetchWipesOutData_failingTests
CAY-2904 disjoint prefetch returns incorrect data
2 parents 80e1d47 + 47f8af6 commit ef7f245

File tree

4 files changed

+203
-1
lines changed

4 files changed

+203
-1
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,20 @@ public boolean startDisjointByIdPrefetch(PrefetchTreeNode node) {
114114
return true;
115115
}
116116

117+
PrefetchProcessorNode processorNode = (PrefetchProcessorNode) node;
118+
PrefetchProcessorNode parentProcessorNode = (PrefetchProcessorNode) processorNode.getParent();
119+
120+
// If parent is a joint node, defer processing until the joint node is processed
121+
if (parentProcessorNode.getSemantics() == PrefetchTreeNode.JOINT_PREFETCH_SEMANTICS) {
122+
// Mark that we need to process this later, but don't fetch data now
123+
return true;
124+
}
125+
126+
return processDisjointByIdNode(node);
127+
}
128+
129+
// Process a disjointById node without checking for deferral
130+
private boolean processDisjointByIdNode(PrefetchTreeNode node) {
117131
PrefetchProcessorNode processorNode = (PrefetchProcessorNode) node;
118132
PrefetchProcessorNode parentProcessorNode = (PrefetchProcessorNode) processorNode.getParent();
119133
ObjRelationship relationship = processorNode.getIncoming().getRelationship();
@@ -137,6 +151,12 @@ public boolean startDisjointByIdPrefetch(PrefetchTreeNode node) {
137151
parentDataRows = parentProcessorNode.getDataRows();
138152
}
139153

154+
// If parent data rows is null or empty, there's nothing to prefetch
155+
if (parentDataRows == null || parentDataRows.isEmpty()) {
156+
processorNode.setDataRows(new ArrayList<>());
157+
return true;
158+
}
159+
140160
int maxIdQualifierSize = context.getParentDataDomain().getMaxIdQualifierSize();
141161
List<DbJoin> joins = getDbJoins(relationship);
142162
Map<DbJoin, String> joinToDataRowKey = getDataRowKeys(joins, pathPrefix);
@@ -362,6 +382,17 @@ public boolean startJointPrefetch(PrefetchTreeNode node) {
362382
processorNode.getResolvedRows(),
363383
queryMetadata.isRefreshingObjects());
364384

385+
// Now process any deferred disjointById children
386+
DisjointByIdProcessor byIdProcessor = new DisjointByIdProcessor();
387+
for (PrefetchTreeNode child : node.getChildren()) {
388+
if (child.isDisjointByIdPrefetch()) {
389+
// Now that the joint parent has been processed, we can fetch the disjointById data
390+
byIdProcessor.processDisjointByIdNode(child);
391+
// And resolve the objects
392+
startDisjointPrefetch(child);
393+
}
394+
}
395+
365396
return true;
366397
}
367398

@@ -444,6 +475,10 @@ public boolean startJointPrefetch(PrefetchTreeNode node) {
444475
}
445476

446477
// linking by parent needed even if an object is already there (many-to-many case)
478+
// we need the row for parent attachment even if object was already resolved
479+
if (row == null) {
480+
row = processorNode.rowFromFlatRow(currentFlatRow);
481+
}
447482
processorNode.getParentAttachmentStrategy().linkToParent(row, object);
448483

449484
processorNode.setLastResolved(object);

‎cayenne/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/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/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 RuntimeCase {
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)