Centralize definition of query parameters and default values

4 files changed, 50 insertions, 29 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java
index b76a856bb66..36a8b7812ba 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingQueryParser.java
@@ -12,7 +12,6 @@ import com.yahoo.search.Result;
 import com.yahoo.search.Searcher;
 import com.yahoo.search.grouping.request.GroupingOperation;
 import com.yahoo.search.query.Select;
-import com.yahoo.search.query.properties.DefaultProperties;
 import com.yahoo.search.searchchain.Execution;
 import com.yahoo.search.searchchain.PhaseNames;
 
@@ -21,6 +20,9 @@ import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.OptionalDouble;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 import java.util.TimeZone;
 
 /**
@@ -42,13 +44,14 @@ public class GroupingQueryParser extends Searcher {
     @Beta public static final CompoundName PARAM_DEFAULT_MAX_HITS = new CompoundName("grouping.defaultMaxHits");
     @Beta public static final CompoundName PARAM_DEFAULT_MAX_GROUPS = new CompoundName("grouping.defaultMaxGroups");
     @Beta public static final CompoundName PARAM_DEFAULT_PRECISION_FACTOR = new CompoundName("grouping.defaultPrecisionFactor");
+    @Beta public static final CompoundName GROUPING_GLOBAL_MAX_GROUPS = new CompoundName("grouping.globalMaxGroups");
     private static final ThreadLocal<ZoneCache> zoneCache = new ThreadLocal<>();
 
     @Override
     public Result search(Query query, Execution execution) {
         try {
-            if (query.getHttpRequest().getProperty(DefaultProperties.GROUPING_GLOBAL_MAX_GROUPS.toString()) != null) {
-                throw new IllegalInputException(DefaultProperties.GROUPING_GLOBAL_MAX_GROUPS + " must be specified in a query profile.");
+            if (query.getHttpRequest().getProperty(GROUPING_GLOBAL_MAX_GROUPS.toString()) != null) {
+                throw new IllegalInputException(GROUPING_GLOBAL_MAX_GROUPS + " must be specified in a query profile.");
             }
 
             String reqParam = query.properties().getString(PARAM_REQUEST);
@@ -61,10 +64,10 @@ public class GroupingQueryParser extends Searcher {
                 grpRequest.setRootOperation(op);
                 grpRequest.setTimeZone(zone);
                 grpRequest.continuations().addAll(continuations);
-                grpRequest.setDefaultMaxGroups(query.properties().getInteger(PARAM_DEFAULT_MAX_GROUPS, -1));
-                grpRequest.setDefaultMaxHits(query.properties().getInteger(PARAM_DEFAULT_MAX_HITS, -1));
-                grpRequest.setGlobalMaxGroups(query.properties().getLong(DefaultProperties.GROUPING_GLOBAL_MAX_GROUPS));
-                grpRequest.setDefaultPrecisionFactor(query.properties().getDouble(PARAM_DEFAULT_PRECISION_FACTOR, 0.0));
+                intProperty(query, PARAM_DEFAULT_MAX_GROUPS).ifPresent(grpRequest::setDefaultMaxGroups);
+                intProperty(query, PARAM_DEFAULT_MAX_HITS).ifPresent(grpRequest::setDefaultMaxHits);
+                longProperty(query, GROUPING_GLOBAL_MAX_GROUPS).ifPresent(grpRequest::setGlobalMaxGroups);
+                doubleProperty(query, PARAM_DEFAULT_PRECISION_FACTOR).ifPresent(grpRequest::setDefaultPrecisionFactor);
             }
             return execution.search(query);
         }
@@ -98,6 +101,21 @@ public class GroupingQueryParser extends Searcher {
         return timeZone;
     }
 
+    private static OptionalInt intProperty(Query q, CompoundName name) {
+        Integer val = q.properties().getInteger(name);
+        return val != null ? OptionalInt.of(val) : OptionalInt.empty();
+    }
+
+    private static OptionalLong longProperty(Query q, CompoundName name) {
+        Long val = q.properties().getLong(name);
+        return val != null ? OptionalLong.of(val) : OptionalLong.empty();
+    }
+
+    private static OptionalDouble doubleProperty(Query q, CompoundName name) {
+        Double val = q.properties().getDouble(name);
+        return val != null ? OptionalDouble.of(val) : OptionalDouble.empty();
+    }
+
     @SuppressWarnings("serial")
     private static class ZoneCache extends LinkedHashMap<String, TimeZone> {
 
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
index 5250f264423..a211d4960f4 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
@@ -34,10 +34,10 @@ public class GroupingRequest {
     private final List<Continuation> continuations = new ArrayList<>();
     private GroupingOperation root;
     private TimeZone timeZone;
-    private int defaultMaxHits = -1;
-    private int defaultMaxGroups = -1;
-    private long globalMaxGroups = -1;
-    private double defaultPrecisionFactor = -1;
+    private Integer defaultMaxHits;
+    private Integer defaultMaxGroups;
+    private Long globalMaxGroups;
+    private Double defaultPrecisionFactor;
 
     private GroupingRequest(Select parent) {
         this.parent = parent;
@@ -47,9 +47,10 @@ public class GroupingRequest {
                             List<Continuation> continuations,
                             GroupingOperation root,
                             TimeZone timeZone,
-                            int defaultMaxHits,
-                            int defaultMaxGroups,
-                            long globalMaxGroups) {
+                            Integer defaultMaxHits,
+                            Integer defaultMaxGroups,
+                            Long globalMaxGroups,
+                            Double defaultPrecisionFactor) {
         this.parent = parent;
         continuations.forEach(item -> this.continuations.add(item.copy()));
         this.root = root != null ? root.copy(null) : null;
@@ -57,11 +58,13 @@ public class GroupingRequest {
         this.defaultMaxHits = defaultMaxHits;
         this.defaultMaxGroups = defaultMaxGroups;
         this.globalMaxGroups = globalMaxGroups;
+        this.defaultPrecisionFactor = defaultPrecisionFactor;
     }
 
     /** Returns a deep copy of this */
     public GroupingRequest copy(Select parentOfCopy) {
-        return new GroupingRequest(parentOfCopy, continuations, root, timeZone, defaultMaxHits, defaultMaxGroups, globalMaxGroups);
+        return new GroupingRequest(parentOfCopy, continuations, root, timeZone, defaultMaxHits, defaultMaxGroups,
+                globalMaxGroups, defaultPrecisionFactor);
     }
 
     /**
@@ -147,28 +150,28 @@ public class GroupingRequest {
 
     @Beta
     public OptionalInt defaultMaxHits() {
-        return defaultMaxHits >= 0 ? OptionalInt.of(defaultMaxHits) : OptionalInt.empty();
+        return defaultMaxHits != null ? OptionalInt.of(defaultMaxHits) : OptionalInt.empty();
     }
 
     @Beta public void setDefaultMaxHits(int v) { this.defaultMaxHits = v; }
 
     @Beta
     public OptionalInt defaultMaxGroups() {
-        return defaultMaxGroups >= 0 ? OptionalInt.of(defaultMaxGroups) : OptionalInt.empty();
+        return defaultMaxGroups != null ? OptionalInt.of(defaultMaxGroups) : OptionalInt.empty();
     }
 
     @Beta public void setDefaultMaxGroups(int v) { this.defaultMaxGroups = v; }
 
     @Beta
     public OptionalLong globalMaxGroups() {
-        return globalMaxGroups >= 0 ? OptionalLong.of(globalMaxGroups) : OptionalLong.empty();
+        return globalMaxGroups != null ? OptionalLong.of(globalMaxGroups) : OptionalLong.empty();
     }
 
     @Beta public void setGlobalMaxGroups(long v) { this.globalMaxGroups = v; }
 
     @Beta
     public OptionalDouble defaultPrecisionFactor() {
-        return defaultPrecisionFactor > 0 ? OptionalDouble.of(defaultPrecisionFactor) : OptionalDouble.empty();
+        return defaultPrecisionFactor != null ? OptionalDouble.of(defaultPrecisionFactor) : OptionalDouble.empty();
     }
 
     @Beta void setDefaultPrecisionFactor(double v) { this.defaultPrecisionFactor = v; }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
index 32601c4faa4..b3c10a67091 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
@@ -48,6 +48,12 @@ public class GroupingExecutor extends Searcher {
     private final static CompoundName PROP_GROUPINGLIST = newCompoundName(GROUPING_LIST);
     private final static Logger log = Logger.getLogger(GroupingExecutor.class.getName());
 
+    // TODO Vespa 8: Modify defaults
+    private static final double DEFAULT_PRECISION_FACTOR = 1;
+    private static final int DEFAULT_MAX_GROUPS = -1;
+    private static final int DEFAULT_MAX_HITS = -1;
+    private static final long DEFAULT_GLOBAL_MAX_GROUPS = -1;
+
     /**
      * Constructs a new instance of this searcher without configuration.
      * This makes the searcher completely useless for searching purposes,
@@ -150,10 +156,10 @@ public class GroupingExecutor extends Searcher {
         builder.setDefaultSummaryName(query.getPresentation().getSummary());
         builder.setTimeZone(req.getTimeZone());
         builder.addContinuations(req.continuations());
-        req.defaultMaxGroups().ifPresent(builder::setDefaultMaxGroups);
-        req.defaultMaxHits().ifPresent(builder::setDefaultMaxHits);
-        req.globalMaxGroups().ifPresent(builder::setGlobalMaxGroups);
-        req.defaultPrecisionFactor().ifPresent(builder::setDefaultPrecisionFactor);
+        builder.setDefaultMaxGroups(req.defaultMaxGroups().orElse(DEFAULT_MAX_GROUPS));
+        builder.setDefaultMaxHits(req.defaultMaxHits().orElse(DEFAULT_MAX_HITS));
+        builder.setGlobalMaxGroups(req.globalMaxGroups().orElse(DEFAULT_GLOBAL_MAX_GROUPS));
+        builder.setDefaultPrecisionFactor(req.defaultPrecisionFactor().orElse(DEFAULT_PRECISION_FACTOR));
         builder.build();
 
         RequestContext ctx = new RequestContext(req, builder.getTransform());
diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/DefaultProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/DefaultProperties.java
index f25b61c3265..b94ddde4733 100644
--- a/container-search/src/main/java/com/yahoo/search/query/properties/DefaultProperties.java
+++ b/container-search/src/main/java/com/yahoo/search/query/properties/DefaultProperties.java
@@ -1,12 +1,10 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.query.properties;
 
-import com.yahoo.api.annotations.Beta;
 import com.yahoo.processing.request.CompoundName;
 import com.yahoo.search.query.Properties;
 import com.yahoo.search.query.profile.types.FieldDescription;
 import com.yahoo.search.query.profile.types.QueryProfileType;
-import com.yahoo.search.query.profile.types.QueryProfileTypeRegistry;
 
 import java.util.Map;
 
@@ -19,7 +17,6 @@ public final class DefaultProperties extends Properties  {
 
     public static final CompoundName MAX_OFFSET = new CompoundName("maxOffset");
     public static final CompoundName MAX_HITS = new CompoundName("maxHits");
-    @Beta public static final CompoundName GROUPING_GLOBAL_MAX_GROUPS = new CompoundName("grouping.globalMaxGroups");
 
     public static final QueryProfileType argumentType = new QueryProfileType("DefaultProperties");
 
@@ -28,7 +25,6 @@ public final class DefaultProperties extends Properties  {
 
         argumentType.addField(new FieldDescription(MAX_OFFSET.toString(), "integer"));
         argumentType.addField(new FieldDescription(MAX_HITS.toString(), "integer"));
-        argumentType.addField(new FieldDescription(GROUPING_GLOBAL_MAX_GROUPS.toString(), "long"), new QueryProfileTypeRegistry());
 
         argumentType.freeze();
     }
@@ -39,8 +35,6 @@ public final class DefaultProperties extends Properties  {
             return 1000;
         } else if (MAX_HITS.equals(name)) {
             return 400;
-        } else if (GROUPING_GLOBAL_MAX_GROUPS.equals(name)) {
-            return -1; // TODO Vespa 8: use default from Vespa 8 release notes
         } else {
             return super.get(name, context, substitution);
         }