diff options
author | Harald Musum <musum@verizonmedia.com> | 2021-05-27 06:46:20 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-27 06:46:20 +0200 |
commit | 40db354a2d0d8f8344bd2017bfbf861886a1550d (patch) | |
tree | 13a6474a24250ac0aed771045c793e2c0f972ac2 | |
parent | 38485f4c84ee0e9946e32166b3036514b13b2f49 (diff) | |
parent | 012c64ae3f8850e1e69a2b150540d24107ca058a (diff) |
Merge pull request #17999 from vespa-engine/balder/take-deploy-logger-during-construction
Wire the deploylogger in the constructor of SearchBuilder to avoid pa…
9 files changed, 72 insertions, 72 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 a77dd65c608..3fd2d009412 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 @@ -432,7 +432,7 @@ public class DeployState implements ConfigDefinitionStore { RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); QueryProfiles queryProfiles = new QueryProfilesBuilder().build(applicationPackage, logger); SemanticRules semanticRules = new SemanticRuleBuilder().build(applicationPackage); - SearchDocumentModel searchDocumentModel = createSearchDocumentModel(rankProfileRegistry, logger, queryProfiles, validationParameters); + SearchDocumentModel searchDocumentModel = createSearchDocumentModel(rankProfileRegistry, queryProfiles, validationParameters); return new DeployState(applicationPackage, searchDocumentModel, rankProfileRegistry, @@ -458,16 +458,15 @@ public class DeployState implements ConfigDefinitionStore { } private SearchDocumentModel createSearchDocumentModel(RankProfileRegistry rankProfileRegistry, - DeployLogger logger, QueryProfiles queryProfiles, ValidationParameters validationParameters) { Collection<NamedReader> readers = applicationPackage.getSearchDefinitions(); Map<String, String> names = new LinkedHashMap<>(); - SearchBuilder builder = new SearchBuilder(applicationPackage, rankProfileRegistry, queryProfiles.getRegistry()); + SearchBuilder builder = new SearchBuilder(applicationPackage, logger, rankProfileRegistry, queryProfiles.getRegistry()); for (NamedReader reader : readers) { try { String readerName = reader.getName(); - String topLevelName = builder.importReader(reader, readerName, logger); + String topLevelName = builder.importReader(reader, readerName); String sdName = stripSuffix(readerName, ApplicationPackage.SD_NAME_SUFFIX); names.put(topLevelName, sdName); if ( ! sdName.equals(topLevelName)) { @@ -483,7 +482,7 @@ public class DeployState implements ConfigDefinitionStore { closeIgnoreException(reader.getReader()); } } - builder.build(! validationParameters.ignoreValidationErrors(), logger); + builder.build(! validationParameters.ignoreValidationErrors()); return SearchDocumentModel.fromBuilderAndNames(builder, names); } diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java index 2594c64b951..7200c3211ba 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java @@ -6,6 +6,7 @@ import com.yahoo.config.application.api.ComponentInfo; import com.yahoo.config.application.api.UnparsedConfigDefinition; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.component.Version; +import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.InstanceName; @@ -116,6 +117,7 @@ public class MockApplicationPackage implements ApplicationPackage { public List<NamedReader> getSearchDefinitions() { ArrayList<NamedReader> readers = new ArrayList<>(); SearchBuilder searchBuilder = new SearchBuilder(this, + new BaseDeployLogger(), new RankProfileRegistry(), queryProfileRegistry); for (String sd : schemas) { 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 a1ad727a01b..fc110504f4d 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java @@ -41,10 +41,11 @@ import java.util.List; public class SearchBuilder { private final DocumentTypeManager docTypeMgr = new DocumentTypeManager(); - private final ApplicationPackage app; private final DocumentModel model = new DocumentModel(); + private final ApplicationPackage app; private final RankProfileRegistry rankProfileRegistry; private final QueryProfileRegistry queryProfileRegistry; + private final DeployLogger deployLogger; private List<Search> searchList = new LinkedList<>(); private boolean isBuilt = false; @@ -53,41 +54,59 @@ public class SearchBuilder { /** For testing only */ public SearchBuilder() { - this(MockApplicationPackage.createEmpty(), new RankProfileRegistry(), new QueryProfileRegistry()); + this(new RankProfileRegistry(), new QueryProfileRegistry()); + } + + /** For testing only */ + public SearchBuilder(DeployLogger deployLogger) { + this(MockApplicationPackage.createEmpty(), deployLogger); + } + + /** For testing only */ + public SearchBuilder(DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry) { + this(MockApplicationPackage.createEmpty(), deployLogger, rankProfileRegistry); } /** Used for generating documents for typed access to document fields in Java */ public SearchBuilder(boolean documentsOnly) { - this(MockApplicationPackage.createEmpty(), new RankProfileRegistry(), new QueryProfileRegistry(), documentsOnly); + this(MockApplicationPackage.createEmpty(), new BaseDeployLogger(), new RankProfileRegistry(), new QueryProfileRegistry(), documentsOnly); } /** For testing only */ - public SearchBuilder(ApplicationPackage app) { - this(app, new RankProfileRegistry(), new QueryProfileRegistry()); + public SearchBuilder(ApplicationPackage app, DeployLogger deployLogger) { + this(app, deployLogger, new RankProfileRegistry(), new QueryProfileRegistry()); + } + + /** For testing only */ + public SearchBuilder(ApplicationPackage app, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry) { + this(app, deployLogger, rankProfileRegistry, new QueryProfileRegistry()); } /** For testing only */ public SearchBuilder(RankProfileRegistry rankProfileRegistry) { - this(MockApplicationPackage.createEmpty(), rankProfileRegistry, new QueryProfileRegistry()); + this(rankProfileRegistry, new QueryProfileRegistry()); } /** For testing only */ public SearchBuilder(RankProfileRegistry rankProfileRegistry, QueryProfileRegistry queryProfileRegistry) { - this(MockApplicationPackage.createEmpty(), rankProfileRegistry, queryProfileRegistry); + this(MockApplicationPackage.createEmpty(), new BaseDeployLogger(), rankProfileRegistry, queryProfileRegistry); } public SearchBuilder(ApplicationPackage app, + DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfileRegistry queryProfileRegistry) { - this(app, rankProfileRegistry, queryProfileRegistry, false); + this(app, deployLogger, rankProfileRegistry, queryProfileRegistry, false); } - public SearchBuilder(ApplicationPackage app, - RankProfileRegistry rankProfileRegistry, - QueryProfileRegistry queryProfileRegistry, - boolean documentsOnly) { + private SearchBuilder(ApplicationPackage app, + DeployLogger deployLogger, + RankProfileRegistry rankProfileRegistry, + QueryProfileRegistry queryProfileRegistry, + boolean documentsOnly) { this.app = app; this.rankProfileRegistry = rankProfileRegistry; this.queryProfileRegistry = queryProfileRegistry; + this.deployLogger = deployLogger; this.documentsOnly = documentsOnly; } @@ -95,29 +114,17 @@ public class SearchBuilder { * Import search definition. * * @param fileName The name of the file to import. - * @param deployLogger Logger for deploy messages. * @return The name of the imported object. * @throws IOException Thrown if the file can not be read for some reason. * @throws ParseException Thrown if the file does not contain a valid search definition. ``` */ - public String importFile(String fileName, DeployLogger deployLogger) throws IOException, ParseException { + public String importFile(String fileName) throws IOException, ParseException { File file = new File(fileName); - return importString(IOUtils.readFile(file), file.getAbsoluteFile().getParent(), deployLogger); + return importString(IOUtils.readFile(file), file.getAbsoluteFile().getParent()); } - /** - * Import search definition. - * - * @param fileName The name of the file to import. - * @return The name of the imported object. - * @throws IOException Thrown if the file can not be read for some reason. - * @throws ParseException Thrown if the file does not contain a valid search definition. - */ - public String importFile(String fileName) throws IOException, ParseException { - return importFile(fileName, new BaseDeployLogger()); - } - public String importFile(Path file) throws IOException, ParseException { - return importFile(file.toString(), new BaseDeployLogger()); + private String importFile(Path file) throws IOException, ParseException { + return importFile(file.toString()); } /** @@ -129,8 +136,8 @@ public class SearchBuilder { * @return The name of the imported object. * @throws ParseException Thrown if the file does not contain a valid search definition. */ - public String importReader(NamedReader reader, String searchDefDir, DeployLogger deployLogger) throws IOException, ParseException { - return importString(IOUtils.readAll(reader), searchDefDir, deployLogger); + public String importReader(NamedReader reader, String searchDefDir) throws IOException, ParseException { + return importString(IOUtils.readAll(reader), searchDefDir); } /** @@ -141,21 +148,10 @@ public class SearchBuilder { * @throws ParseException thrown if the file does not contain a valid search definition. */ public String importString(String str) throws ParseException { - return importString(str, null, new BaseDeployLogger()); - } - - /** - * Import search definition. - * - * @param str the string to parse. - * @return the name of the imported object. - * @throws ParseException thrown if the file does not contain a valid search definition. - */ - public String importString(String str, DeployLogger logger) throws ParseException { - return importString(str, null, logger); + return importString(str, null); } - private String importString(String str, String searchDefDir, DeployLogger deployLogger) throws ParseException { + private String importString(String str, String searchDefDir) throws ParseException { SimpleCharStream stream = new SimpleCharStream(str); try { return importRawSearch(new SDParser(stream, deployLogger, app, rankProfileRegistry, documentsOnly) @@ -199,7 +195,7 @@ public class SearchBuilder { * @throws IllegalStateException Thrown if this method has already been called. */ public void build() { - build(true, new BaseDeployLogger()); + build(true); } /** @@ -207,9 +203,8 @@ public class SearchBuilder { * #getSearch(String)} method. * * @throws IllegalStateException Thrown if this method has already been called. - * @param deployLogger The logger to use during build */ - public void build(boolean validate, DeployLogger deployLogger) { + public void build(boolean validate) { if (isBuilt) throw new IllegalStateException("Model already built"); List<Search> built = new ArrayList<>(); @@ -316,9 +311,9 @@ public class SearchBuilder { } public static SearchBuilder createFromString(String sd, DeployLogger logger) throws ParseException { - SearchBuilder builder = new SearchBuilder(MockApplicationPackage.createEmpty()); - builder.importString(sd, logger); - builder.build(true, logger); + SearchBuilder builder = new SearchBuilder(logger); + builder.importString(sd); + builder.build(true); return builder; } @@ -377,12 +372,13 @@ public class SearchBuilder { QueryProfileRegistry queryprofileRegistry) throws IOException, ParseException { SearchBuilder builder = new SearchBuilder(MockApplicationPackage.createEmpty(), + deployLogger, rankProfileRegistry, queryprofileRegistry); for (String fileName : fileNames) { builder.importFile(fileName); } - builder.build(true, deployLogger); + builder.build(true); return builder; } @@ -404,15 +400,16 @@ public class SearchBuilder { public static SearchBuilder createFromDirectory(String dir, RankProfileRegistry rankProfileRegistry, QueryProfileRegistry queryProfileRegistry, - DeployLogger logger, + DeployLogger deployLogger, ApplicationPackage applicationPackage) throws IOException, ParseException { SearchBuilder builder = new SearchBuilder(applicationPackage, + deployLogger, rankProfileRegistry, queryProfileRegistry); for (Iterator<Path> i = Files.list(new File(dir).toPath()).filter(p -> p.getFileName().toString().endsWith(".sd")).iterator(); i.hasNext(); ) { builder.importFile(i.next()); } - builder.build(true, logger); + builder.build(true); return builder; } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java index b208a5dffcd..91e8640308a 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankProfileTestCase.java @@ -173,7 +173,7 @@ public class RankProfileTestCase extends SchemaTestCase { " rank-profile p1 {}\n" + " rank-profile p2 {}\n" + "}"); - builder.build(true, new BaseDeployLogger()); + builder.build(true); Search search = builder.getSearch(); assertEquals(4, registry.all().size()); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java index e6616ce0dd1..683242c1333 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankProfileSearchFixture.java @@ -4,6 +4,7 @@ package com.yahoo.searchdefinition.processing; import com.google.common.collect.ImmutableList; import com.yahoo.config.application.api.ApplicationPackage; import ai.vespa.rankingexpression.importer.configmodelview.MlModelImporter; +import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.path.Path; import com.yahoo.search.query.profile.QueryProfileRegistry; @@ -62,7 +63,7 @@ class RankProfileSearchFixture { String rankProfiles, String constant, String field) throws ParseException { this.queryProfileRegistry = queryProfileRegistry; - SearchBuilder builder = new SearchBuilder(applicationpackage, rankProfileRegistry, queryProfileRegistry); + SearchBuilder builder = new SearchBuilder(applicationpackage, new BaseDeployLogger(), rankProfileRegistry, queryProfileRegistry); String sdContent = "search test {\n" + " " + (constant != null ? constant : "") + "\n" + " document test {\n" + diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java index 12fe7e151c0..e3d81be6743 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeResolverTestCase.java @@ -384,7 +384,7 @@ public class RankingExpressionTypeResolverTestCase { @Test public void undeclaredQueryFeaturesAreAccepted() throws Exception { InspectableDeployLogger logger = new InspectableDeployLogger(); - SearchBuilder builder = new SearchBuilder(); + SearchBuilder builder = new SearchBuilder(logger); builder.importString(joinLines( "search test {", " document test { ", @@ -401,8 +401,8 @@ public class RankingExpressionTypeResolverTestCase { " }", " }", "}" - ), logger); - builder.build(true, logger); + )); + builder.build(true); String message = logger.findMessage("The following query features"); assertNull(message); } @@ -410,7 +410,7 @@ public class RankingExpressionTypeResolverTestCase { @Test public void undeclaredQueryFeaturesAreAcceptedWithWarningWhenUsingTensors() throws Exception { InspectableDeployLogger logger = new InspectableDeployLogger(); - SearchBuilder builder = new SearchBuilder(); + SearchBuilder builder = new SearchBuilder(logger); builder.importString(joinLines( "search test {", " document test { ", @@ -427,8 +427,8 @@ public class RankingExpressionTypeResolverTestCase { " }", " }", "}" - ), logger); - builder.build(true, logger); + )); + builder.build(true); String message = logger.findMessage("The following query features"); assertNotNull(message); assertEquals("WARNING: The following query features used in 'my_rank_profile' are not declared in query profile types and " + @@ -439,7 +439,7 @@ public class RankingExpressionTypeResolverTestCase { @Test public void noWarningWhenUsingTensorsWhenQueryFeaturesAreDeclared() throws Exception { InspectableDeployLogger logger = new InspectableDeployLogger(); - SearchBuilder builder = new SearchBuilder(); + SearchBuilder builder = new SearchBuilder(logger); QueryProfileType myType = new QueryProfileType("mytype"); myType.addField(new FieldDescription("rank.feature.query(foo)", new TensorFieldType(TensorType.fromSpec("tensor(d[2])"))), @@ -467,8 +467,8 @@ public class RankingExpressionTypeResolverTestCase { " }", " }", "}" - ), logger); - builder.build(true, logger); + )); + builder.build(true); String message = logger.findMessage("The following query features"); assertNull(message); } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTransformerTokensTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTransformerTokensTestCase.java index c64dbcdef03..2ae6ffbf343 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTransformerTokensTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTransformerTokensTestCase.java @@ -1,6 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchdefinition.processing; +import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.search.query.profile.QueryProfileRegistry; import com.yahoo.searchdefinition.RankProfile; @@ -84,7 +85,7 @@ public class RankingExpressionWithTransformerTokensTestCase { " document test {}\n" + " rank-profile my_profile inherits default {}\n" + "}"; - SearchBuilder searchBuilder = new SearchBuilder(application, rankProfileRegistry, queryProfileRegistry); + SearchBuilder searchBuilder = new SearchBuilder(application, new BaseDeployLogger(), rankProfileRegistry, queryProfileRegistry); searchBuilder.importString(sdContent); searchBuilder.build(); Search search = searchBuilder.getSearch(); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ReservedRankingExpressionFunctionNamesTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ReservedRankingExpressionFunctionNamesTestCase.java index eecab3c03d7..b0c65b3ea76 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ReservedRankingExpressionFunctionNamesTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ReservedRankingExpressionFunctionNamesTestCase.java @@ -21,7 +21,7 @@ public class ReservedRankingExpressionFunctionNamesTestCase { public void requireThatFunctionsWithReservedNamesIssueAWarning() throws ParseException { TestDeployLogger deployLogger = new TestDeployLogger(); RankProfileRegistry rankProfileRegistry = new RankProfileRegistry(); - SearchBuilder builder = new SearchBuilder(rankProfileRegistry); + SearchBuilder builder = new SearchBuilder(deployLogger, rankProfileRegistry); builder.importString( "search test {\n" + " document test { \n" + @@ -50,7 +50,7 @@ public class ReservedRankingExpressionFunctionNamesTestCase { " }\n" + " }\n" + "}\n"); - builder.build(true, deployLogger); + builder.build(true); assertTrue(deployLogger.log.contains("sigmoid") && deployLogger.log.contains("test_rank_profile")); assertTrue(deployLogger.log.contains("sigmoid") && deployLogger.log.contains("test_rank_profile_2")); 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 8308b638497..6ab74ef2eae 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 @@ -195,7 +195,7 @@ public class TensorTransformTestCase extends SchemaTestCase { " }\n" + " }\n" + "}\n"); - builder.build(true, new BaseDeployLogger()); + builder.build(true); Search s = builder.getSearch(); RankProfile test = rankProfileRegistry.get(s, "test").compile(queryProfiles, new ImportedMlModels()); List<Pair<String, String>> testRankProperties = new RawRankProfile(test, |