Repository: cayenne
Updated Branches:
  refs/heads/STABLE-4.0 a3e5f1412 -> 3ed2301bf


CAY-2349 Cache issue: 'SelectQuery' with prefetches loses relationships


Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo
Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/3ed2301b
Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/3ed2301b
Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/3ed2301b

Branch: refs/heads/STABLE-4.0
Commit: 3ed2301bf0deee0c03179586d271254872994173
Parents: a3e5f14
Author: Nikita Timofeev <stari...@gmail.com>
Authored: Wed Aug 16 12:47:20 2017 +0300
Committer: Nikita Timofeev <stari...@gmail.com>
Committed: Wed Aug 16 12:47:20 2017 +0300

----------------------------------------------------------------------
 .../cayenne/query/SelectQueryMetadata.java      |  56 ++++++++-
 .../cayenne/access/DataContextPrefetchIT.java   | 126 +++++++++++++++++--
 .../query/SelectQueryMetadataCacheKeyTest.java  | 123 ++++++++++++++++++
 docs/doc/src/main/resources/RELEASE-NOTES.txt   |   1 +
 4 files changed, 293 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cayenne/blob/3ed2301b/cayenne-server/src/main/java/org/apache/cayenne/query/SelectQueryMetadata.java
----------------------------------------------------------------------
diff --git 
a/cayenne-server/src/main/java/org/apache/cayenne/query/SelectQueryMetadata.java
 
b/cayenne-server/src/main/java/org/apache/cayenne/query/SelectQueryMetadata.java
index 1c5b04e..37a0301 100644
--- 
a/cayenne-server/src/main/java/org/apache/cayenne/query/SelectQueryMetadata.java
+++ 
b/cayenne-server/src/main/java/org/apache/cayenne/query/SelectQueryMetadata.java
@@ -107,10 +107,9 @@ class SelectQueryMetadata extends BaseQueryMetadata {
                }
 
                if(query.getColumns() != null && !query.getColumns().isEmpty()) 
{
-                       key.append("/");
                        traversalHandler = new 
ToCacheKeyTraversalHandler(resolver.getValueObjectTypeRegistry(), key);
                        for(Property<?> property : query.getColumns()) {
-                               key.append("c:");
+                               key.append("/c:");
                                
property.getExpression().traverse(traversalHandler);
                        }
                }
@@ -146,6 +145,11 @@ class SelectQueryMetadata extends BaseQueryMetadata {
                        }
                }
 
+               // add prefetch to cache key per CAY-2349
+               if(query.getPrefetchTree() != null) {
+                       query.getPrefetchTree().traverse(new 
ToCacheKeyPrefetchProcessor(key));
+               }
+
                return key.toString();
        }
 
@@ -474,5 +478,51 @@ class SelectQueryMetadata extends BaseQueryMetadata {
                                }
                        }
                }
-       };
+       }
+
+       /**
+        * Prefetch processor that append prefetch tree into cache key.
+        * @since 4.0
+        */
+       static class ToCacheKeyPrefetchProcessor implements PrefetchProcessor {
+
+               StringBuilder out;
+
+               ToCacheKeyPrefetchProcessor(StringBuilder out) {
+                       this.out = out;
+               }
+
+               @Override
+               public boolean startPhantomPrefetch(PrefetchTreeNode node) {
+                       return true;
+               }
+
+               @Override
+               public boolean startDisjointPrefetch(PrefetchTreeNode node) {
+                       out.append("/pd:").append(node.getPath());
+                       return true;
+               }
+
+               @Override
+               public boolean startDisjointByIdPrefetch(PrefetchTreeNode node) 
{
+                       out.append("/pi:").append(node.getPath());
+                       return true;
+               }
+
+               @Override
+               public boolean startJointPrefetch(PrefetchTreeNode node) {
+                       out.append("/pj:").append(node.getPath());
+                       return true;
+               }
+
+               @Override
+               public boolean startUnknownPrefetch(PrefetchTreeNode node) {
+                       out.append("/pu:").append(node.getPath());
+                       return true;
+               }
+
+               @Override
+               public void finishPrefetch(PrefetchTreeNode node) {
+               }
+       }
 }

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3ed2301b/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextPrefetchIT.java
----------------------------------------------------------------------
diff --git 
a/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextPrefetchIT.java
 
b/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextPrefetchIT.java
index 792d815..dce94a4 100644
--- 
a/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextPrefetchIT.java
+++ 
b/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextPrefetchIT.java
@@ -20,6 +20,7 @@
 package org.apache.cayenne.access;
 
 import org.apache.cayenne.Cayenne;
