diff options
| author | Jon Bratseth <bratseth@gmail.com> | 2020-08-18 11:15:50 +0200 |
|---|---|---|
| committer | Jon Bratseth <bratseth@gmail.com> | 2020-08-18 11:15:50 +0200 |
| commit | c30bbdb0fa50cedc56eec71feeadc969ba5a3edf (patch) | |
| tree | 6935b4d042618900ee0f7fd291c14ed55ae06cf8 /container-search/src/main/java/com/yahoo | |
| parent | 529694a88d48270298171fdcb87d1439f183202b (diff) | |
Skip logging only for IllegalInputException
- Add IllegalInputException to signal cases where we know the exception
is caused by illegal input received from the requestor.
- Only skip logging for IllegalInputException instead of the superclass
IllegalArgumentException as that is also used to signal illegal
arguments to methods due to bugs which are otherwise hard to debug.
- Throw IllegalInputException rather than IllegalArgumentException
where appropriate.
- Deprecated QueryException as it was only used to be able to separate
between query string and query parameter exceptions, and not doing
that consistently, and is in a package we don't want more use of.
- Clean up some cases where the wrong exception was thrown.
Diffstat (limited to 'container-search/src/main/java/com/yahoo')
58 files changed, 376 insertions, 346 deletions
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/BoolItem.java b/container-search/src/main/java/com/yahoo/prelude/query/BoolItem.java index 27045629780..542df9d4b8b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/BoolItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/BoolItem.java @@ -1,6 +1,8 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.query; +import com.yahoo.processing.IllegalInputException; + import java.nio.ByteBuffer; /** @@ -59,7 +61,7 @@ public class BoolItem extends TermItem { switch (stringValue.toLowerCase()) { case "true" : return true; case "false" : return false; - default: throw new IllegalArgumentException("Expected 'true' or 'false', got '" + stringValue + "'"); + default: throw new IllegalInputException("Expected 'true' or 'false', got '" + stringValue + "'"); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java b/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java index 64f759dcf9c..4609edb7446 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java @@ -46,7 +46,7 @@ public abstract class CompositeItem extends Item { Item possibleCycle = i.next(); if (this == possibleCycle) { - throw new QueryException("Cannot add " + item + " to " + this + " as it would create a cycle"); + throw new IllegalArgumentException("Cannot add " + item + " to " + this + " as it would create a cycle"); } else if (possibleCycle instanceof CompositeItem) { ensureNotInSubtree((CompositeItem) possibleCycle); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java b/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java index 8202c8fb279..ba5270e7af7 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/GeoLocationItem.java @@ -22,7 +22,7 @@ public class GeoLocationItem extends TermItem { public GeoLocationItem(Location location) { this(location, location.getAttribute()); if (! location.hasAttribute()) { - throw new IllegalArgumentException("missing attribute on location: "+location); + throw new IllegalArgumentException("Missing attribute on location: " + location); } } @@ -34,13 +34,14 @@ public class GeoLocationItem extends TermItem { public GeoLocationItem(Location location, String fieldName) { super(fieldName, false); if (location.hasAttribute() && ! location.getAttribute().equals(fieldName)) { - throw new IllegalArgumentException("inconsistent attribute on location: "+location.getAttribute()+" versus fieldName: "+fieldName); + throw new IllegalArgumentException("Inconsistent attribute on location: " + location.getAttribute() + + " versus fieldName: " + fieldName); } if (! location.isGeoCircle()) { - throw new IllegalArgumentException("GeoLocationItem only supports Geo Circles, got: "+location); + throw new IllegalArgumentException("GeoLocationItem only supports Geo Circles, got: " + location); } if (location.hasBoundingBox()) { - throw new IllegalArgumentException("GeoLocationItem does not support bounding box yet, got: "+location); + throw new IllegalArgumentException("GeoLocationItem does not support bounding box, got: " + location); } this.location = new Location(location.toString()); this.location.setAttribute(null); // keep this in (superclass) indexName only diff --git a/container-search/src/main/java/com/yahoo/prelude/query/IntItem.java b/container-search/src/main/java/com/yahoo/prelude/query/IntItem.java index 714e8f9cb5e..1591d31f749 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/IntItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/IntItem.java @@ -111,7 +111,7 @@ public class IntItem extends TermItem { } catch (IllegalArgumentException e) { throw new IllegalArgumentException("'" + expression + "' is not an int item expression: " + - "Expected NUMBER, '<'NUMBER, '>'NUMBER or ('['|'<')NUMBER;NUMBER(;NUMBER)?(']'|'>')", e); + "Expected NUMBER, '<'NUMBER, '>'NUMBER or ('['|'<')NUMBER;NUMBER(;NUMBER)?(']'|'>')", e); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/QueryException.java b/container-search/src/main/java/com/yahoo/prelude/query/QueryException.java index 34898827c2e..7d029d69077 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/QueryException.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/QueryException.java @@ -4,10 +4,11 @@ package com.yahoo.prelude.query; /** * Runtime exception to mark errors in query parsing. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen + * @deprecated no methods throw this */ +@Deprecated // TODO: Remove on Vespa 8 public class QueryException extends RuntimeException { - private static final long serialVersionUID = -2975856668328596533L; public QueryException(String message) { super(message); @@ -16,4 +17,5 @@ public class QueryException extends RuntimeException { public QueryException(String message, Throwable cause) { super(message, cause); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/RegExpItem.java b/container-search/src/main/java/com/yahoo/prelude/query/RegExpItem.java index 2a5c6135d71..ec3744306ed 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/RegExpItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/RegExpItem.java @@ -7,9 +7,10 @@ import java.util.regex.Pattern; /** * Match a field with the contained regular expression. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class RegExpItem extends TermItem { + private String expression; private Pattern regexp; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java index 1c3eb261f90..f70bf8021ff 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/SegmentItem.java @@ -102,7 +102,7 @@ public abstract class SegmentItem extends CompositeItem implements BlockItem { } private void dontAdd() { - throw new QueryException("Tried to add item to an immutable segment."); + throw new IllegalArgumentException("Tried to add item to an immutable segment."); } public Item removeItem(int index) { @@ -120,7 +120,7 @@ public abstract class SegmentItem extends CompositeItem implements BlockItem { } private void dontRemove() { - throw new QueryException("Tried to remove an item from an immutable segment."); + throw new IllegalArgumentException("Tried to remove an item from an immutable segment."); } // TODO: Add a getItemIterator which is safe for immutability diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java index e3d1b280a5a..e2dc5df5242 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AdvancedParser.java @@ -206,7 +206,7 @@ public class AdvancedParser extends StructuredParser { if (!tokens.currentIs(LBRACE)) return 0; tokens.skip(LBRACE); if (!tokens.currentIsNoIgnore(NUMBER)) throw new IllegalArgumentException("Expected an integer argument"); - int distance=Integer.valueOf(tokens.next().image); + int distance = Integer.valueOf(tokens.next().image); if (!tokens.skip(Token.Kind.RBRACE)) throw new IllegalArgumentException("Expected a right brace following the argument"); return distance; } diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/PhraseMatcher.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/PhraseMatcher.java index f49e49c1771..be33c0ee9e1 100644 --- a/container-search/src/main/java/com/yahoo/prelude/querytransform/PhraseMatcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/PhraseMatcher.java @@ -64,9 +64,9 @@ public class PhraseMatcher { * @throws IllegalArgumentException if FSA is null */ public PhraseMatcher(FSA phraseAutomatonFSA,boolean ignorePluralForm) { - if(phraseAutomatonFSA==null) throw new IllegalArgumentException("FSA is null"); - this.ignorePluralForm=ignorePluralForm; - phraseFSA=phraseAutomatonFSA; + if (phraseAutomatonFSA == null) throw new NullPointerException("FSA is null"); + this.ignorePluralForm = ignorePluralForm; + phraseFSA = phraseAutomatonFSA; } public boolean isEmpty() { return phraseFSA == null; } diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java index 318912eab04..eb31b75cd6f 100644 --- a/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java @@ -359,7 +359,7 @@ public class StemmingSearcher extends Searcher { default: throw new IllegalArgumentException("Unknown segmenting rule: " + current.getSegmentingRule() + ". This is a bug in Vespa, as the implementation has gotten out of sync." + - " Please create a ticket as soon as possible."); + " Please create an issue."); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java index 37561d3a0f5..7063a14a389 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java @@ -75,7 +75,7 @@ public class PosSearcher extends Searcher { loc.setAttribute(posAttribute); try { - if (ll == null && xy == null && bb != null) { + if (ll == null && xy == null) { parseBoundingBox(bb, loc); } else { if (ll != null && xy != null) { diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index 9e3f6d20e36..4995927f7a2 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -10,7 +10,6 @@ import com.yahoo.fs4.MapEncoder; import java.util.logging.Level; import com.yahoo.prelude.fastsearch.DocumentDatabase; import com.yahoo.prelude.query.Highlight; -import com.yahoo.prelude.query.QueryException; import com.yahoo.prelude.query.textualrepresentation.TextualQueryRepresentation; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.dispatch.Dispatcher; @@ -413,11 +412,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { if (field.getType() == FieldType.genericQueryProfileType) { // Generic map CompoundName fullName = prefix.append(field.getName()); for (Map.Entry<String, Object> entry : originalProperties.listProperties(fullName, context).entrySet()) { - try { - properties().set(fullName.append(entry.getKey()), entry.getValue(), context); - } catch (IllegalArgumentException e) { - throw new QueryException("Invalid request parameter", e); - } + properties().set(fullName.append(entry.getKey()), entry.getValue(), context); } } else if (field.getType() instanceof QueryProfileFieldType) { // Nested arguments @@ -427,11 +422,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { CompoundName fullName = prefix.append(field.getName()); Object value = originalProperties.get(fullName, context); if (value != null) { - try { - properties().set(fullName, value, context); - } catch (IllegalArgumentException e) { - throw new QueryException("Invalid request parameter", e); - } + properties().set(fullName, value, context); } } } @@ -440,13 +431,8 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { /** Calls properties.set on all entries in requestMap */ private void setPropertiesFromRequestMap(Map<String, String> requestMap, Properties properties, boolean ignoreSelect) { for (var entry : requestMap.entrySet()) { - try { - if (ignoreSelect && entry.getKey().equals(Select.SELECT)) continue; - properties.set(entry.getKey(), entry.getValue(), requestMap); - } - catch (IllegalArgumentException e) { - throw new QueryException("Invalid request parameter", e); - } + if (ignoreSelect && entry.getKey().equals(Select.SELECT)) continue; + properties.set(entry.getKey(), entry.getValue(), requestMap); } } diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index 60c5d42c531..0c38b38179d 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java @@ -14,6 +14,7 @@ import com.yahoo.concurrent.CopyOnWriteHashMap; import com.yahoo.errorhandling.Results; import com.yahoo.errorhandling.Results.Builder; import com.yahoo.prelude.IndexFacts; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -545,8 +546,8 @@ public class FederationSearcher extends ForkingSearcher { private ComponentSpecification asSourceSpec(String source) { try { return new ComponentSpecification(source); - } catch(Exception e) { - throw new IllegalArgumentException("The source ref '" + source + "' used for federation is not valid.", e); + } catch (Exception e) { + throw new IllegalInputException("The source ref '" + source + "' used for federation is not valid.", e); } } 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 9924a05bb46..b9e0825ab03 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,8 +12,14 @@ import com.yahoo.search.grouping.request.GroupingOperation; import com.yahoo.search.query.Select; import com.yahoo.search.searchchain.Execution; import com.yahoo.search.searchchain.PhaseNames; +import com.yahoo.processing.IllegalInputException; -import java.util.*; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.TimeZone; /** * This searcher is responsible for turning the "select" parameter into a corresponding {@link GroupingRequest}. It will @@ -35,19 +41,23 @@ public class GroupingQueryParser extends Searcher { @Override public Result search(Query query, Execution execution) { - String reqParam = query.properties().getString(PARAM_REQUEST); - if (reqParam == null) { + try { + String reqParam = query.properties().getString(PARAM_REQUEST); + if (reqParam == null) return execution.search(query); + + List<Continuation> continuations = getContinuations(query.properties().getString(PARAM_CONTINUE)); + TimeZone zone = getTimeZone(query.properties().getString(PARAM_TIMEZONE, "utc")); + for (GroupingOperation op : GroupingOperation.fromStringAsList(reqParam)) { + GroupingRequest grpRequest = GroupingRequest.newInstance(query); + grpRequest.setRootOperation(op); + grpRequest.setTimeZone(zone); + grpRequest.continuations().addAll(continuations); + } return execution.search(query); } - List<Continuation> continuations = getContinuations(query.properties().getString(PARAM_CONTINUE)); - TimeZone zone = getTimeZone(query.properties().getString(PARAM_TIMEZONE, "utc")); - for (GroupingOperation op : GroupingOperation.fromStringAsList(reqParam)) { - GroupingRequest grpRequest = GroupingRequest.newInstance(query); - grpRequest.setRootOperation(op); - grpRequest.setTimeZone(zone); - grpRequest.continuations().addAll(continuations); + catch (IllegalArgumentException e) { + throw new IllegalInputException(e); } - return execution.search(query); } private List<Continuation> getContinuations(String param) { diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java index 06b030dbc78..cd8578cd728 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java @@ -5,6 +5,7 @@ import com.google.inject.Inject; import com.yahoo.component.chain.dependencies.After; import com.yahoo.component.chain.dependencies.Before; import com.yahoo.component.chain.dependencies.Provides; +import com.yahoo.processing.IllegalInputException; import com.yahoo.search.grouping.request.AttributeMapLookupValue; import com.yahoo.vespa.config.search.AttributesConfig; import com.yahoo.container.QrSearchersConfig; @@ -80,14 +81,14 @@ public class GroupingValidator extends Searcher { AttributesConfig.Attribute keyAttribute = attributes.get(keyAttributeName); AttributesConfig.Attribute keySourceAttribute = attributes.get(keySourceAttributeName); if (!keySourceAttribute.datatype().equals(keyAttribute.datatype())) { - throw new IllegalArgumentException("Grouping request references key source attribute '" + - keySourceAttributeName + "' with data type '" + keySourceAttribute.datatype() + - "' that is different than data type '" + keyAttribute.datatype() + "' of key attribute '" + - keyAttributeName + "'"); + throw new IllegalInputException("Grouping request references key source attribute '" + + keySourceAttributeName + "' with data type '" + keySourceAttribute.datatype() + + "' that is different than data type '" + keyAttribute.datatype() + "' of key attribute '" + + keyAttributeName + "'"); } if (!keySourceAttribute.collectiontype().equals(AttributesConfig.Attribute.Collectiontype.Enum.SINGLE)) { - throw new IllegalArgumentException("Grouping request references key source attribute '" + - keySourceAttributeName + "' which is not of single value type"); + throw new IllegalInputException("Grouping request references key source attribute '" + + keySourceAttributeName + "' which is not of single value type"); } } diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/AddFunction.java b/container-search/src/main/java/com/yahoo/search/grouping/request/AddFunction.java index 420861d2f6c..60805aacd5f 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/request/AddFunction.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/request/AddFunction.java @@ -38,9 +38,9 @@ public class AddFunction extends FunctionNode { /** * Constructs a new instance of this class from a list of arguments. * - * @param args The arguments to pass to the constructor. - * @return The created instance. - * @throws IllegalArgumentException Thrown if the number of arguments is less than 2. + * @param args the arguments to pass to the constructor. + * @return the created instance. + * @throws IllegalArgumentException thrown if the number of arguments is less than 2. */ public static AddFunction newInstance(List<GroupingExpression> args) { if (args.size() < 2) { diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/BucketResolver.java b/container-search/src/main/java/com/yahoo/search/grouping/request/BucketResolver.java index c36c8af5c34..6c6b20973ab 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/request/BucketResolver.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/request/BucketResolver.java @@ -24,9 +24,9 @@ public class BucketResolver { * Pushes the given expression onto this bucket resolver. Once all buckets have been pushed using this method, call * {@link #resolve(GroupingExpression)} to retrieve to combined grouping expression. * - * @param val The expression to push. - * @param inclusive Whether or not the value is inclusive or not. - * @throws IllegalArgumentException Thrown if the expression is incompatible. + * @param val the expression to push + * @param inclusive whether or not the value is inclusive or not + * @throws IllegalArgumentException thrown if the expression is incompatible */ public BucketResolver push(ConstantValue<?> val, boolean inclusive) { if (prev == null) { diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java b/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java index c825f3c61de..8b73fa01128 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java @@ -228,8 +228,8 @@ public abstract class GroupingOperation extends GroupingNode { * method verifies the input level against the operation type, and recursively resolves the level of all argument * expressions. * - * @param level The level of the input data. - * @throws IllegalArgumentException Thrown if a contained expression is invalid for the given level. + * @param level the level of the input data + * @throws IllegalArgumentException thrown if a contained expression is invalid for the given level */ public void resolveLevel(int level) { if (groupBy != null) { @@ -322,12 +322,7 @@ public abstract class GroupingOperation extends GroupingNode { return this; } - /** - * Return the accuracy of this. - * - * @return The accuracy value. - * @see #setAccuracy(double) - */ + /** Return the accuracy of this. */ public double getAccuracy() { return accuracy; } @@ -335,8 +330,8 @@ public abstract class GroupingOperation extends GroupingNode { /** * Adds an expression to the order-by clause of this operation. * - * @param exp The expressions to add to this. - * @return This, to allow chaining. + * @param exp the expressions to add to this + * @return this, to allow chaining */ public GroupingOperation addOrderBy(GroupingExpression exp) { orderBy.add(exp); @@ -346,11 +341,11 @@ public abstract class GroupingOperation extends GroupingNode { /** * Convenience method to call {@link #addOrderBy(GroupingExpression)} for each element in the given list. * - * @param lst The list of expressions to add. - * @return This, to allow chaining. + * @param list the list of expressions to add + * @return this, to allow chaining */ - public GroupingOperation addOrderBy(List<GroupingExpression> lst) { - for (GroupingExpression exp : lst) { + public GroupingOperation addOrderBy(List<GroupingExpression> list) { + for (GroupingExpression exp : list) { addOrderBy(exp); } return this; @@ -359,7 +354,7 @@ public abstract class GroupingOperation extends GroupingNode { /** * Returns the number of expressions in the order-by clause of this. * - * @return The expression count. + * @return the expression count */ public int getNumOrderBy() { return orderBy.size(); @@ -368,9 +363,9 @@ public abstract class GroupingOperation extends GroupingNode { /** * Returns the group-by expression at the given index. * - * @param i The index of the expression to return. - * @return The expression at the given index. - * @throws IndexOutOfBoundsException If the index is out of range. + * @param i the index of the expression to return + * @return the expression at the given index + * @throws IndexOutOfBoundsException if the index is out of range */ public GroupingExpression getOrderBy(int i) { return orderBy.get(i); @@ -379,7 +374,7 @@ public abstract class GroupingOperation extends GroupingNode { /** * Returns an immutable view to the order-by clause of this. * - * @return The expression list. + * @return the expression list */ public List<GroupingExpression> getOrderBy() { return Collections.unmodifiableList(orderBy); @@ -388,8 +383,8 @@ public abstract class GroupingOperation extends GroupingNode { /** * Adds an expression to the output clause of this operation. * - * @param exp The expressions to add to this. - * @return This, to allow chaining. + * @param exp the expressions to add to this + * @return this, to allow chaining */ public GroupingOperation addOutput(GroupingExpression exp) { outputs.add(exp); @@ -399,8 +394,8 @@ public abstract class GroupingOperation extends GroupingNode { /** * Convenience method to call {@link #addOutput(GroupingExpression)} for each element in the given list. * - * @param lst The list of expressions to add. - * @return This, to allow chaining. + * @param lst the list of expressions to add + * @return this, to allow chaining */ public GroupingOperation addOutputs(List<GroupingExpression> lst) { for (GroupingExpression exp : lst) { @@ -412,7 +407,7 @@ public abstract class GroupingOperation extends GroupingNode { /** * Returns the number of expressions in the output clause of this. * - * @return The expression count. + * @return the expression count */ public int getNumOutputs() { return outputs.size(); @@ -421,9 +416,9 @@ public abstract class GroupingOperation extends GroupingNode { /** * Returns the output expression at the given index. * - * @param i The index of the expression to return. - * @return The expression at the given index. - * @throws IndexOutOfBoundsException If the index is out of range. + * @param i the index of the expression to return + * @return the expression at the given index + * @throws IndexOutOfBoundsException If the index is out of range */ public GroupingExpression getOutput(int i) { return outputs.get(i); @@ -432,7 +427,7 @@ public abstract class GroupingOperation extends GroupingNode { /** * Returns an immutable view to the output clause of this. * - * @return The expression list. + * @return the expression list */ public List<GroupingExpression> getOutputs() { return Collections.unmodifiableList(outputs); @@ -443,8 +438,8 @@ public abstract class GroupingOperation extends GroupingNode { * during expression evaluation to give the dispatch-node more data to consider when selecting the N groups that are * to be evaluated further. * - * @param precision The precision to set. - * @return This, to allow chaining. + * @param precision the precision to set + * @return this, to allow chaining * @see #setMax(int) */ public GroupingOperation setPrecision(int precision) { @@ -452,11 +447,7 @@ public abstract class GroupingOperation extends GroupingNode { return this; } - /** - * Returns the precision clause of this. - * - * @return The precision. - */ + /** Returns the precision clause of this. */ public int getPrecision() { return precision; } @@ -464,11 +455,11 @@ public abstract class GroupingOperation extends GroupingNode { /** * Assigns a string as the where clause of this operation. * - * @param str The string to assign to this. - * @return This, to allow chaining. + * @param string the string to assign to this + * @return this, to allow chaining */ - public GroupingOperation setWhere(String str) { - where = str; + public GroupingOperation setWhere(String string) { + where = string; return this; } @@ -590,9 +581,9 @@ public abstract class GroupingOperation extends GroupingNode { * Convenience method to call {@link #fromStringAsList(String)} and assert that the list contains exactly one * grouping operation. * - * @param str The string to parse. - * @return A grouping operation that corresponds to the string. - * @throws IllegalArgumentException Thrown if the string could not be parsed as a single operation. + * @param str the string to parse + * @return a grouping operation that corresponds to the string + * @throws IllegalArgumentException thrown if the string could not be parsed as a single operation */ public static GroupingOperation fromString(String str) { List<GroupingOperation> lst = fromStringAsList(str); @@ -606,15 +597,15 @@ public abstract class GroupingOperation extends GroupingNode { * Parses the given string as a list of grouping operations. This method never returns null, it either returns a * list of valid grouping requests or it throws an exception. * - * @param str The string to parse. - * @return A list of grouping operations that corresponds to the string. - * @throws IllegalArgumentException Thrown if the string could not be parsed. + * @param string the string to parse + * @return a list of grouping operations that corresponds to the string + * @throws IllegalArgumentException thrown if the string could not be parsed */ - public static List<GroupingOperation> fromStringAsList(String str) { - if (str == null || str.trim().length() == 0) { + public static List<GroupingOperation> fromStringAsList(String string) { + if (string == null || string.trim().length() == 0) { return Collections.emptyList(); } - GroupingParserInput input = new GroupingParserInput(str); + GroupingParserInput input = new GroupingParserInput(string); try { return new GroupingParser(input).requestList(); } catch (ParseException | TokenMgrException e) { diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java index c658d404adb..b0c8fbba059 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java +++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java @@ -21,8 +21,8 @@ import com.yahoo.language.Linguistics; import java.util.logging.Level; import com.yahoo.net.HostName; import com.yahoo.net.UriTools; -import com.yahoo.prelude.query.QueryException; import com.yahoo.prelude.query.parser.ParseException; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.rendering.Renderer; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.query.context.QueryContext; @@ -61,7 +61,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.logging.Level; import java.util.logging.Logger; /** @@ -229,10 +228,8 @@ public class SearchHandler extends LoggingRequestHandler { try { try { return handleBody(request); - } catch (QueryException e) { - return (e.getCause() instanceof IllegalArgumentException) - ? invalidParameterResponse(request, e) - : illegalQueryResponse(request, e); + } catch (IllegalInputException e) { + return illegalQueryResponse(request, e); } catch (RuntimeException e) { // Make sure we generate a valid response even on unexpected errors log.log(Level.WARNING, "Failed handling " + request, e); return internalServerErrorResponse(request, e); @@ -263,10 +260,6 @@ public class SearchHandler extends LoggingRequestHandler { return new HttpSearchResponse(getHttpResponseStatus(request, result), result, query, renderer); } - private HttpResponse invalidParameterResponse(HttpRequest request, RuntimeException e) { - return errorResponse(request, ErrorMessage.createInvalidQueryParameter(Exceptions.toMessageString(e))); - } - private HttpResponse illegalQueryResponse(HttpRequest request, RuntimeException e) { return errorResponse(request, ErrorMessage.createIllegalQuery(Exceptions.toMessageString(e))); } @@ -275,7 +268,6 @@ public class SearchHandler extends LoggingRequestHandler { return errorResponse(request, ErrorMessage.createInternalServerError(Exceptions.toMessageString(e))); } - private HttpSearchResponse handleBody(HttpRequest request) { Map<String, String> requestMap = requestMapFromRequest(request); @@ -450,19 +442,15 @@ public class SearchHandler extends LoggingRequestHandler { + Exceptions.toMessageString(e)); log.log(Level.FINE, error::getDetailedMessage); return new Result(query, error); + } catch (IllegalInputException e) { + ErrorMessage error = ErrorMessage.createBadRequest("Invalid request [" + request + "]: " + + Exceptions.toMessageString(e)); + log.log(Level.FINE, error::getDetailedMessage); + return new Result(query, error); } catch (IllegalArgumentException e) { - if ("Comparison method violates its general contract!".equals(e.getMessage())) { - // This is an error in application components or Vespa code - log(request, query, e); - return new Result(query, ErrorMessage.createUnspecifiedError("Failed searching: " + - Exceptions.toMessageString(e), e)); - } - else { - ErrorMessage error = ErrorMessage.createBadRequest("Invalid search request [" + request + "]: " - + Exceptions.toMessageString(e)); - log.log(Level.FINE, error::getDetailedMessage); - return new Result(query, error); - } + log(request, query, e); + return new Result(query, ErrorMessage.createUnspecifiedError("Failed: " + + Exceptions.toMessageString(e), e)); } catch (LinkageError | StackOverflowError e) { // LinkageError should have been an Exception in an OSGi world - typical bundle dependency issue problem // StackOverflowError is recoverable @@ -472,7 +460,7 @@ public class SearchHandler extends LoggingRequestHandler { return new Result(query, error); } catch (Exception e) { log(request, query, e); - return new Result(query, ErrorMessage.createUnspecifiedError("Failed searching: " + + return new Result(query, ErrorMessage.createUnspecifiedError("Failed: " + Exceptions.toMessageString(e), e)); } } @@ -579,8 +567,8 @@ public class SearchHandler extends LoggingRequestHandler { byte[] byteArray = IOUtils.readBytes(request.getData(), 1 << 20); inspector = SlimeUtils.jsonToSlime(byteArray).get(); if (inspector.field("error_message").valid()) { - throw new QueryException("Illegal query: " + inspector.field("error_message").asString() + " at: '" + - new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8) + "'"); + throw new IllegalInputException("Illegal query: " + inspector.field("error_message").asString() + " at: '" + + new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8) + "'"); } } catch (IOException e) { @@ -593,9 +581,9 @@ public class SearchHandler extends LoggingRequestHandler { requestMap.putAll(request.propertyMap()); if (requestMap.containsKey("yql") && (requestMap.containsKey("select.where") || requestMap.containsKey("select.grouping")) ) - throw new QueryException("Illegal query: Query contains both yql and select parameter"); + throw new IllegalInputException("Illegal query: Query contains both yql and select parameter"); if (requestMap.containsKey("query") && (requestMap.containsKey("select.where") || requestMap.containsKey("select.grouping")) ) - throw new QueryException("Illegal query: Query contains both query and select parameter"); + throw new IllegalInputException("Illegal query: Query contains both query and select parameter"); return requestMap; } diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/PageTemplateSearcher.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/PageTemplateSearcher.java index 0ec04bf99de..2074fce19bd 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/PageTemplateSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/PageTemplateSearcher.java @@ -2,9 +2,9 @@ package com.yahoo.search.pagetemplates; import com.google.inject.Inject; -import com.yahoo.component.ComponentId; import com.yahoo.component.chain.dependencies.Provides; import com.yahoo.component.provider.ComponentRegistry; +import com.yahoo.processing.IllegalInputException; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; @@ -13,8 +13,6 @@ import com.yahoo.search.pagetemplates.config.PageTemplateConfigurer; import com.yahoo.search.pagetemplates.engine.Organizer; import com.yahoo.search.pagetemplates.engine.Resolution; import com.yahoo.search.pagetemplates.engine.Resolver; -import com.yahoo.search.pagetemplates.engine.resolvers.DeterministicResolver; -import com.yahoo.search.pagetemplates.engine.resolvers.RandomResolver; import com.yahoo.search.pagetemplates.engine.resolvers.ResolverRegistry; import com.yahoo.search.pagetemplates.model.Choice; import com.yahoo.search.pagetemplates.model.PageElement; @@ -23,7 +21,13 @@ import com.yahoo.processing.request.CompoundName; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.searchchain.Execution; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.Map; /** * Enables page optimization templates. @@ -107,7 +111,7 @@ public class PageTemplateSearcher extends Searcher { @Override public Result search(Query query, Execution execution) { // Pre execution: Choose template and sources - List<PageElement> pages=selectPageTemplates(query); + List<PageElement> pages = selectPageTemplates(query); if (pages.isEmpty()) return execution.search(query); // Bypass if no page template chosen addSources(pages,query); @@ -115,12 +119,12 @@ public class PageTemplateSearcher extends Searcher { query.properties().set(pagePageTemplateListName, pages); // Execute - Result result=execution.search(query); + Result result = execution.search(query); // Post execution: Resolve choices and organize the result as dictated by the resolved template - Choice pageTemplateChoice=Choice.createSingletons(pages); - Resolution resolution=selectResolver(query).resolve(pageTemplateChoice,query,result); - organizer.organize(pageTemplateChoice,resolution,result); + Choice pageTemplateChoice = Choice.createSingletons(pages); + Resolution resolution = selectResolver(query).resolve(pageTemplateChoice, query, result); + organizer.organize(pageTemplateChoice, resolution, result); return result; } @@ -132,23 +136,23 @@ public class PageTemplateSearcher extends Searcher { // Determine the list of page template ids @SuppressWarnings("unchecked") List<String> pageIds = (List<String>) query.properties().get(pageIdListName); - if (pageIds==null) { - String pageIdString=query.properties().getString(pageIdName,"").trim(); - if (pageIdString.length()>0) - pageIds=Arrays.asList(pageIdString.split(" ")); + if (pageIds == null) { + String pageIdString = query.properties().getString(pageIdName,"").trim(); + if (pageIdString.length() > 0) + pageIds = Arrays.asList(pageIdString.split(" ")); } // If none set, just return the default or null if none - if (pageIds==null) { + if (pageIds == null) { PageElement defaultPage=templateRegistry.getComponent("default"); - return (defaultPage==null ? Collections.<PageElement>emptyList() : Collections.singletonList(defaultPage)); + return (defaultPage == null ? Collections.<PageElement>emptyList() : Collections.singletonList(defaultPage)); } // Resolve the id list to page templates - List<PageElement> pages=new ArrayList<>(pageIds.size()); + List<PageElement> pages = new ArrayList<>(pageIds.size()); for (String pageId : pageIds) { - PageTemplate page=templateRegistry.getComponent(pageId); - if (page==null) + PageTemplate page = templateRegistry.getComponent(pageId); + if (page == null) query.errors().add(ErrorMessage.createInvalidQueryParameter("Could not resolve requested page template '" + pageId + "'")); else @@ -159,17 +163,17 @@ public class PageTemplateSearcher extends Searcher { } private Resolver selectResolver(Query query) { - String resolverId=query.properties().getString(pageResolverName); - if (resolverId==null) return resolverRegistry.defaultResolver(); - Resolver resolver=resolverRegistry.getComponent(resolverId); - if (resolver==null) throw new IllegalArgumentException("No page template resolver '" + resolverId + "'"); + String resolverId = query.properties().getString(pageResolverName); + if (resolverId == null) return resolverRegistry.defaultResolver(); + Resolver resolver = resolverRegistry.getComponent(resolverId); + if (resolver == null) throw new IllegalInputException("No page template resolver '" + resolverId + "'"); return resolver; } /** Sets query.getModel().getSources() to the right value and add source parameters specified in templates */ - private void addSources(List<PageElement> pages,Query query) { + private void addSources(List<PageElement> pages, Query query) { // Determine all wanted sources - Set<Source> pageSources=new HashSet<>(); + Set<Source> pageSources = new HashSet<>(); for (PageElement page : pages) pageSources.addAll(((PageTemplate)page).getSources()); @@ -177,34 +181,34 @@ public class PageTemplateSearcher extends Searcher { if (query.getModel().getSources().size() > 0) { // Add properties if the source list is set explicitly, but do not modify otherwise - addParametersForIncludedSources(pageSources,query); + addParametersForIncludedSources(pageSources, query); return; } if (pageSources.contains(Source.any)) { - IntentModel intentModel=IntentModel.getFrom(query); - if (intentModel!=null) { + IntentModel intentModel = IntentModel.getFrom(query); + if (intentModel != null) { query.getModel().getSources().addAll(intentModel.getSourceNames()); - addPageTemplateSources(pageSources,query); + addPageTemplateSources(pageSources, query); } // otherwise leave empty to search all } else { // Let the page templates decide - addPageTemplateSources(pageSources,query); + addPageTemplateSources(pageSources, query); } } private void addPageTemplateSources(Set<Source> pageSources,Query query) { for (Source pageSource : pageSources) { - if (pageSource==Source.any) continue; + if (pageSource == Source.any) continue; query.getModel().getSources().add(pageSource.getName()); addParameters(pageSource,query); } } - private void addParametersForIncludedSources(Set<Source> sources,Query query) { + private void addParametersForIncludedSources(Set<Source> sources, Query query) { for (Source source : sources) { - if (source.parameters().size()>0 && query.getModel().getSources().contains(source.getName())) + if (source.parameters().size() > 0 && query.getModel().getSources().contains(source.getName())) addParameters(source,query); } } @@ -220,8 +224,8 @@ public class PageTemplateSearcher extends Searcher { * is not supported. (Same parameter sets in multiple templates is supported, * and will be just one entry in this set). */ - private void addErrorIfSameSourceMultipleTimes(List<PageElement> pages,Set<Source> sources,Query query) { - Set<String> sourceNames=new HashSet<>(); + private void addErrorIfSameSourceMultipleTimes(List<PageElement> pages, Set<Source> sources, Query query) { + Set<String> sourceNames = new HashSet<>(); for (Source source : sources) { if (sourceNames.contains(source.getName())) query.errors().add(ErrorMessage.createInvalidQueryParameter( diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderMappingVisitor.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderMappingVisitor.java index c29e9615fe8..c37ea5667c0 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderMappingVisitor.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderMappingVisitor.java @@ -19,16 +19,16 @@ import java.util.Map; */ class PlaceholderMappingVisitor extends PageTemplateVisitor { - private Map<String, MapChoice> placeholderIdToChoice=new LinkedHashMap<>(); + private final Map<String, MapChoice> placeholderIdToChoice = new LinkedHashMap<>(); @Override public void visit(MapChoice mapChoice) { - List<String> placeholderIds=mapChoice.placeholderIds(); + List<String> placeholderIds = mapChoice.placeholderIds(); for (String placeholderId : placeholderIds) { - MapChoice existingChoice=placeholderIdToChoice.put(placeholderId,mapChoice); - if (existingChoice!=null) + MapChoice existingChoice = placeholderIdToChoice.put(placeholderId,mapChoice); + if (existingChoice != null) throw new IllegalArgumentException("placeholder id '" + placeholderId + "' is referenced by both " + - mapChoice + " and " + existingChoice + ": Only one reference is allowed"); + mapChoice + " and " + existingChoice + ": Only one reference is allowed"); } } diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderReferenceCreatingVisitor.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderReferenceCreatingVisitor.java index e8870f4f11a..5ef507201cb 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderReferenceCreatingVisitor.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/PlaceholderReferenceCreatingVisitor.java @@ -1,9 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.pagetemplates; -import com.yahoo.search.pagetemplates.model.*; +import com.yahoo.search.pagetemplates.model.MapChoice; +import com.yahoo.search.pagetemplates.model.PageTemplateVisitor; +import com.yahoo.search.pagetemplates.model.Placeholder; -import java.util.HashMap; import java.util.Map; /** @@ -14,16 +15,16 @@ import java.util.Map; */ class PlaceholderReferenceCreatingVisitor extends PageTemplateVisitor { - private Map<String, MapChoice> placeholderIdToChoice=new HashMap<>(); + private final Map<String, MapChoice> placeholderIdToChoice; public PlaceholderReferenceCreatingVisitor(Map<String, MapChoice> placeholderIdToChoice) { - this.placeholderIdToChoice=placeholderIdToChoice; + this.placeholderIdToChoice = placeholderIdToChoice; } @Override public void visit(Placeholder placeholder) { - MapChoice choice=placeholderIdToChoice.get(placeholder.getId()); - if (choice==null) + MapChoice choice = placeholderIdToChoice.get(placeholder.getId()); + if (choice == null) throw new IllegalArgumentException(placeholder + " is not referenced by any choice"); placeholder.setValueContainer(choice); } diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/config/PageTemplateXMLReader.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/config/PageTemplateXMLReader.java index beffd12b22a..0359432a819 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/config/PageTemplateXMLReader.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/config/PageTemplateXMLReader.java @@ -38,18 +38,18 @@ public class PageTemplateXMLReader { * @throws RuntimeException if <code>directory</code> is not a readable directory, or if there is some error in the XML */ public PageTemplateRegistry read(String directory) { - List<NamedReader> pageReaders=new ArrayList<>(); + List<NamedReader> pageReaders = new ArrayList<>(); try { - File dir=new File(directory); - if ( !dir.isDirectory() ) throw new IllegalArgumentException("Could not read page templates: '" + - directory + "' is not a valid directory."); + File dir = new File(directory); + if ( ! dir.isDirectory() ) throw new IllegalArgumentException("Could not read page templates: '" + + directory + "' is not a valid directory."); for (File file : sortFiles(dir)) { if ( ! file.getName().endsWith(".xml")) continue; - pageReaders.add(new NamedReader(file.getName(),new FileReader(file))); + pageReaders.add(new NamedReader(file.getName(), new FileReader(file))); } - return read(pageReaders,true); + return read(pageReaders, true); } catch (IOException e) { throw new IllegalArgumentException("Could not read page templates from '" + directory + "'",e); @@ -67,18 +67,18 @@ public class PageTemplateXMLReader { * @throws RuntimeException if <code>fileName</code> is not a readable file, or if there is some error in the XML */ public PageTemplate readFile(String fileName) { - NamedReader pageReader=null; + NamedReader pageReader = null; try { - File file=new File(fileName); - pageReader=new NamedReader(fileName,new FileReader(file)); - String firstName=file.getName().substring(0,file.getName().length()-4); - return read(Collections.singletonList(pageReader),true).getComponent(firstName); + File file = new File(fileName); + pageReader = new NamedReader(fileName,new FileReader(file)); + String firstName = file.getName().substring(0, file.getName().length() - 4); + return read(Collections.singletonList(pageReader), true).getComponent(firstName); } catch (IOException e) { - throw new IllegalArgumentException("Could not read the page template '" + fileName + "'",e); + throw new IllegalArgumentException("Could not read the page template '" + fileName + "'", e); } finally { - if (pageReader!=null) + if (pageReader != null) try { pageReader.close(); } catch (IOException e) { } } } @@ -130,11 +130,11 @@ public class PageTemplateXMLReader { } /** Throws an exception if the name is not corresponding to the id */ - private void validateFileName(final String actualName,ComponentId id,String artifactName) { - String expectedCanonicalFileName=id.toFileName(); - String fileName=new File(actualName).getName(); - fileName=stripXmlEnding(fileName); - String canonicalFileName=ComponentId.fromFileName(fileName).toFileName(); + private void validateFileName(String actualName, ComponentId id, String artifactName) { + String expectedCanonicalFileName = id.toFileName(); + String fileName = new File(actualName).getName(); + fileName = stripXmlEnding(fileName); + String canonicalFileName = ComponentId.fromFileName(fileName).toFileName(); if ( ! canonicalFileName.equals(expectedCanonicalFileName)) throw new IllegalArgumentException("The file name of " + artifactName + " '" + id + "' must be '" + expectedCanonicalFileName + ".xml' but was '" + actualName + "'"); @@ -144,14 +144,14 @@ public class PageTemplateXMLReader { if (!fileName.endsWith(".xml")) throw new IllegalArgumentException("'" + fileName + "' should have a .xml ending"); else - return fileName.substring(0,fileName.length()-4); + return fileName.substring(0, fileName.length() - 4); } private void readPages() { for (Map.Entry<ComponentId,Element> pageElement : pageElementsByPageId.entrySet()) { try { - PageTemplate page=registry.getComponent(pageElement.getValue().getAttribute("id")); - readPageContent(pageElement.getValue(),page); + PageTemplate page = registry.getComponent(pageElement.getValue().getAttribute("id")); + readPageContent(pageElement.getValue(), page); } catch (IllegalArgumentException e) { throw new IllegalArgumentException("Could not read page template '" + pageElement.getKey() + "'",e); @@ -159,16 +159,16 @@ public class PageTemplateXMLReader { } } - private void readPageContent(Element pageElement,PageTemplate page) { + private void readPageContent(Element pageElement, PageTemplate page) { if (page.isFrozen()) return; // Already read - Section rootSection=new Section(page.getId().toString()); - readSection(pageElement,rootSection); + Section rootSection = new Section(page.getId().toString()); + readSection(pageElement, rootSection); page.setSection(rootSection); page.freeze(); } /** Fills a section with attributes and sub-elements from a "section" or "page" element */ - private Section readSection(Element sectionElement,Section section) { + private Section readSection(Element sectionElement, Section section) { section.setLayout(Layout.fromString(sectionElement.getAttribute("layout"))); section.setRegion(sectionElement.getAttribute("region")); section.setOrder(Sorting.fromString(sectionElement.getAttribute("order"))); @@ -198,10 +198,10 @@ public class PageTemplateXMLReader { /** Reads the direct descendant elements of an include */ private List<PageElement> readInclude(Element element) { - PageTemplate included=registry.getComponent(element.getAttribute("idref")); - if (included==null) + PageTemplate included = registry.getComponent(element.getAttribute("idref")); + if (included == null) throw new IllegalArgumentException("Could not find page template '" + element.getAttribute("idref")); - readPageContent(pageElementsByPageId.get(included.getId()),included); + readPageContent(pageElementsByPageId.get(included.getId()), included); return included.getSection().elements(Section.class); } @@ -223,9 +223,9 @@ public class PageTemplateXMLReader { } private List<Source> readSourceAttribute(Element sectionElement) { - List<Source> sources=new ArrayList<>(); - String sourceAttributeString=sectionElement.getAttribute("source"); - if (sourceAttributeString!=null) { + List<Source> sources = new ArrayList<>(); + String sourceAttributeString = sectionElement.getAttribute("source"); + if (sourceAttributeString != null) { for (String sourceName : sourceAttributeString.split(" ")) { if (sourceName.isEmpty()) continue; if ("*".equals(sourceName)) diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java index 3e6e82a5584..051103cba06 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java @@ -9,9 +9,7 @@ import com.yahoo.search.query.Sorting; import com.yahoo.search.result.*; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; -import java.util.Map; /** * Reorganizes and prunes a result as prescribed by a resolved template. @@ -29,13 +27,13 @@ public class Organizer { * @param result the result to organize */ public void organize(Choice templateChoice, Resolution resolution, Result result) { - PageTemplate template=(PageTemplate)templateChoice.get(resolution.getResolution(templateChoice)).get(0); - SectionHitGroup sectionGroup =toGroup(template.getSection(),resolution,result); - ErrorHit errors=result.hits().getErrorHit(); + PageTemplate template = (PageTemplate)templateChoice.get(resolution.getResolution(templateChoice)).get(0); + SectionHitGroup sectionGroup = toGroup(template.getSection(), resolution, result); + ErrorHit errors = result.hits().getErrorHit(); // transfer state from existing hit sectionGroup.setQuery(result.hits().getQuery()); - if (errors!=null && errors instanceof DefaultErrorHit) + if (errors instanceof DefaultErrorHit) sectionGroup.add((DefaultErrorHit)errors); result.hits().forEachField((name, value) -> sectionGroup.setField(name, value)); result.setHits(sectionGroup); diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Resolution.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Resolution.java index e0a3821e10c..f36ebe56e21 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Resolution.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Resolution.java @@ -40,10 +40,10 @@ public class Resolution { * been resolved in this */ public int getResolution(Choice choice) { - if (choice.alternatives().size()==1) return 0; + if (choice.alternatives().size() == 1) return 0; if (choice.isEmpty()) throw new IllegalArgumentException("Cannot return a resolution of empty " + choice); - Integer resolution=choiceResolutions.get(choice); - if (resolution==null) throw new IllegalArgumentException(this + " has no resolution of " + choice); + Integer resolution = choiceResolutions.get(choice); + if (resolution == null) throw new IllegalArgumentException(this + " has no resolution of " + choice); return resolution; } diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/result/PageTemplatesXmlRenderer.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/result/PageTemplatesXmlRenderer.java index f2e2e1b034d..47914792da8 100644 --- a/container-search/src/main/java/com/yahoo/search/pagetemplates/result/PageTemplatesXmlRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/result/PageTemplatesXmlRenderer.java @@ -271,10 +271,10 @@ public class PageTemplatesXmlRenderer extends AsynchronousSectionedRenderer<Resu private Result getResult() { try { - return (Result) getResponse(); + return (Result)getResponse(); } catch (ClassCastException e) { - throw new IllegalArgumentException("PageTemplatesXmlRenderer attempted used outside a search context, got a " + - getResponse().getClass().getName()); + throw new IllegalStateException("PageTemplatesXmlRenderer attempted used outside a search context, got a " + + getResponse().getClass().getName()); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/Model.java b/container-search/src/main/java/com/yahoo/search/query/Model.java index f06aab09a3d..637873aa375 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Model.java +++ b/container-search/src/main/java/com/yahoo/search/query/Model.java @@ -7,6 +7,7 @@ import com.yahoo.language.LocaleFactory; import com.yahoo.prelude.query.CompositeItem; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.TaggableItem; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.query.parser.Parsable; @@ -236,10 +237,14 @@ public class Model implements Cloneable { */ public QueryTree getQueryTree() { if (queryTree == null) { - Parser parser = ParserFactory.newInstance(type, ParserEnvironment.fromExecutionContext(execution.context())); - queryTree = parser.parse(Parsable.fromQueryModel(this)); - if (parent.getTraceLevel() >= 2) { - parent.trace("Query parsed to: " + parent.yqlRepresentation(), 2); + try { + Parser parser = ParserFactory.newInstance(type, ParserEnvironment.fromExecutionContext(execution.context())); + queryTree = parser.parse(Parsable.fromQueryModel(this)); + if (parent.getTraceLevel() >= 2) + parent.trace("Query parsed to: " + parent.yqlRepresentation(), 2); + } + catch (IllegalArgumentException e) { + throw new IllegalInputException("Failed parsing query", e); } } return queryTree; diff --git a/container-search/src/main/java/com/yahoo/search/query/ParameterParser.java b/container-search/src/main/java/com/yahoo/search/query/ParameterParser.java index d358fa06977..3addc1fdb4e 100644 --- a/container-search/src/main/java/com/yahoo/search/query/ParameterParser.java +++ b/container-search/src/main/java/com/yahoo/search/query/ParameterParser.java @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.query; +import com.yahoo.processing.IllegalInputException; + import static com.yahoo.container.util.Util.quote; /** @@ -25,13 +27,8 @@ public class ParameterParser { * representation cannot be parsed as a number followed optionally by time unit */ public static Long asMilliSeconds(Object value, Long defaultValue) { - if (value == null) { - return defaultValue; - } - if (value instanceof Number) { - Number n = (Number) value; - return Long.valueOf(n.longValue() * 1000L); - } + if (value == null) return defaultValue; + if (value instanceof Number) return ((Number)value).longValue() * 1000L; return parseTime(value.toString()); } @@ -43,7 +40,7 @@ public class ParameterParser { double multiplier = parseUnit(time.substring(unitOffset)); return (long) (measure * multiplier); } catch (RuntimeException e) { - throw new IllegalArgumentException("Error parsing " + quote(time), e); + throw new IllegalInputException("Error parsing " + quote(time), e); } } @@ -58,7 +55,7 @@ public class ParameterParser { } } if (unitOffset == 0) { - throw new NumberFormatException("Invalid number " + quote(time)); + throw new IllegalInputException("Invalid number " + quote(time)); } return unitOffset; } diff --git a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java index 0d9acea7643..30d741f465c 100644 --- a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java +++ b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java @@ -2,6 +2,7 @@ package com.yahoo.search.query; import com.google.common.base.Preconditions; +import com.yahoo.processing.IllegalInputException; import com.yahoo.collections.LazyMap; import com.yahoo.geo.DistanceParser; import com.yahoo.geo.ParsedDegree; @@ -27,7 +28,6 @@ import com.yahoo.prelude.query.OrItem; import com.yahoo.prelude.query.PhraseItem; import com.yahoo.prelude.query.PredicateQueryItem; import com.yahoo.prelude.query.PrefixItem; -import com.yahoo.prelude.query.QueryException; import com.yahoo.prelude.query.RangeItem; import com.yahoo.prelude.query.RankItem; import com.yahoo.prelude.query.RegExpItem; @@ -72,10 +72,6 @@ import static com.yahoo.search.yql.YqlParser.AND_SEGMENTING; import static com.yahoo.search.yql.YqlParser.ANNOTATIONS; import static com.yahoo.search.yql.YqlParser.APPROXIMATE; import static com.yahoo.search.yql.YqlParser.ASCENDING_HITS_ORDER; -import static com.yahoo.search.yql.YqlParser.BOUNDS; -import static com.yahoo.search.yql.YqlParser.BOUNDS_LEFT_OPEN; -import static com.yahoo.search.yql.YqlParser.BOUNDS_OPEN; -import static com.yahoo.search.yql.YqlParser.BOUNDS_RIGHT_OPEN; import static com.yahoo.search.yql.YqlParser.CONNECTION_ID; import static com.yahoo.search.yql.YqlParser.CONNECTION_WEIGHT; import static com.yahoo.search.yql.YqlParser.CONNECTIVITY; @@ -83,7 +79,6 @@ import static com.yahoo.search.yql.YqlParser.DEFAULT_TARGET_NUM_HITS; import static com.yahoo.search.yql.YqlParser.DESCENDING_HITS_ORDER; import static com.yahoo.search.yql.YqlParser.DISTANCE; import static com.yahoo.search.yql.YqlParser.DOT_PRODUCT; -import static com.yahoo.search.yql.YqlParser.END_ANCHOR; import static com.yahoo.search.yql.YqlParser.EQUIV; import static com.yahoo.search.yql.YqlParser.FILTER; import static com.yahoo.search.yql.YqlParser.GEO_LOCATION; @@ -109,7 +104,6 @@ import static com.yahoo.search.yql.YqlParser.RANKED; import static com.yahoo.search.yql.YqlParser.SAME_ELEMENT; import static com.yahoo.search.yql.YqlParser.SCORE_THRESHOLD; import static com.yahoo.search.yql.YqlParser.SIGNIFICANCE; -import static com.yahoo.search.yql.YqlParser.START_ANCHOR; import static com.yahoo.search.yql.YqlParser.STEM; import static com.yahoo.search.yql.YqlParser.SUBSTRING; import static com.yahoo.search.yql.YqlParser.SUFFIX; @@ -117,7 +111,6 @@ import static com.yahoo.search.yql.YqlParser.TARGET_HITS; import static com.yahoo.search.yql.YqlParser.TARGET_NUM_HITS; import static com.yahoo.search.yql.YqlParser.THRESHOLD_BOOST_FACTOR; import static com.yahoo.search.yql.YqlParser.UNIQUE_ID; -import static com.yahoo.search.yql.YqlParser.URI; import static com.yahoo.search.yql.YqlParser.USE_POSITION_DATA; import static com.yahoo.search.yql.YqlParser.USER_INPUT_LANGUAGE; import static com.yahoo.search.yql.YqlParser.WAND; @@ -169,13 +162,18 @@ public class SelectParser implements Parser { private QueryTree buildTree() { Inspector inspector = SlimeUtils.jsonToSlime(this.query.getSelect().getWhereString()).get(); if (inspector.field("error_message").valid()) { - throw new QueryException("Illegal query: " + inspector.field("error_message").asString() + + throw new IllegalInputException("Illegal query: " + inspector.field("error_message").asString() + " at: '" + new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8) + "'"); } - Item root = walkJson(inspector); - connectItems(); - return new QueryTree(root); + try { + Item root = walkJson(inspector); + connectItems(); + return new QueryTree(root); + } + catch (IllegalArgumentException e) { + throw new IllegalInputException("Illegal JSON query", e); + } } private Item walkJson(Inspector inspector) { @@ -229,8 +227,8 @@ public class SelectParser implements Parser { private List<String> toGroupingRequests(String groupingJson) { Inspector inspector = SlimeUtils.jsonToSlime(groupingJson).get(); if (inspector.field("error_message").valid()) { - throw new QueryException("Illegal query: " + inspector.field("error_message").asString() + - " at: '" + new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8) + "'"); + throw new IllegalInputException("Illegal query: " + inspector.field("error_message").asString() + + " at: '" + new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8) + "'"); } List<String> operations = new ArrayList<>(); @@ -1199,9 +1197,10 @@ public class SelectParser implements Parser { private void connectItems() { for (ConnectedItem entry : connectedItems) { TaggableItem to = identifiedItems.get(entry.toId); - Preconditions.checkNotNull(to, - "Item '%s' was specified to connect to item with ID %s, which does not " - + "exist in the query.", entry.fromItem, entry.toId); + if (to == null) + throw new IllegalArgumentException("Item '" + entry.fromItem + + "' was specified to connect to item with ID " + entry.toId + + ", which does not exist in the query."); entry.fromItem.setConnectivity((Item) to, entry.weight); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/Sorting.java b/container-search/src/main/java/com/yahoo/search/query/Sorting.java index 6518dcd5b6d..0a0ae65d524 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Sorting.java +++ b/container-search/src/main/java/com/yahoo/search/query/Sorting.java @@ -3,6 +3,7 @@ package com.yahoo.search.query; import com.ibm.icu.text.Collator; import com.ibm.icu.util.ULocale; +import com.yahoo.processing.IllegalInputException; import com.yahoo.text.Utf8; import java.nio.ByteBuffer; @@ -88,7 +89,7 @@ public class Sorting implements Cloneable { } else if (STRENGTH_IDENTICAL.equalsIgnoreCase(s)) { strength = UcaSorter.Strength.IDENTICAL; } else { - throw new IllegalArgumentException("Unknown collation strength: '" + s + "'"); + throw new IllegalInputException("Unknown collation strength: '" + s + "'"); } sorter = new UcaSorter(sortString.substring(startPar+1, commaPos), sortString.substring(commaPos+1, commaopt), strength); } else { @@ -99,9 +100,9 @@ public class Sorting implements Cloneable { } } else { if (funcName.isEmpty()) { - throw new IllegalArgumentException("No sort function specified"); + throw new IllegalInputException("No sort function specified"); } else { - throw new IllegalArgumentException("Unknown sort function '" + funcName + "'"); + throw new IllegalInputException("Unknown sort function '" + funcName + "'"); } } } else { @@ -196,7 +197,7 @@ public class Sorting implements Cloneable { if (legalAttributeName.matcher(fieldName).matches()) { this.fieldName = fieldName; } else { - throw new IllegalArgumentException("Illegal attribute name '" + fieldName + "' for sorting. Requires '" + legalAttributeName.pattern() + "'"); + throw new IllegalInputException("Illegal attribute name '" + fieldName + "' for sorting. Requires '" + legalAttributeName.pattern() + "'"); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java index b6b03d37da8..6008b046d1a 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java @@ -4,6 +4,7 @@ package com.yahoo.search.query.profile; import com.google.common.collect.ImmutableList; import com.yahoo.component.ComponentId; import com.yahoo.component.provider.FreezableSimpleComponent; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.processing.request.Properties; import com.yahoo.search.query.profile.compiled.CompiledQueryProfile; @@ -451,7 +452,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable setNode(name, value, null, binding, registry); } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Could not set '" + name + "' to '" + value + "'", e); + throw new IllegalInputException("Could not set '" + name + "' to '" + value + "'", e); } } @@ -688,15 +689,15 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable FieldDescription fieldDescription = type.getField(localName); if (fieldDescription == null) { if (type.isStrict()) - throw new IllegalArgumentException("'" + localName + "' is not declared in " + type + ", and the type is strict"); + throw new IllegalInputException("'" + localName + "' is not declared in " + type + ", and the type is strict"); return value; } if (registry == null && (fieldDescription.getType() instanceof QueryProfileFieldType)) - throw new IllegalArgumentException("A registry was not passed: Query profile references is not supported"); + throw new IllegalInputException("A registry was not passed: Query profile references is not supported"); Object convertedValue = fieldDescription.getType().convertFrom(value, registry); if (convertedValue == null) - throw new IllegalArgumentException("'" + value + "' is not a " + fieldDescription.getType().toInstanceDescription()); + throw new IllegalInputException("'" + value + "' is not a " + fieldDescription.getType().toInstanceDescription()); return convertedValue; } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java index d199ee44c9c..41272d695ac 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java @@ -2,6 +2,7 @@ package com.yahoo.search.query.profile; import com.yahoo.collections.Pair; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.processing.request.properties.PropertyMap; import com.yahoo.protect.Validator; @@ -107,7 +108,7 @@ public class QueryProfileProperties extends Properties { String localName = name.get(i); FieldDescription fieldDescription = type.getField(localName); if (fieldDescription == null && type.isStrict()) - throw new IllegalArgumentException("'" + localName + "' is not declared in " + type + ", and the type is strict"); + throw new IllegalInputException("'" + localName + "' is not declared in " + type + ", and the type is strict"); // TODO: In addition to strictness, check legality along the way @@ -115,7 +116,7 @@ public class QueryProfileProperties extends Properties { if (i == name.size() - 1) { // at the end of the path, check the assignment type value = fieldDescription.getType().convertFrom(value, profile.getRegistry()); if (value == null) - throw new IllegalArgumentException("'" + value + "' is not a " + + throw new IllegalInputException("'" + value + "' is not a " + fieldDescription.getType().toInstanceDescription()); } else if (fieldDescription.getType() instanceof QueryProfileFieldType) { @@ -129,12 +130,12 @@ public class QueryProfileProperties extends Properties { if (value instanceof String && value.toString().startsWith("ref:")) { if (profile.getRegistry() == null) - throw new IllegalArgumentException("Runtime query profile references does not work when the " + + throw new IllegalInputException("Runtime query profile references does not work when the " + "QueryProfileProperties are constructed without a registry"); String queryProfileId = value.toString().substring(4); value = profile.getRegistry().findQueryProfile(queryProfileId); if (value == null) - throw new IllegalArgumentException("Query profile '" + queryProfileId + "' is not found"); + throw new IllegalInputException("Query profile '" + queryProfileId + "' is not found"); } if (value instanceof CompiledQueryProfile) { // this will be due to one of the two clauses above @@ -149,7 +150,7 @@ public class QueryProfileProperties extends Properties { } } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Could not set '" + name + "' to '" + value + "'", e); + throw new IllegalInputException("Could not set '" + name + "' to '" + value + "'", e); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java index 8638a99172f..e873c80add1 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java @@ -33,7 +33,6 @@ public class Binding implements Comparable<Binding> { private final int hashCode; - @SuppressWarnings("unchecked") public static final Binding nullBinding = new Binding(Integer.MAX_VALUE, Collections.<String,String>emptyMap()); public static Binding createFrom(DimensionBinding dimensionBinding) { diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java index 6c30f1a8b05..daab5f6a378 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java @@ -97,7 +97,8 @@ public class FieldDescription implements Comparable<FieldDescription> { this.type = type; // Forbidden until we can figure out the right semantics - if (name.isCompound() && ! aliases.isEmpty()) throw new IllegalArgumentException("Aliases are not allowed with compound names"); + if (name.isCompound() && ! aliases.isEmpty()) + throw new IllegalArgumentException("Aliases are not allowed with compound names"); this.aliases = ImmutableList.copyOf(aliases); this.mandatory = mandatory; diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java index d5f88ba99ec..6186f588a21 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. 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.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; @@ -317,7 +318,7 @@ public class QueryProperties extends Properties { if (e.getMessage() != null && e.getMessage().startsWith("Could not set")) throw e; else - throw new IllegalArgumentException("Could not set '" + key + "' to '" + value + "'", e); + throw new IllegalInputException("Could not set '" + key + "' to '" + value + "'", e); } } @@ -364,8 +365,8 @@ public class QueryProperties extends Properties { } private void throwIllegalParameter(String key,String namespace) { - throw new IllegalArgumentException("'" + key + "' is not a valid property in '" + namespace + - "'. See the query api for valid keys starting by '" + namespace + "'."); + throw new IllegalInputException("'" + key + "' is not a valid property in '" + namespace + + "'. See the query api for valid keys starting by '" + namespace + "'."); } @Override diff --git a/container-search/src/main/java/com/yahoo/search/query/ranking/MatchPhase.java b/container-search/src/main/java/com/yahoo/search/query/ranking/MatchPhase.java index 794247863bf..72a6533e946 100644 --- a/container-search/src/main/java/com/yahoo/search/query/ranking/MatchPhase.java +++ b/container-search/src/main/java/com/yahoo/search/query/ranking/MatchPhase.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.query.ranking; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.query.Ranking; import com.yahoo.search.query.profile.types.FieldDescription; @@ -84,8 +85,9 @@ public class MatchPhase implements Cloneable { public void setMaxFilterCoverage(double maxFilterCoverage) { if ((maxFilterCoverage < 0.0) || (maxFilterCoverage > 1.0)) { - throw new IllegalArgumentException("maxFilterCoverage must be in the range [0.0, 1.0]. It is " + maxFilterCoverage); + throw new IllegalInputException("maxFilterCoverage must be in the range [0.0, 1.0]. It is " + maxFilterCoverage); } + this.maxFilterCoverage = maxFilterCoverage; } diff --git a/container-search/src/main/java/com/yahoo/search/query/ranking/Matching.java b/container-search/src/main/java/com/yahoo/search/query/ranking/Matching.java index fb3f2acfadd..14e4e006b39 100644 --- a/container-search/src/main/java/com/yahoo/search/query/ranking/Matching.java +++ b/container-search/src/main/java/com/yahoo/search/query/ranking/Matching.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.query.ranking; +import com.yahoo.processing.IllegalInputException; import com.yahoo.search.query.Ranking; import com.yahoo.search.query.profile.types.FieldDescription; import com.yahoo.search.query.profile.types.QueryProfileType; @@ -59,7 +60,7 @@ public class Matching implements Cloneable { public void setTermwiselimit(double value) { if ((value < 0.0) || (value > 1.0)) { - throw new IllegalArgumentException("termwiselimit must be in the range [0.0, 1.0]. It is " + value); + throw new IllegalInputException("termwiselimit must be in the range [0.0, 1.0]. It is " + value); } termwiseLimit = value; } diff --git a/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java b/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java index 0d47ef77ce5..43c26692221 100644 --- a/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java +++ b/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.query.ranking; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.query.Ranking; import com.yahoo.search.query.profile.types.FieldDescription; @@ -54,7 +55,7 @@ public class SoftTimeout implements Cloneable { /** Override the adaptive factor determined on the content nodes */ public void setFactor(double factor) { if ((factor < 0.0) || (factor > 1.0)) { - throw new IllegalArgumentException("factor must be in the range [0.0, 1.0], got " + factor); + throw new IllegalInputException("factor must be in the range [0.0, 1.0], got " + factor); } this.factor = factor; } @@ -64,7 +65,7 @@ public class SoftTimeout implements Cloneable { /** Override the tail cost factor determined on the content nodes */ public void setTailcost(double tailcost) { if ((tailcost < 0.0) || (tailcost > 1.0)) { - throw new IllegalArgumentException("tailcost must be in the range [0.0, 1.0], got " + tailcost); + throw new IllegalInputException("tailcost must be in the range [0.0, 1.0], got " + tailcost); } this.tailcost = tailcost; } diff --git a/container-search/src/main/java/com/yahoo/search/query/rewrite/rewriters/GenericExpansionRewriter.java b/container-search/src/main/java/com/yahoo/search/query/rewrite/rewriters/GenericExpansionRewriter.java index 794217e688f..9a2374da4c9 100644 --- a/container-search/src/main/java/com/yahoo/search/query/rewrite/rewriters/GenericExpansionRewriter.java +++ b/container-search/src/main/java/com/yahoo/search/query/rewrite/rewriters/GenericExpansionRewriter.java @@ -76,9 +76,8 @@ public class GenericExpansionRewriter extends QueryRewriteSearcher { HashMap<String, File> fileList) { logger = Logger.getLogger(GenericExpansionRewriter.class.getName()); FSA fsa = (FSA)rewriterDicts.get(GENERIC_EXPAND_DICT); - if(fsa==null) { - RewriterUtils.error(logger, "Error retrieving FSA dictionary: " + - GENERIC_EXPAND_DICT); + if (fsa==null) { + RewriterUtils.error(logger, "Error retrieving FSA dictionary: " + GENERIC_EXPAND_DICT); return false; } // Create Phrase Matcher diff --git a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/ItemContext.java b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/ItemContext.java index e739608387d..b6c886c91d7 100644 --- a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/ItemContext.java +++ b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/ItemContext.java @@ -12,6 +12,7 @@ import java.util.Map; * @author Tony Vaagenes */ public class ItemContext { + private class Connectivity { final String id; final double strength; @@ -43,7 +44,8 @@ public class ItemContext { private Item getItem(String id) { Item item = itemById.get(id); if (item == null) - throw new IllegalArgumentException("No item with id '" + id + "'."); + throw new IllegalArgumentException("No item with id '" + id + "'"); return item; } + } diff --git a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TermConverter.java b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TermConverter.java index 04c01d7acc1..503987af027 100644 --- a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TermConverter.java +++ b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TermConverter.java @@ -10,6 +10,7 @@ import com.yahoo.search.query.textserialize.serializer.ItemIdMapper; * @author Tony Vaagenes */ public abstract class TermConverter implements ItemFormConverter { + @Override public Object formToItem(String name, ItemArguments arguments, ItemContext context) { ensureOnlyOneChild(arguments); @@ -22,7 +23,6 @@ public abstract class TermConverter implements ItemFormConverter { abstract TermItem newTermItem(String word); - private void ensureOnlyOneChild(ItemArguments arguments) { if (arguments.children.size() != 1) { throw new IllegalArgumentException("Expected exactly one argument, got '" + @@ -50,4 +50,5 @@ public abstract class TermConverter implements ItemFormConverter { } protected abstract String getValue(TermItem item); + } diff --git a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TypeCheck.java b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TypeCheck.java index b7843a300dc..70eba9d8b50 100644 --- a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TypeCheck.java +++ b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/TypeCheck.java @@ -7,6 +7,7 @@ import com.yahoo.protect.Validator; * @author Tony Vaagenes */ public class TypeCheck { + public static void ensureInstanceOf(Object object, Class<?> c) { Validator.ensureInstanceOf(expectationString(c.getName(), object.getClass().getSimpleName()), object, c); @@ -24,4 +25,5 @@ public class TypeCheck { private static String expectationString(String expected, String got) { return "Expected " + expected + ", but got " + got; } + } diff --git a/container-search/src/main/java/com/yahoo/search/query/textserialize/serializer/Serializer.java b/container-search/src/main/java/com/yahoo/search/query/textserialize/serializer/Serializer.java index d4e499bab79..d849e744184 100644 --- a/container-search/src/main/java/com/yahoo/search/query/textserialize/serializer/Serializer.java +++ b/container-search/src/main/java/com/yahoo/search/query/textserialize/serializer/Serializer.java @@ -14,6 +14,7 @@ import static com.yahoo.search.query.textserialize.item.ListUtil.first; * @author Tony Vaagenes */ class Serializer { + static String serialize(Object child, ItemIdMapper itemIdMapper) { if (child instanceof DispatchForm) { return ((DispatchForm) child).serialize(itemIdMapper); @@ -76,4 +77,5 @@ class Serializer { static String serializeItem(Item item, ItemIdMapper itemIdMapper) { return ItemExecutorRegistry.getByType(item.getItemType()).itemToForm(item, itemIdMapper).serialize(itemIdMapper); } + } diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/BooleanAttributeParser.java b/container-search/src/main/java/com/yahoo/search/querytransform/BooleanAttributeParser.java index fd76cc4397a..532de1270da 100644 --- a/container-search/src/main/java/com/yahoo/search/querytransform/BooleanAttributeParser.java +++ b/container-search/src/main/java/com/yahoo/search/querytransform/BooleanAttributeParser.java @@ -14,10 +14,10 @@ import java.math.BigInteger; * a 64-bit hex number <code>0x1234</code> or a list of bits <code>[0, 2, 43, * 22, ...]</code>. * - * @author <a href="mailto:magnarn@yahoo-inc.com">Magnar Nedland</a> - * @since 5.1.15 + * @author Magnar Nedland */ abstract class BooleanAttributeParser extends SimpleMapParser { + private boolean isMap = true; @Override diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/BooleanSearcher.java b/container-search/src/main/java/com/yahoo/search/querytransform/BooleanSearcher.java index f77301f587c..de01773c27c 100644 --- a/container-search/src/main/java/com/yahoo/search/querytransform/BooleanSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/querytransform/BooleanSearcher.java @@ -4,6 +4,7 @@ package com.yahoo.search.querytransform; import com.yahoo.component.chain.dependencies.After; import com.yahoo.component.chain.dependencies.Provides; import com.yahoo.prelude.query.PredicateQueryItem; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -26,6 +27,7 @@ import static com.yahoo.yolean.Exceptions.toMessageString; @After({ STEMMING, ACCENT_REMOVAL }) @Provides(BooleanSearcher.PREDICATE) public class BooleanSearcher extends Searcher { + private static final CompoundName FIELD = new CompoundName("boolean.field"); private static final CompoundName ATTRIBUTES = new CompoundName("boolean.attributes"); private static final CompoundName RANGE_ATTRIBUTES = new CompoundName("boolean.rangeAttributes"); @@ -61,7 +63,11 @@ public class BooleanSearcher extends Searcher { } catch (TokenMgrException e) { return new Result(query, ErrorMessage.createInvalidQueryParameter(toMessageString(e))); } - } else { + catch (IllegalArgumentException e) { + throw new IllegalInputException("Failed boolean search on field '" + fieldName + "'", e); + } + } + else { if (query.isTraceable(5)) { query.trace("BooleanSearcher: Nothing added to query", false, 5); } @@ -79,7 +85,9 @@ public class BooleanSearcher extends Searcher { } static public class PredicateValueAttributeParser extends BooleanAttributeParser { - private PredicateQueryItem item; + + private final PredicateQueryItem item; + public PredicateValueAttributeParser(PredicateQueryItem item) { this.item = item; } @@ -93,10 +101,13 @@ public class BooleanSearcher extends Searcher { protected void addAttribute(String attribute, String value, BigInteger subQueryMask) { item.addFeature(attribute, value, subQueryMask.longValue()); } + } static private class PredicateRangeAttributeParser extends BooleanAttributeParser { - private PredicateQueryItem item; + + private final PredicateQueryItem item; + public PredicateRangeAttributeParser(PredicateQueryItem item) { this.item = item; } @@ -110,5 +121,7 @@ public class BooleanSearcher extends Searcher { protected void addAttribute(String attribute, String value, BigInteger subQueryMask) { item.addRangeFeature(attribute, Long.parseLong(value), subQueryMask.longValue()); } + } + } diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/WandSearcher.java b/container-search/src/main/java/com/yahoo/search/querytransform/WandSearcher.java index 0b1387a16a2..f3804af1a9e 100644 --- a/container-search/src/main/java/com/yahoo/search/querytransform/WandSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/querytransform/WandSearcher.java @@ -4,6 +4,7 @@ package com.yahoo.search.querytransform; import com.yahoo.prelude.Index; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.query.*; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -92,7 +93,7 @@ public class WandSearcher extends Searcher { private WandType resolveWandType(IndexFacts.Session indexFacts, Query query) { Index index = indexFacts.getIndex(fieldName); if (index.isNull()) { - throw new IllegalArgumentException("Field '" + fieldName + "' was not found in " + indexFacts); + throw new IllegalInputException("Field '" + fieldName + "' was not found in " + indexFacts); } else { return WandType.create(query.properties().getString(WAND_TYPE, "vespa")); } @@ -100,15 +101,15 @@ public class WandSearcher extends Searcher { private int resolveHeapSize(Query query) { String defaultHeapSize = "100"; - return Integer.valueOf(query.properties().getString(WAND_HEAP_SIZE, defaultHeapSize)); + return Integer.parseInt(query.properties().getString(WAND_HEAP_SIZE, defaultHeapSize)); } private double resolveScoreThreshold(Query query) { - return Double.valueOf(query.properties().getString(WAND_SCORE_THRESHOLD, "0")); + return Double.parseDouble(query.properties().getString(WAND_SCORE_THRESHOLD, "0")); } private double resolveThresholdBoostFactor(Query query) { - return Double.valueOf(query.properties().getString(WAND_THRESHOLD_BOOST_FACTOR, "1")); + return Double.parseDouble(query.properties().getString(WAND_THRESHOLD_BOOST_FACTOR, "1")); } public boolean hasValidData() { @@ -166,7 +167,7 @@ public class WandSearcher extends Searcher { } else if (inputs.getWandType().equals(WandType.DOT_PRODUCT)) { return populate(new DotProductItem(inputs.getFieldName()), inputs.getTokens()); } - throw new IllegalArgumentException("Unknown type '" + inputs.getWandType() + "'"); + throw new IllegalInputException("Unknown type '" + inputs.getWandType() + "'"); } private CompositeItem populate(CompositeItem parent, String fieldName, Map<String,Integer> tokens) { @@ -195,10 +196,12 @@ public class WandSearcher extends Searcher { } private static class IntegerMapParser extends MapParser<Integer> { + @Override protected Integer parseValue(String s) { return Integer.parseInt(s); } + } } diff --git a/container-search/src/main/java/com/yahoo/search/rendering/RendererRegistry.java b/container-search/src/main/java/com/yahoo/search/rendering/RendererRegistry.java index 783045babf4..9540bc20bc5 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/RendererRegistry.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/RendererRegistry.java @@ -4,6 +4,7 @@ package com.yahoo.search.rendering; import com.yahoo.component.ComponentId; import com.yahoo.component.ComponentSpecification; import com.yahoo.component.provider.ComponentRegistry; +import com.yahoo.processing.IllegalInputException; import com.yahoo.processing.rendering.Renderer; import com.yahoo.search.Result; import com.yahoo.search.pagetemplates.result.PageTemplatesXmlRenderer; @@ -103,8 +104,8 @@ public final class RendererRegistry extends ComponentRegistry<com.yahoo.processi com.yahoo.processing.rendering.Renderer<Result> renderer = getComponent(format); if (renderer == null) - throw new IllegalArgumentException("No renderer with id or alias '" + format + "'. " + - "Available renderers are: [" + rendererNames() + "]."); + throw new IllegalInputException("No renderer with id or alias '" + format + "'. " + + "Available renderers are: [" + rendererNames() + "]."); return renderer; } diff --git a/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java index 62ee16993fd..f3b0cd3a61e 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/XmlRenderer.java @@ -352,8 +352,8 @@ public final class XmlRenderer extends AsynchronousSectionedRenderer<Result> { try { return (Result) getResponse(); } catch (ClassCastException e) { - throw new IllegalArgumentException("XmlRenderer attempted used outside a search context, got a " + - getResponse().getClass().getName()); + throw new IllegalStateException("XmlRenderer attempted used outside a search context, got a " + + getResponse().getClass().getName()); } } diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java b/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java index 4c36ca9b4da..8db690678bb 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/model/federation/LocalProviderSpec.java @@ -56,7 +56,7 @@ public class LocalProviderSpec { this.cacheSize = cacheSize; if (clusterName == null) - throw new IllegalArgumentException("Missing cluster name."); + throw new IllegalArgumentException("Missing cluster name"); } public static boolean includesType(String type) { diff --git a/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java b/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java index 3c0453f8900..5e15a8ba14b 100644 --- a/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/searchers/InputCheckingSearcher.java @@ -19,11 +19,13 @@ import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.PhraseItem; import com.yahoo.prelude.query.TermItem; import com.yahoo.prelude.query.WordItem; +import com.yahoo.processing.IllegalInputException; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.searchchain.Execution; +import com.yahoo.yolean.Exceptions; /** * Check whether the query tree seems to be "well formed". In other words, run heurestics against @@ -50,10 +52,8 @@ public class InputCheckingSearcher extends Searcher { public Result search(Query query, Execution execution) { try { checkQuery(query); - } catch (IllegalArgumentException e) { - if (log.isLoggable(Level.FINE)) { - log.log(Level.FINE, "Rejected query \"" + query.toString() + "\" on cause of: " + e.getMessage()); - } + } catch (IllegalInputException e) { + log.log(Level.FINE, () -> "Rejected query '" + query.toString() + "' on cause of: " + Exceptions.toMessageString(e)); return new Result(query, ErrorMessage.createIllegalQuery(e.getMessage())); } return execution.search(query); @@ -92,8 +92,9 @@ public class InputCheckingSearcher extends Searcher { repeatedCount++; if (repeatedCount >= MAX_REPEATED_CONSECUTIVE_TERMS_IN_PHRASE) { repeatedConsecutiveTermsInPhraseRejections.add(); - throw new IllegalArgumentException("More than " + MAX_REPEATED_CONSECUTIVE_TERMS_IN_PHRASE + - " ocurrences of term '" + current + "' in a row detected in phrase : " + phrase.toString()); + throw new IllegalInputException("More than " + MAX_REPEATED_CONSECUTIVE_TERMS_IN_PHRASE + + " occurrences of term '" + current + + "' in a row detected in phrase : " + phrase.toString()); } } else { repeatedCount = 0; @@ -125,8 +126,8 @@ public class InputCheckingSearcher extends Searcher { if (count != null) { if (count.get() >= MAX_REPEATED_TERMS_IN_PHRASE) { repeatedTermsInPhraseRejections.add(); - throw new IllegalArgumentException("Phrase contains more than " + MAX_REPEATED_TERMS_IN_PHRASE + - " occurrences of term '" + current + "' in phrase : " + phrase.toString()); + throw new IllegalInputException("Phrase contains more than " + MAX_REPEATED_TERMS_IN_PHRASE + + " occurrences of term '" + current + "' in phrase : " + phrase.toString()); } count.inc(); } else { @@ -169,8 +170,8 @@ public class InputCheckingSearcher extends Searcher { return; } utfRejections.add(); - throw new IllegalArgumentException("The user input has been determined to be double encoded UTF-8." - + " Please investigate whether this is a false positive."); + throw new IllegalInputException("The user input has been determined to be double encoded UTF-8." + + " Please investigate whether this is a false positive."); } private int countSingleCharacterUserTerms(Item queryItem) { diff --git a/container-search/src/main/java/com/yahoo/search/statistics/PeakQpsSearcher.java b/container-search/src/main/java/com/yahoo/search/statistics/PeakQpsSearcher.java index 108e46fb68e..6d4fcdaf8fb 100644 --- a/container-search/src/main/java/com/yahoo/search/statistics/PeakQpsSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/statistics/PeakQpsSearcher.java @@ -137,8 +137,8 @@ public class PeakQpsSearcher extends Searcher { useMetaHit = false; propertyName = null; } else { - throw new IllegalArgumentException("Config definition out of sync with implementation." + - " No way to create output for method " + method + "."); + throw new IllegalStateException("Config definition out of sync with implementation." + + " No way to create output for method " + method + "."); } } diff --git a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java index 8b2438e3ec2..b863440ea5b 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java +++ b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java @@ -5,6 +5,7 @@ import com.google.common.annotations.Beta; import com.google.inject.Inject; import com.yahoo.language.Linguistics; import com.yahoo.language.simple.SimpleLinguistics; +import com.yahoo.processing.IllegalInputException; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; @@ -28,6 +29,7 @@ import java.util.logging.Logger; * * @author Steinar Knutsen */ +// TODO: The query model should do this @Beta @Provides(MinimalQueryInserter.EXTERNAL_YQL) @Before(PhaseNames.TRANSFORMED_QUERY) @@ -40,19 +42,22 @@ public class MinimalQueryInserter extends Searcher { private static final CompoundName MAX_HITS = new CompoundName("maxHits"); private static final CompoundName MAX_OFFSET = new CompoundName("maxOffset"); - private static Logger log = Logger.getLogger(MinimalQueryInserter.class.getName()); + private static final Logger log = Logger.getLogger(MinimalQueryInserter.class.getName()); @Inject public MinimalQueryInserter(Linguistics linguistics) { // Warmup is needed to avoid a large 400ms init cost during first execution of yql code. warmup(linguistics); } + public MinimalQueryInserter() { this(new SimpleLinguistics()); } + static boolean warmup() { return warmup(new SimpleLinguistics()); } + private static boolean warmup(Linguistics linguistics) { Query query = new Query("search/?yql=select%20*%20from%20sources%20where%20title%20contains%20'xyz';"); Result result = insertQuery(query, new ParserEnvironment().setLinguistics(linguistics)); @@ -67,6 +72,18 @@ public class MinimalQueryInserter extends Searcher { return true; } + @Override + public Result search(Query query, Execution execution) { + try { + if (query.properties().get(YQL) == null) return execution.search(query); + Result result = insertQuery(query, ParserEnvironment.fromExecutionContext(execution.context())); + return (result == null) ? execution.search(query) : result; + } + catch (IllegalArgumentException e) { + throw new IllegalInputException("Illegal YQL query", e); + } + } + private static Result insertQuery(Query query, ParserEnvironment env) { YqlParser parser = (YqlParser) ParserFactory.newInstance(Query.Type.YQL, env); parser.setQueryParser(false); @@ -116,12 +133,4 @@ public class MinimalQueryInserter extends Searcher { return null; } - @Override - public Result search(Query query, Execution execution) { - if (query.properties().get(YQL) == null) return execution.search(query); - - Result result = insertQuery(query, ParserEnvironment.fromExecutionContext(execution.context())); - return (result == null) ? execution.search(query) : result; - } - } diff --git a/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java b/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java index e3ef4b5d4df..f529de32fe9 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java +++ b/container-search/src/main/java/com/yahoo/search/yql/OperatorNode.java @@ -162,7 +162,7 @@ final class OperatorNode<T extends Operator> { } // we are aware only of types used in our logical operator trees -- OperatorNode, List, and constant values - private static final Function<Object, Object> COPY = new Function<Object, Object>() { + private static final Function<Object, Object> COPY = new Function<>() { @Override public Object apply(Object input) { if (input instanceof List) { diff --git a/container-search/src/main/java/com/yahoo/search/yql/ProgramParser.java b/container-search/src/main/java/com/yahoo/search/yql/ProgramParser.java index a270d935a97..e287a368d04 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/ProgramParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/ProgramParser.java @@ -622,19 +622,19 @@ final class ProgramParser { } private OperatorNode<SequenceOperator> convertMerge(List<Merge_componentContext> mergeComponentList, Scope scope) { - Preconditions.checkArgument(mergeComponentList != null); - List<OperatorNode<SequenceOperator>> sources = Lists.newArrayListWithExpectedSize(mergeComponentList.size()); - for (Merge_componentContext mergeComponent:mergeComponentList) { - Select_statementContext selectContext = mergeComponent.select_statement(); - Source_statementContext sourceContext = mergeComponent.source_statement(); - if (selectContext != null) { - sources.add(convertQuery(selectContext, scope.getRoot())); - } else { - sources.add(convertQuery(sourceContext, scope.getRoot())); - } - } - return OperatorNode.create(SequenceOperator.MERGE, sources); - } + Preconditions.checkArgument(mergeComponentList != null); + List<OperatorNode<SequenceOperator>> sources = Lists.newArrayListWithExpectedSize(mergeComponentList.size()); + for (Merge_componentContext mergeComponent:mergeComponentList) { + Select_statementContext selectContext = mergeComponent.select_statement(); + Source_statementContext sourceContext = mergeComponent.source_statement(); + if (selectContext != null) { + sources.add(convertQuery(selectContext, scope.getRoot())); + } else { + sources.add(convertQuery(sourceContext, scope.getRoot())); + } + } + return OperatorNode.create(SequenceOperator.MERGE, sources); + } private OperatorNode<SequenceOperator> convertQuery(ParseTree node, Scope scope) { if (node instanceof Select_statementContext @@ -642,7 +642,7 @@ final class ProgramParser { || node instanceof Update_statementContext || node instanceof Delete_statementContext) { return convertSelectOrInsertOrUpdateOrDelete(node, scope.getRoot()); - } else if (node instanceof Source_statementContext) { //for pipe + } else if (node instanceof Source_statementContext) { // for pipe Source_statementContext sourceStatementContext = (Source_statementContext)node; return convertPipe(sourceStatementContext.query_statement(), sourceStatementContext.pipeline_step(), scope); } else if (node instanceof Merge_statementContext) { diff --git a/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java b/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java index 22328fb026e..a38e48fd89d 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java +++ b/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java @@ -142,7 +142,7 @@ public class VespaSerializer { escape(((WordItem) current).getIndexedString(), destination).append('"'); } else { throw new IllegalArgumentException("Serializing of " + current.getClass().getSimpleName() - + " in segment AND expressions not implemented, please report this as a bug."); + + " in segment AND expressions not implemented, please report this as a bug."); } } } @@ -643,7 +643,7 @@ public class VespaSerializer { WordAlternativesSerializer.serialize(destination, (WordAlternativesItem) current, false); } else { throw new IllegalArgumentException("Serializing of " + current.getClass().getSimpleName() + - " in phrases not implemented, please report this as a bug."); + " in phrases not implemented, please report this as a bug."); } } destination.append(')'); @@ -1158,7 +1158,7 @@ public class VespaSerializer { Serializer doIt = dispatch.get(item.getClass()); if (doIt == null) { - throw new IllegalArgumentException(item.getClass() + " not supported for YQL+ marshalling."); + throw new IllegalArgumentException(item.getClass() + " not supported for YQL marshalling."); } if (state.peekFirst() != null && state.peekFirst().subItems > 0) { diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java index 31a22fd3b58..3875018609e 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java @@ -67,6 +67,7 @@ import com.yahoo.prelude.query.WeakAndItem; import com.yahoo.prelude.query.WeightedSetItem; import com.yahoo.prelude.query.WordAlternativesItem; import com.yahoo.prelude.query.WordItem; +import com.yahoo.processing.IllegalInputException; import com.yahoo.search.Query; import com.yahoo.search.grouping.Continuation; import com.yahoo.search.grouping.request.GroupingOperation; @@ -318,10 +319,10 @@ public class YqlParser implements Parser { private void connectItems() { for (ConnectedItem entry : connectedItems) { TaggableItem to = identifiedItems.get(entry.toId); - Preconditions.checkNotNull(to, - "Item '%s' was specified to connect to item with ID %s, which does not " - + "exist in the query.", entry.fromItem, - entry.toId); + if (to == null) + throw new IllegalArgumentException("Item '" + entry.fromItem + + "' was specified to connect to item with ID " + entry.toId + + ", which does not exist in the query."); entry.fromItem.setConnectivity((Item) to, entry.weight); } } @@ -782,7 +783,7 @@ public class YqlParser implements Parser { try { ast = new ProgramParser().parse("query", currentlyParsing.getQuery()); } catch (Exception e) { - throw new IllegalArgumentException(e); + throw new IllegalInputException(e); } assertHasOperator(ast, StatementOperator.PROGRAM); Preconditions.checkArgument(ast.getArguments().length == 1, diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java index 8bd7bdac8a5..d06c4cd6f01 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java @@ -163,7 +163,7 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { query.toString(), elapsedMillis / 1000.0))); } return new Result(query, ErrorMessage.createTimeout(e.getMessage())); - } catch (InterruptedException|IllegalArgumentException e) { + } catch (InterruptedException | IllegalArgumentException e) { return new Result(query, ErrorMessage.createBackendCommunicationError(e.getMessage())); } diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java index 2b0d8adc8ef..b05ddfa3bb2 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java @@ -346,9 +346,9 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { log.log(Level.FINE, "VdsVisitor completed successfully for " + query + " with selection " + params.getDocumentSelection()); } } else { - throw new IllegalArgumentException("Query failed: " // TODO: Is it necessary to use a runtime exception? - + params.getControlHandler().getResult().code + ": " - + params.getControlHandler().getResult().message); + throw new IllegalArgumentException("Query failed: " + + params.getControlHandler().getResult().code + ": " + + params.getControlHandler().getResult().message); } } @@ -425,7 +425,8 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { BufferSerializer buf = new BufferSerializer( new GrowableByteBuffer(ByteBuffer.wrap(value)) ); newGrouping.deserialize(buf); if (buf.getBuf().hasRemaining()) { - throw new IllegalArgumentException("Failed deserializing grouping. There are still data left. Position = " + buf.position() + ", limit = " + buf.getBuf().limit()); + throw new IllegalArgumentException("Failed deserializing grouping. There is still data left. " + + "Position = " + buf.position() + ", limit = " + buf.getBuf().limit()); } synchronized (groupingMap) { |