diff options
6 files changed, 84 insertions, 44 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 43d3fafdb78..24e68965e9c 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -47,8 +47,7 @@ import java.util.Set; /** * Contains various state during deploy that should be available in all builders of a {@link com.yahoo.config.model.ConfigModel} * - * @author lulf - * @since 5.8 + * @author Ulf Lilleengen */ public class DeployState implements ConfigDefinitionStore { @@ -211,9 +210,9 @@ public class DeployState implements ConfigDefinitionStore { public QueryProfiles getQueryProfiles() { return queryProfiles; } public SemanticRules getSemanticRules() { return semanticRules; } - + public Version getWantedNodeVespaVersion() { return wantedNodeVespaVersion; } - + public Instant now() { return now; } public boolean disableFiledistributor() { return disableFiledistributor; } @@ -288,7 +287,7 @@ public class DeployState implements ConfigDefinitionStore { this.now = now; return this; } - + public Builder wantedNodeVespaVersion(Version version) { this.wantedNodeVespaVersion = version; return this; @@ -321,7 +320,8 @@ public class DeployState implements ConfigDefinitionStore { names.put(searchName, sdName); if (!sdName.equals(searchName)) { throw new IllegalArgumentException("Search definition file name ('" + sdName + "') and name of " + - "search element ('" + searchName + "') are not equal for file '" + readerName + "'"); + "search element ('" + searchName + + "') are not equal for file '" + readerName + "'"); } } catch (ParseException e) { throw new IllegalArgumentException("Could not parse search definition file '" + getSearchDefinitionRelativePath(reader.getName()) + "': " + e.getMessage(), e); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java index ba0ede77b55..0bccbc9c433 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java @@ -309,7 +309,7 @@ public class SearchBuilder { builder.build(); return builder; } - + /** * Convenience factory method to import and build a {@link Search} object from a file. Only for testing. * @@ -353,7 +353,7 @@ public class SearchBuilder { } // TODO: The build methods below just call the create methods above - remove - + /** * Convenience factory method to import and build a {@link Search} object from a file. Only for testing. * @@ -394,7 +394,7 @@ public class SearchBuilder { throws IOException, ParseException { return createFromFile(fileName, deployLogger, rankProfileRegistry).getSearch(); } - + /** * Convenience factory method to import and build a {@link Search} object from a raw object. * @@ -423,4 +423,5 @@ public class SearchBuilder { public RankProfileRegistry getRankProfileRegistry() { return rankProfileRegistry; } + } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java index 3faebbfeae3..3ee64094274 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/multifieldresolver/RankProfileTypeSettingsProcessor.java @@ -14,11 +14,10 @@ import com.yahoo.searchdefinition.document.ImportedField; import com.yahoo.searchdefinition.document.ImportedFields; import com.yahoo.searchdefinition.document.SDField; import com.yahoo.searchdefinition.processing.Processor; +import com.yahoo.searchlib.rankingexpression.evaluation.FeatureNames; import com.yahoo.vespa.model.container.search.QueryProfiles; import java.util.Map; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.Optional; /** @@ -30,8 +29,6 @@ import java.util.Optional; */ public class RankProfileTypeSettingsProcessor extends Processor { - private static final Pattern queryFeaturePattern = Pattern.compile("query\\((\\w+)\\)$"); - public RankProfileTypeSettingsProcessor(Search search, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) { super(search, deployLogger, rankProfileRegistry, queryProfiles); } @@ -41,7 +38,6 @@ public class RankProfileTypeSettingsProcessor extends Processor { processAttributeFields(); processImportedFields(); processQueryProfileTypes(); - } private void processAttributeFields() { @@ -54,18 +50,18 @@ public class RankProfileTypeSettingsProcessor extends Processor { } private void processImportedFields() { - Optional<ImportedFields> importedFields = search.importedFields(); - if (importedFields.isPresent()) { - importedFields.get().fields().forEach((fieldName, field) -> processImportedField(field)); - } + Optional<ImportedFields> importedFields = search.importedFields(); + if (importedFields.isPresent()) { + importedFields.get().fields().forEach((fieldName, field) -> processImportedField(field)); + } } private void processImportedField(ImportedField field) { - SDField targetField = field.targetField(); - Attribute attribute = targetField.getAttributes().get(targetField.getName()); - if (attribute != null && attribute.tensorType().isPresent()) { - addAttributeTypeToRankProfiles(field.fieldName(), attribute.tensorType().get().toString()); - } + SDField targetField = field.targetField(); + Attribute attribute = targetField.getAttributes().get(targetField.getName()); + if (attribute != null && attribute.tensorType().isPresent()) { + addAttributeTypeToRankProfiles(field.fieldName(), attribute.tensorType().get().toString()); + } } private void addAttributeTypeToRankProfiles(String attributeName, String attributeType) { @@ -87,11 +83,8 @@ public class RankProfileTypeSettingsProcessor extends Processor { FieldType fieldType = fieldDescription.getType(); if (fieldType instanceof TensorFieldType) { TensorFieldType tensorFieldType = (TensorFieldType)fieldType; - Matcher matcher = queryFeaturePattern.matcher(fieldName); - if (matcher.matches()) { - String queryFeature = matcher.group(1); - addQueryFeatureTypeToRankProfiles(queryFeature, tensorFieldType.type().toString()); - } + FeatureNames.argumentOf(fieldName).ifPresent(argument -> + addQueryFeatureTypeToRankProfiles(argument, tensorFieldType.type().toString())); } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java index 475fee62177..6458f3b9a6d 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/TensorTransformTestCase.java @@ -91,7 +91,7 @@ public class TensorTransformTestCase extends SearchDefinitionTestCase { private void assertContainsExpression(String expr, String transformedExpression) throws ParseException { - assertTrue("Expected expression '" + transformedExpression + "' not found", + assertTrue("Expected expression '" + transformedExpression + "' found", containsExpression(expr, transformedExpression)); } @@ -100,6 +100,7 @@ public class TensorTransformTestCase extends SearchDefinitionTestCase { String rankProperty = rankPropertyExpression.getFirst(); if (rankProperty.equals("rankingExpression(firstphase).rankingScript")) { String rankExpression = censorBindingHash(rankPropertyExpression.getSecond().replace(" ","")); + System.out.println("--> " + rankExpression); return rankExpression.equals(transformedExpression); } } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNames.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNames.java index 80028519f67..8912d6e55b6 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNames.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNames.java @@ -3,11 +3,12 @@ package com.yahoo.searchlib.rankingexpression.evaluation; import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.regex.Pattern; import java.util.stream.Collectors; /** - * Utility methods for working with rank feature names + * Utility methods for query, document and constant rank feature names * * @author bratseth */ @@ -16,30 +17,52 @@ public class FeatureNames { private static final Pattern identifierRegexp = Pattern.compile("[A-Za-z0-9_][A-Za-z0-9_-]*"); /** - * Returns the given feature in canonical form. - * A feature name consists of a feature shortname, followed by zero or more arguments enclosed in quotes - * and an optional output prefixed by a dot: shortname[(argument-ist)][.output] - * Arguments may be identifiers or any strings single or double quoted. + * <p>Returns the given query, document or constant feature in canonical form. + * A feature name consists of a feature type name (query, attribute or constant), + * followed by one argument enclosed in quotes. + * The argument may be an identifier or any string single or double quoted.</p> * - * Argument string values may not contain comma, single quote nor double quote characters. + * <p>Argument string values may not contain comma, single quote nor double quote characters.</p> * - * <i>The canonical form use no quotes for arguments which are identifiers, and double quotes otherwise.</i> + * <p><i>The canonical form use no quotes for arguments which are identifiers, and double quotes otherwise.</i></p> + * + * <p>Note that the above definition is not true for features in general, which accept any ranking expression + * as argument.</p> + * + * @throws IllegalArgumentException if the feature name is not valid */ + // Note that this implementation is more general than what is described above: + // It accepts any number of arguments and an optional output public static String canonicalize(String feature) { + return canonicalizeIfValid(feature).orElseThrow(() -> + new IllegalArgumentException("A feature name must be on the form query(name), attribute(name) or " + + "constant(name), but was '" + feature + "'" + )); + } + + /** + * Canonicalizes the given argument as in canonicalize, but returns empty instead of throwing an exception if + * the argument is not a valid feature + */ + public static Optional<String> canonicalizeIfValid(String feature) { int startParenthesis = feature.indexOf('('); + if (startParenthesis < 0) + return Optional.empty(); int endParenthesis = feature.lastIndexOf(')'); - if (startParenthesis < 1) return feature; // No arguments + String featureType = feature.substring(0, startParenthesis); + if ( ! ( featureType.equals("query") || featureType.equals("attribute") || featureType.equals("constant"))) + return Optional.empty(); + if (startParenthesis < 1) return Optional.of(feature); // No arguments if (endParenthesis < startParenthesis) - throw new IllegalArgumentException("A feature name must be on the form shortname[(argument-ist)][.output], " + - "but was '" + feature + "'"); + return Optional.empty(); String argumentString = feature.substring(startParenthesis + 1, endParenthesis); List<String> canonicalizedArguments = Arrays.stream(argumentString.split(",")) - .map(FeatureNames::canonicalizeArgument) - .collect(Collectors.toList()); - return feature.substring(0, startParenthesis + 1) + - canonicalizedArguments.stream().collect(Collectors.joining(",")) + - feature.substring(endParenthesis); + .map(FeatureNames::canonicalizeArgument) + .collect(Collectors.toList()); + return Optional.of(featureType + "(" + + canonicalizedArguments.stream().collect(Collectors.joining(",")) + + feature.substring(endParenthesis)); } /** Canomicalizes a single argument */ @@ -74,4 +97,20 @@ public class FeatureNames { return canonicalize("query(\"" + propertyName + "\")"); } + /** + * Returns the single argument of the given feature name, without any quotes, + * or empty if it is not a valid query, attribute or constant feature name + */ + public static Optional<String> argumentOf(String feature) { + return canonicalizeIfValid(feature).map(f -> { + int startParenthesis = f.indexOf("("); + int endParenthesis = f.indexOf(")"); + String possiblyQuotedArgument = f.substring(startParenthesis + 1, endParenthesis); + if (possiblyQuotedArgument.startsWith("\"")) + return possiblyQuotedArgument.substring(1, possiblyQuotedArgument.length() - 1); + else + return possiblyQuotedArgument; + }); + } + } diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNamesTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNamesTestCase.java index cf390171fa8..55319e4ec93 100644 --- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNamesTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNamesTestCase.java @@ -32,6 +32,12 @@ public class FeatureNamesTestCase { } @Test + public void testArgument() { + assertEquals("bar", FeatureNames.argumentOf("foo(bar)")); + assertEquals("bar.baz", FeatureNames.argumentOf("foo(bar.baz)")); + } + + @Test public void testConstantFeature() { assertEquals("constant(\"foo/bar\")", FeatureNames.asConstantFeature("foo/bar")); } |