+import org.apache.cayenne.Fault;
 import org.apache.cayenne.PersistenceState;
 import org.apache.cayenne.ValueHolder;
 import org.apache.cayenne.di.Inject;
@@ -28,6 +29,7 @@ import org.apache.cayenne.exp.ExpressionFactory;
 import org.apache.cayenne.exp.Property;
 import org.apache.cayenne.map.ObjEntity;
 import org.apache.cayenne.map.ObjRelationship;
+import org.apache.cayenne.query.ObjectSelect;
 import org.apache.cayenne.query.QueryCacheStrategy;
 import org.apache.cayenne.query.SelectQuery;
 import org.apache.cayenne.test.jdbc.DBHelper;
@@ -35,6 +37,7 @@ import org.apache.cayenne.test.jdbc.TableHelper;
 import org.apache.cayenne.testdo.testmap.ArtGroup;
 import org.apache.cayenne.testdo.testmap.Artist;
 import org.apache.cayenne.testdo.testmap.ArtistExhibit;
+import org.apache.cayenne.testdo.testmap.Gallery;
 import org.apache.cayenne.testdo.testmap.Painting;
 import org.apache.cayenne.testdo.testmap.PaintingInfo;
 import org.apache.cayenne.unit.di.DataChannelInterceptor;
@@ -81,8 +84,8 @@ public class DataContextPrefetchIT extends ServerCase {
                tArtist.setColumns("ARTIST_ID", "ARTIST_NAME");
 
                tPainting = new TableHelper(dbHelper, "PAINTING");
-               tPainting.setColumns("PAINTING_ID", "PAINTING_TITLE", 
"ARTIST_ID", "ESTIMATED_PRICE").setColumnTypes(
-                               Types.INTEGER, Types.VARCHAR, Types.BIGINT, 
Types.DECIMAL);
+               tPainting.setColumns("PAINTING_ID", "PAINTING_TITLE", 
"ARTIST_ID", "ESTIMATED_PRICE", "GALLERY_ID").setColumnTypes(
+                               Types.INTEGER, Types.VARCHAR, Types.BIGINT, 
Types.DECIMAL, Types.INTEGER);
 
                tPaintingInfo = new TableHelper(dbHelper, "PAINTING_INFO");
                tPaintingInfo.setColumns("PAINTING_ID", "TEXT_REVIEW");
@@ -106,15 +109,15 @@ public class DataContextPrefetchIT extends ServerCase {
        protected void createTwoArtistsAndTwoPaintingsDataSet() throws 
Exception {
                tArtist.insert(11, "artist2");
                tArtist.insert(101, "artist3");
-               tPainting.insert(6, "p_artist3", 101, 1000);
-               tPainting.insert(7, "p_artist2", 11, 2000);
+               tPainting.insert(6, "p_artist3", 101, 1000, null);
+               tPainting.insert(7, "p_artist2", 11, 2000, null);
        }
 
        protected void createArtistWithTwoPaintingsAndTwoInfosDataSet() throws 
Exception {
                tArtist.insert(11, "artist2");
 
-               tPainting.insert(6, "p_artist2", 11, 1000);
-               tPainting.insert(7, "p_artist3", 11, 2000);
+               tPainting.insert(6, "p_artist2", 11, 1000, null);
+               tPainting.insert(7, "p_artist3", 11, 2000, null);
 
                tPaintingInfo.insert(6, "xYs");
        }
@@ -545,9 +548,9 @@ public class DataContextPrefetchIT extends ServerCase {
                tArtGroup.insert(1, "AG");
                tArtist.insert(11, "artist2");
                tArtist.insert(101, "artist3");
-               tPainting.insert(6, "p_artist3", 101, 1000);
-               tPainting.insert(7, "p_artist21", 11, 2000);
-               tPainting.insert(8, "p_artist22", 11, 3000);
+               tPainting.insert(6, "p_artist3", 101, 1000, null);
+               tPainting.insert(7, "p_artist21", 11, 2000, null);
+               tPainting.insert(8, "p_artist22", 11, 3000, null);
 
                // flattened join matches an object that is NOT the one we are 
looking
                // for
@@ -657,7 +660,7 @@ public class DataContextPrefetchIT extends ServerCase {
        @Test
        public void testPrefetchingToOneNull() throws Exception {
 
-               tPainting.insert(6, "p_Xty", null, 1000);
+               tPainting.insert(6, "p_Xty", null, 1000, null);
 
                SelectQuery q = new SelectQuery(Painting.class);
                q.addPrefetch(Painting.TO_ARTIST.disjoint());
@@ -842,4 +845,107 @@ public class DataContextPrefetchIT extends ServerCase {
                });
        }
 
+       /**
+        * This test and next one is the result of CAY-2349 fix
+        */
+       @Test
+       public void testPrefetchWithLocalCache() throws Exception {
+               tArtist.deleteAll();
+               tGallery.deleteAll();
+               tPainting.deleteAll();
+               tArtist.insert(1, "artist1");
+               tGallery.insert(1, "gallery1");
+               tPainting.insert(1, "painting1", 1, 100, 1);
+
+               List<Painting> paintings = ObjectSelect.query(Painting.class)
+                               .localCache("g1").select(context);
+               assertEquals(1, paintings.size());
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Fault);
+
+               paintings = ObjectSelect.query(Painting.class)
+                               .prefetch(Painting.TO_ARTIST.joint())
+                               .localCache("g1").select(context);
+               assertEquals(1, paintings.size());
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Artist);
+
+               queryInterceptor.runWithQueriesBlocked(new UnitTestClosure() {
+
+                       public void execute() {
+                               List<Painting> paintings = 
ObjectSelect.query(Painting.class)
+                                               
.prefetch(Painting.TO_ARTIST.joint())
+                                               
.localCache("g1").select(context);
+                               assertEquals(1, paintings.size());
+                               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Artist);
+                       }
+               });
+       }
+
+       @Test
+       public void testPrefetchWithSharedCache() throws Exception {
+               tArtist.deleteAll();
+               tGallery.deleteAll();
+               tPainting.deleteAll();
+               tArtist.insert(1, "artist1");
+               tGallery.insert(1, "gallery1");
+               tPainting.insert(1, "painting1", 1, 100, 1);
+
+               final ObjectSelect<Painting> s1 = 
ObjectSelect.query(Painting.class)
+                               .sharedCache("g1");
+
+               final ObjectSelect<Painting> s2 = 
ObjectSelect.query(Painting.class)
+                               .prefetch(Painting.TO_ARTIST.disjoint())
+                               .sharedCache("g1");
+
+               final ObjectSelect<Painting> s3 = 
ObjectSelect.query(Painting.class)
+                               .prefetch(Painting.TO_GALLERY.joint())
+                               .sharedCache("g1");
+
+               final ObjectSelect<Painting> s4 = 
ObjectSelect.query(Painting.class)
+                               .prefetch(Painting.TO_ARTIST.disjoint())
+                               .prefetch(Painting.TO_GALLERY.joint())
+                               .sharedCache("g1");
+
+               // first iteration select from DB and cache
+               List<Painting> paintings = s1.select(context);
+               assertEquals(1, paintings.size());
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Fault);
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_GALLERY.getName()) 
instanceof Fault);
+
+               paintings = s2.select(context);
+               assertEquals(1, paintings.size());
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Artist);
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_GALLERY.getName()) 
instanceof Fault);
+
+               paintings = s3.select(context);
+               assertEquals(1, paintings.size());
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Fault);
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_GALLERY.getName()) 
instanceof Gallery);
+
+               paintings = s4.select(context);
+               assertEquals(1, paintings.size());
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Artist);
+               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_GALLERY.getName()) 
instanceof Gallery);
+
+               queryInterceptor.runWithQueriesBlocked(new UnitTestClosure() {
+
+                       public void execute() {
+                               // select from cache
+                               List<Painting> paintings = s2.select(context);
+                               assertEquals(1, paintings.size());
+                               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Artist);
+                               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_GALLERY.getName()) 
instanceof Fault);
+
+                               paintings = s3.select(context);
+                               assertEquals(1, paintings.size());
+                               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Fault);
+                               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_GALLERY.getName()) 
instanceof Gallery);
+
+                               paintings = s4.select(context);
+                               assertEquals(1, paintings.size());
+                               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_ARTIST.getName()) 
instanceof Artist);
+                               
assertTrue(paintings.get(0).readPropertyDirectly(Painting.TO_GALLERY.getName()) 
instanceof Gallery);
+                       }
+               });
+       }
+
 }

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3ed2301b/cayenne-server/src/test/java/org/apache/cayenne/query/SelectQueryMetadataCacheKeyTest.java
----------------------------------------------------------------------
diff --git 
a/cayenne-server/src/test/java/org/apache/cayenne/query/SelectQueryMetadataCacheKeyTest.java
 
