ISPN-7339 Fix Ickle's support for named parameters

client/hotrod-client/src/main/java/org/infinispan/client/hotrod/event/ContinuousQueryImpl.java

@@ -68,7 +68,7 @@ public <C> void addContinuousQueryListener(String queryString, Map<String, Objec
     */
    public <C> void addContinuousQueryListener(Query query, ContinuousQueryListener<K, C> listener) {
       BaseQuery baseQuery = (BaseQuery) query;
-      addContinuousQueryListener(baseQuery.getQueryString(), baseQuery.getNamedParameters(), listener);
+      addContinuousQueryListener(baseQuery.getQueryString(), baseQuery.getParameters(), listener);
    }
 
    public void removeContinuousQueryListener(ContinuousQueryListener<K, ?> listener) {

client/hotrod-client/src/main/java/org/infinispan/client/hotrod/filter/Filters.java

@@ -40,6 +40,6 @@ public static Object[] makeFactoryParams(String queryString, Map<String, Object>
 
    public static Object[] makeFactoryParams(Query query) {
       BaseQuery baseQuery = (BaseQuery) query;
-      return makeFactoryParams(baseQuery.getQueryString(), baseQuery.getNamedParameters());
+      return makeFactoryParams(baseQuery.getQueryString(), baseQuery.getParameters());
    }
 }

client/hotrod-client/src/main/java/org/infinispan/client/hotrod/impl/operations/QueryOperation.java

@@ -75,7 +75,7 @@ protected QueryResponse executeOperation(Transport transport) {
    }
 
    private List<QueryRequest.NamedParameter> getNamedParameters() {
-      Map<String, Object> namedParameters = remoteQuery.getNamedParameters();
+      Map<String, Object> namedParameters = remoteQuery.getParameters();
       if (namedParameters == null || namedParameters.isEmpty()) {
          return null;
       }

client/hotrod-client/src/main/java/org/infinispan/client/hotrod/impl/query/RemoteQuery.java

@@ -9,8 +9,6 @@
 import org.infinispan.client.hotrod.exceptions.HotRodClientException;
 import org.infinispan.client.hotrod.impl.RemoteCacheImpl;
 import org.infinispan.client.hotrod.impl.operations.QueryOperation;
-import org.infinispan.client.hotrod.logging.Log;
-import org.infinispan.client.hotrod.logging.LogFactory;
 import org.infinispan.protostream.ProtobufUtil;
 import org.infinispan.protostream.SerializationContext;
 import org.infinispan.protostream.WrappedMessage;
@@ -24,14 +22,19 @@
  */
 public final class RemoteQuery extends BaseQuery {
 
-   private static final Log log = LogFactory.getLog(RemoteQuery.class, Log.class);
-
    private final RemoteCacheImpl cache;
    private final SerializationContext serializationContext;
 
-   private List results = null;
+   private List<?> results = null;
    private int totalResults;
 
+   RemoteQuery(QueryFactory queryFactory, RemoteCacheImpl cache, SerializationContext serializationContext,
+               String queryString) {
+      super(queryFactory, queryString);
+      this.cache = cache;
+      this.serializationContext = serializationContext;
+   }
+
    RemoteQuery(QueryFactory queryFactory, RemoteCacheImpl cache, SerializationContext serializationContext,
                String queryString, Map<String, Object> namedParameters, String[] projection, long startOffset, int maxResults) {
       super(queryFactory, queryString, namedParameters, projection, startOffset, maxResults);
@@ -59,7 +62,7 @@ public int getResultSize() {
 
    private void executeQuery() {
       if (results == null) {
-         checkParameters();
+         validateNamedParameters();
 
          QueryOperation op = cache.getOperationsFactory().newQueryOperation(this);
          QueryResponse response = op.execute();
@@ -97,16 +100,6 @@ private List<Object> unwrapResults(int projectionSize, List<WrappedMessage> resu
       return unwrappedResults;
    }
 
-   private void checkParameters() {
-      if (namedParameters != null) {
-         for (Map.Entry<String, Object> e : namedParameters.entrySet()) {
-            if (e.getValue() == null) {
-               throw log.queryParameterNotSet(e.getKey());
-            }
-         }
-      }
-   }
-
    public SerializationContext getSerializationContext() {
       return serializationContext;
    }

client/hotrod-client/src/main/java/org/infinispan/client/hotrod/impl/query/RemoteQueryFactory.java

@@ -19,12 +19,6 @@ public final class RemoteQueryFactory extends BaseQueryFactory {
    private final RemoteCacheImpl cache;
    private final SerializationContext serializationContext;
 
-   @Override
-   public Query create(String queryString) {
-      //todo [anistor] some params come from a builder, some from parsing the query string
-      return new RemoteQuery(this, cache, serializationContext, queryString, null, null, -1, -1);
-   }
-
    public RemoteQueryFactory(RemoteCacheImpl cache) {
       serializationContext = ProtoStreamMarshaller.getSerializationContext(cache.getRemoteCacheManager());
 
@@ -40,7 +34,12 @@ public RemoteQueryFactory(RemoteCacheImpl cache) {
    }
 
    @Override
-   public QueryBuilder from(Class entityType) {
+   public Query create(String queryString) {
+      return new RemoteQuery(this, cache, serializationContext, queryString);
+   }
+
+   @Override
+   public QueryBuilder from(Class<?> entityType) {
       String typeName = serializationContext.getMarshaller(entityType).getTypeName();
       return new RemoteQueryBuilder(this, cache, serializationContext, typeName);
    }
@@ -49,7 +48,6 @@ public QueryBuilder from(Class entityType) {
    public QueryBuilder from(String entityType) {
       // just check that the type name is valid
       serializationContext.getMarshaller(entityType);
-
       return new RemoteQueryBuilder(this, cache, serializationContext, entityType);
    }
 }

client/hotrod-client/src/main/java/org/infinispan/client/hotrod/logging/Log.java

@@ -226,9 +226,6 @@ public interface Log extends BasicLogger {
    @Message(value = "The client listener must use the '%s' filter/converter factory", id = 4059)
    IncorrectClientListenerException clientListenerMustUseDesignatedFilterConverterFactory(String filterConverterFactoryName);
 
-   @Message(value = "Query parameter '%s' was not set", id = 4060)
-   IllegalStateException queryParameterNotSet(String filterConverterFactoryName);
-
    @LogMessage(level = WARN)
    @Message(value = "Ignoring error when closing iteration '%s'", id = 4061)
    void ignoringErrorDuringIterationClose(String iterationId, @Cause Exception e);

client/hotrod-client/src/test/java/org/infinispan/client/hotrod/query/RemoteQueryDslConditionsTest.java

@@ -306,14 +306,14 @@ public void testDuplicateDateProjection() throws Exception {
       assertEquals(makeDate("2013-02-27").getTime(), list.get(0)[2]);
    }
 
-   @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "ISPN004060: Query parameter 'param2' was not set")
+   @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "ISPN014825: Query parameter 'param2' was not set")
    @Override
    public void testMissingParamWithParameterMap() throws Exception {
       // exception message code is different because it is generated by a different logger
       super.testMissingParamWithParameterMap();
    }
 
-   @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "ISPN004060: Query parameter 'param2' was not set")
+   @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "ISPN014825: Query parameter 'param2' was not set")
    @Override
    public void testMissingParam() throws Exception {
       // exception message code is different because it is generated by a different logger

object-filter/src/main/java/org/infinispan/objectfilter/impl/BaseMatcher.java

@@ -169,7 +169,7 @@ public FilterSubscription registerFilter(Query query, FilterCallback callback, O
    @Override
    public FilterSubscription registerFilter(Query query, FilterCallback callback, boolean isDeltaFilter, Object... eventType) {
       BaseQuery baseQuery = (BaseQuery) query;
-      return registerFilter(baseQuery.getQueryString(), baseQuery.getNamedParameters(), callback, isDeltaFilter, eventType);
+      return registerFilter(baseQuery.getQueryString(), baseQuery.getParameters(), callback, isDeltaFilter, eventType);
    }
 
    @Override

object-filter/src/test/java/org/infinispan/objectfilter/test/FilterQueryFactory.java

@@ -32,12 +32,11 @@ final class FilterQueryFactory extends BaseQueryFactory {
 
    @Override
    public Query create(String queryString) {
-      //todo [anistor] some params come from a builder, some from parsing the query string
       return new FilterQuery(this, queryString, null, null, -1, -1);
    }
 
    @Override
-   public QueryBuilder from(Class entityType) {
+   public QueryBuilder from(Class<?> entityType) {
       if (serializationContext != null) {
          serializationContext.getMarshaller(entityType);
       }

query-dsl/src/main/java/org/infinispan/query/dsl/QueryFactory.java

@@ -10,6 +10,11 @@
  */
 public interface QueryFactory {
 
+   /**
+    * Creates a Query based on an Ickle query string.
+    *
+    * @return a query
+    */
    Query create(String queryString);
 
    /**
@@ -18,7 +23,7 @@ public interface QueryFactory {
     * @param entityType the Class of the entity
     * @return a builder capable of creating queries for the specified entity type
     */
-   QueryBuilder from(Class entityType);
+   QueryBuilder from(Class<?> entityType);
 
    /**
     * Creates a QueryBuilder for the given entity type.
@@ -28,6 +33,12 @@ public interface QueryFactory {
     */
    QueryBuilder from(String entityType);
 
+   /**
+    * Creates a condition on the given attribute path that is to be completed later by using it as a sub-condition.
+    *
+    * @param expression a path Expression
+    * @return the incomplete sub-condition
+    */
    FilterConditionEndContext having(Expression expression);
 
    /**

query-dsl/src/main/java/org/infinispan/query/dsl/impl/BaseQuery.java

@@ -2,6 +2,7 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -22,7 +23,9 @@ public abstract class BaseQuery implements Query {
 
    protected final String queryString;
 
-   protected final Map<String, Object> namedParameters;
+   private final boolean paramsDefined;
+
+   protected Map<String, Object> namedParameters;
 
    protected final String[] projection;
 
@@ -31,8 +34,9 @@ public abstract class BaseQuery implements Query {
    protected int maxResults;
 
    //todo [anistor] can startOffset really be a long or it really has to be int due to limitations in query module?
-   protected BaseQuery(QueryFactory queryFactory, String queryString, Map<String, Object> namedParameters, String[] projection,
-                       long startOffset, int maxResults) {
+   protected BaseQuery(QueryFactory queryFactory, String queryString,
+                       Map<String, Object> namedParameters, String[] projection, long startOffset, int maxResults) {
+      this.paramsDefined = true;
       this.queryFactory = queryFactory;
       this.queryString = queryString;
       this.namedParameters = namedParameters;
@@ -41,8 +45,13 @@ protected BaseQuery(QueryFactory queryFactory, String queryString, Map<String, O
       this.maxResults = maxResults;
    }
 
-   public QueryFactory getQueryFactory() {
-      return queryFactory;
+   protected BaseQuery(QueryFactory queryFactory, String queryString) {
+      this.paramsDefined = false;
+      this.queryFactory = queryFactory;
+      this.queryString = queryString;
+      this.projection = null;
+      this.startOffset = 0;
+      this.maxResults = -1;
    }
 
    /**
@@ -67,20 +76,25 @@ public String getQueryString() {
 
    @Override
    public Map<String, Object> getParameters() {
-      return Collections.unmodifiableMap(namedParameters);
+      return namedParameters != null ? Collections.unmodifiableMap(namedParameters) : null;
    }
 
    @Override
    public Query setParameter(String paramName, Object paramValue) {
-      if (namedParameters == null) {
-         throw log.queryDoesNotHaveParameters();
-      }
       if (paramName == null || paramName.isEmpty()) {
          throw log.parameterNameCannotBeNulOrEmpty();
       }
-      if (!namedParameters.containsKey(paramName)) {
-         throw log.parameterNotFound(paramName);
+      if (paramsDefined) {
+         if (namedParameters == null) {
+            throw log.queryDoesNotHaveParameters();
+         }
+         if (!namedParameters.containsKey(paramName)) {
+            throw log.parameterNotFound(paramName);
+         }
+      } else if (namedParameters == null) {
+         namedParameters = new HashMap<>(5);
       }
+
       namedParameters.put(paramName, paramValue);
 
       // reset the query to force a new execution
@@ -94,24 +108,29 @@ public Query setParameters(Map<String, Object> paramValues) {
       if (paramValues == null) {
          throw log.argumentCannotBeNull("paramValues");
       }
-      if (namedParameters == null) {
-         throw log.queryDoesNotHaveParameters();
-      }
-      List<String> unknownParams = null;
-      for (String paramName : paramValues.keySet()) {
-         if (paramName == null || paramName.isEmpty()) {
-            throw log.parameterNameCannotBeNulOrEmpty();
+      if (paramsDefined) {
+         if (namedParameters == null) {
+            throw log.queryDoesNotHaveParameters();
          }
-         if (!namedParameters.containsKey(paramName)) {
-            if (unknownParams == null) {
-               unknownParams = new ArrayList<>();
+         List<String> unknownParams = null;
+         for (String paramName : paramValues.keySet()) {
+            if (paramName == null || paramName.isEmpty()) {
+               throw log.parameterNameCannotBeNulOrEmpty();
+            }
+            if (!namedParameters.containsKey(paramName)) {
+               if (unknownParams == null) {
+                  unknownParams = new ArrayList<>();
+               }
+               unknownParams.add(paramName);
             }
-            unknownParams.add(paramName);
          }
+         if (unknownParams != null) {
+            throw log.parametersNotFound(unknownParams.toString());
+         }
+      } else if (namedParameters == null) {
+         namedParameters = new HashMap<>(5);
       }
-      if (unknownParams != null) {
-         throw log.parametersNotFound(unknownParams.toString());
-      }
+
       namedParameters.putAll(paramValues);
 
       // reset the query to force a new execution
@@ -121,13 +140,22 @@ public Query setParameters(Map<String, Object> paramValues) {
    }
 
    /**
-    * Reset internal state after query parameters are modified. This is needed to ensure the next execution of the query
-    * uses the new parameter values.
+    * Reset internal state after pagination or query parameters are modified. This is needed to ensure the next
+    * execution of the query uses the new values.
     */
    public abstract void resetQuery();
 
-   public Map<String, Object> getNamedParameters() {
-      return namedParameters;
+   /**
+    * Ensure all named parameters have non-null values.
+    */
+   public void validateNamedParameters() {
+      if (namedParameters != null) {
+         for (Map.Entry<String, Object> e : namedParameters.entrySet()) {
+            if (e.getValue() == null) {
+               throw log.queryParameterNotSet(e.getKey());
+            }
+         }
+      }
    }
 
    public String[] getProjection() {
@@ -144,13 +172,15 @@ public int getMaxResults() {
 
    @Override
    public Query startOffset(long startOffset) {
-      this.startOffset = (int) startOffset;    //todo [anistor] why????
+      this.startOffset = (int) startOffset;    //todo [anistor] why int?
+      resetQuery();
       return this;
    }
 
    @Override
    public Query maxResults(int maxResults) {
       this.maxResults = maxResults;
+      resetQuery();
       return this;
    }
 }

query-dsl/src/main/java/org/infinispan/query/dsl/impl/logging/Log.java

@@ -85,4 +85,7 @@ public interface Log extends BasicLogger {
 
    @Message(value = "startOffset cannot be less than 0", id = 14824)
    IllegalArgumentException startOffsetCannotBeLessThanZero();
+
+   @Message(value = "Query parameter '%s' was not set", id = 14825)
+   IllegalStateException queryParameterNotSet(String paramName);
 }

query-dsl/src/test/java/org/infinispan/query/dsl/impl/DummyQueryFactory.java

@@ -14,7 +14,7 @@ public Query create(String queryString) {
    }
 
    @Override
-   public DummyQueryBuilder from(Class entityType) {
+   public DummyQueryBuilder from(Class<?> entityType) {
       return new DummyQueryBuilder(this, entityType.getName());
    }
 

query/src/main/java/org/infinispan/query/Search.java

@@ -42,7 +42,7 @@ public static <K, V> CacheEventFilterConverter<K, V, ObjectFilter.FilterResult>
 
    public static <K, V> CacheEventFilterConverter<K, V, ObjectFilter.FilterResult> makeFilter(Query query) {
       BaseQuery baseQuery = (BaseQuery) query;
-      return makeFilter(baseQuery.getQueryString(), baseQuery.getNamedParameters());
+      return makeFilter(baseQuery.getQueryString(), baseQuery.getParameters());
    }
 
    public static QueryFactory getQueryFactory(Cache<?, ?> cache) {