diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-06-05 13:10:17 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-05 13:10:17 +0200 |
commit | ec755c18cfe7ef1c2ffbb1f9b78a857746bf9484 (patch) | |
tree | c7527668bbc143f14db44cac0b91c6af7e810212 | |
parent | 25e0c392a20dbe8f2cbfdc04870a97f567d0720a (diff) | |
parent | 97f959c44ce20db622e6267205051be365d25168 (diff) |
Merge pull request #18142 from vespa-engine/revert-18138-revert-18129-hmusum/cleanup-12
Reapply "Searchdefinition/schemas cleanup"
15 files changed, 49 insertions, 51 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index db875f669fe..81fbc764bb6 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -301,7 +301,7 @@ public class FilesApplicationPackage implements ApplicationPackage { } @Override - public Collection<NamedReader> searchDefinitionContents() { + public Collection<NamedReader> getSchemas() { Set<NamedReader> ret = new LinkedHashSet<>(); try { for (File f : getSearchDefinitionFiles()) { @@ -575,11 +575,6 @@ public class FilesApplicationPackage implements ApplicationPackage { IOUtils.writeFile(metaFile, metaData.asJsonBytes()); } - @Override - public Collection<NamedReader> getSearchDefinitions() { - return searchDefinitionContents(); - } - private void preprocessXML(File destination, File inputXml, Zone zone) throws IOException { if ( ! inputXml.exists()) return; try { diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index bdf2b53bc92..58d2693aace 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -103,7 +103,7 @@ "public abstract java.io.Reader getHosts()", "public java.util.List getUserIncludeDirs()", "public void validateIncludeDir(java.lang.String)", - "public abstract java.util.Collection searchDefinitionContents()", + "public java.util.Collection searchDefinitionContents()", "public abstract java.util.Map getAllExistingConfigDefs()", "public abstract java.util.List getFiles(com.yahoo.path.Path, java.lang.String, boolean)", "public java.util.List getFiles(com.yahoo.path.Path, java.lang.String)", @@ -127,7 +127,8 @@ "public void writeMetaData()", "public java.util.Optional getAllocatedHosts()", "public java.util.Map getFileRegistries()", - "public abstract java.util.Collection getSearchDefinitions()", + "public java.util.Collection getSearchDefinitions()", + "public abstract java.util.Collection getSchemas()", "public com.yahoo.config.application.api.ApplicationPackage preprocess(com.yahoo.config.provision.Zone, com.yahoo.config.application.api.DeployLogger)" ], "fields": [ diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java index d97ff5ca774..bc7dbbe2069 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ApplicationPackage.java @@ -79,7 +79,7 @@ public interface ApplicationPackage { * @return the name of the application (i.e the directory where the application package was deployed from) * @deprecated do not use */ - @Deprecated // TODO: Remove on Vespa 8 + @Deprecated // TODO: Remove in Vespa 8 String getApplicationName(); ApplicationId getApplicationId(); @@ -113,9 +113,12 @@ public interface ApplicationPackage { /** * Readers for all the search definition files for this. + * @deprecated use {@link #getSchemas()} instead * @return a list of readers for search definitions */ - Collection<NamedReader> searchDefinitionContents(); + @Deprecated + // TODO: Remove in Vespa 8 + default Collection<NamedReader> searchDefinitionContents() { return getSchemas(); } /** * Returns all the config definitions available in this package as unparsed data. @@ -235,7 +238,18 @@ public interface ApplicationPackage { return Collections.emptyMap(); } - Collection<NamedReader> getSearchDefinitions(); + /** + * @deprecated use {@link #getSchemas()} instead + */ + @Deprecated + // TODO: Remove in Vespa 8 + default Collection<NamedReader> getSearchDefinitions() { return getSchemas(); } + + /** + * Readers for all the schema files. + * @return a collection of readers for schemas + */ + Collection<NamedReader> getSchemas(); /** * Preprocess an application for a given zone and return a new application package pointing to the preprocessed 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 1d0541b67d1..68924dde3e1 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 @@ -22,7 +22,7 @@ import com.yahoo.config.model.api.ValidationParameters; import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.model.provision.HostsXmlProvisioner; -import com.yahoo.config.model.provision.SingleNodeProvisioner; +import com.yahoo.config.model.provision .SingleNodeProvisioner; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.DockerImage; import com.yahoo.config.provision.Zone; @@ -460,7 +460,7 @@ public class DeployState implements ConfigDefinitionStore { private SearchDocumentModel createSearchDocumentModel(RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles, ValidationParameters validationParameters) { - Collection<NamedReader> readers = applicationPackage.getSearchDefinitions(); + Collection<NamedReader> readers = applicationPackage.getSchemas(); Map<String, String> names = new LinkedHashMap<>(); SearchBuilder builder = new SearchBuilder(applicationPackage, logger, properties, rankProfileRegistry, queryProfiles.getRegistry()); for (NamedReader reader : readers) { @@ -470,14 +470,14 @@ public class DeployState implements ConfigDefinitionStore { String sdName = stripSuffix(readerName, ApplicationPackage.SD_NAME_SUFFIX); names.put(topLevelName, sdName); if ( ! sdName.equals(topLevelName)) { - throw new IllegalArgumentException("Schema definition file name ('" + sdName + "') and name of " + + throw new IllegalArgumentException("Schema file name ('" + sdName + "') and name of " + "top level element ('" + topLevelName + "') are not equal for file '" + readerName + "'"); } } catch (ParseException e) { - throw new IllegalArgumentException("Could not parse sd file '" + reader.getName() + "'", e); + throw new IllegalArgumentException("Could not parse schema file '" + reader.getName() + "'", e); } catch (IOException e) { - throw new IllegalArgumentException("Could not read sd file '" + reader.getName() + "'", e); + throw new IllegalArgumentException("Could not read schema file '" + reader.getName() + "'", e); } finally { closeIgnoreException(reader.getReader()); } 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 e9fe0824f30..411a37bb70a 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 @@ -115,7 +115,7 @@ public class MockApplicationPackage implements ApplicationPackage { } @Override - public List<NamedReader> getSearchDefinitions() { + public List<NamedReader> getSchemas() { ArrayList<NamedReader> readers = new ArrayList<>(); SearchBuilder searchBuilder = new SearchBuilder(this, new BaseDeployLogger(), @@ -134,11 +134,6 @@ public class MockApplicationPackage implements ApplicationPackage { } @Override - public List<NamedReader> searchDefinitionContents() { - return new ArrayList<>(); - } - - @Override public Map<ConfigDefinitionKey, UnparsedConfigDefinition> getAllExistingConfigDefs() { return Collections.emptyMap(); } diff --git a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java index e7622816603..8f751631fb5 100644 --- a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java @@ -54,7 +54,7 @@ public class ApplicationDeployTest { @Test public void testVespaModel() throws SAXException, IOException { ApplicationPackageTester tester = ApplicationPackageTester.create(TESTDIR + "app1"); - VespaModel model = new VespaModel(tester.app()); + new VespaModel(tester.app()); List<NamedSchema> schemas = tester.getSchemas(); assertEquals(schemas.size(), 5); for (NamedSchema searchDefinition : schemas) { diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index b1b386924d1..b0ddadf11bd 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -49,6 +49,7 @@ import static com.yahoo.config.model.test.TestUtil.joinLines; import static com.yahoo.vespa.defaults.Defaults.getDefaults; import static com.yahoo.vespa.model.search.NodeResourcesTuning.GB; import static com.yahoo.vespa.model.search.NodeResourcesTuning.reservedMemoryGb; +import static com.yahoo.vespa.model.test.utils.ApplicationPackageUtils.generateSchemas; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -2015,7 +2016,7 @@ public class ModelProvisioningTest { } private VespaModel createNonProvisionedModel(boolean multitenant, String hosts, String services) { - VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(hosts, services, ApplicationPackageUtils.generateSearchDefinition("type1")); + VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(hosts, services, generateSchemas("type1")); ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg; DeployState deployState = new DeployState.Builder().applicationPackage(appPkg). properties((new TestProperties()).setMultitenant(multitenant)). @@ -2023,7 +2024,7 @@ public class ModelProvisioningTest { return modelCreatorWithMockPkg.create(false, deployState); } - private int physicalMemoryPercentage(ContainerCluster cluster) { + private int physicalMemoryPercentage(ContainerCluster<?> cluster) { QrStartConfig.Builder b = new QrStartConfig.Builder(); cluster.getConfig(b); return b.build().jvm().heapSizeAsPercentageOfPhysicalMemory(); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java index 76f34cf4a81..341a90c6618 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java @@ -15,6 +15,7 @@ import org.junit.rules.ExpectedException; import org.xml.sax.SAXException; import java.io.IOException; +import java.util.List; import static com.yahoo.config.model.test.TestUtil.joinLines; @@ -99,17 +100,17 @@ public class ComplexAttributeFieldsValidatorTestCase { "}")); } - private static void createModelAndValidate(String searchDefinition) throws IOException, SAXException { - DeployState deployState = createDeployState(servicesXml(), searchDefinition); + private static void createModelAndValidate(String schema) throws IOException, SAXException { + DeployState deployState = createDeployState(servicesXml(), schema); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); ValidationParameters validationParameters = new ValidationParameters(CheckRouting.FALSE); Validation.validate(model, validationParameters, deployState); } - private static DeployState createDeployState(String servicesXml, String searchDefinition) { + private static DeployState createDeployState(String servicesXml, String schema) { ApplicationPackage app = new MockApplicationPackage.Builder() .withServices(servicesXml) - .withSearchDefinition(searchDefinition) + .withSchemas(List.of(schema)) .build(); return new DeployState.Builder().applicationPackage(app).build(); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java index 7c93b4ef02b..afeffbbc875 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java @@ -15,9 +15,7 @@ import java.util.List; import static com.yahoo.config.model.test.TestUtil.joinLines; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; /** * @author Simon Thoresen Hult @@ -179,7 +177,7 @@ public class ClusterTest { " </tuning>", " </content>", "</services>")) - .withSchemas(ApplicationPackageUtils.generateSearchDefinition("my_document")) + .withSchemas(ApplicationPackageUtils.generateSchemas("my_document")) .build(); List<Content> contents = new TestDriver().buildModel(app).getConfigModels(Content.class); assertEquals(1, contents.size()); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java index 8673d862ac8..2b36bfc47b2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java @@ -192,7 +192,7 @@ public class VespaModelTestCase { " </documents>" + "</content>" + "</services>", - ApplicationPackageUtils.generateSearchDefinition("music")) + ApplicationPackageUtils.generateSchemas("music")) .create(); MessagebusConfig.Builder mBusB = new MessagebusConfig.Builder(); model.getConfig(mBusB, "client"); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java index ba975e52d1a..7e34e9efbbf 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java @@ -21,7 +21,6 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.test.utils.ApplicationPackageUtils; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import java.util.ArrayList; @@ -31,6 +30,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import static com.yahoo.vespa.model.test.utils.ApplicationPackageUtils.generateSchemas; + /** * Helper class which sets up a system with multiple hosts. * Usage: @@ -168,7 +169,7 @@ public class VespaModelTester { boolean alwaysReturnOneNode, int startIndexForClusters, Optional<VespaModel> previousModel, DeployState.Builder deployStatebuilder, String ... retiredHostNames) { - VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, ApplicationPackageUtils.generateSearchDefinition("type1")); + VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, generateSchemas("type1")); ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg; provisioner = hosted ? new ProvisionerAdapter(new InMemoryProvisioner(hostsByResources, diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.java index df62a3bff07..1f7deaf1991 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/ApplicationPackageUtils.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.test.utils; import java.util.ArrayList; @@ -43,10 +43,6 @@ public class ApplicationPackageUtils { "}"; } - public static List<String> generateSearchDefinition(String name) { - return generateSchemas(name); - } - public static List<String> generateSchemas(String ... sdNames) { return generateSchemas(Arrays.asList(sdNames)); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java index ca7489012e2..42f4dbd5762 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClient.java @@ -80,7 +80,7 @@ public class ZooKeeperClient { try { writeUserDefs(app); writeSomeOf(app); - writeSearchDefinitions(app); + writeSchemas(app); writeUserIncludeDirs(app, app.getUserIncludeDirs()); write(app.getMetaData()); } catch (Exception e) { @@ -90,8 +90,8 @@ public class ZooKeeperClient { } } - private void writeSearchDefinitions(ApplicationPackage app) throws IOException { - Collection<NamedReader> sds = app.getSearchDefinitions(); + private void writeSchemas(ApplicationPackage app) throws IOException { + Collection<NamedReader> sds = app.getSchemas(); if (sds.isEmpty()) return; Path zkPath = getZooKeeperAppPath(USERAPP_ZK_SUBPATH).append(SCHEMAS_DIR); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java index 48c47587c8b..9f1bfa0b4e4 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java @@ -132,13 +132,13 @@ public class ZKApplicationPackage implements ApplicationPackage { } @Override - public List<NamedReader> searchDefinitionContents() { + public List<NamedReader> getSchemas() { List<NamedReader> schemas = new ArrayList<>(); for (String sd : zkApplication.getChildren(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SCHEMAS_DIR)) { if (sd.endsWith(SD_NAME_SUFFIX)) schemas.add(new NamedReader(sd, new StringReader(zkApplication.getData(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SCHEMAS_DIR, sd)))); } - // TODO: Remove when everything is written to SCHEMAS_DIR (July 2021) + // TODO: Remove when 7.414.19 is oldest version in use for (String sd : zkApplication.getChildren(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SEARCH_DEFINITIONS_DIR)) { if (sd.endsWith(SD_NAME_SUFFIX)) schemas.add(new NamedReader(sd, new StringReader(zkApplication.getData(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SEARCH_DEFINITIONS_DIR, sd)))); @@ -165,11 +165,6 @@ public class ZKApplicationPackage implements ApplicationPackage { return fileRegistry; } - @Override - public List<NamedReader> getSearchDefinitions() { - return searchDefinitionContents(); - } - private Reader retrieveConfigDefReader(String def) { try { return zkApplication.getDataReader(ConfigCurator.DEFCONFIGS_ZK_SUBPATH, def); @@ -263,6 +258,7 @@ public class ZKApplicationPackage implements ApplicationPackage { @Override public Reader getRankingExpression(String name) { Optional<Reader> reader = zkApplication.getOptionalDataReader(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SCHEMAS_DIR, name); + // TODO: Remove when 7.414.19 is oldest version in use return reader.orElseGet(() -> zkApplication.getDataReader(ConfigCurator.USERAPP_ZK_SUBPATH + "/" + SEARCH_DEFINITIONS_DIR, name)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java index 80d01fa4d36..a71f75f5035 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java @@ -78,7 +78,7 @@ public class ZKApplicationPackageTest { assertTrue(Pattern.compile(".*<slobroks>.*",Pattern.MULTILINE+Pattern.DOTALL).matcher(IOUtils.readAll(zkApp.getFile(Path.fromString("services.xml")).createReader())).matches()); DeployState deployState = new DeployState.Builder().applicationPackage(zkApp).build(); assertEquals(deployState.getSchemas().size(), 5); - assertEquals(zkApp.searchDefinitionContents().size(), 5); + assertEquals(zkApp.getSchemas().size(), 5); assertEquals(IOUtils.readAll(zkApp.getRankingExpression("foo.expression")), "foo()+1\n"); assertEquals(zkApp.getFiles(Path.fromString(""), "xml").size(), 3); assertEquals(zkApp.getFileReference(Path.fromString("components/file.txt")).getAbsolutePath(), "/home/vespa/test/file.txt"); |