ISPN-4066 org.infinispan.query.dsl.Query.getResultSize() gives wrong …

…results when used in remote mode * totalResults was missing from QueryResponse * relocated query.proto file to obey OSGi rules * renamed query proto package to match new location * add tests for max results
Cotton-Ben · Mar 10, 2014 · ce7d74a · ce7d74a
1 parent a904730
commit ce7d74a
Show file tree

Hide file tree

Showing 17 changed files with 280 additions and 53 deletions.
diff --git a/client/hotrod-client/src/main/java/org/infinispan/client/hotrod/impl/query/RemoteQuery.java b/client/hotrod-client/src/main/java/org/infinispan/client/hotrod/impl/query/RemoteQuery.java
@@ -30,7 +30,7 @@ public final class RemoteQuery implements Query {
    private final int maxResults;
 
    private List results = null;
-   private int numResults;
+   private int totalResults;
 
    public RemoteQuery(RemoteCacheImpl cache, SerializationContext serializationContext,
                       String jpqlString, List<SortCriteria> sortCriteria, long startOffset, int maxResults) {
@@ -77,7 +77,7 @@ private List<Object> executeQuery() {
 
       QueryOperation op = cache.getOperationsFactory().newQueryOperation(this);
       QueryResponse response = op.execute();
-      numResults = response.getNumResults();
+      totalResults = (int) response.getTotalResults();
       if (response.getProjectionSize() > 0) {
          results = new ArrayList<Object>(response.getResults().size() / response.getProjectionSize());
          Iterator<WrappedMessage> it = response.getResults().iterator();
@@ -108,7 +108,7 @@ private List<Object> executeQuery() {
    @Override
    public int getResultSize() {
       list();
-      return numResults;
+      return totalResults;
    }
 
    public SerializationContext getSerializationContext() {

diff --git a/...client/src/test/java/org/infinispan/client/hotrod/query/RemoteQueryDslConditionsTest.java b/...client/src/test/java/org/infinispan/client/hotrod/query/RemoteQueryDslConditionsTest.java
@@ -1033,8 +1033,7 @@ public void testSampleDomainQuery16() throws Exception {
             .toBuilder().build();
 
       List<Transaction> list = q.list();
-      assertEquals(10, q.getResultSize());
-
+      assertEquals(50, q.getResultSize());
       assertEquals(10, list.size());
       for (int i = 0; i < 10; i++) {
          assertEquals("Expensive shoes " + (20 + i), list.get(i).getDescription());

diff --git a/...-client/src/test/java/org/infinispan/client/hotrod/query/RemoteQueryDslIterationTest.java b/...-client/src/test/java/org/infinispan/client/hotrod/query/RemoteQueryDslIterationTest.java
@@ -0,0 +1,206 @@
+package org.infinispan.client.hotrod.query;
+
+import org.infinispan.client.hotrod.RemoteCache;
+import org.infinispan.client.hotrod.RemoteCacheManager;
+import org.infinispan.client.hotrod.Search;
+import org.infinispan.client.hotrod.TestHelper;
+import org.infinispan.client.hotrod.marshall.ProtoStreamMarshaller;
+import org.infinispan.configuration.cache.ConfigurationBuilder;
+import org.infinispan.manager.EmbeddedCacheManager;
+import org.infinispan.protostream.sampledomain.User;
+import org.infinispan.protostream.sampledomain.marshallers.MarshallerRegistration;
+import org.infinispan.query.dsl.Query;
+import org.infinispan.query.dsl.QueryFactory;
+import org.infinispan.query.dsl.SortOrder;
+import org.infinispan.query.remote.ProtobufMetadataManager;
+import org.infinispan.server.hotrod.HotRodServer;
+import org.infinispan.test.SingleCacheManagerTest;
+import org.infinispan.test.fwk.TestCacheManagerFactory;
+import org.testng.annotations.AfterTest;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import java.util.List;
+
+import static org.infinispan.client.hotrod.test.HotRodClientTestingUtil.killRemoteCacheManager;
+import static org.infinispan.client.hotrod.test.HotRodClientTestingUtil.killServers;
+import static org.infinispan.server.hotrod.test.HotRodTestingUtil.hotRodCacheConfiguration;
+import static org.testng.AssertJUnit.*;
+
+/**
+ * Test for orderBy, max results, start offset and projections.
+ *
+ * @author [email protected]
+ * @author [email protected]
+ * @since 7.0
+ */
+@Test(groups = "functional", testName = "client.hotrod.query.RemoteQueryDslIterationTest")
+public class RemoteQueryDslIterationTest extends SingleCacheManagerTest {
+
+   protected HotRodServer hotRodServer;
+   protected RemoteCacheManager remoteCacheManager;
+   protected RemoteCache<String, Object> remoteCache;
+
+   @Override
+   protected EmbeddedCacheManager createCacheManager() throws Exception {
+      ConfigurationBuilder builder = getConfigurationBuilder();
+
+      cacheManager = TestCacheManagerFactory.createCacheManager(builder);
+      cache = cacheManager.getCache();
+
+      hotRodServer = TestHelper.startHotRodServer(cacheManager);
+
+      org.infinispan.client.hotrod.configuration.ConfigurationBuilder clientBuilder = new org.infinispan.client.hotrod.configuration.ConfigurationBuilder();
+      clientBuilder.addServer().host("127.0.0.1").port(hotRodServer.getPort());
+      clientBuilder.marshaller(new ProtoStreamMarshaller());
+      remoteCacheManager = new RemoteCacheManager(clientBuilder.build());
+      remoteCache = remoteCacheManager.getCache();
+
+      //initialize server-side serialization context
+      cacheManager.getGlobalComponentRegistry().getComponent(ProtobufMetadataManager.class).registerProtofile("/sample_bank_account/bank.protobin");
+
+      //initialize client-side serialization context
+      MarshallerRegistration.registerMarshallers(ProtoStreamMarshaller.getSerializationContext(remoteCacheManager));
+
+      return cacheManager;
+   }
+
+   protected ConfigurationBuilder getConfigurationBuilder() {
+      ConfigurationBuilder builder = hotRodCacheConfiguration();
+      builder.indexing().enable()
+            .addProperty("default.directory_provider", getLuceneDirectoryProvider())
+            .addProperty("lucene_version", "LUCENE_CURRENT");
+
+      return builder;
+   }
+
+   protected String getLuceneDirectoryProvider() {
+      return "ram";
+   }
+
+   @AfterTest
+   public void release() {
+      killRemoteCacheManager(remoteCacheManager);
+      killServers(hotRodServer);
+   }
+
+   @BeforeMethod(alwaysRun = true)
+   protected void populateCache() throws Exception {
+      User user1 = new User();
+      user1.setId(1);
+      user1.setName("John");
+      user1.setSurname("White");
+
+      User user2 = new User();
+      user2.setId(2);
+      user2.setName("Jack");
+      user2.setSurname("Black");
+
+      User user3 = new User();
+      user3.setId(3);
+      user3.setName("John");
+      user3.setSurname("Brown");
+
+      User user4 = new User();
+      user4.setId(4);
+      user4.setName("Michael");
+      user4.setSurname("Black");
+
+      remoteCache.put("user_" + user1.getId(), user1);
+      remoteCache.put("user_" + user2.getId(), user2);
+      remoteCache.put("user_" + user3.getId(), user3);
+      remoteCache.put("user_" + user4.getId(), user4);
+   }
+
+   public void testOrderByAsc() throws Exception {
+      QueryFactory qf = Search.getQueryFactory(remoteCache);
+
+      Query q = qf.from(User.class)
+            .orderBy("name", SortOrder.ASC).build();
+
+      assertEquals(4, q.getResultSize());
+
+      List<User> list = q.list();
+      assertEquals(4, list.size());
+      checkNameOrder(list, true);
+   }
+
+   public void testOrderByDesc() throws Exception {
+      QueryFactory qf = Search.getQueryFactory(remoteCache);
+
+      Query q = qf.from(User.class)
+            .orderBy("surname", SortOrder.DESC).build();
+
+      assertEquals(4, q.getResultSize());
+
+      List<User> list = q.list();
+      assertEquals(4, list.size());
+      checkSurnameOrder(list, false);
+   }
+
+   public void testMaxResults() throws Exception {
+      QueryFactory qf = Search.getQueryFactory(remoteCache);
+
+      Query q = qf.from(User.class)
+            .orderBy("name", SortOrder.ASC).maxResults(2).build();
+
+      assertEquals(4, q.getResultSize());
+
+      List<User> list = q.list();
+      assertEquals(2, list.size());
+      checkNameOrder(list, true);
+   }
+
+   public void testStartOffset() throws Exception {
+      QueryFactory qf = Search.getQueryFactory(remoteCache);
+
+      Query q = qf.from(User.class)
+            .orderBy("name", SortOrder.ASC).startOffset(2).build();
+
+      assertEquals(4, q.getResultSize());
+
+      List<User> list = q.list();
+      assertEquals(2, list.size());
+      checkNameOrder(list, true);
+   }
+
+   public void testProjection() throws Exception {
+      QueryFactory qf = Search.getQueryFactory(remoteCache);
+
+      Query q = qf.from(User.class)
+            .setProjection("id", "name").maxResults(3).build();
+
+      assertEquals(4, q.getResultSize());
+
+      List<Object[]> list = q.list();
+      assertEquals(3, list.size());
+      for (Object[] u : list) {
+         assertNotNull(u[1]);
+         assertTrue(u[0] instanceof Integer);
+      }
+   }
+
+   private void checkNameOrder(List<User> list, boolean isAsc) {
+      String prevName = null;
+      for (User u : list) {
+         assertNotNull(u.getName());
+         if (prevName != null) {
+            int comp = u.getName().compareTo(prevName);
+            assertTrue(isAsc ? comp >= 0 : comp <= 0);
+         }
+         prevName = u.getName();
+      }
+   }
+
+   private void checkSurnameOrder(List<User> list, boolean isAsc) {
+      String prevSurname = null;
+      for (User u : list) {
+         assertNotNull(u.getSurname());
+         if (prevSurname != null) {
+            int comp = u.getSurname().compareTo(prevSurname);
+            assertTrue(isAsc ? comp >= 0 : comp <= 0);
+         }
+         prevSurname = u.getSurname();
+      }
+   }
+}
diff --git a/query-dsl/src/main/java/org/infinispan/query/dsl/Query.java b/query-dsl/src/main/java/org/infinispan/query/dsl/Query.java
@@ -25,7 +25,7 @@ public interface Query {
     *
     * @return total number of results.
     */
-   int getResultSize();
+   int getResultSize(); //todo [anistor] this should probably be a long?
 
    //todo [anistor] also add long getStartOffset() ?
 }
diff --git a/query/src/test/java/org/infinispan/query/dsl/embedded/ClusteredQueryDslConditionsTest.java b/query/src/test/java/org/infinispan/query/dsl/embedded/ClusteredQueryDslConditionsTest.java
@@ -1070,6 +1070,7 @@ public void testSampleDomainQuery16() throws Exception {
             .toBuilder().build();
 
       List<Transaction> list = q.list();
+      assertEquals(50, q.getResultSize());
       assertEquals(10, list.size());
       for (int i = 0; i < 10; i++) {
          assertEquals("Expensive shoes " + (20 + i), list.get(i).getDescription());

diff --git a/query/src/test/java/org/infinispan/query/dsl/embedded/QueryDslConditionsTest.java b/query/src/test/java/org/infinispan/query/dsl/embedded/QueryDslConditionsTest.java
@@ -1025,6 +1025,7 @@ public void testSampleDomainQuery16() throws Exception {
             .toBuilder().build();
 
       List<Transaction> list = q.list();
+      assertEquals(50, q.getResultSize());
       assertEquals(10, list.size());
       for (int i = 0; i < 10; i++) {
          assertEquals("Expensive shoes " + (20 + i), list.get(i).getDescription());

diff --git a/query/src/test/java/org/infinispan/query/dsl/embedded/QueryDslIterationTest.java b/query/src/test/java/org/infinispan/query/dsl/embedded/QueryDslIterationTest.java
@@ -64,18 +64,7 @@ public void testOrderByAsc() throws Exception {
 
       List<User> list = q.list();
       assertEquals(4, list.size());
-      checkNamesAsc(list);
-   }
-
-   private void checkNamesAsc(List<User> list) {
-      String prevName = null;
-      for (User u : list) {
-         assertNotNull(u.getName());
-         if (prevName != null) {
-            assertTrue(u.getName().compareTo(prevName) >= 0);
-         }
-         prevName = u.getName();
-      }
+      checkNameOrder(list, true);
    }
 
    public void testOrderByDesc() throws Exception {
@@ -88,14 +77,7 @@ public void testOrderByDesc() throws Exception {
 
       List<User> list = q.list();
       assertEquals(4, list.size());
-      String prevName = null;
-      for (User u : list) {
-         assertNotNull(u.getSurname());
-         if (prevName != null) {
-            assertTrue(u.getSurname().compareTo(prevName) <= 0);
-         }
-         prevName = u.getSurname();
-      }
+      checkSurnameOrder(list, false);
    }
 
    public void testMaxResults() throws Exception {
@@ -108,7 +90,7 @@ public void testMaxResults() throws Exception {
 
       List<User> list = q.list();
       assertEquals(2, list.size());
-      checkNamesAsc(list);
+      checkNameOrder(list, true);
    }
 
    public void testStartOffset() throws Exception {
@@ -121,7 +103,7 @@ public void testStartOffset() throws Exception {
 
       List<User> list = q.list();
       assertEquals(2, list.size());
-      checkNamesAsc(list);
+      checkNameOrder(list, true);
    }
 
    public void testProjection1() throws Exception {
@@ -183,4 +165,28 @@ private void checkIterator(int expected, ResultIterator iterator) {
       }
       assertEquals(expected, elements);
    }
+
+   private void checkNameOrder(List<User> list, boolean isAsc) {
+      String prevName = null;
+      for (User u : list) {
+         assertNotNull(u.getName());
+         if (prevName != null) {
+            int comp = u.getName().compareTo(prevName);
+            assertTrue(isAsc ? comp >= 0 : comp <= 0);
+         }
+         prevName = u.getName();
+      }
+   }
+
+   private void checkSurnameOrder(List<User> list, boolean isAsc) {
+      String prevSurname = null;
+      for (User u : list) {
+         assertNotNull(u.getSurname());
+         if (prevSurname != null) {
+            int comp = u.getSurname().compareTo(prevSurname);
+            assertTrue(isAsc ? comp >= 0 : comp <= 0);
+         }
+         prevSurname = u.getSurname();
+      }
+   }
 }
diff --git a/remote-query/remote-query-client/pom.xml b/remote-query/remote-query-client/pom.xml
@@ -33,7 +33,7 @@
             <configuration>
                <instructions>
                   <Export-Package>
-                     ${project.groupId}.query.remote.*;version=${project.version};-split-package:=error,
+                     ${project.groupId}.query.remote.client.*;version=${project.version};-split-package:=error,
                   </Export-Package>
                </instructions>
             </configuration>

diff --git a/...query-client/src/main/java/org/infinispan/query/remote/client/MarshallerRegistration.java b/...query-client/src/main/java/org/infinispan/query/remote/client/MarshallerRegistration.java
@@ -12,7 +12,7 @@
 public class MarshallerRegistration {
 
    public static void registerMarshallers(SerializationContext ctx) throws IOException, Descriptors.DescriptorValidationException {
-      ctx.registerProtofile(MarshallerRegistration.class.getResourceAsStream("/query.protobin"));
+      ctx.registerProtofile(MarshallerRegistration.class.getResourceAsStream("/org/infinispan/query/remote/client/query.protobin"));
       ctx.registerMarshaller(QueryRequest.class, new QueryRequestMarshaller());
       ctx.registerMarshaller(QueryRequest.SortCriteria.class, new SortCriteriaMarshaller());
       ctx.registerMarshaller(QueryResponse.class, new QueryResponseMarshaller());

diff --git a/...query-client/src/main/java/org/infinispan/query/remote/client/QueryRequestMarshaller.java b/...query-client/src/main/java/org/infinispan/query/remote/client/QueryRequestMarshaller.java
@@ -36,6 +36,6 @@ public Class<? extends QueryRequest> getJavaClass() {
 
    @Override
    public String getTypeName() {
-      return "org.infinispan.client.hotrod.impl.query.QueryRequest";
+      return "org.infinispan.query.remote.client.QueryRequest";
    }
 }
diff --git a/...y/remote-query-client/src/main/java/org/infinispan/query/remote/client/QueryResponse.java b/...y/remote-query-client/src/main/java/org/infinispan/query/remote/client/QueryResponse.java
@@ -16,6 +16,8 @@ public class QueryResponse {
 
    private List<WrappedMessage> results;
 
+   private long totalResults;
+
    public int getNumResults() {
       return numResults;
    }
@@ -39,4 +41,12 @@ public List<WrappedMessage> getResults() {
    public void setResults(List<WrappedMessage> results) {
       this.results = results;
    }
+
+   public long getTotalResults() {
+      return totalResults;
+   }
+
+   public void setTotalResults(long totalResults) {
+      this.totalResults = totalResults;
+   }
 }