diff options
author | Jon Bratseth <bratseth@verizonmedia.com> | 2019-06-28 08:56:48 -0500 |
---|---|---|
committer | Jon Bratseth <bratseth@verizonmedia.com> | 2019-06-28 08:56:48 -0500 |
commit | 781d6dfd8afdcb1da0ce29ef37ccbc0c45dc155d (patch) | |
tree | 18846b64a9ea079196591f93bd2856a8b5459b4d | |
parent | 38be09692fbc8e286ed1b669771e1c2d66e1db6c (diff) | |
parent | 4f9ad5d65f0e7fac3796830b90c2f8b9128523ce (diff) |
Merge with master
83 files changed, 1468 insertions, 1193 deletions
diff --git a/build_settings.cmake b/build_settings.cmake index 59c5a66fcd3..7d8ba11f8a1 100644 --- a/build_settings.cmake +++ b/build_settings.cmake @@ -117,6 +117,6 @@ else() set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--no-undefined") set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-undefined") -# Enable cppunit tests in shared libraries +# Enable GTest unit tests in shared libraries set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-as-needed") endif() diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java index f69330eb196..3e9790a81a2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java @@ -20,6 +20,7 @@ import com.yahoo.config.model.api.ValidationParameters; import com.yahoo.config.model.application.provider.ApplicationPackageXmlFilesValidator; import com.yahoo.config.model.builder.xml.ConfigModelBuilder; import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.config.provision.TransientException; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.config.VespaVersion; import com.yahoo.vespa.model.application.validation.Validation; @@ -166,7 +167,7 @@ public class VespaModelFactory implements ModelFactory { private List<ConfigChangeAction> validateModel(VespaModel model, DeployState deployState, ValidationParameters validationParameters) { try { return Validation.validate(model, validationParameters, deployState); - } catch (IllegalArgumentException e) { + } catch (IllegalArgumentException | TransientException e) { rethrowUnlessIgnoreErrors(e, validationParameters.ignoreValidationErrors()); } catch (Exception e) { throw new RuntimeException(e); @@ -174,7 +175,7 @@ public class VespaModelFactory implements ModelFactory { return new ArrayList<>(); } - private static void rethrowUnlessIgnoreErrors(IllegalArgumentException e, boolean ignoreValidationErrors) { + private static void rethrowUnlessIgnoreErrors(RuntimeException e, boolean ignoreValidationErrors) { if (!ignoreValidationErrors) { throw e; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultPublicMetrics.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultPublicMetrics.java index dc250179460..cec7f796302 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultPublicMetrics.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/DefaultPublicMetrics.java @@ -29,6 +29,8 @@ public class DefaultPublicMetrics { private static Set<Metric> getAllMetrics() { return ImmutableSet.<Metric>builder() + .addAll(getContentMetrics()) + .addAll(getStorageMetrics()) .addAll(getContainerMetrics()) .addAll(getQrserverMetrics()) .build(); @@ -64,6 +66,55 @@ public class DefaultPublicMetrics { return metrics; } + private static Set<Metric> getContentMetrics() { + Set<Metric> metrics = new LinkedHashSet<>(); + + metrics.add(new Metric("content.proton.docsum.docs.rate")); + metrics.add(new Metric("content.proton.docsum.latency.average")); + + metrics.add(new Metric("content.proton.transport.query.count.rate")); + metrics.add(new Metric("content.proton.transport.query.latency.average")); + + metrics.add(new Metric("content.proton.documentdb.documents.total.last")); + metrics.add(new Metric("content.proton.documentdb.documents.ready.last")); + metrics.add(new Metric("content.proton.documentdb.documents.active.last")); + metrics.add(new Metric("content.proton.documentdb.index.docs_in_memory.last")); + metrics.add(new Metric("content.proton.documentdb.disk_usage.last")); + + metrics.add(new Metric("content.proton.documentdb.job.total.average")); + metrics.add(new Metric("content.proton.documentdb.job.attribute_flush.average")); + metrics.add(new Metric("content.proton.documentdb.job.disk_index_fusion.average")); + metrics.add(new Metric("content.proton.documentdb.job.document_store_compact.average")); + metrics.add(new Metric("content.proton.documentdb.job.memory_index_flush.average")); + + metrics.add(new Metric("content.proton.resource_usage.disk.average")); + metrics.add(new Metric("content.proton.resource_usage.disk_utilization.average")); + metrics.add(new Metric("content.proton.resource_usage.memory.average")); + + metrics.add(new Metric("content.proton.documentdb.ready.document_store.memory_usage.allocated_bytes.average")); + metrics.add(new Metric("content.proton.documentdb.ready.document_store.cache.hit_rate.average")); + metrics.add(new Metric("content.proton.documentdb.index.memory_usage.allocated_bytes.average")); + + metrics.add(new Metric("content.proton.documentdb.matching.docs_matched.rate")); + metrics.add(new Metric("content.proton.documentdb.matching.docs_reranked.rate")); + metrics.add(new Metric("content.proton.documentdb.matching.rank_profile.query_latency.average")); + metrics.add(new Metric("content.proton.documentdb.matching.rank_profile.rerank_time.average")); + + return metrics; + } + + private static Set<Metric> getStorageMetrics() { + Set<Metric> metrics = new LinkedHashSet<>(); + + metrics.add(new Metric("vds.filestor.alldisks.allthreads.put.sum.count.rate")); + metrics.add(new Metric("vds.filestor.alldisks.allthreads.remove.sum.count.rate")); + metrics.add(new Metric("vds.filestor.alldisks.allthreads.get.sum.count.rate")); + metrics.add(new Metric("vds.filestor.alldisks.allthreads.update.sum.count.rate")); + metrics.add(new Metric("vds.filestor.alldisks.allthreads.visit.sum.count.rate")); + + return metrics; + } + private DefaultPublicMetrics() { } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/TlsSecretsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/TlsSecretsValidator.java new file mode 100644 index 00000000000..1018099cf05 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/TlsSecretsValidator.java @@ -0,0 +1,17 @@ +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.config.model.api.TlsSecrets; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.config.provision.CertificateNotReadyException; +import com.yahoo.vespa.model.VespaModel; + +public class TlsSecretsValidator extends Validator { + + /** This check is delayed until validation to allow node provisioning to complete while we are waiting for cert */ + @Override + public void validate(VespaModel model, DeployState deployState) { + if (deployState.tlsSecrets().isPresent() && deployState.tlsSecrets().get() == TlsSecrets.MISSING) { + throw new CertificateNotReadyException("TLS enabled, but could not retrieve certificate yet"); + } + } +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index e44acf61466..042c7cc867c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -56,6 +56,7 @@ public class Validation { new DeploymentFileValidator().validate(model, deployState); new RankingConstantsValidator().validate(model, deployState); new SecretStoreValidator().validate(model, deployState); + new TlsSecretsValidator().validate(model, deployState); List<ConfigChangeAction> result = Collections.emptyList(); if (deployState.getProperties().isFirstTimeDeployment()) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java index 78c1c5931a2..48c3e9fdda9 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java @@ -134,7 +134,7 @@ public class MetricsProxyContainerClusterTest { } @Test - public void public_is_a_reserved_consumer_id() { + public void default_is_a_reserved_consumer_id() { assertReservedConsumerId("default"); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/TlsSecretsValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/TlsSecretsValidatorTest.java new file mode 100644 index 00000000000..cdb4ce955e2 --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/TlsSecretsValidatorTest.java @@ -0,0 +1,88 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.model.NullConfigModelRegistry; +import com.yahoo.config.model.api.TlsSecrets; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.config.model.deploy.TestProperties; +import com.yahoo.config.model.test.MockApplicationPackage; +import com.yahoo.config.provision.CertificateNotReadyException; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.Zone; +import com.yahoo.vespa.model.VespaModel; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import java.util.Optional; + +import static com.yahoo.config.model.test.TestUtil.joinLines; +import static org.junit.Assert.assertTrue; + +/** + * @author andreer + */ +public class TlsSecretsValidatorTest { + @Rule + public final ExpectedException exceptionRule = ExpectedException.none(); + + private static String servicesXml() { + return joinLines("<services version='1.0'>", + " <container id='default' version='1.0'>", + " </container>", + "</services>"); + } + + private static String deploymentXml() { + return joinLines("<deployment version='1.0' >", + " <prod />", + "</deployment>"); + } + + @Test + public void missing_certificate_fails_validation() throws Exception { + DeployState deployState = deployState(servicesXml(), deploymentXml(), Optional.of(TlsSecrets.MISSING)); + VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); + + exceptionRule.expect(CertificateNotReadyException.class); + exceptionRule.expectMessage("TLS enabled, but could not retrieve certificate yet"); + + new TlsSecretsValidator().validate(model, deployState); + } + + @Test + public void validation_succeeds_with_certificate() throws Exception { + DeployState deployState = deployState(servicesXml(), deploymentXml(), Optional.of(new TlsSecrets("cert", "key"))); + VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); + + new TlsSecretsValidator().validate(model, deployState); + } + + @Test + public void validation_succeeds_without_certificate() throws Exception { + DeployState deployState = deployState(servicesXml(), deploymentXml(), Optional.empty()); + VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); + + new TlsSecretsValidator().validate(model, deployState); + } + + private static DeployState deployState(String servicesXml, String deploymentXml, Optional<TlsSecrets> tlsSecrets) { + ApplicationPackage app = new MockApplicationPackage.Builder() + .withServices(servicesXml) + .withDeploymentSpec(deploymentXml) + .build(); + DeployState.Builder builder = new DeployState.Builder() + .applicationPackage(app) + .zone(new Zone(Environment.prod, RegionName.from("foo"))) + .properties( + new TestProperties() + .setHostedVespa(true) + .setTlsSecrets(tlsSecrets)); + final DeployState deployState = builder.build(); + + assertTrue("Test must emulate a hosted deployment.", deployState.isHosted()); + return deployState; + } +} diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index ffeb5dce112..a9b45fe6bb4 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -149,6 +149,17 @@ ], "fields": [] }, + "com.yahoo.config.provision.CertificateNotReadyException": { + "superClass": "com.yahoo.config.provision.TransientException", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(java.lang.String)" + ], + "fields": [] + }, "com.yahoo.config.provision.CloudName": { "superClass": "java.lang.Object", "interfaces": [ diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/CertificateNotReadyException.java b/config-provisioning/src/main/java/com/yahoo/config/provision/CertificateNotReadyException.java new file mode 100644 index 00000000000..0d88a7aa435 --- /dev/null +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/CertificateNotReadyException.java @@ -0,0 +1,17 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.config.provision; + +/** + * Exception thrown when trying to validate an application which is configured + * with a certificate that is not yet retrievable + * + * @author andreer + * + */ +public class CertificateNotReadyException extends TransientException { + + public CertificateNotReadyException(String message) { + super(message); + } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java index a29891ae764..3d2ecd4a2ca 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java @@ -48,7 +48,8 @@ public class HttpErrorResponse extends HttpResponse { OUT_OF_CAPACITY, REQUEST_TIMEOUT, UNKNOWN_VESPA_VERSION, - PARENT_HOST_NOT_READY + PARENT_HOST_NOT_READY, + CERTIFICATE_NOT_READY } public static HttpErrorResponse notFoundError(String msg) { @@ -95,6 +96,10 @@ public class HttpErrorResponse extends HttpResponse { return new HttpErrorResponse(CONFLICT, errorCodes.PARENT_HOST_NOT_READY.name(), msg); } + public static HttpErrorResponse certificateNotReady(String msg) { + return new HttpErrorResponse(CONFLICT, errorCodes.CERTIFICATE_NOT_READY.name(), msg); + } + @Override public void render(OutputStream stream) throws IOException { new JsonFormat(true).encode(stream, slime); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java index cd2052653ed..20ee77be9fe 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.config.provision.ApplicationLockException; +import com.yahoo.config.provision.CertificateNotReadyException; import com.yahoo.config.provision.ParentHostUnavailableException; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; @@ -64,6 +65,8 @@ public class HttpHandler extends LoggingRequestHandler { return HttpErrorResponse.applicationLockFailure(getMessage(e, request)); } catch (ParentHostUnavailableException e) { return HttpErrorResponse.parentHostNotReady(getMessage(e, request)); + } catch (CertificateNotReadyException e) { + return HttpErrorResponse.certificateNotReady(getMessage(e, request)); } catch (Exception e) { log.log(LogLevel.WARNING, "Unexpected exception handling a config server request", e); return HttpErrorResponse.internalServerError(getMessage(e, request)); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java index 07c06f22497..34dcefe05bd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/modelfactory/ModelsBuilder.java @@ -12,6 +12,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.component.Version; +import com.yahoo.config.provision.TransientException; import com.yahoo.config.provision.Zone; import com.yahoo.lang.SettableOptional; import com.yahoo.log.LogLevel; @@ -111,7 +112,7 @@ public abstract class ModelsBuilder<MODELRESULT extends ModelResult> { break; buildLatestModelForThisMajor = false; // We have successfully built latest model version, do it only for this major } - catch (OutOfCapacityException | ApplicationLockException e) { + catch (OutOfCapacityException | ApplicationLockException | TransientException e) { // Don't wrap this exception, and don't try to load other model versions as this is (most likely) // caused by the state of the system, not the model version/application combination throw e; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java index 88baf1b8d74..651dde375ee 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionPreparerTest.java @@ -9,6 +9,7 @@ import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.application.provider.FilesApplicationPackage; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.CertificateNotReadyException; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.Rotation; import com.yahoo.config.provision.TenantName; @@ -276,31 +277,21 @@ public class SessionPreparerTest { assertEquals("CERT", tlsSecrets.get().certificate()); } - @Test + @Test(expected = CertificateNotReadyException.class) public void require_that_tlssecretkey_is_missing_when_not_in_secretstore() throws IOException { var tlskey = "vespa.tlskeys.tenant1--app1"; var applicationId = applicationId("test"); var params = new PrepareParams.Builder().applicationId(applicationId).tlsSecretsKeyName(tlskey).build(); prepare(new File("src/test/resources/deploy/hosted-app"), params); - - // Read from zk and verify key/cert is missing - Optional<TlsSecrets> tlsSecrets = new TlsSecretsKeys(curator, tenantPath, secretStore).readTlsSecretsKeyFromZookeeper(applicationId); - assertTrue(tlsSecrets.isPresent()); - assertTrue(tlsSecrets.get().isMissing()); } - @Test + @Test(expected = CertificateNotReadyException.class) public void require_that_tlssecretkey_is_missing_when_certificate_not_in_secretstore() throws IOException { var tlskey = "vespa.tlskeys.tenant1--app1"; var applicationId = applicationId("test"); var params = new PrepareParams.Builder().applicationId(applicationId).tlsSecretsKeyName(tlskey).build(); secretStore.put(tlskey+"-key", "KEY"); prepare(new File("src/test/resources/deploy/hosted-app"), params); - - // Read from zk and verify key/cert is missing - Optional<TlsSecrets> tlsSecrets = new TlsSecretsKeys(curator, tenantPath, secretStore).readTlsSecretsKeyFromZookeeper(applicationId); - assertTrue(tlsSecrets.isPresent()); - assertTrue(tlsSecrets.get().isMissing()); } private void prepare(File app) throws IOException { diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 06713d14d88..cc06710480d 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -436,6 +436,7 @@ "public void lock()", "public boolean isLocked()", "public int getTermCount()", + "public java.util.Optional extractSingleChild()", "public bridge synthetic com.yahoo.prelude.query.Item clone()", "public bridge synthetic java.lang.Object clone()" ], @@ -854,7 +855,8 @@ "abstract" ], "methods": [ - "public void <init>()" + "public void <init>()", + "public java.util.Optional extractSingleChild()" ], "fields": [] }, @@ -944,6 +946,7 @@ "public void addItem(com.yahoo.prelude.query.Item)", "public void addItem(int, com.yahoo.prelude.query.Item)", "public com.yahoo.prelude.query.Item setItem(int, com.yahoo.prelude.query.Item)", + "public java.util.Optional extractSingleChild()", "public com.yahoo.prelude.query.WordItem getWordItem(int)", "public com.yahoo.prelude.query.BlockItem getBlockItem(int)", "protected void encodeThis(java.nio.ByteBuffer)", @@ -974,6 +977,7 @@ "public void setIndexName(java.lang.String)", "public void setWeight(int)", "public void addItem(com.yahoo.prelude.query.Item)", + "public java.util.Optional extractSingleChild()", "public com.yahoo.prelude.query.WordItem getWordItem(int)", "protected void encodeThis(java.nio.ByteBuffer)", "public int encode(java.nio.ByteBuffer)", @@ -1239,6 +1243,7 @@ "protected void appendHeadingString(java.lang.StringBuilder)", "protected void appendBodyString(java.lang.StringBuilder)", "protected void adding(com.yahoo.prelude.query.Item)", + "public java.util.Optional extractSingleChild()", "public com.yahoo.prelude.query.Item$ItemType getItemType()", "public java.lang.String getName()", "public java.lang.String getFieldName()" @@ -7146,12 +7151,14 @@ "methods": [ "public void <init>(com.yahoo.data.access.Inspector)", "public com.yahoo.data.access.Inspector inspect()", - "public java.lang.String toString()", "public java.lang.String toJson()", "public java.lang.StringBuilder writeJson(java.lang.StringBuilder)", "public java.lang.Double getDouble(java.lang.String)", "public com.yahoo.tensor.Tensor getTensor(java.lang.String)", - "public java.util.Set featureNames()" + "public java.util.Set featureNames()", + "public java.lang.String toString()", + "public int hashCode()", + "public boolean equals(java.lang.Object)" ], "fields": [] }, diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java index 3f45e8e8f00..5130cf7ff34 100644 --- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java @@ -145,7 +145,6 @@ public class ClusterSearcher extends Searcher { documentDbConfig); addBackendSearcher(searcher); } else { - System.out.println("Dispatchers: " + searchClusterConfig.dispatcher().size()); for (int dispatcherIndex = 0; dispatcherIndex < searchClusterConfig.dispatcher().size(); dispatcherIndex++) { try { if ( ! isRemote(searchClusterConfig.dispatcher(dispatcherIndex).host())) { 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 907eabe60ce..64f759dcf9c 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 @@ -13,6 +13,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.ListIterator; +import java.util.Optional; /** @@ -379,4 +380,13 @@ public abstract class CompositeItem extends Item { return terms; } + /** + * Will return its single child if itself can safely be omitted. + * + * @return a valid Item or empty Optional if it can not be done + */ + public Optional<Item> extractSingleChild() { + return getItemCount() == 1 ? Optional.of(getItem(0)) : Optional.empty(); + } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.java b/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.java index 84aa177369a..97d724953ea 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/NonReducibleCompositeItem.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.prelude.query; +import java.util.Optional; + /** * A composite item which specifies semantics which are not maintained * if an instance with a single child is replaced by the single child. @@ -12,4 +14,8 @@ package com.yahoo.prelude.query; * @author bratseth */ public abstract class NonReducibleCompositeItem extends CompositeItem { + @Override + public Optional<Item> extractSingleChild() { + return Optional.empty(); + } } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java b/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java index 26da5eec7eb..4de0af1f408 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/PhraseItem.java @@ -5,6 +5,7 @@ import com.yahoo.prelude.query.textualrepresentation.Discloser; import java.nio.ByteBuffer; import java.util.Iterator; +import java.util.Optional; /** * A term which contains a phrase - a collection of word terms @@ -127,6 +128,13 @@ public class PhraseItem extends CompositeIndexedItem { } } + @Override + public Optional<Item> extractSingleChild() { + Optional<Item> extracted = super.extractSingleChild(); + extracted.ifPresent(e -> e.setWeight(this.getWeight())); + return extracted; + } + private void addIndexedItem(IndexedItem word) { word.setIndexName(this.getIndexName()); super.addItem((Item) word); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java index a19a6e53963..53a57a968f5 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/PhraseSegmentItem.java @@ -5,6 +5,7 @@ import com.yahoo.prelude.query.textualrepresentation.Discloser; import java.nio.ByteBuffer; import java.util.Iterator; +import java.util.Optional; /** @@ -55,10 +56,12 @@ public class PhraseSegmentItem extends IndexedSegmentItem { super(rawWord, current, isFromQuery, stemmed, substring); } + @Override public ItemType getItemType() { return ItemType.PHRASE; } + @Override public String getName() { return "SPHRASE"; } @@ -87,6 +90,7 @@ public class PhraseSegmentItem extends IndexedSegmentItem { * * @throws IllegalArgumentException if the given item is not a WordItem or PhraseItem */ + @Override public void addItem(Item item) { if (item instanceof WordItem) { addWordItem((WordItem) item); @@ -95,6 +99,13 @@ public class PhraseSegmentItem extends IndexedSegmentItem { } } + @Override + public Optional<Item> extractSingleChild() { + Optional<Item> extracted = super.extractSingleChild(); + extracted.ifPresent(e -> e.setWeight(this.getWeight())); + return extracted; + } + private void addWordItem(WordItem word) { word.setIndexName(this.getIndexName()); super.addItem(word); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java b/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java index 31e69e5b7cd..88bae76b26d 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java @@ -4,7 +4,10 @@ package com.yahoo.prelude.query; import com.yahoo.search.Query; import com.yahoo.search.query.QueryTree; -import java.util.*; +import java.util.HashSet; +import java.util.ListIterator; +import java.util.Optional; +import java.util.Set; /** * Query normalizer and sanity checker. @@ -82,15 +85,11 @@ public class QueryCanonicalizer { if (composite.getItemCount() == 0) parentIterator.remove(); - if (composite.getItemCount() == 1 && ! (composite instanceof NonReducibleCompositeItem)) { - if (composite instanceof PhraseItem || composite instanceof PhraseSegmentItem) - composite.getItem(0).setWeight(composite.getWeight()); - parentIterator.set(composite.getItem(0)); - } + composite.extractSingleChild().ifPresent(extractedChild -> parentIterator.set(extractedChild)); return CanonicalizationResult.success(); } - + private static void collapseLevels(CompositeItem composite) { if (composite instanceof RankItem || composite instanceof NotItem) { collapseLevels(composite, composite.getItemIterator()); // collapse the first item only diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java index aa446140da0..d2c19339298 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/SameElementItem.java @@ -6,6 +6,7 @@ import com.yahoo.protect.Validator; import java.nio.ByteBuffer; import java.util.Iterator; +import java.util.Optional; /** * This represents a query where all terms are required to match in the same element id. @@ -54,6 +55,16 @@ public class SameElementItem extends NonReducibleCompositeItem { Validator.ensureNonEmpty("Struct fieldname", asTerm.getIndexName()); Validator.ensureNonEmpty("Query term", asTerm.getIndexedString()); } + + @Override + public Optional<Item> extractSingleChild() { + if (getItemCount() == 1) { + WordItem child = (WordItem) getItem(0); + child.setIndexName(getFieldName() + "." + child.getIndexName()); + return Optional.of(child); + } + return Optional.empty(); + } @Override public ItemType getItemType() { diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java index 2d941681f2a..84c793a6df1 100644 --- a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java +++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java @@ -6,7 +6,6 @@ import com.yahoo.prelude.query.CompositeItem; import com.yahoo.prelude.query.EquivItem; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.NearItem; -import com.yahoo.prelude.query.NonReducibleCompositeItem; import com.yahoo.prelude.query.NotItem; import com.yahoo.prelude.query.NullItem; import com.yahoo.prelude.query.OrItem; @@ -215,7 +214,7 @@ public class QueryRewrite { parent.setItem(i, newChild); } } - return ((numChildren == 1) && !(parent instanceof NonReducibleCompositeItem)) ? parent.getItem(0) : item; + return parent.extractSingleChild().orElse(item); } private static Item rewriteSddocname(Item item) { 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 1bca3df4d77..0d47ef77ce5 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 @@ -45,7 +45,11 @@ public class SoftTimeout implements Cloneable { this.enabled = enable; } - public Boolean getEnable() { return enabled; } + /** Returns whether softtimeout is enabled. Defauyt is true. */ + public Boolean getEnable() { + if (enabled == null) return Boolean.TRUE; + return enabled; + } /** Override the adaptive factor determined on the content nodes */ public void setFactor(double factor) { diff --git a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java index 2f241f9c7a3..4bcb48447db 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java @@ -18,6 +18,7 @@ import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.document.datatypes.TensorFieldValue; import com.yahoo.document.json.JsonWriter; import com.yahoo.lang.MutableBoolean; +import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.processing.Response; import com.yahoo.processing.execution.Execution.Trace; import com.yahoo.processing.rendering.AsynchronousSectionedRenderer; @@ -39,6 +40,7 @@ import com.yahoo.search.result.Coverage; import com.yahoo.search.result.DefaultErrorHit; import com.yahoo.search.result.ErrorHit; import com.yahoo.search.result.ErrorMessage; +import com.yahoo.search.result.FeatureData; import com.yahoo.search.result.Hit; import com.yahoo.search.result.HitGroup; import com.yahoo.search.result.NanNumber; @@ -781,7 +783,7 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { } private void renderFieldContents(Object field) throws IOException { - if (field instanceof Inspectable) { + if (field instanceof Inspectable && ! (field instanceof FeatureData)) { renderInspector(((Inspectable)field).inspect()); } else { renderFieldContentsDirect(field); @@ -799,6 +801,8 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { generator.writeTree((TreeNode) field); } else if (field instanceof Tensor) { renderTensor(Optional.of((Tensor)field)); + } else if (field instanceof FeatureData) { + generator.writeRawValue(((FeatureData)field).toJson()); } else if (field instanceof Inspectable) { renderInspectorDirect(((Inspectable)field).inspect()); } else if (field instanceof JsonProducer) { @@ -811,8 +815,7 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { // the null below is the field which has already been written ((FieldValue) field).serialize(null, new JsonWriter(generator)); } else if (field instanceof JSONArray || field instanceof JSONObject) { - // org.json returns null if the object would not result in - // syntactically correct JSON + // org.json returns null if the object would not result in syntactically correct JSON String s = field.toString(); if (s == null) { generator.writeNull(); diff --git a/container-search/src/main/java/com/yahoo/search/rendering/Renderer.java b/container-search/src/main/java/com/yahoo/search/rendering/Renderer.java index c9b890e64f5..a5b51e60861 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/Renderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/Renderer.java @@ -40,7 +40,7 @@ abstract public class Renderer extends com.yahoo.processing.rendering.Renderer<R public final ListenableFuture<Boolean> render(OutputStream stream, Result response, Execution execution, Request request) { Writer writer = null; try { - writer = createWriter(stream,response); + writer = createWriter(stream, response); render(writer, response); } catch (IOException e) { @@ -50,7 +50,7 @@ abstract public class Renderer extends com.yahoo.processing.rendering.Renderer<R if (writer !=null) try { writer.close(); } catch (IOException e2) {}; } - SettableFuture<Boolean> completed=SettableFuture.create(); + SettableFuture<Boolean> completed = SettableFuture.create(); completed.set(true); return completed; } diff --git a/container-search/src/main/java/com/yahoo/search/result/FeatureData.java b/container-search/src/main/java/com/yahoo/search/result/FeatureData.java index 7e5d6b12f30..1fd8f6e7e17 100644 --- a/container-search/src/main/java/com/yahoo/search/result/FeatureData.java +++ b/container-search/src/main/java/com/yahoo/search/result/FeatureData.java @@ -18,7 +18,7 @@ import java.util.Set; /** * A wrapper for structured data representing feature values: A map of floats and tensors. - * This class is not thread safe even when it is only consumed. + * This class is immutable but not thread safe. */ public class FeatureData implements Inspectable, JsonProducer { @@ -26,6 +26,8 @@ public class FeatureData implements Inspectable, JsonProducer { private Set<String> featureNames = null; + private String jsonForm = null; + public FeatureData(Inspector value) { this.value = value; } @@ -39,14 +41,11 @@ public class FeatureData implements Inspectable, JsonProducer { public Inspector inspect() { return value; } @Override - public String toString() { - if (value.type() == Type.EMPTY) return ""; - return toJson(); - } - - @Override public String toJson() { - return writeJson(new StringBuilder()).toString(); + if (jsonForm != null) return jsonForm; + + jsonForm = writeJson(new StringBuilder()).toString(); + return jsonForm; } @Override @@ -95,6 +94,22 @@ public class FeatureData implements Inspectable, JsonProducer { return featureNames; } + @Override + public String toString() { + if (value.type() == Type.EMPTY) return ""; + return toJson(); + } + + @Override + public int hashCode() { return toJson().hashCode(); } + + @Override + public boolean equals(Object other) { + if (other == this) return true; + if ( ! (other instanceof FeatureData)) return false; + return ((FeatureData)other).toJson().equals(this.toJson()); + } + /** A JSON encoder which encodes DATA as a tensor */ private static class Encoder extends JsonRender.StringEncoder { diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java index ad4d0cf82e5..e6cc3ac9e54 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java @@ -3,9 +3,11 @@ package com.yahoo.prelude.fastsearch; import com.google.common.collect.ImmutableSet; import com.yahoo.config.subscription.ConfigGetter; +import com.yahoo.data.access.slime.SlimeAdapter; import com.yahoo.prelude.hitfield.RawData; import com.yahoo.prelude.hitfield.XMLString; import com.yahoo.prelude.hitfield.JSONString; +import com.yahoo.search.result.FeatureData; import com.yahoo.search.result.Hit; import com.yahoo.search.result.NanNumber; import com.yahoo.search.result.StructuredData; @@ -17,9 +19,11 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import com.yahoo.slime.BinaryFormat; import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.serialization.TypedBinaryFormat; @@ -62,6 +66,7 @@ public class SlimeSummaryTestCase { assertNull(hit.getField("jsonstring_field")); assertNull(hit.getField("tensor_field1")); assertNull(hit.getField("tensor_field2")); + assertNull(hit.getField("summaryfeatures")); } @Test @@ -75,7 +80,7 @@ public class SlimeSummaryTestCase { @Test public void testDecoding() { Tensor tensor1 = Tensor.from("tensor(x{},y{}):{{x:foo,y:bar}:0.1}"); - Tensor tensor2 = Tensor.from("tensor(x[],y[1]):{{x:0,y:0}:-0.3}"); + Tensor tensor2 = Tensor.from("tensor(x[1],y[1]):{{x:0,y:0}:-0.3}"); DocsumDefinitionSet docsum = createDocsumDefinitionSet(summary_cf); FastHit hit = new FastHit(); assertNull(docsum.lazyDecode("default", fullSummary(tensor1, tensor2), hit)); @@ -111,6 +116,12 @@ public class SlimeSummaryTestCase { } assertEquals(tensor1, hit.getField("tensor_field1")); assertEquals(tensor2, hit.getField("tensor_field2")); + FeatureData featureData = (FeatureData)hit.getField("summaryfeatures"); + assertEquals("double_feature,tensor1_feature,tensor2_feature", + featureData.featureNames().stream().sorted().collect(Collectors.joining(","))); + assertEquals(0.5, featureData.getDouble("double_feature"), 0.00000001); + assertEquals(tensor1, featureData.getTensor("tensor1_feature")); + assertEquals(tensor2, featureData.getTensor("tensor2_feature")); } @Test @@ -238,7 +249,9 @@ public class SlimeSummaryTestCase { assertFields(expected, hit); // --- Add full summary - assertNull(fullDocsum.lazyDecode("default", fullishSummary(), hit)); + Tensor tensor1 = Tensor.from("tensor(x{},y{}):{{x:foo,y:bar}:0.1}"); + Tensor tensor2 = Tensor.from("tensor(x[1],y[1]):{{x:0,y:0}:-0.3}"); + assertNull(fullDocsum.lazyDecode("default", fullishSummary(tensor1, tensor2), hit)); expected.put("integer_field", 4); expected.put("short_field", (short)2); expected.put("byte_field", (byte)1); @@ -247,7 +260,15 @@ public class SlimeSummaryTestCase { expected.put("int64_field", 8L); expected.put("string_field", "string_value"); expected.put("longstring_field", "longstring_value"); - assertFields(expected, hit); + expected.put("tensor_field1", tensor1); + expected.put("tensor_field2", tensor2); + + Slime slime = new Slime(); + Cursor summaryFeatures = slime.setObject(); + summaryFeatures.setDouble("double_feature", 0.5); + summaryFeatures.setData("tensor1_feature", TypedBinaryFormat.encode(tensor1)); + summaryFeatures.setData("tensor2_feature", TypedBinaryFormat.encode(tensor2)); + expected.put("summaryfeatures", new FeatureData(new SlimeAdapter(slime.get()))); hit.removeField("string_field"); hit.removeField("integer_field"); @@ -272,7 +293,7 @@ public class SlimeSummaryTestCase { fail("Multiple callbacks for " + name); traversed.put(name, value); }); - assertEquals(expected, traversed); + assertEqualMaps(expected, traversed); // raw utf8 field traverser Map<String, Object> traversedUtf8 = new HashMap<>(); hit.forEachFieldAsRaw(new Utf8FieldTraverser(traversedUtf8)); @@ -288,7 +309,7 @@ public class SlimeSummaryTestCase { // fieldKeys assertEquals(expected.keySet(), hit.fieldKeys()); // fields - assertEquals(expected, hit.fields()); + assertEqualMaps(expected, hit.fields()); // fieldIterator int fieldIteratorFieldCount = 0; for (Iterator<Map.Entry<String, Object>> i = hit.fieldIterator(); i.hasNext(); ) { @@ -302,6 +323,15 @@ public class SlimeSummaryTestCase { assertEquals(field.getValue(), hit.getField(field.getKey())); } + private void assertEqualMaps(Map<String, Object> expected, Map<String, Object> actual) { + assertEquals("Map sizes", expected.size(), actual.size()); + assertEquals("Keys", expected.keySet(), actual.keySet()); + for (var expectedEntry : expected.entrySet()) { + assertEquals("Key '" + expectedEntry.getKey() + "'", + expectedEntry.getValue(), actual.get(expectedEntry.getKey())); + } + } + private byte[] emptySummary() { Slime slime = new Slime(); slime.setObject(); @@ -339,7 +369,7 @@ public class SlimeSummaryTestCase { return encode((slime)); } - private byte[] fullishSummary() { + private byte[] fullishSummary(Tensor tensor1, Tensor tensor2) { Slime slime = new Slime(); Cursor docsum = slime.setObject(); docsum.setLong("integer_field", 4); @@ -352,6 +382,7 @@ public class SlimeSummaryTestCase { //docsum.setData("data_field", "data_value".getBytes(StandardCharsets.UTF_8)); docsum.setString("longstring_field", "longstring_value"); //docsum.setData("longdata_field", "longdata_value".getBytes(StandardCharsets.UTF_8)); + addTensors(tensor1, tensor2, docsum); return encode((slime)); } @@ -374,11 +405,23 @@ public class SlimeSummaryTestCase { field.setLong("foo", 1); field.setLong("bar", 2); } + + addTensors(tensor1, tensor2, docsum); + return encode((slime)); + } + + private void addTensors(Tensor tensor1, Tensor tensor2, Cursor docsum) { if (tensor1 != null) docsum.setData("tensor_field1", TypedBinaryFormat.encode(tensor1)); if (tensor2 != null) docsum.setData("tensor_field2", TypedBinaryFormat.encode(tensor2)); - return encode((slime)); + + if (tensor1 !=null && tensor2 != null) { + Cursor summaryFeatures = docsum.setObject("summaryfeatures"); + summaryFeatures.setDouble("double_feature", 0.5); + summaryFeatures.setData("tensor1_feature", TypedBinaryFormat.encode(tensor1)); + summaryFeatures.setData("tensor2_feature", TypedBinaryFormat.encode(tensor2)); + } } private byte[] encode(Slime slime) { diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/summary.cfg b/container-search/src/test/java/com/yahoo/prelude/fastsearch/summary.cfg index e46904b17d0..e074eadcbc2 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/summary.cfg +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/summary.cfg @@ -3,7 +3,7 @@ documentdb[0].name test documentdb[0].summaryclass[1] documentdb[0].summaryclass[0].name default documentdb[0].summaryclass[0].id 0 -documentdb[0].summaryclass[0].fields[14] +documentdb[0].summaryclass[0].fields[15] documentdb[0].summaryclass[0].fields[0].name integer_field documentdb[0].summaryclass[0].fields[0].type integer documentdb[0].summaryclass[0].fields[1].name short_field @@ -32,3 +32,5 @@ documentdb[0].summaryclass[0].fields[12].name tensor_field1 documentdb[0].summaryclass[0].fields[12].type tensor documentdb[0].summaryclass[0].fields[13].name tensor_field2 documentdb[0].summaryclass[0].fields[13].type tensor +documentdb[0].summaryclass[0].fields[14].name summaryfeatures +documentdb[0].summaryclass[0].fields[14].type featuredata diff --git a/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java b/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java index 6754494ba4e..dff6d4c26c3 100644 --- a/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java @@ -13,7 +13,7 @@ public class SoftTimeoutTestCase { @Test public void testDefaultsInQuery() { Query query=new Query("?query=test"); - assertNull(query.getRanking().getSoftTimeout().getEnable()); + assertTrue(query.getRanking().getSoftTimeout().getEnable()); assertNull(query.getRanking().getSoftTimeout().getFactor()); assertNull(query.getRanking().getSoftTimeout().getTailcost()); } @@ -21,7 +21,7 @@ public class SoftTimeoutTestCase { @Test public void testQueryOverride() { Query query=new Query("?query=test&ranking.softtimeout.factor=0.7&ranking.softtimeout.tailcost=0.3"); - assertNull(query.getRanking().getSoftTimeout().getEnable()); + assertTrue(query.getRanking().getSoftTimeout().getEnable()); assertEquals(Double.valueOf(0.7), query.getRanking().getSoftTimeout().getFactor()); assertEquals(Double.valueOf(0.3), query.getRanking().getSoftTimeout().getTailcost()); query.prepare(); diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index 9fb2e627e9c..a245d61bafb 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -36,6 +36,7 @@ import com.yahoo.search.grouping.result.RootGroup; import com.yahoo.search.grouping.result.StringId; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorMessage; +import com.yahoo.search.result.FeatureData; import com.yahoo.search.result.Hit; import com.yahoo.search.result.HitGroup; import com.yahoo.search.result.NanNumber; @@ -51,6 +52,7 @@ import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorType; +import com.yahoo.tensor.serialization.TypedBinaryFormat; import com.yahoo.text.Utf8; import com.yahoo.yolean.trace.TraceNode; import org.json.JSONArray; @@ -123,7 +125,7 @@ public class JsonRendererTestCase { } @Test - public void testDataTypes() throws IOException, InterruptedException, ExecutionException, JSONException { + public void testDataTypes() throws IOException, InterruptedException, ExecutionException { String expected = "{" + " \"root\": {" + " \"children\": [" @@ -139,7 +141,13 @@ public class JsonRendererTestCase { + " \"predicate\": \"a in [b]\"," + " \"tensor1\": { \"cells\": [ { \"address\": {\"x\": \"a\"}, \"value\":2.0 } ] }," + " \"tensor2\": { \"cells\": [] }," - + " \"tensor3\": { \"cells\": [ { \"address\": {\"x\": \"a\", \"y\": \"0\"}, \"value\":2.0 }, { \"address\": {\"x\": \"a\", \"y\": \"1\"}, \"value\":-1.0 } ] }" + + " \"tensor3\": { \"cells\": [ { \"address\": {\"x\": \"a\", \"y\": \"0\"}, \"value\":2.0 }, { \"address\": {\"x\": \"a\", \"y\": \"1\"}, \"value\":-1.0 } ] }," + + " \"summaryfeatures\": {" + + " \"scalar1\":1.5," + + " \"scalar2\":2.5," + + " \"tensor1\":{\"type\":\"tensor(x[3])\",\"cells\":[{\"address\":{\"x\":\"0\"},\"value\":1.5},{\"address\":{\"x\":\"1\"},\"value\":2.0},{\"address\":{\"x\":\"2\"},\"value\":2.5}]}," + + " \"tensor2\":{\"type\":\"tensor()\",\"cells\":[{\"address\":{},\"value\":0.5}]}" + + " }" + " }," + " \"id\": \"datatypestuff\"," + " \"relevance\": 1.0" @@ -166,12 +174,24 @@ public class JsonRendererTestCase { h.setField("tensor2", new TensorFieldValue(TensorType.empty)); h.setField("tensor3", Tensor.from("{ {x:a, y:0}: 2.0, {x:a, y:1}: -1 }")); h.setField("object", new Thingie()); + h.setField("summaryfeatures", createSummaryFeatures()); r.hits().add(h); r.setTotalHitCount(1L); String summary = render(r); assertEqualJson(expected, summary); } + private FeatureData createSummaryFeatures() { + Slime slime = new Slime(); + Cursor features = slime.setObject(); + features.setDouble("scalar1", 1.5); + features.setDouble("scalar2", 2.5); + Tensor tensor1 = Tensor.from("tensor(x[3]):[1.5, 2, 2.5]"); + features.setData("tensor1", TypedBinaryFormat.encode(tensor1)); + Tensor tensor2 = Tensor.from(0.5); + features.setData("tensor2", TypedBinaryFormat.encode(tensor2)); + return new FeatureData(new SlimeAdapter(slime.get())); + } @Test public void testTracing() throws IOException, InterruptedException, ExecutionException { @@ -679,12 +699,10 @@ public class JsonRendererTestCase { + "}"; Result r = newEmptyResult(); Hit h = new Hit("moredatatypestuff"); - h.setField("byte", Byte.valueOf((byte) 8)); - h.setField("short", Short.valueOf((short) 16)); - h.setField("bigInteger", new BigInteger( - "340282366920938463463374607431768211455")); - h.setField("bigDecimal", new BigDecimal( - "340282366920938463463374607431768211456.5")); + h.setField("byte", (byte)8); + h.setField("short", (short)16); + h.setField("bigInteger", new BigInteger("340282366920938463463374607431768211455")); + h.setField("bigDecimal", new BigDecimal("340282366920938463463374607431768211456.5")); h.setField("nanNumber", NanNumber.NaN); r.hits().add(h); r.setTotalHitCount(1L); diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java index 70d50b23bed..00a17f963c6 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java @@ -303,7 +303,7 @@ public class YqlParserTestCase { assertCanonicalParse("select foo from bar where baz contains sameElement(key contains \"a\", value.f2 = 10);", "baz:{key:a value.f2:10}"); assertCanonicalParse("select foo from bar where baz contains sameElement(key contains \"a\");", - "baz:{key:a}"); + "baz.key:a"); } @Test diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java index e4d0c8246d9..dbcb44d1711 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificate.java @@ -1,29 +1,36 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.certificates; -import java.security.cert.X509Certificate; -import java.util.List; +import java.util.Objects; /** - * Represents a certificate chain and a reference to the private key used for generating the certificate + * Represents a reference to a certificate and private key. * * @author mortent * @author andreer */ public class ApplicationCertificate { - private final List<X509Certificate> certificateChain; - private final KeyId keyId; - public ApplicationCertificate(List<X509Certificate> certificateChain, KeyId keyId) { - this.certificateChain = certificateChain; - this.keyId = keyId; + private final String secretsKeyNamePrefix; + + public ApplicationCertificate(String secretsKeyNamePrefix) { + this.secretsKeyNamePrefix = secretsKeyNamePrefix; + } + + public String secretsKeyNamePrefix() { + return secretsKeyNamePrefix; } - public List<X509Certificate> certificateChain() { - return certificateChain; + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ApplicationCertificate that = (ApplicationCertificate) o; + return Objects.equals(secretsKeyNamePrefix, that.secretsKeyNamePrefix); } - public KeyId keyId() { - return keyId; + @Override + public int hashCode() { + return Objects.hash(secretsKeyNamePrefix); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificateProvider.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificateProvider.java new file mode 100644 index 00000000000..fa489a6b754 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/ApplicationCertificateProvider.java @@ -0,0 +1,12 @@ +package com.yahoo.vespa.hosted.controller.api.integration.certificates; + +import com.yahoo.config.provision.ApplicationId; + +/** + * Generates a certificate. + * + * @author andreer + */ +public interface ApplicationCertificateProvider { + ApplicationCertificate requestCaSignedCertificate(ApplicationId applicationId); +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/CertificateProvider.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/CertificateProvider.java deleted file mode 100644 index d2462eb574f..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/CertificateProvider.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.yahoo.vespa.hosted.controller.api.integration.certificates; - -import java.security.KeyPair; -import java.security.cert.X509Certificate; -import java.util.List; - -/** - * Generates a certificate. - * - * @author andreer - */ -public interface CertificateProvider { - List<X509Certificate> requestCaSignedCertificate(KeyPair keyPair, List<String> domains); -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/KeyId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/KeyId.java deleted file mode 100644 index 3ab22d4a5b7..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/KeyId.java +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration.certificates; - -/** - * Identifier for a key pair. Used for persisting/retrieving a key pair. - * - * @author mortent - * @author andreer - */ -public class KeyId { - private final String name; - private final int version; - - public KeyId(String name, int version) { - this.name = name; - this.version = version; - } -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/KeyPairProvider.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/KeyPairProvider.java deleted file mode 100644 index a872bf63343..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/KeyPairProvider.java +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration.certificates; - -import com.yahoo.config.provision.ApplicationId; - -/** - * Provides a key pair. Generates and persists the key pair if not found. - * - * @author mortent - * @author andreer - */ -public interface KeyPairProvider { - VersionedKeyPair getKeyPair(ApplicationId applicationId); -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/VersionedKeyPair.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/VersionedKeyPair.java deleted file mode 100644 index c95303b9497..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/VersionedKeyPair.java +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.integration.certificates; - -import java.security.KeyPair; - -/** - * Represents a key pair and an unique persistence identifier - * - * @author mortent - * @author andreer - */ -public class VersionedKeyPair { - private final KeyId keyId; - private final KeyPair keyPair; - - public VersionedKeyPair(KeyId keyId, KeyPair keyPair) { - this.keyId = keyId; - this.keyPair = keyPair; - } - - public KeyId keyId() { - return keyId; - } - - public KeyPair keyPair() { - return keyPair; - } -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/package-info.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/package-info.java new file mode 100644 index 00000000000..0ba13524d33 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.controller.api.integration.certificates; + +import com.yahoo.osgi.annotation.ExportPackage;
\ No newline at end of file diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index 20469e6449a..ba00203ec34 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions; import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import java.io.IOException; @@ -29,7 +30,7 @@ public interface ConfigServer { } PreparedApplication deploy(DeploymentId deployment, DeployOptions deployOptions, Set<String> rotationNames, - List<ContainerEndpoint> containerEndpoints, byte[] content); + List<ContainerEndpoint> containerEndpoints, ApplicationCertificate applicationCertificate, byte[] content); void restart(DeploymentId deployment, Optional<Hostname> hostname); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java index a2d199a38a8..20599e92aa9 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServerException.java @@ -37,7 +37,8 @@ public class ConfigServerException extends RuntimeException { OUT_OF_CAPACITY, REQUEST_TIMEOUT, UNKNOWN_VESPA_VERSION, - PARENT_HOST_NOT_READY + PARENT_HOST_NOT_READY, + CERTIFICATE_NOT_READY } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index c168d09a15a..9ca73d27120 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -11,6 +11,7 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.ApplicationMetrics; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; import com.yahoo.vespa.hosted.controller.api.integration.organization.User; @@ -59,6 +60,7 @@ public class Application { private final Optional<String> pemDeployKey; private final List<AssignedRotation> rotations; private final Map<HostName, RotationStatus> rotationStatus; + private final Optional<ApplicationCertificate> applicationCertificate; /** Creates an empty application */ public Application(ApplicationId id, Instant now) { @@ -66,7 +68,7 @@ public class Application { new DeploymentJobs(OptionalLong.empty(), Collections.emptyList(), Optional.empty(), false), Change.empty(), Change.empty(), Optional.empty(), Optional.empty(), OptionalInt.empty(), new ApplicationMetrics(0, 0), - Optional.empty(), Collections.emptyList(), Collections.emptyMap()); + Optional.empty(), Collections.emptyList(), Collections.emptyMap(), Optional.empty()); } /** Used from persistence layer: Do not use */ @@ -74,18 +76,19 @@ public class Application { List<Deployment> deployments, DeploymentJobs deploymentJobs, Change change, Change outstandingChange, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Optional<String> pemDeployKey, - List<AssignedRotation> rotations, Map<HostName, RotationStatus> rotationStatus) { + List<AssignedRotation> rotations, Map<HostName, RotationStatus> rotationStatus, + Optional<ApplicationCertificate> applicationCertificate) { this(id, createdAt, deploymentSpec, validationOverrides, deployments.stream().collect(Collectors.toMap(Deployment::zone, Function.identity())), deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } Application(ApplicationId id, Instant createdAt, DeploymentSpec deploymentSpec, ValidationOverrides validationOverrides, Map<ZoneId, Deployment> deployments, DeploymentJobs deploymentJobs, Change change, Change outstandingChange, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Optional<String> pemDeployKey, - List<AssignedRotation> rotations, Map<HostName, RotationStatus> rotationStatus) { + List<AssignedRotation> rotations, Map<HostName, RotationStatus> rotationStatus, Optional<ApplicationCertificate> applicationCertificate) { this.id = Objects.requireNonNull(id, "id cannot be null"); this.createdAt = Objects.requireNonNull(createdAt, "instant of creation cannot be null"); this.deploymentSpec = Objects.requireNonNull(deploymentSpec, "deploymentSpec cannot be null"); @@ -101,6 +104,7 @@ public class Application { this.pemDeployKey = pemDeployKey; this.rotations = List.copyOf(Objects.requireNonNull(rotations, "rotations cannot be null")); this.rotationStatus = ImmutableMap.copyOf(Objects.requireNonNull(rotationStatus, "rotationStatus cannot be null")); + this.applicationCertificate = Objects.requireNonNull(applicationCertificate, "applicationCertificate cannot be null"); } public ApplicationId id() { return id; } @@ -241,6 +245,10 @@ public class Application { .orElse(RotationStatus.unknown); } + public Optional<ApplicationCertificate> applicationCertificate() { + return applicationCertificate; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 8345aa91685..197dda8c409 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -26,6 +26,8 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.BuildService; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; @@ -125,6 +127,8 @@ public class ApplicationController { private final Clock clock; private final BooleanFlag redirectLegacyDnsFlag; private final DeploymentTrigger deploymentTrigger; + private final BooleanFlag provisionApplicationCertificate; + private final ApplicationCertificateProvider applicationCertificateProvider; ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, RotationsConfig rotationsConfig, @@ -145,6 +149,9 @@ public class ApplicationController { this.rotationRepository = new RotationRepository(rotationsConfig, this, curator); this.deploymentTrigger = new DeploymentTrigger(controller, buildService, clock); + this.provisionApplicationCertificate = Flags.PROVISION_APPLICATION_CERTIFICATE.bindTo(controller.flagSource()); + this.applicationCertificateProvider = controller.applicationCertificateProvider(); + // Update serialization format of all applications Once.after(Duration.ofMinutes(1), () -> { Instant start = clock.instant(); @@ -287,6 +294,7 @@ public class ApplicationController { ApplicationVersion applicationVersion; ApplicationPackage applicationPackage; Set<String> rotationNames = new HashSet<>(); + ApplicationCertificate applicationCertificate; try (Lock lock = lock(applicationId)) { LockedApplication application = new LockedApplication(require(applicationId), lock); @@ -332,6 +340,10 @@ public class ApplicationController { // Include rotation ID to ensure that deployment can respond to health checks with rotation ID as Host header app.rotations().stream().map(RotationId::asString).forEach(rotationNames::add); + // Get application certificate (provisions a new certificate if missing) + application = withApplicationCertificate(application); + applicationCertificate = application.get().applicationCertificate().orElse(null); + // Update application with information from application package if ( ! preferOldestVersion && ! application.get().deploymentJobs().deployedInternally() @@ -342,7 +354,7 @@ public class ApplicationController { // Carry out deployment without holding the application lock. options = withVersion(platformVersion, options); - ActivateResult result = deploy(applicationId, applicationPackage, zone, options, rotationNames); + ActivateResult result = deploy(applicationId, applicationPackage, zone, options, rotationNames, applicationCertificate); lockOrThrow(applicationId, application -> store(application.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant(), @@ -409,7 +421,7 @@ public class ApplicationController { artifactRepository.getSystemApplicationPackage(application.id(), zone, version) ); DeployOptions options = withVersion(version, DeployOptions.none()); - return deploy(application.id(), applicationPackage, zone, options, Set.of()); + return deploy(application.id(), applicationPackage, zone, options, Set.of(), /* No application cert */ null); } else { throw new RuntimeException("This system application does not have an application package: " + application.id().toShortString()); } @@ -417,16 +429,16 @@ public class ApplicationController { /** Deploys the given tester application to the given zone. */ public ActivateResult deployTester(TesterId tester, ApplicationPackage applicationPackage, ZoneId zone, DeployOptions options) { - return deploy(tester.id(), applicationPackage, zone, options, Set.of()); + return deploy(tester.id(), applicationPackage, zone, options, Set.of(), /* No application cert for tester*/ null); } private ActivateResult deploy(ApplicationId application, ApplicationPackage applicationPackage, ZoneId zone, DeployOptions deployOptions, - Set<String> rotationNames) { + Set<String> rotationNames, ApplicationCertificate applicationCertificate) { DeploymentId deploymentId = new DeploymentId(application, zone); try { ConfigServer.PreparedApplication preparedApplication = - configServer.deploy(deploymentId, deployOptions, rotationNames, List.of(), applicationPackage.zippedContent()); + configServer.deploy(deploymentId, deployOptions, rotationNames, List.of(), applicationCertificate, applicationPackage.zippedContent()); return new ActivateResult(new RevisionId(applicationPackage.hash()), preparedApplication.prepareResponse(), applicationPackage.zippedContent().length); } finally { @@ -463,6 +475,18 @@ public class ApplicationController { return application; } + private LockedApplication withApplicationCertificate(LockedApplication application) { + ApplicationId applicationId = application.get().id(); + + // TODO: Verify that the application is deploying to a zone where certificate provisioning is enabled + boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value(); + if (provisionCertificate) { + application = application.withApplicationCertificate( + Optional.of(applicationCertificateProvider.requestCaSignedCertificate(applicationId))); + } + return application; + } + private ActivateResult unexpectedDeployment(ApplicationId application, ZoneId zone) { Log logEntry = new Log(); logEntry.level = "WARNING"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index d87d52f2c12..ed81d08c533 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -9,12 +9,12 @@ import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.ZoneApi; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.controller.api.integration.BuildService; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; import com.yahoo.vespa.hosted.controller.api.integration.RunDataStore; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider; import com.yahoo.vespa.hosted.controller.api.integration.chef.Chef; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationStore; @@ -84,6 +84,7 @@ public class Controller extends AbstractComponent { private final AuditLogger auditLogger; private final FlagSource flagSource; private final NameServiceForwarder nameServiceForwarder; + private final ApplicationCertificateProvider applicationCertificateProvider; private final MavenRepository mavenRepository; /** @@ -98,12 +99,12 @@ public class Controller extends AbstractComponent { AccessControl accessControl, ArtifactRepository artifactRepository, ApplicationStore applicationStore, TesterCloud testerCloud, BuildService buildService, RunDataStore runDataStore, Mailer mailer, FlagSource flagSource, - MavenRepository mavenRepository) { + MavenRepository mavenRepository, ApplicationCertificateProvider applicationCertificateProvider) { this(curator, rotationsConfig, gitHub, zoneRegistry, configServer, metricsService, routingGenerator, chef, Clock.systemUTC(), accessControl, artifactRepository, applicationStore, testerCloud, buildService, runDataStore, com.yahoo.net.HostName::getLocalhost, mailer, flagSource, - mavenRepository); + mavenRepository, applicationCertificateProvider); } public Controller(CuratorDb curator, RotationsConfig rotationsConfig, GitHub gitHub, @@ -113,7 +114,7 @@ public class Controller extends AbstractComponent { AccessControl accessControl, ArtifactRepository artifactRepository, ApplicationStore applicationStore, TesterCloud testerCloud, BuildService buildService, RunDataStore runDataStore, Supplier<String> hostnameSupplier, - Mailer mailer, FlagSource flagSource, MavenRepository mavenRepository) { + Mailer mailer, FlagSource flagSource, MavenRepository mavenRepository, ApplicationCertificateProvider applicationCertificateProvider) { this.hostnameSupplier = Objects.requireNonNull(hostnameSupplier, "HostnameSupplier cannot be null"); this.curator = Objects.requireNonNull(curator, "Curator cannot be null"); @@ -126,6 +127,7 @@ public class Controller extends AbstractComponent { this.mailer = Objects.requireNonNull(mailer, "Mailer cannot be null"); this.flagSource = Objects.requireNonNull(flagSource, "FlagSource cannot be null"); this.nameServiceForwarder = new NameServiceForwarder(curator); + this.applicationCertificateProvider = Objects.requireNonNull(applicationCertificateProvider); this.mavenRepository = Objects.requireNonNull(mavenRepository, "MavenRepository cannot be null"); jobController = new JobController(this, runDataStore, Objects.requireNonNull(testerCloud)); @@ -304,6 +306,10 @@ public class Controller extends AbstractComponent { return auditLogger; } + public ApplicationCertificateProvider applicationCertificateProvider() { + return applicationCertificateProvider; + } + /** Returns all other roles the given tenant role implies. */ public Set<Role> impliedRoles(TenantRole role) { return Stream.concat(Roles.tenantRoles(role.tenant()).stream(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java index 02b0afdd48f..294dc10d0bd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java @@ -10,6 +10,7 @@ import com.yahoo.config.provision.HostName; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.ApplicationMetrics; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.organization.IssueId; @@ -59,6 +60,7 @@ public class LockedApplication { private final Optional<String> pemDeployKey; private final List<AssignedRotation> rotations; private final Map<HostName, RotationStatus> rotationStatus; + private final Optional<ApplicationCertificate> applicationCertificate; /** * Used to create a locked application @@ -72,7 +74,7 @@ public class LockedApplication { application.deployments(), application.deploymentJobs(), application.change(), application.outstandingChange(), application.ownershipIssueId(), application.owner(), application.majorVersion(), application.metrics(), - application.pemDeployKey(), application.assignedRotations(), application.rotationStatus()); + application.pemDeployKey(), application.assignedRotations(), application.rotationStatus(), application.applicationCertificate()); } private LockedApplication(Lock lock, ApplicationId id, Instant createdAt, @@ -80,7 +82,7 @@ public class LockedApplication { Map<ZoneId, Deployment> deployments, DeploymentJobs deploymentJobs, Change change, Change outstandingChange, Optional<IssueId> ownershipIssueId, Optional<User> owner, OptionalInt majorVersion, ApplicationMetrics metrics, Optional<String> pemDeployKey, - List<AssignedRotation> rotations, Map<HostName, RotationStatus> rotationStatus) { + List<AssignedRotation> rotations, Map<HostName, RotationStatus> rotationStatus, Optional<ApplicationCertificate> applicationCertificate) { this.lock = lock; this.id = id; this.createdAt = createdAt; @@ -97,41 +99,42 @@ public class LockedApplication { this.pemDeployKey = pemDeployKey; this.rotations = rotations; this.rotationStatus = rotationStatus; + this.applicationCertificate = applicationCertificate; } /** Returns a read-only copy of this */ public Application get() { return new Application(id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication withBuiltInternally(boolean builtInternally) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withBuiltInternally(builtInternally), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication withProjectId(OptionalLong projectId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withProjectId(projectId), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication withDeploymentIssueId(IssueId issueId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.with(issueId), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication withJobPause(JobType jobType, OptionalLong pausedUntil) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withPause(jobType, pausedUntil), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication withJobCompletion(long projectId, JobType jobType, JobStatus.JobRun completion, @@ -139,14 +142,14 @@ public class LockedApplication { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withCompletion(projectId, jobType, completion, jobError), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, - pemDeployKey, rotations, rotationStatus); + pemDeployKey, rotations, rotationStatus, applicationCertificate); } public LockedApplication withJobTriggering(JobType jobType, JobStatus.JobRun job) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.withTriggering(jobType, job), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication withNewDeployment(ZoneId zone, ApplicationVersion applicationVersion, Version version, @@ -197,45 +200,45 @@ public class LockedApplication { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs.without(jobType), change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication with(DeploymentSpec deploymentSpec) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } public LockedApplication with(ValidationOverrides validationOverrides) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } public LockedApplication withChange(Change change) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } public LockedApplication withOutstandingChange(Change outstandingChange) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } public LockedApplication withOwnershipIssueId(IssueId issueId) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, Optional.ofNullable(issueId), owner, - majorVersion, metrics, pemDeployKey, rotations, rotationStatus); + majorVersion, metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } public LockedApplication withOwner(User owner) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, Optional.ofNullable(owner), majorVersion, metrics, pemDeployKey, - rotations, rotationStatus); + rotations, rotationStatus, applicationCertificate); } /** Set a major version for this, or set to null to remove any major version override */ @@ -243,33 +246,40 @@ public class LockedApplication { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion == null ? OptionalInt.empty() : OptionalInt.of(majorVersion), - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } public LockedApplication with(MetricsService.ApplicationMetrics metrics) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } public LockedApplication withPemDeployKey(String pemDeployKey) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, Optional.ofNullable(pemDeployKey), rotations, rotationStatus); + metrics, Optional.ofNullable(pemDeployKey), rotations, rotationStatus, applicationCertificate); } public LockedApplication with(List<AssignedRotation> assignedRotations) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, assignedRotations, rotationStatus); + metrics, pemDeployKey, assignedRotations, rotationStatus, applicationCertificate); } public LockedApplication withRotationStatus(Map<HostName, RotationStatus> rotationStatus) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } + public LockedApplication withApplicationCertificate(Optional<ApplicationCertificate> applicationCertificate) { + return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, + deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); + } + + /** Don't expose non-leaf sub-objects. */ private LockedApplication with(Deployment deployment) { Map<ZoneId, Deployment> deployments = new LinkedHashMap<>(this.deployments); @@ -280,7 +290,7 @@ public class LockedApplication { private LockedApplication with(Map<ZoneId, Deployment> deployments) { return new LockedApplication(lock, id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, change, outstandingChange, ownershipIssueId, owner, majorVersion, - metrics, pemDeployKey, rotations, rotationStatus); + metrics, pemDeployKey, rotations, rotationStatus, applicationCertificate); } @Override diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index bf2460284ab..ce0e7c0dbab 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -61,6 +61,7 @@ import static com.yahoo.log.LogLevel.DEBUG; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.ACTIVATION_CONFLICT; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.APPLICATION_LOCK_FAILURE; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.BAD_REQUEST; +import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.CERTIFICATE_NOT_READY; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.INVALID_APPLICATION_PACKAGE; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.OUT_OF_CAPACITY; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException.ErrorCode.PARENT_HOST_NOT_READY; @@ -231,7 +232,8 @@ public class InternalStepRunner implements StepRunner { if ( e.getErrorCode() == OUT_OF_CAPACITY && type.isTest() || e.getErrorCode() == ACTIVATION_CONFLICT || e.getErrorCode() == APPLICATION_LOCK_FAILURE - || e.getErrorCode() == PARENT_HOST_NOT_READY) { + || e.getErrorCode() == PARENT_HOST_NOT_READY + || e.getErrorCode() == CERTIFICATE_NOT_READY) { logger.log("Will retry, because of '" + e.getErrorCode() + "' deploying:\n" + e.getMessage()); return Optional.empty(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index 271221e15b2..6ecf60e7404 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -15,6 +15,7 @@ import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService.ApplicationMetrics; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; @@ -87,6 +88,7 @@ public class ApplicationSerializer { private final String rotationsField = "endpoints"; private final String deprecatedRotationField = "rotation"; private final String rotationStatusField = "rotationStatus"; + private final String applicationCertificateField = "applicationCertificate"; // Deployment fields private final String zoneField = "zone"; @@ -181,6 +183,7 @@ public class ApplicationSerializer { rotationsToSlime(application.assignedRotations(), root, rotationsField); assignedRotationsToSlime(application.assignedRotations(), root, assignedRotationsField); toSlime(application.rotationStatus(), root.setArray(rotationStatusField)); + application.applicationCertificate().ifPresent(cert -> root.setString(applicationCertificateField, cert.secretsKeyNamePrefix())); return slime; } @@ -363,10 +366,11 @@ public class ApplicationSerializer { Optional<String> pemDeployKey = optionalString(root.field(pemDeployKeyField)); List<AssignedRotation> assignedRotations = assignedRotationsFromSlime(deploymentSpec, root); Map<HostName, RotationStatus> rotationStatus = rotationStatusFromSlime(root.field(rotationStatusField)); + Optional<ApplicationCertificate> applicationCertificate = optionalString(root.field(applicationCertificateField)).map(ApplicationCertificate::new); return new Application(id, createdAt, deploymentSpec, validationOverrides, deployments, deploymentJobs, deploying, outstandingChange, ownershipIssueId, owner, majorVersion, metrics, - pemDeployKey, assignedRotations, rotationStatus); + pemDeployKey, assignedRotations, rotationStatus, applicationCertificate); } private List<Deployment> deploymentsFromSlime(Inspector array) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index 5748ad4f55c..dbf983a5bab 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.slime.Slime; import com.yahoo.test.ManualClock; import com.yahoo.vespa.athenz.api.AthenzDomain; @@ -35,11 +36,11 @@ import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMavenRepository; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockRunDataStore; import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud; -import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock; import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock; +import com.yahoo.vespa.hosted.controller.integration.ApplicationCertificateMock; import com.yahoo.vespa.hosted.controller.integration.ApplicationStoreMock; import com.yahoo.vespa.hosted.controller.integration.ArtifactRepositoryMock; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; @@ -353,7 +354,8 @@ public final class ControllerTester { () -> "test-controller", new MockMailer(), new InMemoryFlagSource(), - new MockMavenRepository()); + new MockMavenRepository(), + new ApplicationCertificateMock()); // Calculate initial versions controller.updateVersionStatus(VersionStatus.compute(controller)); return controller; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java new file mode 100644 index 00000000000..3246a260217 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java @@ -0,0 +1,14 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.integration; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider; + +public class ApplicationCertificateMock implements ApplicationCertificateProvider { + + @Override + public ApplicationCertificate requestCaSignedCertificate(ApplicationId applicationId) { + return new ApplicationCertificate(String.format("vespa.tls.%s.%s", applicationId.tenant(),applicationId.application())); + } +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index d4df9c20ead..cdbc45c4d8f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.identifiers.Identifier; import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ContainerEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.configserver.LoadBalancer; @@ -225,7 +226,7 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer @Override public PreparedApplication deploy(DeploymentId deployment, DeployOptions deployOptions, Set<String> rotationNames, - List<ContainerEndpoint> containerEndpoints, byte[] content) { + List<ContainerEndpoint> containerEndpoints, ApplicationCertificate applicationCertificate, byte[] content) { lastPrepareVersion = deployOptions.vespaVersion.map(Version::fromString).orElse(null); if (prepareException != null) { RuntimeException prepareException = this.prepareException; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 67b6b1ac61c..347fe6064df 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -10,6 +10,7 @@ import com.yahoo.config.provision.HostName; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.MetricsService; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; @@ -119,7 +120,8 @@ public class ApplicationSerializerTest { new MetricsService.ApplicationMetrics(0.5, 0.9), Optional.of("-----BEGIN PUBLIC KEY-----\n∠( ᐛ 」∠)_\n-----END PUBLIC KEY-----"), List.of(new AssignedRotation(new ClusterSpec.Id("foo"), EndpointId.default_(), new RotationId("my-rotation"))), - rotationStatus); + rotationStatus, + Optional.of(new ApplicationCertificate("vespa.certificate"))); Application serialized = applicationSerializer.fromSlime(applicationSerializer.toSlime(original)); @@ -157,6 +159,8 @@ public class ApplicationSerializerTest { assertEquals(original.rotations(), serialized.rotations()); assertEquals(original.rotationStatus(), serialized.rotationStatus()); + assertEquals(original.applicationCertificate(), serialized.applicationCertificate()); + // Test cluster utilization assertEquals(0, serialized.deployments().get(zone1).clusterUtils().size()); assertEquals(3, serialized.deployments().get(zone2).clusterUtils().size()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java index 6f612005524..53476a2e42f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java @@ -92,6 +92,7 @@ public class ControllerContainerTest { " <component id='com.yahoo.vespa.hosted.controller.integration.ApplicationStoreMock'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer'/>\n" + + " <component id='com.yahoo.vespa.hosted.controller.integration.ApplicationCertificateMock'/>\n" + " <component id='com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMavenRepository'/>\n" + " <handler id='com.yahoo.vespa.hosted.controller.restapi.deployment.DeploymentApiHandler'>\n" + " <binding>http://*/deployment/v1/*</binding>\n" + diff --git a/dist/vespa.spec b/dist/vespa.spec index 99b24c2ff8e..3426eff459d 100644 --- a/dist/vespa.spec +++ b/dist/vespa.spec @@ -70,7 +70,6 @@ BuildRequires: java-11-openjdk-devel BuildRequires: openssl-devel BuildRequires: rpm-build BuildRequires: make -BuildRequires: vespa-cppunit-devel >= 1.12.1-6 BuildRequires: systemd BuildRequires: flex >= 2.5.0 BuildRequires: bison >= 3.0.0 diff --git a/docproc/abi-spec.json b/docproc/abi-spec.json index 65ca886efaf..dee2d2172e4 100644 --- a/docproc/abi-spec.json +++ b/docproc/abi-spec.json @@ -11,7 +11,8 @@ "public abstract java.util.Map documentTypes()", "public abstract java.util.Map structTypes()", "public abstract java.util.Map annotationTypes()", - "public abstract com.yahoo.document.Document getDocumentCopy(java.lang.String, com.yahoo.document.datatypes.StructuredFieldValue, com.yahoo.document.DocumentId)" + "public abstract com.yahoo.document.Document getDocumentCopy(java.lang.String, com.yahoo.document.datatypes.StructuredFieldValue, com.yahoo.document.DocumentId)", + "public com.yahoo.document.datatypes.FieldValue optionallyUpgrade(com.yahoo.document.Field, com.yahoo.document.datatypes.FieldValue)" ], "fields": [] }, diff --git a/docproc/src/main/java/com/yahoo/docproc/AbstractConcreteDocumentFactory.java b/docproc/src/main/java/com/yahoo/docproc/AbstractConcreteDocumentFactory.java index e9a821c66ac..f23e1f33d8f 100644 --- a/docproc/src/main/java/com/yahoo/docproc/AbstractConcreteDocumentFactory.java +++ b/docproc/src/main/java/com/yahoo/docproc/AbstractConcreteDocumentFactory.java @@ -2,9 +2,16 @@ package com.yahoo.docproc; import java.util.Map; + +import com.yahoo.document.DataType; import com.yahoo.document.Document; +import com.yahoo.document.DocumentId; +import com.yahoo.document.Field; import com.yahoo.document.annotation.Annotation; +import com.yahoo.document.datatypes.Array; +import com.yahoo.document.datatypes.FieldValue; import com.yahoo.document.datatypes.Struct; +import com.yahoo.document.datatypes.StructuredFieldValue; /** * Subtyped by factory classes for concrete document types. The factory classes are auto-generated @@ -23,8 +30,37 @@ public abstract class AbstractConcreteDocumentFactory extends com.yahoo.componen * * @return a concrete document instance */ - public abstract com.yahoo.document.Document getDocumentCopy(java.lang.String type, - com.yahoo.document.datatypes.StructuredFieldValue src, - com.yahoo.document.DocumentId id); + public abstract Document getDocumentCopy(java.lang.String type, StructuredFieldValue src, DocumentId id); + + /** + * If the FieldValue is a StructuredFieldValue it will upgrade to the concrete type + * @param field + * @param fv + * @return fv or upgraded fv + */ + public FieldValue optionallyUpgrade(Field field, FieldValue fv) { + return optionallyUpgrade(field.getDataType(), fv); + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + private FieldValue optionallyUpgrade(DataType dataType, FieldValue fv) { + if (fv instanceof StructuredFieldValue) { + try { + return structTypes().get(dataType.getName()) + .getConstructor(StructuredFieldValue.class) + .newInstance(fv); + } catch (java.lang.Exception ex) { + throw new RuntimeException(ex); + } + } else if (fv instanceof Array) { + Array<FieldValue> array = (Array<FieldValue>) fv; + DataType nestedType = array.getDataType().getNestedType(); + for (int i=0; i < array.size(); i++) { + array.set(i, optionallyUpgrade(nestedType, array.get(i))); + } + } + // TODO We also need special handling for weighted set/map. Limiting to array until verified. + return fv; + } } diff --git a/documentgen-test/src/test/java/com/yahoo/vespa/config/DocumentGenPluginTest.java b/documentgen-test/src/test/java/com/yahoo/vespa/config/DocumentGenPluginTest.java index 0cb365d421f..25277e9cdfe 100644 --- a/documentgen-test/src/test/java/com/yahoo/vespa/config/DocumentGenPluginTest.java +++ b/documentgen-test/src/test/java/com/yahoo/vespa/config/DocumentGenPluginTest.java @@ -38,6 +38,8 @@ import com.yahoo.document.datatypes.WeightedSet; import com.yahoo.document.serialization.DocumentDeserializerFactory; import com.yahoo.document.serialization.DocumentSerializer; import com.yahoo.document.serialization.DocumentSerializerFactory; +import com.yahoo.document.serialization.VespaDocumentDeserializerHead; +import com.yahoo.document.serialization.VespaDocumentSerializerHead; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.searchdefinition.derived.Deriver; import com.yahoo.tensor.Tensor; @@ -283,9 +285,7 @@ public class DocumentGenPluginTest { assertEquals(twelve, new IntegerFieldValue(12)); } - @Test - public void testArrayOfStruct() { - Book book = getBook(); + private void verifyArrayOfStruct(Book book) { assertEquals(book.getMysinglestructarray().get(0).getS1(), "YEPS"); assertEquals(book.getMysinglestructarray().get(1).getI1(), (Integer)456); Struct s1 = (Struct) ((Array)book.getFieldValue("mysinglestructarray")).get(0); @@ -300,8 +300,25 @@ public class DocumentGenPluginTest { assertEquals(ifv2.getInteger(), 456); s2.setFieldValue("i1", new IntegerFieldValue(123)); assertEquals(book.getMysinglestructarray().get(1).getI1(), (Integer)123); - book.getMysinglestructarray().remove(0); + Book.Ss1 prev = book.getMysinglestructarray().remove(0); assertEquals(book.getMysinglestructarray().get(0).getI1(), (Integer)123); + book.getMysinglestructarray().add(0, prev); + assertEquals(book.getMysinglestructarray().get(1).getI1(), (Integer)123); + s2.setFieldValue("i1", new IntegerFieldValue(456)); + } + + private static Document copyBySerialization(Document orig) { + return roundtripSerialize(orig, typeManagerForBookType()); + } + private Book toBook(Document doc) { + return (Book) new ConcreteDocumentFactory().getDocumentCopy(doc.getDataType().getName(), doc, doc.getId()); + } + + @Test + public void testArrayOfStruct() { + Book book = getBook(); + verifyArrayOfStruct(book); + verifyArrayOfStruct(toBook(copyBySerialization(book))); } @Test @@ -489,13 +506,13 @@ public class DocumentGenPluginTest { } } - private DocumentTypeManager typeManagerFromSDs(String... files) { + private static DocumentTypeManager typeManagerFromSDs(String... files) { final DocumentTypeManager mgr = new DocumentTypeManager(); mgr.configure("raw:" + getDocumentConfig(Arrays.asList(files))); return mgr; } - private DocumentTypeManager typeManagerForBookType() { + private static DocumentTypeManager typeManagerForBookType() { return typeManagerFromSDs("etc/complex/common.sd", "etc/complex/parent.sd", "etc/complex/book.sd"); } @@ -511,7 +528,7 @@ public class DocumentGenPluginTest { assertEquals(NUM_BOOKS, manyGenericBooks.size()); } - private String getDocumentConfig(List<String> sds) { + private static String getDocumentConfig(List<String> sds) { return new DocumentmanagerConfig(Deriver.getDocumentManagerConfig(sds)).toString(); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 1b7ed1fb21e..b969e328419 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -69,12 +69,6 @@ public class Flags { "Takes effect on next node agent tick. Change is orchestrated, but does NOT require container restart", HOSTNAME, APPLICATION_ID); - public static final UnboundBooleanFlag SUPPORT_DHCPV6_IN_AWS = defineFeatureFlag( - "support-dhcpv6-in-aws", true, - "Whether to open up for DHCPv6 traffic in AWS. Old behavior is false.", - "Takes effect on next tick in host-admin, except FirewallTask which requires a restart of host-admin", - HOSTNAME); - public static final UnboundStringFlag TLS_INSECURE_MIXED_MODE = defineStringFlag( "tls-insecure-mixed-mode", "tls_client_mixed_server", "TLS insecure mixed mode. Allowed values: ['plaintext_client_mixed_server', 'tls_client_mixed_server', 'tls_client_tls_server']", @@ -151,6 +145,11 @@ public class Flags { "Configserver RPC authorizer. Allowed values: ['disable', 'log-only', 'enforce']", "Takes effect on restart of configserver"); + public static final UnboundBooleanFlag PROVISION_APPLICATION_CERTIFICATE = defineFeatureFlag( + "provision-application-certificate", false, + "Privision certificate from CA and include reference in deployment", + "Takes effect on deployment through controller", + APPLICATION_ID); /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, String description, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index fea30d3660c..e0b192de74d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -62,7 +62,6 @@ public class CuratorDatabaseClient { private static final Path root = Path.fromString("/provision/v1"); private static final Path lockRoot = root.append("locks"); private static final Path loadBalancersRoot = root.append("loadBalancers"); - private static final Path flagsRoot = root.append("flags"); private static final Duration defaultLockTimeout = Duration.ofMinutes(2); private final NodeSerializer nodeSerializer; @@ -75,7 +74,6 @@ public class CuratorDatabaseClient { public CuratorDatabaseClient(NodeFlavors flavors, Curator curator, Clock clock, Zone zone, boolean useCache) { this.nodeSerializer = new NodeSerializer(flavors); this.zone = zone; - curator.delete(flagsRoot); // TODO: Remove after 7.42 has been released this.curatorDatabase = new CuratorDatabase(curator, root, useCache); this.clock = clock; this.provisionIndexCounter = new CuratorCounter(curator, root.append("provisionIndexCounter").getAbsolute()); @@ -473,23 +471,28 @@ public class CuratorDatabaseClient { // Load balancers - public Map<LoadBalancerId, LoadBalancer> readLoadBalancers() { + public List<LoadBalancerId> readLoadBalancerIds() { return curatorDatabase.getChildren(loadBalancersRoot).stream() .map(LoadBalancerId::fromSerializedForm) - .map(this::readLoadBalancer) - .filter(Optional::isPresent) - .map(Optional::get) - .collect(collectingAndThen(toMap(LoadBalancer::id, Function.identity()), - Collections::unmodifiableMap)); + .collect(Collectors.toUnmodifiableList()); + } + + public Map<LoadBalancerId, LoadBalancer> readLoadBalancers() { + return readLoadBalancerIds().stream() + .map(this::readLoadBalancer) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(collectingAndThen(toMap(LoadBalancer::id, Function.identity()), + Collections::unmodifiableMap)); } - private Optional<LoadBalancer> readLoadBalancer(LoadBalancerId id) { + public Optional<LoadBalancer> readLoadBalancer(LoadBalancerId id) { return read(loadBalancerPath(id), LoadBalancerSerializer::fromJson); } public void writeLoadBalancer(LoadBalancer loadBalancer) { NestedTransaction transaction = new NestedTransaction(); - writeLoadBalancers(Collections.singletonList(loadBalancer), transaction); + writeLoadBalancers(List.of(loadBalancer), transaction); transaction.commit(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 0828f3369a2..4f0081b6a7f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -48,6 +48,13 @@ public class LoadBalancerProvisioner { this.nodeRepository = nodeRepository; this.db = nodeRepository.database(); this.service = service; + // Read and write all load balancers to make sure they are stored in the latest version of the serialization format + try (var lock = db.lockLoadBalancers()) { + for (var id : db.readLoadBalancerIds()) { + var loadBalancer = db.readLoadBalancer(id); + loadBalancer.ifPresent(db::writeLoadBalancer); + } + } } /** @@ -103,18 +110,19 @@ public class LoadBalancerProvisioner { try (var loadBalancersLock = db.lockLoadBalancers()) { var id = new LoadBalancerId(application, clusterId); var now = nodeRepository.clock().instant(); - var loadBalancer = db.readLoadBalancers().get(id); - if (loadBalancer == null && activate) return; // Nothing to activate as this load balancer was never prepared + var loadBalancer = db.readLoadBalancer(id); + if (loadBalancer.isEmpty() && activate) return; // Nothing to activate as this load balancer was never prepared - var force = loadBalancer != null && loadBalancer.state() != LoadBalancer.State.active; + var force = loadBalancer.isPresent() && loadBalancer.get().state() != LoadBalancer.State.active; var instance = create(application, clusterId, allocatedContainers(application, clusterId), force); - if (loadBalancer == null) { - loadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now); + LoadBalancer newLoadBalancer; + if (loadBalancer.isEmpty()) { + newLoadBalancer = new LoadBalancer(id, instance, LoadBalancer.State.reserved, now); } else { - var newState = activate ? LoadBalancer.State.active : loadBalancer.state(); - loadBalancer = loadBalancer.with(instance).with(newState, now); + var newState = activate ? LoadBalancer.State.active : loadBalancer.get().state(); + newLoadBalancer = loadBalancer.get().with(instance).with(newState, now); } - db.writeLoadBalancer(loadBalancer); + db.writeLoadBalancer(newLoadBalancer); } } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index f02bc99a645..9ddf1ff8ac9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -226,6 +226,8 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl total_matching_time.start(); MatchingStats my_stats; SearchReply::UP reply = std::make_unique<SearchReply>(); + size_t covered = 0; + uint32_t numActiveLids = 0; { // we want to measure full set-up and tear-down time as part of // collateral time GroupingContext groupingContext(_clock, request.getTimeOfDoom(), @@ -289,7 +291,7 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl } reply = std::move(result->_reply); - uint32_t numActiveLids = metaStore.getNumActiveLids(); + numActiveLids = metaStore.getNumActiveLids(); // note: this is actually totalSpace+1, since 0 is reserved uint32_t totalSpace = metaStore.getCommittedDocIdLimit(); LOG(debug, "docid limit = %d", totalSpace); @@ -302,7 +304,7 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl // account for docid 0 reserved spaceEstimate += 1; } - size_t covered = (spaceEstimate * numActiveLids) / totalSpace; + covered = (spaceEstimate * numActiveLids) / totalSpace; LOG(debug, "covered = %zd", covered); SearchReply::Coverage & coverage = reply->coverage; @@ -336,8 +338,9 @@ Matcher::match(const SearchRequest &request, vespalib::ThreadBundle &threadBundl adjustedDuration = 0; } _stats.updatesoftDoomFactor(request.getTimeout(), softLimit, adjustedDuration); - LOG(info, "Triggered softtimeout factor adjustment. request=%1.3f, doomOvertime=%1.3f, limit=%1.3f and duration=%1.3f, rankprofile=%s" + LOG(info, "Triggered softtimeout factor adjustment. Coverage = %lu of %u documents. request=%1.3f, doomOvertime=%1.3f, limit=%1.3f and duration=%1.3f, rankprofile=%s" ", factor adjusted from %1.3f to %1.3f", + covered, numActiveLids, request.getTimeout().sec(), my_stats.doomOvertime().sec(), softLimit.sec(), duration.sec(), request.ranking.c_str(), old, _stats.softDoomFactor()); } diff --git a/searchlib/src/tests/memoryindex/compact_words_store/CMakeLists.txt b/searchlib/src/tests/memoryindex/compact_words_store/CMakeLists.txt index ee31ef7c7aa..754ff796690 100644 --- a/searchlib/src/tests/memoryindex/compact_words_store/CMakeLists.txt +++ b/searchlib/src/tests/memoryindex/compact_words_store/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchlib_compact_words_store_test_app TEST compact_words_store_test.cpp DEPENDS searchlib + gtest ) vespa_add_test(NAME searchlib_compact_words_store_test_app COMMAND searchlib_compact_words_store_test_app) diff --git a/searchlib/src/tests/memoryindex/compact_words_store/compact_words_store_test.cpp b/searchlib/src/tests/memoryindex/compact_words_store/compact_words_store_test.cpp index bda29115db6..52c85a70160 100644 --- a/searchlib/src/tests/memoryindex/compact_words_store/compact_words_store_test.cpp +++ b/searchlib/src/tests/memoryindex/compact_words_store/compact_words_store_test.cpp @@ -1,10 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/memoryindex/compact_words_store.h> #include <vespa/vespalib/datastore/entryref.h> -#include <vespa/vespalib/stllike/string.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/stllike/hash_map.hpp> +#include <vespa/vespalib/stllike/string.h> #include <iostream> #include <map> @@ -13,9 +13,9 @@ using namespace search::datastore; using namespace search::memoryindex; using vespalib::MemoryUsage; -typedef CompactWordsStore::Builder Builder; -typedef CompactWordsStore::Iterator Iterator; -typedef Builder::WordRefVector WordRefVector; +using Builder = CompactWordsStore::Builder; +using Iterator = CompactWordsStore::Iterator; +using WordRefVector = Builder::WordRefVector; const EntryRef w1(1); const EntryRef w2(2); @@ -52,54 +52,52 @@ toStr(Iterator itr) return oss.str(); } -struct SingleFixture -{ - CompactWordsStore _store; - SingleFixture() : _store() { - _store.insert(Builder(d1).insert(w1).insert(w2).insert(w3)); +struct SingleDocumentTest : public ::testing::Test { + CompactWordsStore store; + SingleDocumentTest() : store() { + store.insert(Builder(d1).insert(w1).insert(w2).insert(w3)); } }; -struct MultiFixture -{ - CompactWordsStore _store; - MultiFixture() : _store() { - _store.insert(Builder(d1).insert(w1)); - _store.insert(Builder(d2).insert(w2)); - _store.insert(Builder(d3).insert(w3)); +struct MultiDocumentTest : public ::testing::Test { + CompactWordsStore store; + MultiDocumentTest() : store() { + store.insert(Builder(d1).insert(w1)); + store.insert(Builder(d2).insert(w2)); + store.insert(Builder(d3).insert(w3)); } }; -TEST_F("require that fields and words can be added for a document", SingleFixture) +TEST_F(SingleDocumentTest, fields_and_words_can_be_added_for_a_document) { - EXPECT_EQUAL("[1,2,3]", toStr(f._store.get(d1))); + EXPECT_EQ("[1,2,3]", toStr(store.get(d1))); } -TEST_F("require that multiple documents can be added", MultiFixture) +TEST_F(MultiDocumentTest, multiple_documents_can_be_added) { - EXPECT_EQUAL("[1]", toStr(f._store.get(d1))); - EXPECT_EQUAL("[2]", toStr(f._store.get(d2))); - EXPECT_EQUAL("[3]", toStr(f._store.get(d3))); - EXPECT_FALSE(f._store.get(d4).valid()); + EXPECT_EQ("[1]", toStr(store.get(d1))); + EXPECT_EQ("[2]", toStr(store.get(d2))); + EXPECT_EQ("[3]", toStr(store.get(d3))); + EXPECT_FALSE(store.get(d4).valid()); } -TEST_F("require that documents can be removed", MultiFixture) +TEST_F(MultiDocumentTest, documents_can_be_removed) { - f._store.remove(d2); - EXPECT_TRUE(f._store.get(d1).valid()); - EXPECT_FALSE(f._store.get(d2).valid()); - EXPECT_TRUE(f._store.get(d3).valid()); + store.remove(d2); + EXPECT_TRUE(store.get(d1).valid()); + EXPECT_FALSE(store.get(d2).valid()); + EXPECT_TRUE(store.get(d3).valid()); } -TEST_F("require that documents can be removed and re-inserted", MultiFixture) +TEST_F(MultiDocumentTest, documents_can_be_removed_and_reinserted) { - f._store.remove(d2); - f._store.insert(Builder(d2).insert(w4)); - EXPECT_EQUAL("[4]", toStr(f._store.get(d2))); + store.remove(d2); + store.insert(Builder(d2).insert(w4)); + EXPECT_EQ("[4]", toStr(store.get(d2))); } -TEST("require that a lot of words can be inserted, retrieved and removed") +TEST(CompactWordStoreTest, multiple_words_can_be_inserted_retrieved_and_removed) { CompactWordsStore store; for (uint32_t docId = 0; docId < 50; ++docId) { @@ -113,10 +111,10 @@ TEST("require that a lot of words can be inserted, retrieved and removed") } for (uint32_t docId = 0; docId < 50; ++docId) { WordRefVector words = build(store.get(docId)); - EXPECT_EQUAL(20000u, words.size()); + EXPECT_EQ(20000u, words.size()); uint32_t wordRef = 0; for (auto word : words) { - EXPECT_EQUAL(wordRef++, word.ref()); + EXPECT_EQ(wordRef++, word.ref()); } store.remove(docId); MemoryUsage usage = store.getMemoryUsage(); @@ -124,7 +122,7 @@ TEST("require that a lot of words can be inserted, retrieved and removed") } } -TEST("require that initial memory usage is reported") +TEST(CompactWordStoreTest, initial_memory_usage_is_reported) { CompactWordsStore store; CompactWordsStore::DocumentWordsMap docs; @@ -134,24 +132,24 @@ TEST("require that initial memory usage is reported") initExp.incUsedBytes(docs.getMemoryUsed()); initExp.merge(internalStore.getMemoryUsage()); MemoryUsage init = store.getMemoryUsage(); - EXPECT_EQUAL(initExp.allocatedBytes(), init.allocatedBytes()); - EXPECT_EQUAL(initExp.usedBytes(), init.usedBytes()); - EXPECT_GREATER(init.allocatedBytes(), init.usedBytes()); - EXPECT_GREATER(init.allocatedBytes(), 0u); - EXPECT_GREATER(init.usedBytes(), 0u); + EXPECT_EQ(initExp.allocatedBytes(), init.allocatedBytes()); + EXPECT_EQ(initExp.usedBytes(), init.usedBytes()); + EXPECT_GT(init.allocatedBytes(), init.usedBytes()); + EXPECT_GT(init.allocatedBytes(), 0u); + EXPECT_GT(init.usedBytes(), 0u); } -TEST("require that memory usage is updated after insert") +TEST(CompactWordStoreTest, memory_usage_is_updated_after_insert) { CompactWordsStore store; MemoryUsage init = store.getMemoryUsage(); store.insert(Builder(d1).insert(w1)); MemoryUsage after = store.getMemoryUsage(); - EXPECT_GREATER_EQUAL(after.allocatedBytes(), init.allocatedBytes()); - EXPECT_GREATER(after.usedBytes(), init.usedBytes()); + EXPECT_GE(after.allocatedBytes(), init.allocatedBytes()); + EXPECT_GT(after.usedBytes(), init.usedBytes()); } +GTEST_MAIN_RUN_ALL_TESTS() -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/memoryindex/datastore/CMakeLists.txt b/searchlib/src/tests/memoryindex/datastore/CMakeLists.txt index 45507f3b0ae..be1a193cd3c 100644 --- a/searchlib/src/tests/memoryindex/datastore/CMakeLists.txt +++ b/searchlib/src/tests/memoryindex/datastore/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_executable(searchlib_feature_store_test_app TEST feature_store_test.cpp DEPENDS searchlib + gtest ) vespa_add_test(NAME searchlib_feature_store_test_app COMMAND searchlib_feature_store_test_app) vespa_add_executable(searchlib_word_store_test_app TEST @@ -11,5 +12,6 @@ vespa_add_executable(searchlib_word_store_test_app TEST word_store_test.cpp DEPENDS searchlib + gtest ) vespa_add_test(NAME searchlib_word_store_test_app COMMAND searchlib_word_store_test_app) diff --git a/searchlib/src/tests/memoryindex/datastore/feature_store_test.cpp b/searchlib/src/tests/memoryindex/datastore/feature_store_test.cpp index aca83d67a8a..c6368bee6eb 100644 --- a/searchlib/src/tests/memoryindex/datastore/feature_store_test.cpp +++ b/searchlib/src/tests/memoryindex/datastore/feature_store_test.cpp @@ -1,8 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/searchlib/memoryindex/feature_store.h> +#include <vespa/vespalib/gtest/gtest.h> + #include <vespa/log/log.h> LOG_SETUP("feature_store_test"); -#include <vespa/vespalib/testkit/testapp.h> -#include <vespa/searchlib/memoryindex/feature_store.h> using namespace search::btree; using namespace search::datastore; @@ -11,62 +13,55 @@ using namespace search::index; using search::index::schema::CollectionType; using search::index::schema::DataType; -namespace search -{ - +namespace search::memoryindex { -namespace memoryindex -{ +class FeatureStoreTest : public ::testing::Test { +public: + Schema schema; + FeatureStore fs; + Schema make_schema() const; + FeatureStoreTest(); +}; -class Test : public vespalib::TestApp +Schema +FeatureStoreTest::make_schema() const { -private: - Schema _schema; - - const Schema & getSchema() const { return _schema; } - bool assertFeatures(const DocIdAndFeatures &exp, const DocIdAndFeatures &act); - void requireThatFeaturesCanBeAddedAndRetrieved(); - void requireThatNextWordsAreWorking(); - void requireThatAddFeaturesTriggersChangeOfBuffer(); - -public: - Test(); - int Main() override; -}; + Schema result; + result.addIndexField(Schema::IndexField("f0", DataType::STRING)); + result.addIndexField(Schema::IndexField("f1", DataType::STRING, CollectionType::WEIGHTEDSET)); + return result; +} +FeatureStoreTest::FeatureStoreTest() + : schema(make_schema()), + fs(schema) +{ +} -bool -Test::assertFeatures(const DocIdAndFeatures &exp, - const DocIdAndFeatures &act) +void +assertFeatures(const DocIdAndFeatures& exp, + const DocIdAndFeatures& act) { // docid is not encoded as part of features - if (!EXPECT_EQUAL(exp.elements().size(), - act.elements().size())) - return false; + ASSERT_EQ(exp.elements().size(), + act.elements().size()); for (size_t i = 0; i < exp.elements().size(); ++i) { - if (!EXPECT_EQUAL(exp.elements()[i].getElementId(), - act.elements()[i].getElementId())) - return false; - if (!EXPECT_EQUAL(exp.elements()[i].getNumOccs(), - act.elements()[i].getNumOccs())) - return false; - if (!EXPECT_EQUAL(exp.elements()[i].getWeight(), act.elements()[i].getWeight())) - return false; - if (!EXPECT_EQUAL(exp.elements()[i].getElementLen(), - act.elements()[i].getElementLen())) - return false; + EXPECT_EQ(exp.elements()[i].getElementId(), + act.elements()[i].getElementId()); + EXPECT_EQ(exp.elements()[i].getNumOccs(), + act.elements()[i].getNumOccs()); + EXPECT_EQ(exp.elements()[i].getWeight(), act.elements()[i].getWeight()); + EXPECT_EQ(exp.elements()[i].getElementLen(), + act.elements()[i].getElementLen()); } - if (!EXPECT_EQUAL(exp.word_positions().size(), act.word_positions().size())) - return false; + ASSERT_EQ(exp.word_positions().size(), act.word_positions().size()); for (size_t i = 0; i < exp.word_positions().size(); ++i) { - if (!EXPECT_EQUAL(exp.word_positions()[i].getWordPos(), - act.word_positions()[i].getWordPos())) return false; + EXPECT_EQ(exp.word_positions()[i].getWordPos(), + act.word_positions()[i].getWordPos()); } - return true; } - DocIdAndFeatures getFeatures(uint32_t numOccs, int32_t weight, @@ -84,11 +79,8 @@ getFeatures(uint32_t numOccs, return f; } - -void -Test::requireThatFeaturesCanBeAddedAndRetrieved() +TEST_F(FeatureStoreTest, features_can_be_added_and_retrieved) { - FeatureStore fs(getSchema()); DocIdAndFeatures act; EntryRef r1; EntryRef r2; @@ -98,9 +90,9 @@ Test::requireThatFeaturesCanBeAddedAndRetrieved() r = fs.addFeatures(0, f); r1 = r.first; EXPECT_TRUE(r.second > 0); - EXPECT_EQUAL(FeatureStore::RefType::align(1u), - FeatureStore::RefType(r1).offset()); - EXPECT_EQUAL(0u, FeatureStore::RefType(r1).bufferId()); + EXPECT_EQ(FeatureStore::RefType::align(1u), + FeatureStore::RefType(r1).offset()); + EXPECT_EQ(0u, FeatureStore::RefType(r1).bufferId()); LOG(info, "bits(%" PRIu64 "), ref.offset(%zu), ref.bufferId(%u)", r.second, @@ -108,7 +100,7 @@ Test::requireThatFeaturesCanBeAddedAndRetrieved() FeatureStore::RefType(r1).bufferId()); fs.getFeatures(0, r1, act); // weight not encoded for single value - EXPECT_TRUE(assertFeatures(getFeatures(2, 1, 8), act)); + ASSERT_NO_FATAL_FAILURE(assertFeatures(getFeatures(2, 1, 8), act)); } { DocIdAndFeatures f = getFeatures(4, 8, 16); @@ -117,22 +109,19 @@ Test::requireThatFeaturesCanBeAddedAndRetrieved() EXPECT_TRUE(r.second > 0); EXPECT_TRUE(FeatureStore::RefType(r2).offset() > FeatureStore::RefType(r1).offset()); - EXPECT_EQUAL(0u, FeatureStore::RefType(r1).bufferId()); + EXPECT_EQ(0u, FeatureStore::RefType(r1).bufferId()); LOG(info, "bits(%" PRIu64 "), ref.offset(%zu), ref.bufferId(%u)", r.second, FeatureStore::RefType(r2).offset(), FeatureStore::RefType(r2).bufferId()); fs.getFeatures(1, r2, act); - EXPECT_TRUE(assertFeatures(f, act)); + ASSERT_NO_FATAL_FAILURE(assertFeatures(f, act)); } } - -void -Test::requireThatNextWordsAreWorking() +TEST_F(FeatureStoreTest, next_words_are_working) { - FeatureStore fs(getSchema()); DocIdAndFeatures act; EntryRef r1; EntryRef r2; @@ -142,9 +131,9 @@ Test::requireThatNextWordsAreWorking() r = fs.addFeatures(0, f); r1 = r.first; EXPECT_TRUE(r.second > 0); - EXPECT_EQUAL(FeatureStore::RefType::align(1u), - FeatureStore::RefType(r1).offset()); - EXPECT_EQUAL(0u, FeatureStore::RefType(r1).bufferId()); + EXPECT_EQ(FeatureStore::RefType::align(1u), + FeatureStore::RefType(r1).offset()); + EXPECT_EQ(0u, FeatureStore::RefType(r1).bufferId()); LOG(info, "bits(%" PRIu64 "), ref.offset(%zu), ref.bufferId(%u)", r.second, @@ -152,7 +141,7 @@ Test::requireThatNextWordsAreWorking() FeatureStore::RefType(r1).bufferId()); fs.getFeatures(0, r1, act); // weight not encoded for single value - EXPECT_TRUE(assertFeatures(getFeatures(2, 1, 8), act)); + ASSERT_NO_FATAL_FAILURE(assertFeatures(getFeatures(2, 1, 8), act)); } { DocIdAndFeatures f = getFeatures(4, 8, 16); @@ -161,22 +150,19 @@ Test::requireThatNextWordsAreWorking() EXPECT_TRUE(r.second > 0); EXPECT_TRUE(FeatureStore::RefType(r2).offset() > FeatureStore::RefType(r1).offset()); - EXPECT_EQUAL(0u, FeatureStore::RefType(r1).bufferId()); + EXPECT_EQ(0u, FeatureStore::RefType(r1).bufferId()); LOG(info, "bits(%" PRIu64 "), ref.offset(%zu), ref.bufferId(%u)", r.second, FeatureStore::RefType(r2).offset(), FeatureStore::RefType(r2).bufferId()); fs.getFeatures(1, r2, act); - EXPECT_TRUE(assertFeatures(f, act)); + ASSERT_NO_FATAL_FAILURE(assertFeatures(f, act)); } } - -void -Test::requireThatAddFeaturesTriggersChangeOfBuffer() +TEST_F(FeatureStoreTest, add_features_triggers_change_of_buffer) { - FeatureStore fs(getSchema()); size_t cnt = 1; DocIdAndFeatures act; uint32_t lastId = 0; @@ -185,7 +171,7 @@ Test::requireThatAddFeaturesTriggersChangeOfBuffer() DocIdAndFeatures f = getFeatures(numOccs, 1, numOccs + 1); std::pair<EntryRef, uint64_t> r = fs.addFeatures(0, f); fs.getFeatures(0, r.first, act); - EXPECT_TRUE(assertFeatures(f, act)); + ASSERT_NO_FATAL_FAILURE(assertFeatures(f, act)); uint32_t bufferId = FeatureStore::RefType(r.first).bufferId(); if (bufferId > lastId) { LOG(info, @@ -197,36 +183,10 @@ Test::requireThatAddFeaturesTriggersChangeOfBuffer() break; } } - EXPECT_EQUAL(1u, lastId); + EXPECT_EQ(1u, lastId); LOG(info, "Added %zu feature sets in 1 buffer", cnt); } - -Test::Test() - : _schema() -{ - _schema.addIndexField(Schema::IndexField("f0", DataType::STRING)); - _schema.addIndexField(Schema::IndexField("f1", DataType::STRING, CollectionType::WEIGHTEDSET)); } - -int -Test::Main() -{ - TEST_INIT("feature_store_test"); - - requireThatFeaturesCanBeAddedAndRetrieved(); - requireThatNextWordsAreWorking(); - requireThatAddFeaturesTriggersChangeOfBuffer(); - - TEST_DONE(); -} - - -} - - -} - - -TEST_APPHOOK(search::memoryindex::Test); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/memoryindex/datastore/word_store_test.cpp b/searchlib/src/tests/memoryindex/datastore/word_store_test.cpp index b7f454bfdf7..86365287b29 100644 --- a/searchlib/src/tests/memoryindex/datastore/word_store_test.cpp +++ b/searchlib/src/tests/memoryindex/datastore/word_store_test.cpp @@ -1,24 +1,15 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/searchlib/memoryindex/word_store.h> +#include <vespa/vespalib/gtest/gtest.h> + #include <vespa/log/log.h> LOG_SETUP("word_store_test"); -#include <vespa/vespalib/testkit/testapp.h> -#include <vespa/searchlib/memoryindex/word_store.h> using namespace search::datastore; -namespace search { -namespace memoryindex { +namespace search::memoryindex { -class Test : public vespalib::TestApp { -private: - void requireThatWordsCanBeAddedAndRetrieved(); - void requireThatAddWordTriggersChangeOfBuffer(); -public: - int Main() override; -}; - -void -Test::requireThatWordsCanBeAddedAndRetrieved() +TEST(WordStoreTest, words_can_be_added_and_retrieved) { std::string w1 = "require"; std::string w2 = "that"; @@ -32,19 +23,18 @@ Test::requireThatWordsCanBeAddedAndRetrieved() uint32_t w1p = WordStore::RefType::pad(w1s); uint32_t w2s = w2.size() + 1; uint32_t w2p = WordStore::RefType::pad(w2s); - EXPECT_EQUAL(invp, WordStore::RefType(r1).offset()); - EXPECT_EQUAL(invp + w1s + w1p, WordStore::RefType(r2).offset()); - EXPECT_EQUAL(invp + w1s + w1p + w2s + w2p, WordStore::RefType(r3).offset()); - EXPECT_EQUAL(0u, WordStore::RefType(r1).bufferId()); - EXPECT_EQUAL(0u, WordStore::RefType(r2).bufferId()); - EXPECT_EQUAL(0u, WordStore::RefType(r3).bufferId()); - EXPECT_EQUAL(std::string("require"), ws.getWord(r1)); - EXPECT_EQUAL(std::string("that"), ws.getWord(r2)); - EXPECT_EQUAL(std::string("words"), ws.getWord(r3)); + EXPECT_EQ(invp, WordStore::RefType(r1).offset()); + EXPECT_EQ(invp + w1s + w1p, WordStore::RefType(r2).offset()); + EXPECT_EQ(invp + w1s + w1p + w2s + w2p, WordStore::RefType(r3).offset()); + EXPECT_EQ(0u, WordStore::RefType(r1).bufferId()); + EXPECT_EQ(0u, WordStore::RefType(r2).bufferId()); + EXPECT_EQ(0u, WordStore::RefType(r3).bufferId()); + EXPECT_EQ(std::string("require"), ws.getWord(r1)); + EXPECT_EQ(std::string("that"), ws.getWord(r2)); + EXPECT_EQ(std::string("words"), ws.getWord(r3)); } -void -Test::requireThatAddWordTriggersChangeOfBuffer() +TEST(WordStoreTest, add_word_triggers_change_of_buffer) { WordStore ws; size_t word = 0; @@ -54,7 +44,7 @@ Test::requireThatAddWordTriggersChangeOfBuffer() sprintf(wordStr, "%6zu", word); // all words uses 12 bytes (include padding) EntryRef r = ws.addWord(std::string(wordStr)); - EXPECT_EQUAL(std::string(wordStr), ws.getWord(r)); + EXPECT_EQ(std::string(wordStr), ws.getWord(r)); uint32_t bufferId = WordStore::RefType(r).bufferId(); if (bufferId > lastId) { LOG(info, @@ -68,23 +58,11 @@ Test::requireThatAddWordTriggersChangeOfBuffer() } } LOG(info, "Added %zu words in 4 buffers", word); - EXPECT_EQUAL(2047u, word); - EXPECT_EQUAL(4u, lastId); -} - -int -Test::Main() -{ - TEST_INIT("word_store_test"); - - requireThatWordsCanBeAddedAndRetrieved(); - requireThatAddWordTriggersChangeOfBuffer(); - - TEST_DONE(); + EXPECT_EQ(2047u, word); + EXPECT_EQ(4u, lastId); } } -} -TEST_APPHOOK(search::memoryindex::Test); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/memoryindex/document_inverter/CMakeLists.txt b/searchlib/src/tests/memoryindex/document_inverter/CMakeLists.txt index 1058a19d0ce..ecf33ee48fd 100644 --- a/searchlib/src/tests/memoryindex/document_inverter/CMakeLists.txt +++ b/searchlib/src/tests/memoryindex/document_inverter/CMakeLists.txt @@ -5,5 +5,6 @@ vespa_add_executable(searchlib_document_inverter_test_app TEST DEPENDS searchlib_test searchlib + gtest ) vespa_add_test(NAME searchlib_document_inverter_test_app COMMAND searchlib_document_inverter_test_app) diff --git a/searchlib/src/tests/memoryindex/document_inverter/document_inverter_test.cpp b/searchlib/src/tests/memoryindex/document_inverter/document_inverter_test.cpp index 08645f38712..38862dfe94b 100644 --- a/searchlib/src/tests/memoryindex/document_inverter/document_inverter_test.cpp +++ b/searchlib/src/tests/memoryindex/document_inverter/document_inverter_test.cpp @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/* -*- mode: C++; coding: utf-8; -*- */ - +#include <vespa/searchlib/common/sequencedtaskexecutor.h> #include <vespa/searchlib/index/docbuilder.h> #include <vespa/searchlib/index/field_length_calculator.h> #include <vespa/searchlib/memoryindex/document_inverter.h> @@ -10,8 +9,7 @@ #include <vespa/searchlib/memoryindex/i_field_index_collection.h> #include <vespa/searchlib/memoryindex/word_store.h> #include <vespa/searchlib/test/memoryindex/ordered_field_index_inserter.h> -#include <vespa/searchlib/common/sequencedtaskexecutor.h> -#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/gtest/gtest.h> namespace search { @@ -37,7 +35,6 @@ makeDoc10(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc11(DocBuilder &b) { @@ -51,7 +48,6 @@ makeDoc11(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc12(DocBuilder &b) { @@ -62,7 +58,6 @@ makeDoc12(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc13(DocBuilder &b) { @@ -73,7 +68,6 @@ makeDoc13(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc14(DocBuilder &b) { @@ -84,7 +78,6 @@ makeDoc14(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc15(DocBuilder &b) { @@ -94,8 +87,7 @@ makeDoc15(DocBuilder &b) } -class MockFieldIndexCollection : public IFieldIndexCollection -{ +class MockFieldIndexCollection : public IFieldIndexCollection { FieldIndexRemover &_remover; test::OrderedFieldIndexInserter &_inserter; FieldLengthCalculator &_calculator; @@ -122,8 +114,7 @@ public: }; -struct Fixture -{ +struct DocumentInverterTest : public ::testing::Test { Schema _schema; DocBuilder _b; SequencedTaskExecutor _invertThreads; @@ -135,9 +126,7 @@ struct Fixture MockFieldIndexCollection _fic; DocumentInverter _inv; - static Schema - makeSchema() - { + static Schema makeSchema() { Schema schema; schema.addIndexField(Schema::IndexField("f0", DataType::STRING)); schema.addIndexField(Schema::IndexField("f1", DataType::STRING)); @@ -146,7 +135,7 @@ struct Fixture return schema; } - Fixture() + DocumentInverterTest() : _schema(makeSchema()), _b(_schema), _invertThreads(2), @@ -160,9 +149,7 @@ struct Fixture { } - void - pushDocuments() - { + void pushDocuments() { _invertThreads.sync(); uint32_t fieldId = 0; for (auto &inverter : _inv.getInverters()) { @@ -174,153 +161,143 @@ struct Fixture } }; - -TEST_F("requireThatFreshInsertWorks", Fixture) +TEST_F(DocumentInverterTest, require_that_fresh_insert_works) { - f._inv.invertDocument(10, *makeDoc10(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=c,a=10," - "w=d,a=10", - f._inserter.toStr()); + _inv.invertDocument(10, *makeDoc10(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=c,a=10," + "w=d,a=10", + _inserter.toStr()); } - -TEST_F("requireThatMultipleDocsWork", Fixture) +TEST_F(DocumentInverterTest, require_that_multiple_docs_work) { - f._inv.invertDocument(10, *makeDoc10(f._b)); - f._inv.invertDocument(11, *makeDoc11(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10,a=11," - "w=b,a=10,a=11," - "w=c,a=10,w=d,a=10," - "w=e,a=11," - "w=f,a=11," - "f=1,w=a,a=11," - "w=g,a=11", - f._inserter.toStr()); + _inv.invertDocument(10, *makeDoc10(_b)); + _inv.invertDocument(11, *makeDoc11(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10,a=11," + "w=b,a=10,a=11," + "w=c,a=10,w=d,a=10," + "w=e,a=11," + "w=f,a=11," + "f=1,w=a,a=11," + "w=g,a=11", + _inserter.toStr()); } - -TEST_F("requireThatRemoveWorks", Fixture) +TEST_F(DocumentInverterTest, require_that_remove_works) { - f._inv.getInverter(0)->remove("b", 10); - f._inv.getInverter(0)->remove("a", 10); - f._inv.getInverter(0)->remove("b", 11); - f._inv.getInverter(2)->remove("c", 12); - f._inv.getInverter(1)->remove("a", 10); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,r=10," - "w=b,r=10,r=11," - "f=1,w=a,r=10," - "f=2,w=c,r=12", - f._inserter.toStr()); + _inv.getInverter(0)->remove("b", 10); + _inv.getInverter(0)->remove("a", 10); + _inv.getInverter(0)->remove("b", 11); + _inv.getInverter(2)->remove("c", 12); + _inv.getInverter(1)->remove("a", 10); + pushDocuments(); + EXPECT_EQ("f=0,w=a,r=10," + "w=b,r=10,r=11," + "f=1,w=a,r=10," + "f=2,w=c,r=12", + _inserter.toStr()); } - -TEST_F("requireThatReputWorks", Fixture) +TEST_F(DocumentInverterTest, require_that_reput_works) { - f._inv.invertDocument(10, *makeDoc10(f._b)); - f._inv.invertDocument(10, *makeDoc11(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=e,a=10," - "w=f,a=10," - "f=1,w=a,a=10," - "w=g,a=10", - f._inserter.toStr()); + _inv.invertDocument(10, *makeDoc10(_b)); + _inv.invertDocument(10, *makeDoc11(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=e,a=10," + "w=f,a=10," + "f=1,w=a,a=10," + "w=g,a=10", + _inserter.toStr()); } - -TEST_F("requireThatAbortPendingDocWorks", Fixture) +TEST_F(DocumentInverterTest, require_that_abort_pending_doc_works) { - Document::UP doc10 = makeDoc10(f._b); - Document::UP doc11 = makeDoc11(f._b); - Document::UP doc12 = makeDoc12(f._b); - Document::UP doc13 = makeDoc13(f._b); - Document::UP doc14 = makeDoc14(f._b); - - f._inv.invertDocument(10, *doc10); - f._inv.invertDocument(11, *doc11); - f._inv.removeDocument(10); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=11," - "w=b,a=11," - "w=e,a=11," - "w=f,a=11," - "f=1,w=a,a=11," - "w=g,a=11", - f._inserter.toStr()); - - f._inv.invertDocument(10, *doc10); - f._inv.invertDocument(11, *doc11); - f._inv.invertDocument(12, *doc12); - f._inv.invertDocument(13, *doc13); - f._inv.invertDocument(14, *doc14); - f._inv.removeDocument(11); - f._inv.removeDocument(13); - f._inserter.reset(); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=c,a=10," - "w=d,a=10," - "w=doc12,a=12," - "w=doc14,a=14," - "w=h,a=12," - "w=j,a=14", - f._inserter.toStr()); - - f._inv.invertDocument(10, *doc10); - f._inv.invertDocument(11, *doc11); - f._inv.invertDocument(12, *doc12); - f._inv.invertDocument(13, *doc13); - f._inv.invertDocument(14, *doc14); - f._inv.removeDocument(11); - f._inv.removeDocument(12); - f._inv.removeDocument(13); - f._inv.removeDocument(14); - f._inserter.reset(); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=c,a=10," - "w=d,a=10", - f._inserter.toStr()); - - + auto doc10 = makeDoc10(_b); + auto doc11 = makeDoc11(_b); + auto doc12 = makeDoc12(_b); + auto doc13 = makeDoc13(_b); + auto doc14 = makeDoc14(_b); + + _inv.invertDocument(10, *doc10); + _inv.invertDocument(11, *doc11); + _inv.removeDocument(10); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=11," + "w=b,a=11," + "w=e,a=11," + "w=f,a=11," + "f=1,w=a,a=11," + "w=g,a=11", + _inserter.toStr()); + + _inv.invertDocument(10, *doc10); + _inv.invertDocument(11, *doc11); + _inv.invertDocument(12, *doc12); + _inv.invertDocument(13, *doc13); + _inv.invertDocument(14, *doc14); + _inv.removeDocument(11); + _inv.removeDocument(13); + _inserter.reset(); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=c,a=10," + "w=d,a=10," + "w=doc12,a=12," + "w=doc14,a=14," + "w=h,a=12," + "w=j,a=14", + _inserter.toStr()); + + _inv.invertDocument(10, *doc10); + _inv.invertDocument(11, *doc11); + _inv.invertDocument(12, *doc12); + _inv.invertDocument(13, *doc13); + _inv.invertDocument(14, *doc14); + _inv.removeDocument(11); + _inv.removeDocument(12); + _inv.removeDocument(13); + _inv.removeDocument(14); + _inserter.reset(); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=c,a=10," + "w=d,a=10", + _inserter.toStr()); } - -TEST_F("requireThatMixOfAddAndRemoveWorks", Fixture) +TEST_F(DocumentInverterTest, require_that_mix_of_add_and_remove_works) { - f._inv.getInverter(0)->remove("a", 11); - f._inv.getInverter(0)->remove("c", 9); - f._inv.getInverter(0)->remove("d", 10); - f._inv.getInverter(0)->remove("z", 12); - f._inv.invertDocument(10, *makeDoc10(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10,r=11," - "w=b,a=10," - "w=c,r=9,a=10," - "w=d,r=10,a=10," - "w=z,r=12", - f._inserter.toStr()); + _inv.getInverter(0)->remove("a", 11); + _inv.getInverter(0)->remove("c", 9); + _inv.getInverter(0)->remove("d", 10); + _inv.getInverter(0)->remove("z", 12); + _inv.invertDocument(10, *makeDoc10(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10,r=11," + "w=b,a=10," + "w=c,r=9,a=10," + "w=d,r=10,a=10," + "w=z,r=12", + _inserter.toStr()); } - -TEST_F("require that empty document can be inverted", Fixture) +TEST_F(DocumentInverterTest, require_that_empty_document_can_be_inverted) { - f._inv.invertDocument(15, *makeDoc15(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", - f._inserter.toStr()); + _inv.invertDocument(15, *makeDoc15(_b)); + pushDocuments(); + EXPECT_EQ("", + _inserter.toStr()); } +} +} -} // namespace memoryindex -} // namespace search - -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/memoryindex/field_index_remover/CMakeLists.txt b/searchlib/src/tests/memoryindex/field_index_remover/CMakeLists.txt index ef75337c6b6..f18b4ba29cd 100644 --- a/searchlib/src/tests/memoryindex/field_index_remover/CMakeLists.txt +++ b/searchlib/src/tests/memoryindex/field_index_remover/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(searchlib_field_index_remover_test_app TEST field_index_remover_test.cpp DEPENDS searchlib + gtest ) vespa_add_test(NAME searchlib_field_index_remover_test_app COMMAND searchlib_field_index_remover_test_app) diff --git a/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp b/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp index fed6d963b70..c0e8871b80a 100644 --- a/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp +++ b/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp @@ -1,10 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> - #include <vespa/searchlib/memoryindex/field_index_remover.h> #include <vespa/searchlib/memoryindex/i_field_index_remove_listener.h> #include <vespa/searchlib/memoryindex/word_store.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/test/insertion_operators.h> #include <algorithm> @@ -14,8 +13,7 @@ LOG_SETUP("document_remover_test"); using namespace search; using namespace search::memoryindex; -struct WordFieldPair -{ +struct WordFieldPair { vespalib::string _word; uint32_t _fieldId; WordFieldPair(vespalib::stringref word, uint32_t fieldId) @@ -29,7 +27,7 @@ struct WordFieldPair } }; -typedef std::vector<WordFieldPair> WordFieldVector; +using WordFieldVector = std::vector<WordFieldPair>; std::ostream & operator<<(std::ostream &os, const WordFieldPair &val) @@ -38,13 +36,12 @@ operator<<(std::ostream &os, const WordFieldPair &val) return os; } -struct MockRemoveListener : public IFieldIndexRemoveListener -{ +struct MockRemoveListener : public IFieldIndexRemoveListener { WordFieldVector _words; uint32_t _expDocId; uint32_t _fieldId; virtual void remove(const vespalib::stringref word, uint32_t docId) override { - EXPECT_EQUAL(_expDocId, docId); + EXPECT_EQ(_expDocId, docId); _words.emplace_back(word, _fieldId); } void reset(uint32_t expDocId) { @@ -60,13 +57,13 @@ struct MockRemoveListener : public IFieldIndexRemoveListener void setFieldId(uint32_t fieldId) { _fieldId = fieldId; } }; -struct Fixture -{ +struct FieldIndexRemoverTest : public ::testing::Test { MockRemoveListener _listener; std::vector<std::unique_ptr<WordStore>> _wordStores; std::vector<std::map<vespalib::string, datastore::EntryRef>> _wordToRefMaps; std::vector<std::unique_ptr<FieldIndexRemover>> _removers; - Fixture() + + FieldIndexRemoverTest() : _listener(), _wordStores(), _wordToRefMaps(), @@ -91,7 +88,7 @@ struct Fixture } return itr->second; } - Fixture &insert(const vespalib::string &word, uint32_t fieldId, uint32_t docId) { + FieldIndexRemoverTest &insert(const vespalib::string &word, uint32_t fieldId, uint32_t docId) { assert(fieldId < _wordStores.size()); _removers[fieldId]->insert(getWordRef(word, fieldId), docId); return *this; @@ -113,32 +110,31 @@ struct Fixture } }; -TEST_F("require that {word,fieldId} pairs for multiple doc ids can be inserted", Fixture) +TEST_F(FieldIndexRemoverTest, word_field_id_pairs_for_multiple_doc_ids_can_be_inserted) { - f.insert("a", 1, 10).insert("a", 1, 20).insert("a", 1, 30); - f.insert("a", 2, 10).insert("a", 2, 20); - f.insert("b", 1, 20).insert("b", 1, 30); - f.insert("b", 2, 10).insert("b", 2, 30); - f.insert("c", 1, 10); - f.insert("c", 2, 20); - f.insert("c", 3, 30); - f.flush(); + insert("a", 1, 10).insert("a", 1, 20).insert("a", 1, 30); + insert("a", 2, 10).insert("a", 2, 20); + insert("b", 1, 20).insert("b", 1, 30); + insert("b", 2, 10).insert("b", 2, 30); + insert("c", 1, 10); + insert("c", 2, 20); + insert("c", 3, 30); + flush(); - EXPECT_EQUAL("[{a,1},{a,2},{b,2},{c,1}]", f.remove(10)); - EXPECT_EQUAL("[{a,1},{a,2},{b,1},{c,2}]", f.remove(20)); - EXPECT_EQUAL("[{a,1},{b,1},{b,2},{c,3}]", f.remove(30)); + EXPECT_EQ("[{a,1},{a,2},{b,2},{c,1}]", remove(10)); + EXPECT_EQ("[{a,1},{a,2},{b,1},{c,2}]", remove(20)); + EXPECT_EQ("[{a,1},{b,1},{b,2},{c,3}]", remove(30)); } -TEST_F("require that we can insert after flush", Fixture) +TEST_F(FieldIndexRemoverTest, we_can_insert_after_flush) { - f.insert("a", 1, 10).insert("b", 1, 10); - f.flush(); - f.insert("b", 1, 20).insert("b", 2, 20); - f.flush(); + insert("a", 1, 10).insert("b", 1, 10); + flush(); + insert("b", 1, 20).insert("b", 2, 20); + flush(); - EXPECT_EQUAL("[{a,1},{b,1}]", f.remove(10)); - EXPECT_EQUAL("[{b,1},{b,2}]", f.remove(20)); + EXPECT_EQ("[{a,1},{b,1}]", remove(10)); + EXPECT_EQ("[{b,1},{b,2}]", remove(20)); } - -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/memoryindex/field_inverter/CMakeLists.txt b/searchlib/src/tests/memoryindex/field_inverter/CMakeLists.txt index f39e05d6823..6fefada6570 100644 --- a/searchlib/src/tests/memoryindex/field_inverter/CMakeLists.txt +++ b/searchlib/src/tests/memoryindex/field_inverter/CMakeLists.txt @@ -5,5 +5,6 @@ vespa_add_executable(searchlib_field_inverter_test_app TEST DEPENDS searchlib_test searchlib + gtest ) vespa_add_test(NAME searchlib_field_inverter_test_app COMMAND searchlib_field_inverter_test_app) diff --git a/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp b/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp index ff0629d2172..72a8f6ed239 100644 --- a/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp +++ b/searchlib/src/tests/memoryindex/field_inverter/field_inverter_test.cpp @@ -7,7 +7,7 @@ #include <vespa/searchlib/memoryindex/field_inverter.h> #include <vespa/searchlib/memoryindex/word_store.h> #include <vespa/searchlib/test/memoryindex/ordered_field_index_inserter.h> -#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/gtest/gtest.h> namespace search { @@ -21,10 +21,8 @@ using namespace index; namespace memoryindex { - namespace { - Document::UP makeDoc10(DocBuilder &b) { @@ -35,7 +33,6 @@ makeDoc10(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc11(DocBuilder &b) { @@ -49,7 +46,6 @@ makeDoc11(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc12(DocBuilder &b) { @@ -60,7 +56,6 @@ makeDoc12(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc13(DocBuilder &b) { @@ -71,7 +66,6 @@ makeDoc13(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc14(DocBuilder &b) { @@ -82,7 +76,6 @@ makeDoc14(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc15(DocBuilder &b) { @@ -90,7 +83,6 @@ makeDoc15(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc16(DocBuilder &b) { @@ -113,8 +105,7 @@ makeDoc17(DocBuilder &b) } -struct Fixture -{ +struct FieldInverterTest : public ::testing::Test { Schema _schema; DocBuilder _b; WordStore _word_store; @@ -123,9 +114,7 @@ struct Fixture std::vector<std::unique_ptr<FieldLengthCalculator>> _calculators; std::vector<std::unique_ptr<FieldInverter> > _inverters; - static Schema - makeSchema() - { + static Schema makeSchema() { Schema schema; schema.addIndexField(Schema::IndexField("f0", DataType::STRING)); schema.addIndexField(Schema::IndexField("f1", DataType::STRING)); @@ -134,7 +123,7 @@ struct Fixture return schema; } - Fixture() + FieldInverterTest() : _schema(makeSchema()), _b(_schema), _word_store(), @@ -154,9 +143,7 @@ struct Fixture } } - void - invertDocument(uint32_t docId, const Document &doc) - { + void invertDocument(uint32_t docId, const Document &doc) { uint32_t fieldId = 0; for (auto &inverter : _inverters) { vespalib::stringref fieldName = @@ -166,9 +153,7 @@ struct Fixture } } - void - pushDocuments() - { + void pushDocuments() { uint32_t fieldId = 0; for (auto &inverter : _inverters) { _inserter.setFieldId(fieldId); @@ -177,218 +162,207 @@ struct Fixture } } - void - removeDocument(uint32_t docId) { + void removeDocument(uint32_t docId) { for (auto &inverter : _inverters) { inverter->removeDocument(docId); } } void assert_calculator(uint32_t field_id, double exp_avg, uint32_t exp_samples) { - double epsilon = 0.000000001; const auto &calc = *_calculators[field_id]; - EXPECT_APPROX(exp_avg, calc.get_average_field_length(), epsilon); - EXPECT_EQUAL(exp_samples, calc.get_num_samples()); + EXPECT_DOUBLE_EQ(exp_avg, calc.get_average_field_length()); + EXPECT_EQ(exp_samples, calc.get_num_samples()); } }; - -TEST_F("requireThatFreshInsertWorks", Fixture) +TEST_F(FieldInverterTest, require_that_fresh_insert_works) { - f.invertDocument(10, *makeDoc10(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=c,a=10," - "w=d,a=10", - f._inserter.toStr()); + invertDocument(10, *makeDoc10(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=c,a=10," + "w=d,a=10", + _inserter.toStr()); } - -TEST_F("requireThatMultipleDocsWork", Fixture) +TEST_F(FieldInverterTest, require_that_multiple_docs_work) { - f.invertDocument(10, *makeDoc10(f._b)); - f.invertDocument(11, *makeDoc11(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10,a=11," - "w=b,a=10,a=11," - "w=c,a=10,w=d,a=10," - "w=e,a=11," - "w=f,a=11," - "f=1,w=a,a=11," - "w=g,a=11", - f._inserter.toStr()); + invertDocument(10, *makeDoc10(_b)); + invertDocument(11, *makeDoc11(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10,a=11," + "w=b,a=10,a=11," + "w=c,a=10,w=d,a=10," + "w=e,a=11," + "w=f,a=11," + "f=1,w=a,a=11," + "w=g,a=11", + _inserter.toStr()); } - -TEST_F("requireThatRemoveWorks", Fixture) +TEST_F(FieldInverterTest, require_that_remove_works) { - f._inverters[0]->remove("b", 10); - f._inverters[0]->remove("a", 10); - f._inverters[0]->remove("b", 11); - f._inverters[2]->remove("c", 12); - f._inverters[1]->remove("a", 10); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,r=10," - "w=b,r=10,r=11," - "f=1,w=a,r=10," - "f=2,w=c,r=12", - f._inserter.toStr()); + _inverters[0]->remove("b", 10); + _inverters[0]->remove("a", 10); + _inverters[0]->remove("b", 11); + _inverters[2]->remove("c", 12); + _inverters[1]->remove("a", 10); + pushDocuments(); + EXPECT_EQ("f=0,w=a,r=10," + "w=b,r=10,r=11," + "f=1,w=a,r=10," + "f=2,w=c,r=12", + _inserter.toStr()); } - -TEST_F("requireThatReputWorks", Fixture) +TEST_F(FieldInverterTest, require_that_reput_works) { - f.invertDocument(10, *makeDoc10(f._b)); - f.invertDocument(10, *makeDoc11(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=e,a=10," - "w=f,a=10," - "f=1,w=a,a=10," - "w=g,a=10", - f._inserter.toStr()); + invertDocument(10, *makeDoc10(_b)); + invertDocument(10, *makeDoc11(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=e,a=10," + "w=f,a=10," + "f=1,w=a,a=10," + "w=g,a=10", + _inserter.toStr()); } - -TEST_F("requireThatAbortPendingDocWorks", Fixture) +TEST_F(FieldInverterTest, require_that_abort_pending_doc_works) { - Document::UP doc10 = makeDoc10(f._b); - Document::UP doc11 = makeDoc11(f._b); - Document::UP doc12 = makeDoc12(f._b); - Document::UP doc13 = makeDoc13(f._b); - Document::UP doc14 = makeDoc14(f._b); - - f.invertDocument(10, *doc10); - f.invertDocument(11, *doc11); - f.removeDocument(10); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=11," - "w=b,a=11," - "w=e,a=11," - "w=f,a=11," - "f=1,w=a,a=11," - "w=g,a=11", - f._inserter.toStr()); - - f.invertDocument(10, *doc10); - f.invertDocument(11, *doc11); - f.invertDocument(12, *doc12); - f.invertDocument(13, *doc13); - f.invertDocument(14, *doc14); - f.removeDocument(11); - f.removeDocument(13); - f._inserter.reset(); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=c,a=10," - "w=d,a=10," - "w=doc12,a=12," - "w=doc14,a=14," - "w=h,a=12," - "w=j,a=14", - f._inserter.toStr()); - - f.invertDocument(10, *doc10); - f.invertDocument(11, *doc11); - f.invertDocument(12, *doc12); - f.invertDocument(13, *doc13); - f.invertDocument(14, *doc14); - f.removeDocument(11); - f.removeDocument(12); - f.removeDocument(13); - f.removeDocument(14); - f._inserter.reset(); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10," - "w=b,a=10," - "w=c,a=10," - "w=d,a=10", - f._inserter.toStr()); - - + auto doc10 = makeDoc10(_b); + auto doc11 = makeDoc11(_b); + auto doc12 = makeDoc12(_b); + auto doc13 = makeDoc13(_b); + auto doc14 = makeDoc14(_b); + + invertDocument(10, *doc10); + invertDocument(11, *doc11); + removeDocument(10); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=11," + "w=b,a=11," + "w=e,a=11," + "w=f,a=11," + "f=1,w=a,a=11," + "w=g,a=11", + _inserter.toStr()); + + invertDocument(10, *doc10); + invertDocument(11, *doc11); + invertDocument(12, *doc12); + invertDocument(13, *doc13); + invertDocument(14, *doc14); + removeDocument(11); + removeDocument(13); + _inserter.reset(); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=c,a=10," + "w=d,a=10," + "w=doc12,a=12," + "w=doc14,a=14," + "w=h,a=12," + "w=j,a=14", + _inserter.toStr()); + + invertDocument(10, *doc10); + invertDocument(11, *doc11); + invertDocument(12, *doc12); + invertDocument(13, *doc13); + invertDocument(14, *doc14); + removeDocument(11); + removeDocument(12); + removeDocument(13); + removeDocument(14); + _inserter.reset(); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10," + "w=b,a=10," + "w=c,a=10," + "w=d,a=10", + _inserter.toStr()); } - -TEST_F("requireThatMixOfAddAndRemoveWorks", Fixture) +TEST_F(FieldInverterTest, require_that_mix_of_add_and_remove_works) { - f._inverters[0]->remove("a", 11); - f._inverters[0]->remove("c", 9); - f._inverters[0]->remove("d", 10); - f._inverters[0]->remove("z", 12); - f.invertDocument(10, *makeDoc10(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0,w=a,a=10,r=11," - "w=b,a=10," - "w=c,r=9,a=10," - "w=d,r=10,a=10," - "w=z,r=12", - f._inserter.toStr()); + _inverters[0]->remove("a", 11); + _inverters[0]->remove("c", 9); + _inverters[0]->remove("d", 10); + _inverters[0]->remove("z", 12); + invertDocument(10, *makeDoc10(_b)); + pushDocuments(); + EXPECT_EQ("f=0,w=a,a=10,r=11," + "w=b,a=10," + "w=c,r=9,a=10," + "w=d,r=10,a=10," + "w=z,r=12", + _inserter.toStr()); } - -TEST_F("require that empty document can be inverted", Fixture) +TEST_F(FieldInverterTest, require_that_empty_document_can_be_inverted) { - f.invertDocument(15, *makeDoc15(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", - f._inserter.toStr()); + invertDocument(15, *makeDoc15(_b)); + pushDocuments(); + EXPECT_EQ("", + _inserter.toStr()); } -TEST_F("require that multiple words at same position works", Fixture) +TEST_F(FieldInverterTest, require_that_multiple_words_at_same_position_works) { - f.invertDocument(16, *makeDoc16(f._b)); - f._inserter.setVerbose(); - f.pushDocuments(); - EXPECT_EQUAL("f=0," - "w=altbaz,a=16(e=0,w=1,l=5[2])," - "w=alty,a=16(e=0,w=1,l=5[3])," - "w=bar,a=16(e=0,w=1,l=5[1])," - "w=baz,a=16(e=0,w=1,l=5[2])," - "w=foo,a=16(e=0,w=1,l=5[0])," - "w=y,a=16(e=0,w=1,l=5[3])," - "w=z,a=16(e=0,w=1,l=5[4])", - f._inserter.toStr()); + invertDocument(16, *makeDoc16(_b)); + _inserter.setVerbose(); + pushDocuments(); + EXPECT_EQ("f=0," + "w=altbaz,a=16(e=0,w=1,l=5[2])," + "w=alty,a=16(e=0,w=1,l=5[3])," + "w=bar,a=16(e=0,w=1,l=5[1])," + "w=baz,a=16(e=0,w=1,l=5[2])," + "w=foo,a=16(e=0,w=1,l=5[0])," + "w=y,a=16(e=0,w=1,l=5[3])," + "w=z,a=16(e=0,w=1,l=5[4])", + _inserter.toStr()); } -TEST_F("require that interleaved features are calculated", Fixture) +TEST_F(FieldInverterTest, require_that_interleaved_features_are_calculated) { - f.invertDocument(17, *makeDoc17(f._b)); - f._inserter.setVerbose(); - f._inserter.set_show_interleaved_features(); - f.pushDocuments(); - EXPECT_EQUAL("f=1," - "w=bar0,a=17(fl=2,occs=1,e=0,w=1,l=2[1])," - "w=foo0,a=17(fl=2,occs=1,e=0,w=1,l=2[0])," - "f=2," - "w=bar,a=17(fl=3,occs=2,e=0,w=1,l=2[1],e=1,w=1,l=1[0])," - "w=foo,a=17(fl=3,occs=1,e=0,w=1,l=2[0])," - "f=3," - "w=bar2,a=17(fl=3,occs=2,e=0,w=3,l=2[1],e=1,w=4,l=1[0])," - "w=foo2,a=17(fl=3,occs=1,e=0,w=3,l=2[0])", - f._inserter.toStr()); + invertDocument(17, *makeDoc17(_b)); + _inserter.setVerbose(); + _inserter.set_show_interleaved_features(); + pushDocuments(); + EXPECT_EQ("f=1," + "w=bar0,a=17(fl=2,occs=1,e=0,w=1,l=2[1])," + "w=foo0,a=17(fl=2,occs=1,e=0,w=1,l=2[0])," + "f=2," + "w=bar,a=17(fl=3,occs=2,e=0,w=1,l=2[1],e=1,w=1,l=1[0])," + "w=foo,a=17(fl=3,occs=1,e=0,w=1,l=2[0])," + "f=3," + "w=bar2,a=17(fl=3,occs=2,e=0,w=3,l=2[1],e=1,w=4,l=1[0])," + "w=foo2,a=17(fl=3,occs=1,e=0,w=3,l=2[0])", + _inserter.toStr()); } -TEST_F("require that average field length is calculated", Fixture) +TEST_F(FieldInverterTest, require_that_average_field_length_is_calculated) { - f.invertDocument(10, *makeDoc10(f._b)); - f.pushDocuments(); - TEST_DO(f.assert_calculator(0, 4.0, 1)); - TEST_DO(f.assert_calculator(1, 0.0, 0)); - f.invertDocument(11, *makeDoc11(f._b)); - f.pushDocuments(); - TEST_DO(f.assert_calculator(0, (4.0 + 4.0)/2, 2)); - TEST_DO(f.assert_calculator(1, 2.0, 1)); - f.invertDocument(12, *makeDoc12(f._b)); - f.pushDocuments(); - TEST_DO(f.assert_calculator(0, (4.0 + 4.0 + 2.0)/3, 3)); - TEST_DO(f.assert_calculator(1, 2.0, 1)); + invertDocument(10, *makeDoc10(_b)); + pushDocuments(); + assert_calculator(0, 4.0, 1); + assert_calculator(1, 0.0, 0); + invertDocument(11, *makeDoc11(_b)); + pushDocuments(); + assert_calculator(0, (4.0 + 4.0)/2, 2); + assert_calculator(1, 2.0, 1); + invertDocument(12, *makeDoc12(_b)); + pushDocuments(); + assert_calculator(0, (4.0 + 4.0 + 2.0)/3, 3); + assert_calculator(1, 2.0, 1); } -} // namespace memoryindex -} // namespace search +} +} -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/memoryindex/url_field_inverter/CMakeLists.txt b/searchlib/src/tests/memoryindex/url_field_inverter/CMakeLists.txt index 28efc8a861e..db9418b7190 100644 --- a/searchlib/src/tests/memoryindex/url_field_inverter/CMakeLists.txt +++ b/searchlib/src/tests/memoryindex/url_field_inverter/CMakeLists.txt @@ -5,5 +5,6 @@ vespa_add_executable(searchlib_url_field_inverter_test_app TEST DEPENDS searchlib_test searchlib + gtest ) vespa_add_test(NAME searchlib_url_field_inverter_test_app COMMAND searchlib_url_field_inverter_test_app) diff --git a/searchlib/src/tests/memoryindex/url_field_inverter/url_field_inverter_test.cpp b/searchlib/src/tests/memoryindex/url_field_inverter/url_field_inverter_test.cpp index 2ea13a20063..2151a44a66d 100644 --- a/searchlib/src/tests/memoryindex/url_field_inverter/url_field_inverter_test.cpp +++ b/searchlib/src/tests/memoryindex/url_field_inverter/url_field_inverter_test.cpp @@ -1,15 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/* -*- mode: C++; coding: utf-8; -*- */ #include <vespa/document/repo/fixedtyperepo.h> #include <vespa/searchlib/index/docbuilder.h> #include <vespa/searchlib/index/field_length_calculator.h> #include <vespa/searchlib/memoryindex/field_index_remover.h> #include <vespa/searchlib/memoryindex/field_inverter.h> -#include <vespa/searchlib/memoryindex/word_store.h> #include <vespa/searchlib/memoryindex/url_field_inverter.h> +#include <vespa/searchlib/memoryindex/word_store.h> #include <vespa/searchlib/test/memoryindex/ordered_field_index_inserter.h> -#include <vespa/vespalib/testkit/testapp.h> +#include <vespa/vespalib/gtest/gtest.h> namespace search { @@ -56,7 +55,6 @@ makeDoc10Single(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc10Array(DocBuilder &b) { @@ -169,7 +167,6 @@ makeDoc10WeightedSet(DocBuilder &b) return b.endDocument(); } - Document::UP makeDoc10Empty(DocBuilder &b) { @@ -179,8 +176,7 @@ makeDoc10Empty(DocBuilder &b) } -struct Fixture -{ +struct UrlFieldInverterTest : public ::testing::Test { Schema _schema; DocBuilder _b; WordStore _word_store; @@ -191,15 +187,13 @@ struct Fixture std::unique_ptr<UrlFieldInverter> _urlInverter; index::SchemaIndexFields _schemaIndexFields; - static Schema - makeSchema(Schema::CollectionType collectionType) - { + static Schema makeSchema(Schema::CollectionType collectionType) { Schema schema; schema.addUriIndexFields(Schema::IndexField("url", DataType::STRING, collectionType)); return schema; } - Fixture(Schema::CollectionType collectionType) + UrlFieldInverterTest(Schema::CollectionType collectionType) : _schema(makeSchema(collectionType)), _b(_schema), _word_store(), @@ -233,15 +227,11 @@ struct Fixture _inverters[urlField._hostname].get()); } - void - invertDocument(uint32_t docId, const Document &doc) - { + void invertDocument(uint32_t docId, const Document &doc) { _urlInverter->invertField(docId, doc.getValue(url)); } - void - pushDocuments() - { + void pushDocuments() { uint32_t fieldId = 0; for (auto &inverter : _inverters) { _inserter.setFieldId(fieldId); @@ -250,324 +240,330 @@ struct Fixture } } - void - enableAnnotations() - { + void enableAnnotations() { _urlInverter->setUseAnnotations(true); } }; +struct SingleInverterTest : public UrlFieldInverterTest { + SingleInverterTest() : UrlFieldInverterTest(CollectionType::SINGLE) {} +}; + +struct ArrayInverterTest : public UrlFieldInverterTest { + ArrayInverterTest() : UrlFieldInverterTest(CollectionType::ARRAY) {} +}; -TEST_F("requireThatSingleUrlFieldWorks", Fixture(CollectionType::SINGLE)) -{ - f.invertDocument(10, *makeDoc10Single(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0," - "w=2,a=10," - "w=4,a=10," - "w=81,a=10," - "w=ab,a=10," - "w=com,a=10," - "w=example,a=10," - "w=fluke,a=10," - "w=http,a=10," - "w=www,a=10," - "f=1," - "w=http,a=10," - "f=2," - "w=com,a=10," - "w=example,a=10," - "w=www,a=10," - "f=3," - "w=81,a=10," - "f=4," - "w=fluke,a=10," - "f=5," - "w=2,a=10," - "w=ab,a=10," - "f=6," - "w=4,a=10," - "f=7," - "w=EnDhOsT,a=10," - "w=StArThOsT,a=10," - "w=com,a=10," - "w=example,a=10," - "w=www,a=10", - f._inserter.toStr()); -} +struct WeightedSetInverterTest : public UrlFieldInverterTest { + WeightedSetInverterTest() : UrlFieldInverterTest(CollectionType::WEIGHTEDSET) {} +}; -TEST_F("requireThatArrayUrlFieldWorks", Fixture(CollectionType::ARRAY)) +TEST_F(SingleInverterTest, require_that_single_url_field_works) { - f.invertDocument(10, *makeDoc10Array(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0," - "w=2,a=10," - "w=8,a=10," - "w=82,a=10," - "w=9,a=10," - "w=ab,a=10," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=fluke,a=10," - "w=http,a=10," - "w=www,a=10," - "f=1," - "w=http,a=10," - "f=2," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=www,a=10," - "f=3," - "w=82,a=10," - "f=4," - "w=fluke,a=10," - "f=5," - "w=2,a=10," - "w=ab,a=10," - "f=6," - "w=8,a=10," - "w=9,a=10," - "f=7," - "w=EnDhOsT,a=10," - "w=StArThOsT,a=10," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=www,a=10", - f._inserter.toStr()); + invertDocument(10, *makeDoc10Single(_b)); + pushDocuments(); + EXPECT_EQ("f=0," + "w=2,a=10," + "w=4,a=10," + "w=81,a=10," + "w=ab,a=10," + "w=com,a=10," + "w=example,a=10," + "w=fluke,a=10," + "w=http,a=10," + "w=www,a=10," + "f=1," + "w=http,a=10," + "f=2," + "w=com,a=10," + "w=example,a=10," + "w=www,a=10," + "f=3," + "w=81,a=10," + "f=4," + "w=fluke,a=10," + "f=5," + "w=2,a=10," + "w=ab,a=10," + "f=6," + "w=4,a=10," + "f=7," + "w=EnDhOsT,a=10," + "w=StArThOsT,a=10," + "w=com,a=10," + "w=example,a=10," + "w=www,a=10", + _inserter.toStr()); } -TEST_F("requireThatWeightedSetFieldWorks", Fixture(CollectionType::WEIGHTEDSET)) +TEST_F(ArrayInverterTest, require_that_array_url_field_works) { - f.invertDocument(10, *makeDoc10WeightedSet(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0," - "w=12,a=10," - "w=13,a=10," - "w=2,a=10," - "w=83,a=10," - "w=85,a=10," - "w=ab,a=10," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=fluke,a=10," - "w=http,a=10," - "w=www,a=10," - "f=1," - "w=http,a=10," - "f=2," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=www,a=10," - "f=3," - "w=83,a=10," - "w=85,a=10," - "f=4," - "w=fluke,a=10," - "f=5," - "w=2,a=10," - "w=ab,a=10," - "f=6," - "w=12,a=10," - "w=13,a=10," - "f=7," - "w=EnDhOsT,a=10," - "w=StArThOsT,a=10," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=www,a=10", - f._inserter.toStr()); + invertDocument(10, *makeDoc10Array(_b)); + pushDocuments(); + EXPECT_EQ("f=0," + "w=2,a=10," + "w=8,a=10," + "w=82,a=10," + "w=9,a=10," + "w=ab,a=10," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=fluke,a=10," + "w=http,a=10," + "w=www,a=10," + "f=1," + "w=http,a=10," + "f=2," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=www,a=10," + "f=3," + "w=82,a=10," + "f=4," + "w=fluke,a=10," + "f=5," + "w=2,a=10," + "w=ab,a=10," + "f=6," + "w=8,a=10," + "w=9,a=10," + "f=7," + "w=EnDhOsT,a=10," + "w=StArThOsT,a=10," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=www,a=10", + _inserter.toStr()); } -TEST_F("requireThatAnnotatedSingleUrlFieldWorks", Fixture(CollectionType::SINGLE)) +TEST_F(WeightedSetInverterTest, require_that_weighted_set_field_works) { - f.enableAnnotations(); - f.invertDocument(10, *makeDoc10Single(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0," - "w=2,a=10," - "w=4,a=10," - "w=81,a=10," - "w=ab,a=10," - "w=com,a=10," - "w=example,a=10," - "w=fluke,a=10," - "w=http,a=10," - "w=www,a=10," - "f=1," - "w=http,a=10," - "f=2," - "w=com,a=10," - "w=example,a=10," - "w=www,a=10," - "f=3," - "w=81,a=10," - "f=4," - "w=altfluke,a=10," - "w=fluke,a=10," - "f=5," - "w=2,a=10," - "w=ab,a=10," - "f=6," - "w=4,a=10," - "f=7," - "w=EnDhOsT,a=10," - "w=StArThOsT,a=10," - "w=com,a=10," - "w=example,a=10," - "w=www,a=10", - f._inserter.toStr()); + invertDocument(10, *makeDoc10WeightedSet(_b)); + pushDocuments(); + EXPECT_EQ("f=0," + "w=12,a=10," + "w=13,a=10," + "w=2,a=10," + "w=83,a=10," + "w=85,a=10," + "w=ab,a=10," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=fluke,a=10," + "w=http,a=10," + "w=www,a=10," + "f=1," + "w=http,a=10," + "f=2," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=www,a=10," + "f=3," + "w=83,a=10," + "w=85,a=10," + "f=4," + "w=fluke,a=10," + "f=5," + "w=2,a=10," + "w=ab,a=10," + "f=6," + "w=12,a=10," + "w=13,a=10," + "f=7," + "w=EnDhOsT,a=10," + "w=StArThOsT,a=10," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=www,a=10", + _inserter.toStr()); } - -TEST_F("requireThatAnnotatedArrayUrlFieldWorks", Fixture(CollectionType::ARRAY)) +TEST_F(SingleInverterTest, require_that_annotated_single_url_field_works) { - f.enableAnnotations(); - f.invertDocument(10, *makeDoc10Array(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0," - "w=2,a=10," - "w=8,a=10," - "w=82,a=10," - "w=9,a=10," - "w=ab,a=10," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=fluke,a=10," - "w=http,a=10," - "w=www,a=10," - "f=1," - "w=http,a=10," - "f=2," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=www,a=10," - "f=3," - "w=82,a=10," - "f=4," - "w=altfluke,a=10," - "w=fluke,a=10," - "f=5," - "w=2,a=10," - "w=ab,a=10," - "f=6," - "w=8,a=10," - "w=9,a=10," - "f=7," - "w=EnDhOsT,a=10," - "w=StArThOsT,a=10," - "w=com,a=10," - "w=example,a=10," - "w=flickr,a=10," - "w=www,a=10", - f._inserter.toStr()); + enableAnnotations(); + invertDocument(10, *makeDoc10Single(_b)); + pushDocuments(); + EXPECT_EQ("f=0," + "w=2,a=10," + "w=4,a=10," + "w=81,a=10," + "w=ab,a=10," + "w=com,a=10," + "w=example,a=10," + "w=fluke,a=10," + "w=http,a=10," + "w=www,a=10," + "f=1," + "w=http,a=10," + "f=2," + "w=com,a=10," + "w=example,a=10," + "w=www,a=10," + "f=3," + "w=81,a=10," + "f=4," + "w=altfluke,a=10," + "w=fluke,a=10," + "f=5," + "w=2,a=10," + "w=ab,a=10," + "f=6," + "w=4,a=10," + "f=7," + "w=EnDhOsT,a=10," + "w=StArThOsT,a=10," + "w=com,a=10," + "w=example,a=10," + "w=www,a=10", + _inserter.toStr()); } -TEST_F("requireThatAnnotatedWeightedSetFieldWorks", - Fixture(CollectionType::WEIGHTEDSET)) +TEST_F(ArrayInverterTest, require_that_annotated_array_url_field_works) { - f.enableAnnotations(); - f._inserter.setVerbose(); - f.invertDocument(10, *makeDoc10WeightedSet(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("f=0," - "w=12,a=10(e=0,w=4,l=9[8])," - "w=13,a=10(e=1,w=7,l=9[8])," - "w=2,a=10(e=0,w=4,l=9[7],e=1,w=7,l=9[7])," - "w=83,a=10(e=0,w=4,l=9[4])," - "w=85,a=10(e=1,w=7,l=9[4])," - "w=ab,a=10(e=0,w=4,l=9[6],e=1,w=7,l=9[6])," - "w=com,a=10(e=0,w=4,l=9[3],e=1,w=7,l=9[3])," - "w=example,a=10(e=0,w=4,l=9[2])," - "w=flickr,a=10(e=1,w=7,l=9[2])," - "w=fluke,a=10(e=0,w=4,l=9[5],e=1,w=7,l=9[5])," - "w=http,a=10(e=0,w=4,l=9[0],e=1,w=7,l=9[0])," - "w=www,a=10(e=0,w=4,l=9[1],e=1,w=7,l=9[1])," - "f=1," - "w=http,a=10(e=0,w=4,l=1[0],e=1,w=7,l=1[0])," - "f=2," - "w=com,a=10(e=0,w=4,l=3[2],e=1,w=7,l=3[2])," - "w=example,a=10(e=0,w=4,l=3[1])," - "w=flickr,a=10(e=1,w=7,l=3[1])," - "w=www,a=10(e=0,w=4,l=3[0],e=1,w=7,l=3[0])," - "f=3," - "w=83,a=10(e=0,w=4,l=1[0])," - "w=85,a=10(e=1,w=7,l=1[0])," - "f=4," - "w=altfluke,a=10(e=0,w=4,l=1[0])," - "w=fluke,a=10(e=0,w=4,l=1[0],e=1,w=7,l=1[0])," - "f=5," - "w=2,a=10(e=0,w=4,l=2[1],e=1,w=7,l=2[1])," - "w=ab,a=10(e=0,w=4,l=2[0],e=1,w=7,l=2[0])," - "f=6," - "w=12,a=10(e=0,w=4,l=1[0])," - "w=13,a=10(e=1,w=7,l=1[0])," - "f=7," - "w=EnDhOsT,a=10(e=0,w=4,l=5[4],e=1,w=7,l=5[4])," - "w=StArThOsT,a=10(e=0,w=4,l=5[0],e=1,w=7,l=5[0])," - "w=com,a=10(e=0,w=4,l=5[3],e=1,w=7,l=5[3])," - "w=example,a=10(e=0,w=4,l=5[2])," - "w=flickr,a=10(e=1,w=7,l=5[2])," - "w=www,a=10(e=0,w=4,l=5[1],e=1,w=7,l=5[1])", - f._inserter.toStr()); + enableAnnotations(); + invertDocument(10, *makeDoc10Array(_b)); + pushDocuments(); + EXPECT_EQ("f=0," + "w=2,a=10," + "w=8,a=10," + "w=82,a=10," + "w=9,a=10," + "w=ab,a=10," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=fluke,a=10," + "w=http,a=10," + "w=www,a=10," + "f=1," + "w=http,a=10," + "f=2," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=www,a=10," + "f=3," + "w=82,a=10," + "f=4," + "w=altfluke,a=10," + "w=fluke,a=10," + "f=5," + "w=2,a=10," + "w=ab,a=10," + "f=6," + "w=8,a=10," + "w=9,a=10," + "f=7," + "w=EnDhOsT,a=10," + "w=StArThOsT,a=10," + "w=com,a=10," + "w=example,a=10," + "w=flickr,a=10," + "w=www,a=10", + _inserter.toStr()); } +TEST_F(WeightedSetInverterTest, require_that_annotated_weighted_set_field_works) +{ + enableAnnotations(); + _inserter.setVerbose(); + invertDocument(10, *makeDoc10WeightedSet(_b)); + pushDocuments(); + EXPECT_EQ("f=0," + "w=12,a=10(e=0,w=4,l=9[8])," + "w=13,a=10(e=1,w=7,l=9[8])," + "w=2,a=10(e=0,w=4,l=9[7],e=1,w=7,l=9[7])," + "w=83,a=10(e=0,w=4,l=9[4])," + "w=85,a=10(e=1,w=7,l=9[4])," + "w=ab,a=10(e=0,w=4,l=9[6],e=1,w=7,l=9[6])," + "w=com,a=10(e=0,w=4,l=9[3],e=1,w=7,l=9[3])," + "w=example,a=10(e=0,w=4,l=9[2])," + "w=flickr,a=10(e=1,w=7,l=9[2])," + "w=fluke,a=10(e=0,w=4,l=9[5],e=1,w=7,l=9[5])," + "w=http,a=10(e=0,w=4,l=9[0],e=1,w=7,l=9[0])," + "w=www,a=10(e=0,w=4,l=9[1],e=1,w=7,l=9[1])," + "f=1," + "w=http,a=10(e=0,w=4,l=1[0],e=1,w=7,l=1[0])," + "f=2," + "w=com,a=10(e=0,w=4,l=3[2],e=1,w=7,l=3[2])," + "w=example,a=10(e=0,w=4,l=3[1])," + "w=flickr,a=10(e=1,w=7,l=3[1])," + "w=www,a=10(e=0,w=4,l=3[0],e=1,w=7,l=3[0])," + "f=3," + "w=83,a=10(e=0,w=4,l=1[0])," + "w=85,a=10(e=1,w=7,l=1[0])," + "f=4," + "w=altfluke,a=10(e=0,w=4,l=1[0])," + "w=fluke,a=10(e=0,w=4,l=1[0],e=1,w=7,l=1[0])," + "f=5," + "w=2,a=10(e=0,w=4,l=2[1],e=1,w=7,l=2[1])," + "w=ab,a=10(e=0,w=4,l=2[0],e=1,w=7,l=2[0])," + "f=6," + "w=12,a=10(e=0,w=4,l=1[0])," + "w=13,a=10(e=1,w=7,l=1[0])," + "f=7," + "w=EnDhOsT,a=10(e=0,w=4,l=5[4],e=1,w=7,l=5[4])," + "w=StArThOsT,a=10(e=0,w=4,l=5[0],e=1,w=7,l=5[0])," + "w=com,a=10(e=0,w=4,l=5[3],e=1,w=7,l=5[3])," + "w=example,a=10(e=0,w=4,l=5[2])," + "w=flickr,a=10(e=1,w=7,l=5[2])," + "w=www,a=10(e=0,w=4,l=5[1],e=1,w=7,l=5[1])", + _inserter.toStr()); +} -TEST_F("requireThatEmptySingleFieldWorks", Fixture(CollectionType::SINGLE)) +TEST_F(SingleInverterTest, require_that_empty_single_field_works) { - f.invertDocument(10, *makeDoc10Empty(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", f._inserter.toStr()); + invertDocument(10, *makeDoc10Empty(_b)); + pushDocuments(); + EXPECT_EQ("", _inserter.toStr()); } -TEST_F("requireThatEmptyArrayFieldWorks", Fixture(CollectionType::ARRAY)) +TEST_F(ArrayInverterTest, require_that_empty_array_field_works) { - f.invertDocument(10, *makeDoc10Empty(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", - f._inserter.toStr()); + invertDocument(10, *makeDoc10Empty(_b)); + pushDocuments(); + EXPECT_EQ("", + _inserter.toStr()); } -TEST_F("requireThatEmptyWeightedSetFieldWorks", Fixture(CollectionType::WEIGHTEDSET)) +TEST_F(WeightedSetInverterTest, require_that_empty_weighted_set_field_works) { - f.invertDocument(10, *makeDoc10Empty(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", f._inserter.toStr()); + invertDocument(10, *makeDoc10Empty(_b)); + pushDocuments(); + EXPECT_EQ("", _inserter.toStr()); } -TEST_F("requireThatAnnotatedEmptySingleFieldWorks", Fixture(CollectionType::SINGLE)) +TEST_F(SingleInverterTest, require_that_annotated_empty_single_field_works) { - f.enableAnnotations(); - f.invertDocument(10, *makeDoc10Empty(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", f._inserter.toStr()); + enableAnnotations(); + invertDocument(10, *makeDoc10Empty(_b)); + pushDocuments(); + EXPECT_EQ("", _inserter.toStr()); } -TEST_F("requireThatAnnotatedEmptyArrayFieldWorks", Fixture(CollectionType::ARRAY)) +TEST_F(ArrayInverterTest, require_that_annotated_empty_array_field_works) { - f.enableAnnotations(); - f.invertDocument(10, *makeDoc10Empty(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", f._inserter.toStr()); + enableAnnotations(); + invertDocument(10, *makeDoc10Empty(_b)); + pushDocuments(); + EXPECT_EQ("", _inserter.toStr()); } -TEST_F("requireThatAnnotatedEmptyWeightedSetFieldWorks", Fixture(CollectionType::WEIGHTEDSET)) +TEST_F(WeightedSetInverterTest, require_that_annotated_empty_weighted_set_field_works) { - f.enableAnnotations(); - f.invertDocument(10, *makeDoc10Empty(f._b)); - f.pushDocuments(); - EXPECT_EQUAL("", f._inserter.toStr()); + enableAnnotations(); + invertDocument(10, *makeDoc10Empty(_b)); + pushDocuments(); + EXPECT_EQ("", _inserter.toStr()); } -} // namespace memoryindex -} // namespace search +} +} -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.cpp b/storage/src/vespa/storage/distributor/idealstatemanager.cpp index 5ae5d8dc3f8..eea6db8c782 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.cpp +++ b/storage/src/vespa/storage/distributor/idealstatemanager.cpp @@ -32,7 +32,8 @@ IdealStateManager::IdealStateManager( : HtmlStatusReporter("idealstateman", "Ideal state manager"), _metrics(new IdealStateMetricSet), _distributorComponent(owner, bucketSpaceRepo, readOnlyBucketSpaceRepo, compReg, "Ideal state manager"), - _bucketSpaceRepo(bucketSpaceRepo) + _bucketSpaceRepo(bucketSpaceRepo), + _has_logged_phantom_replica_warning(false) { _distributorComponent.registerStatusPage(*this); _distributorComponent.registerMetric(*_metrics); @@ -52,9 +53,7 @@ IdealStateManager::IdealStateManager( _stateCheckers.push_back(StateChecker::SP(new GarbageCollectionStateChecker())); } -IdealStateManager::~IdealStateManager() -{ -} +IdealStateManager::~IdealStateManager() = default; void IdealStateManager::print(std::ostream& out, bool verbose, @@ -143,6 +142,26 @@ IdealStateManager::runStateCheckers(StateChecker::Context& c) const return highestPri; } +void IdealStateManager::verify_only_live_nodes_in_context(const StateChecker::Context& c) const { + if (_has_logged_phantom_replica_warning) { + return; + } + for (const auto& n : c.entry->getRawNodes()) { + const uint16_t index = n.getNode(); + const auto& state = c.systemState.getNodeState(lib::Node(lib::NodeType::STORAGE, index)); + // Only nodes in Up, Initializing or Retired should ever be present in the DB. + if (!state.getState().oneOf("uir")) { + LOG(warning, "%s in bucket DB is on node %u, which is in unavailable state %s. " + "Current cluster state is '%s'", + c.entry.getBucketId().toString().c_str(), + index, + state.getState().toString().c_str(), + c.systemState.toString().c_str()); + _has_logged_phantom_replica_warning = true; + } + } +} + StateChecker::Result IdealStateManager::generateHighestPriority( const document::Bucket &bucket, @@ -160,6 +179,7 @@ IdealStateManager::generateHighestPriority( LOG(spam, "Checking bucket %s", e->toString().c_str()); c.entry = *e; + verify_only_live_nodes_in_context(c); return runStateCheckers(c); } diff --git a/storage/src/vespa/storage/distributor/idealstatemanager.h b/storage/src/vespa/storage/distributor/idealstatemanager.h index 8566c67a51b..10f18a35952 100644 --- a/storage/src/vespa/storage/distributor/idealstatemanager.h +++ b/storage/src/vespa/storage/distributor/idealstatemanager.h @@ -41,7 +41,7 @@ public: DistributorComponentRegister& compReg, bool manageActiveBucketCopies); - ~IdealStateManager(); + ~IdealStateManager() override; void print(std::ostream& out, bool verbose, const std::string& indent) const; @@ -86,6 +86,7 @@ public: const DistributorBucketSpaceRepo &getBucketSpaceRepo() const { return _bucketSpaceRepo; } private: + void verify_only_live_nodes_in_context(const StateChecker::Context& c) const; void fillParentAndChildBuckets(StateChecker::Context& c) const; void fillSiblingBucket(StateChecker::Context& c) const; StateChecker::Result generateHighestPriority( @@ -95,13 +96,6 @@ private: BucketDatabase::Entry* getEntryForPrimaryBucket(StateChecker::Context& c) const; - friend class Operation_TestCase; - friend class RemoveBucketOperation_Test; - friend class MergeOperation_Test; - friend class CreateBucketOperation_Test; - friend class SplitOperation_Test; - friend class JoinOperation_Test; - std::shared_ptr<IdealStateMetricSet> _metrics; document::BucketId _lastPrioritizedBucket; @@ -112,8 +106,7 @@ private: DistributorComponent _distributorComponent; DistributorBucketSpaceRepo &_bucketSpaceRepo; - - std::vector<IdealStateOperation::SP> generateOperationsForBucket(StateChecker::Context& c) const; + mutable bool _has_logged_phantom_replica_warning; bool iAmUp() const; @@ -133,7 +126,6 @@ private: return true; } }; - friend class StatusBucketVisitor; void getBucketStatus(document::BucketSpace bucketSpace, const BucketDatabase::ConstEntryRef& entry, NodeMaintenanceStatsTracker& statsTracker, std::ostream& out) const; diff --git a/tenant-base/pom.xml b/tenant-base/pom.xml index 10c42ba8acd..f087a8019da 100644 --- a/tenant-base/pom.xml +++ b/tenant-base/pom.xml @@ -36,7 +36,6 @@ <target_jdk_version>11</target_jdk_version> <compiler_plugin_version>3.8.0</compiler_plugin_version> <surefire_version>2.22.0</surefire_version> - <junit.jupiter.version>5.4.2</junit.jupiter.version> <endpoint>https://api.vespa-external.aws.oath.cloud:4443</endpoint> </properties> @@ -73,13 +72,6 @@ <version>${vespaversion}</version> <scope>test</scope> </dependency> - - <dependency> - <groupId>org.junit.jupiter</groupId> - <artifactId>junit-jupiter-engine</artifactId> - <version>${junit.jupiter.version}</version> - <scope>test</scope> - </dependency> </dependencies> <profiles> diff --git a/tenant-cd/pom.xml b/tenant-cd/pom.xml index 18c4084a173..829b1de457b 100644 --- a/tenant-cd/pom.xml +++ b/tenant-cd/pom.xml @@ -51,10 +51,8 @@ </dependency> <dependency> - <groupId>junit</groupId> - <artifactId>junit</artifactId> - <version>4.12</version> - <scope>test</scope> + <groupId>org.junit.jupiter</groupId> + <artifactId>junit-jupiter-engine</artifactId> </dependency> </dependencies> diff --git a/tenant-cd/src/main/java/ai/vespa/hosted/cd/FunctionalTest.java b/tenant-cd/src/main/java/ai/vespa/hosted/cd/FunctionalTest.java deleted file mode 100644 index e6beb313d28..00000000000 --- a/tenant-cd/src/main/java/ai/vespa/hosted/cd/FunctionalTest.java +++ /dev/null @@ -1,31 +0,0 @@ -package ai.vespa.hosted.cd; - -/** - * Tests that compare the behaviour of a Vespa application deployment against a fixed specification. - * - * These tests are run whenever a change is pushed to a Vespa application, and whenever the Vespa platform - * is upgraded, and before any deployments to production zones. When these tests fails, the tested change to - * the Vespa application is not rolled out. - * - * A typical functional test is to feed some documents, optionally verifying that the documents have been processed - * as expected, and then to see that queries give the expected results. Another common use is to verify integration - * with external services. - * - * @author jonmv - */ -public interface FunctionalTest { - - // Want to feed some documents. - // Want to verify document processing and routing is as expected. - // Want to check recall on those documents. - // Want to verify queries give expected documents. - // Want to verify searchers. - // Want to verify updates. - // Want to verify deletion. - // May want to verify reprocessing. - // Must likely delete documents between tests. - // Must be able to feed documents, setting route. - // Must be able to search. - // Must be able to visit. - -} diff --git a/tenant-cd/src/main/java/ai/vespa/hosted/cd/ProductionTest.java b/tenant-cd/src/main/java/ai/vespa/hosted/cd/ProductionTest.java index 0dfeab5d327..e9a2be1fb1f 100644 --- a/tenant-cd/src/main/java/ai/vespa/hosted/cd/ProductionTest.java +++ b/tenant-cd/src/main/java/ai/vespa/hosted/cd/ProductionTest.java @@ -1,6 +1,16 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.cd; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + /** * Tests that verify the health of production deployments of Vespa applications. * @@ -14,10 +24,10 @@ package ai.vespa.hosted.cd; * * @author jonmv */ -public interface ProductionTest { - - /** Use with JUnit 5 @Tag to have this run in the production jobs in the pipeline. */ - String name = "ai.vespa.hosted.cd.ProductionTest"; +@Target({TYPE, ANNOTATION_TYPE}) +@Retention(RUNTIME) +@Tag("ai.vespa.hosted.cd.ProductionTest") +public @interface ProductionTest { // Want to verify metrics (Vespa). // Want to verify external metrics (YAMAS, other). diff --git a/tenant-cd/src/main/java/ai/vespa/hosted/cd/StagingTest.java b/tenant-cd/src/main/java/ai/vespa/hosted/cd/StagingTest.java index 6e1487ced0f..b0325efa8d3 100644 --- a/tenant-cd/src/main/java/ai/vespa/hosted/cd/StagingTest.java +++ b/tenant-cd/src/main/java/ai/vespa/hosted/cd/StagingTest.java @@ -1,6 +1,16 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.cd; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + /** * Tests that assert continuity of behaviour for Vespa application deployments, through upgrades. * @@ -16,11 +26,12 @@ package ai.vespa.hosted.cd; * * @author jonmv */ -public interface StagingTest { - - /** Use with JUnit 5 @Tag to have this run in the staging test job in the pipeline. */ - String name = "ai.vespa.hosted.cd.StagingTest"; +@Target({TYPE, ANNOTATION_TYPE}) +@Retention(RUNTIME) +@Tag("ai.vespa.hosted.cd.StagingTest") +public @interface StagingTest { // Want to verify documents are not damaged by upgrade. // May want to verify metrics during upgrade. + } diff --git a/tenant-cd/src/main/java/ai/vespa/hosted/cd/SystemTest.java b/tenant-cd/src/main/java/ai/vespa/hosted/cd/SystemTest.java index f2f06d53515..f27fa01072c 100644 --- a/tenant-cd/src/main/java/ai/vespa/hosted/cd/SystemTest.java +++ b/tenant-cd/src/main/java/ai/vespa/hosted/cd/SystemTest.java @@ -1,6 +1,18 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.cd; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + /** * Tests that compare the behaviour of a Vespa application deployment against a fixed specification. * @@ -14,10 +26,10 @@ package ai.vespa.hosted.cd; * * @author jonmv */ -public interface SystemTest { - - /** Use with JUnit 5 @Tag to have this run in the system test job in the pipeline. */ - String name = "ai.vespa.hosted.cd.SystemTest"; +@Target({TYPE, ANNOTATION_TYPE}) +@Retention(RUNTIME) +@Tag("ai.vespa.hosted.cd.SystemTest") +public @interface SystemTest { // Want to feed some documents. // Want to verify document processing and routing is as expected. @@ -31,4 +43,5 @@ public interface SystemTest { // Must be able to feed documents, setting route. // Must be able to search. // Must be able to visit. + } diff --git a/tenant-cd/src/main/java/ai/vespa/hosted/cd/UpgradeTest.java b/tenant-cd/src/main/java/ai/vespa/hosted/cd/UpgradeTest.java deleted file mode 100644 index 32083fbd5f6..00000000000 --- a/tenant-cd/src/main/java/ai/vespa/hosted/cd/UpgradeTest.java +++ /dev/null @@ -1,23 +0,0 @@ -package ai.vespa.hosted.cd; - -/** - * Tests that assert continuity of behaviour for Vespa application deployments, through upgrades. - * - * These tests are run whenever a change is pushed to a Vespa application, and whenever the Vespa platform - * is upgraded, and before any deployments to production zones. When these tests fails, the tested change to - * the Vespa application is not rolled out. - * - * A typical upgrade test is to do some operations against a test deployment prior to upgrade, like feed and - * search for some documents, perhaps recording some metrics from the deployment, and then to upgrade it, - * repeat the exercise, and compare the results from pre and post upgrade. - * - * TODO Split in platform upgrades and application upgrades? - * - * @author jonmv - */ -public interface UpgradeTest { - - // Want to verify documents are not damaged by upgrade. - // May want to verify metrics during upgrade. - -} diff --git a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java index bc34a4ac3df..3afd84725bf 100644 --- a/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java +++ b/vespa-documentgen-plugin/src/main/java/com/yahoo/vespa/DocumentGenMojo.java @@ -203,7 +203,7 @@ public class DocumentGenMojo extends AbstractMojo { " * Generated by vespa-documentgen-plugin, do not edit.\n" + " * Date: "+new Date()+"\n" + " */\n"); - out.write("@com.yahoo.document.Generated public class ConcreteDocumentFactory extends com.yahoo.docproc.AbstractConcreteDocumentFactory {\n"); + out.write("@com.yahoo.document.Generated\npublic class ConcreteDocumentFactory extends com.yahoo.docproc.AbstractConcreteDocumentFactory {\n"); out.write(ind()+"private static java.util.Map<java.lang.String, java.lang.Class<? extends com.yahoo.document.Document>> dTypes = new java.util.HashMap<java.lang.String, java.lang.Class<? extends com.yahoo.document.Document>>();\n"); out.write(ind()+"private static java.util.Map<java.lang.String, com.yahoo.document.DocumentType> docTypes = new java.util.HashMap<>();\n"); out.write(ind()+"private static java.util.Map<java.lang.String, java.lang.Class<? extends com.yahoo.document.datatypes.Struct>> sTypes = new java.util.HashMap<java.lang.String, java.lang.Class<? extends com.yahoo.document.datatypes.Struct>>();\n"); @@ -316,7 +316,7 @@ public class DocumentGenMojo extends AbstractMojo { " * Input annotation type: "+annType.getName()+"\n" + " * Date: "+new Date()+"\n" + " */\n" + - "@com.yahoo.document.Generated public "+annTypeModifier(annType)+"class "+className+" extends "+getParentAnnotationType(annType)+" {\n\n"); + "@com.yahoo.document.Generated\npublic "+annTypeModifier(annType)+"class "+className+" extends "+getParentAnnotationType(annType)+" {\n\n"); if (annType.getDataType() instanceof StructDataType) { out.write(ind() + "public "+className+"() {\n" + ind(2) + "setType(new com.yahoo.document.annotation.AnnotationType(\""+annType.getName()+"\", Fields.type));\n" + @@ -435,7 +435,7 @@ public class DocumentGenMojo extends AbstractMojo { " * Input document type: "+docType.getName()+"\n" + " * Date: "+new Date()+"\n" + " */\n" + - "@com.yahoo.document.Generated public class "+className+" extends "+superType+" {\n\n"+ + "@com.yahoo.document.Generated\npublic class "+className+" extends "+superType+" {\n\n"+ ind(1)+"/** The doc type of this.*/\n" + ind(1)+"public static final com.yahoo.document.DocumentType type = getDocumentType();\n\n"+ ind(1)+"/** Struct type view of the type of the body of this.*/\n" + @@ -567,19 +567,13 @@ public class DocumentGenMojo extends AbstractMojo { out.write(ind(ind)+"public "+className+"(com.yahoo.document.datatypes.StructuredFieldValue src) {\n"+ ind(ind+1)+"super("+className+".type);\n"); } + out.write(ind() + "ConcreteDocumentFactory factory = new ConcreteDocumentFactory();"); out.write( ind(ind+1)+"for (java.util.Iterator<java.util.Map.Entry<com.yahoo.document.Field, com.yahoo.document.datatypes.FieldValue>>i=src.iterator() ; i.hasNext() ; ) {\n" + ind(ind+2)+"java.util.Map.Entry<com.yahoo.document.Field, com.yahoo.document.datatypes.FieldValue> e = i.next();\n" + ind(ind+2)+"com.yahoo.document.Field f = e.getKey();\n" + ind(ind+2)+"com.yahoo.document.datatypes.FieldValue fv = e.getValue();\n" + - ind(ind+2)+"if (fv instanceof com.yahoo.document.datatypes.StructuredFieldValue) {\n" + - ind(ind+3)+"try {\n" + - ind(ind+4)+"com.yahoo.document.datatypes.StructuredFieldValue newVal = ConcreteDocumentFactory.structTypes.get(f.getDataType().getName()).getConstructor(com.yahoo.document.datatypes.StructuredFieldValue.class).newInstance(fv);\n" + - ind(ind+4)+"setFieldValue(f, newVal);\n" + - ind(ind+3)+"} catch (java.lang.Exception ex) { throw new java.lang.RuntimeException(ex); }\n" + - ind(ind+2)+"} else {\n" + - ind(ind+3)+"setFieldValue(f, fv);\n" + - ind(ind+2)+"}\n" + + ind(ind+2)+"setFieldValue(f, factory.optionallyUpgrade(f, fv));\n" + ind(ind+1)+"}\n"+ ind(ind)+"}\n\n"); } diff --git a/vespabase/desc.vespa_base_dev b/vespabase/desc.vespa_base_dev index de19051806b..1f05be94257 100644 --- a/vespabase/desc.vespa_base_dev +++ b/vespabase/desc.vespa_base_dev @@ -33,5 +33,3 @@ Development tools and 3rd party libraries: * vespa_boost_dev: Boost headers, prepared for use with the Vespa compiler. - * vespa_cppunit_dev: Headers and libraries prepared with the - Vespa compiler. |