b/cayenne-server/src/test/java/org/apache/cayenne/query/SelectQueryMetadataCacheKeyTest.java
index efd03c4..dfbb23e 100644
--- 
a/cayenne-server/src/test/java/org/apache/cayenne/query/SelectQueryMetadataCacheKeyTest.java
+++ 
b/cayenne-server/src/test/java/org/apache/cayenne/query/SelectQueryMetadataCacheKeyTest.java
@@ -25,6 +25,8 @@ import org.apache.cayenne.access.types.ValueObjectType;
 import org.apache.cayenne.access.types.ValueObjectTypeRegistry;
 import org.apache.cayenne.exp.ExpressionFactory;
 import org.apache.cayenne.exp.TraversalHandler;
+import org.apache.cayenne.testdo.testmap.Artist;
+import org.apache.cayenne.testdo.testmap.Painting;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -209,10 +211,131 @@ public class SelectQueryMetadataCacheKeyTest {
         assertNotEquals(s6, s7);
     }
 
+    @Test
+    public void testPrefetchEmpty() {
+        PrefetchTreeNode prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        assertTrue(cacheKey.toString().isEmpty());
+    }
+
+    @Test
+    public void testPrefetchSingle() {
+        PrefetchTreeNode prefetchTreeNode;
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s1 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s2 = cacheKey.toString();
+
+        assertFalse(s1.isEmpty());
+        assertEquals(s1, s2);
+    }
+
+    @Test
+    public void testPrefetchSemantics() {
+        PrefetchTreeNode prefetchTreeNode;
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s1 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.disjoint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s2 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.disjointById());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s3 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.disjoint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s4 = cacheKey.toString();
+
+        assertNotEquals(s1, s2);
+        assertNotEquals(s2, s3);
+        assertEquals(s2, s4);
+    }
+
+    @Test
+    public void testPrefetchMultiNodes() {
+        PrefetchTreeNode prefetchTreeNode;
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s1 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.merge(Artist.GROUP_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s2 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.merge(Artist.GROUP_ARRAY.joint());
+        prefetchTreeNode.merge(Artist.ARTIST_EXHIBIT_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s3 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.merge(Artist.GROUP_ARRAY.joint());
+        prefetchTreeNode.merge(Artist.ARTIST_EXHIBIT_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s4 = cacheKey.toString();
+
+        assertNotEquals(s1, s2);
+        assertNotEquals(s2, s3);
+        assertEquals(s3, s4);
+    }
+
+    @Test
+    public void testPrefetchLongPaths() {
+        PrefetchTreeNode prefetchTreeNode;
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        prefetchTreeNode.merge(Artist.PAINTING_ARRAY.joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s1 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        
prefetchTreeNode.merge(Artist.PAINTING_ARRAY.dot(Painting.TO_ARTIST).joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s2 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        
prefetchTreeNode.merge(Artist.PAINTING_ARRAY.dot(Painting.TO_ARTIST).dot(Artist.GROUP_ARRAY).joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s3 = cacheKey.toString();
+
+        prefetchTreeNode = new PrefetchTreeNode();
+        
prefetchTreeNode.merge(Artist.PAINTING_ARRAY.dot(Painting.TO_ARTIST).dot(Artist.GROUP_ARRAY).joint());
+        prefetchTreeNode.traverse(newPrefetchProcessor());
+        String s4 = cacheKey.toString();
+
+        assertNotEquals(s1, s2);
+        assertNotEquals(s2, s3);
+        assertEquals(s3, s4);
+    }
+
     private TraversalHandler newHandler() {
         return new SelectQueryMetadata.ToCacheKeyTraversalHandler(registry, 
cacheKey = new StringBuilder());
     }
 
+    private PrefetchProcessor newPrefetchProcessor() {
+        return new SelectQueryMetadata.ToCacheKeyPrefetchProcessor(cacheKey = 
new StringBuilder());
+    }
+
     /* ************* Test types *************** */
 
     /**

http://git-wip-us.apache.org/repos/asf/cayenne/blob/3ed2301b/docs/doc/src/main/resources/RELEASE-NOTES.txt
----------------------------------------------------------------------
diff --git a/docs/doc/src/main/resources/RELEASE-NOTES.txt 
b/docs/doc/src/main/resources/RELEASE-NOTES.txt
index 544a276..a8d263f 100644
--- a/docs/doc/src/main/resources/RELEASE-NOTES.txt
+++ b/docs/doc/src/main/resources/RELEASE-NOTES.txt
@@ -17,6 +17,7 @@ Bug Fixes:
 CAY-2319 Modeler: Embeddable > Attributes. Undo does not cancel pasted objects
 CAY-2323 Modeler: Graph. No warning while saving the image with existing name
 CAY-2331 cgen: broken templates for data map
+CAY-2349 Cache issue: 'SelectQuery' with prefetches loses relationships
 
 ----------------------------------
 Release: 4.0.B1

Reply via email to