diff options
4 files changed, 125 insertions, 7 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/MacroShadower.java b/config-model/src/main/java/com/yahoo/searchdefinition/MacroShadower.java index c43e00b7a34..edf0ce69819 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/MacroShadower.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/MacroShadower.java @@ -1,14 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchdefinition; -import com.yahoo.config.application.api.DeployLogger; -import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.searchlib.rankingexpression.RankingExpression; import com.yahoo.searchlib.rankingexpression.rule.*; import com.yahoo.searchlib.rankingexpression.transform.ExpressionTransformer; import java.util.Map; -import java.util.logging.Level; /** * Transforms function nodes to reference nodes if a macro shadows a built-in function. @@ -25,8 +22,6 @@ import java.util.logging.Level; */ class MacroShadower extends ExpressionTransformer { - private static final DeployLogger logger = new BaseDeployLogger(); - private final Map<String, RankProfile.Macro> macros; public MacroShadower(Map<String, RankProfile.Macro> macros) { @@ -60,11 +55,9 @@ class MacroShadower extends ExpressionTransformer { int functionArity = function.getFunction().arity(); int macroArity = macro.getFormalParams() != null ? macro.getFormalParams().size() : 0; if (functionArity != macroArity) { - logger.log(Level.WARNING, "Macro \"" + name + "\" has the same name as a built-in function. Due to different number of arguments, the built-in function will be used."); return transformChildren(function); } - logger.log(Level.WARNING, "Macro \"" + name + "\" shadows the built-in function with the same name."); ReferenceNode node = new ReferenceNode(name, function.children(), null); return transformChildren(node); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java index 536d4b4d67c..26f98026d4f 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java @@ -74,6 +74,7 @@ public class Processing { RankProfileTypeSettingsProcessor::new, ReferenceFieldsProcessor::new, FastAccessValidator::new, + ReservedMacroNames::new, // These two should be last. IndexingValidation::new, diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ReservedMacroNames.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ReservedMacroNames.java new file mode 100644 index 00000000000..19063b8e7f9 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ReservedMacroNames.java @@ -0,0 +1,50 @@ +package com.yahoo.searchdefinition.processing; + +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.searchdefinition.RankProfile; +import com.yahoo.searchdefinition.RankProfileRegistry; +import com.yahoo.searchdefinition.Search; +import com.yahoo.searchlib.rankingexpression.parser.RankingExpressionParserConstants; +import com.yahoo.vespa.model.container.search.QueryProfiles; + +import java.util.HashSet; +import java.util.Set; +import java.util.logging.Level; + +/** + * Issues a warning if some macro has a reserved name. This is not necessarily + * an error, as a macro can shadow a built-in function. + * + * @author lesters + */ +public class ReservedMacroNames extends Processor { + + public ReservedMacroNames(Search search, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) { + super(search, deployLogger, rankProfileRegistry, queryProfiles); + } + + @Override + public void process() { + Set<String> reservedNames = getReservedNames(); + for (RankProfile rp : rankProfileRegistry.allRankProfiles()) { + for (String macroName : rp.getMacros().keySet()) { + if (reservedNames.contains(macroName)) { + deployLogger.log(Level.WARNING, "Macro \"" + macroName + "\" " + + "in rank profile \"" + rp.getName() + "\" " + + "has a reserved name. This might mean that the macro shadows " + + "the built-in function with the same name." + ); + } + } + } + } + + private Set<String> getReservedNames() { + Set<String> names = new HashSet<>(); + for (String token : RankingExpressionParserConstants.tokenImage) { + String tokenWithoutQuotes = token.substring(1, token.length()-1); + names.add(tokenWithoutQuotes); + } + return names; + } +} diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ReservedMacroNamesTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ReservedMacroNamesTestCase.java new file mode 100644 index 00000000000..ba641dd4a3e --- /dev/null +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ReservedMacroNamesTestCase.java @@ -0,0 +1,74 @@ +package com.yahoo.searchdefinition.processing; + +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.searchdefinition.RankProfileRegistry; +import com.yahoo.searchdefinition.Search; +import com.yahoo.searchdefinition.SearchBuilder; +import com.yahoo.searchdefinition.parser.ParseException; +import org.junit.Test; + +import java.util.logging.Level; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author lesters + */ +public class ReservedMacroNamesTestCase { + + @Test + public void requireThatMacrosWithReservedNamesIssueAWarning() throws ParseException { + RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); + SearchBuilder builder = new SearchBuilder(rankProfileRegistry); + builder.importString( + "search test {\n" + + " document test { \n" + + " field a type string { \n" + + " indexing: index \n" + + " }\n" + + " }\n" + + " \n" + + " rank-profile test_rank_profile {\n" + + " macro not_a_reserved_name(x) {\n" + + " expression: x + x\n" + + " }\n" + + " macro sigmoid(x) {\n" + + " expression: x * x\n" + + " }\n" + + " first-phase {\n" + + " expression: sigmoid(2) + not_a_reserved_name(1)\n" + + " }\n" + + " }\n" + + " rank-profile test_rank_profile_2 inherits test_rank_profile {\n" + + " macro sin(x) {\n" + + " expression: x * x\n" + + " }\n" + + " first-phase {\n" + + " expression: sigmoid(2) + sin(1)\n" + + " }\n" + + " }\n" + + "}\n"); + builder.build(); + Search search = builder.getSearch(); + TestDeployLogger deployLogger = new TestDeployLogger(); + ReservedMacroNames processor = new ReservedMacroNames(search, deployLogger, rankProfileRegistry, null); + processor.process(); + + assertTrue(deployLogger.log.contains("sigmoid") && deployLogger.log.contains("test_rank_profile")); + assertTrue(deployLogger.log.contains("sigmoid") && deployLogger.log.contains("test_rank_profile_2")); + assertTrue(deployLogger.log.contains("sin") && deployLogger.log.contains("test_rank_profile_2")); + assertFalse(deployLogger.log.contains("not_a_reserved_name") && deployLogger.log.contains("test_rank_profile")); + assertFalse(deployLogger.log.contains("not_a_reserved_name") && deployLogger.log.contains("test_rank_profile_2")); + + } + + public static class TestDeployLogger implements DeployLogger { + public String log = ""; + @Override + public void log(Level level, String message) { + log += message; + } + } + +} |