diff options
65 files changed, 586 insertions, 237 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 1f4ecfd3793..1b494379413 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -209,8 +209,8 @@ "public com.yahoo.config.provision.Environment environment()", "public java.util.Optional region()", "public boolean active()", - "public java.util.Optional athenzService()", "public java.util.Optional testerFlavor()", + "public java.util.Optional athenzService()", "public java.util.List zones()", "public boolean deploysTo(com.yahoo.config.provision.Environment, java.util.Optional)", "public int hashCode()", @@ -487,16 +487,17 @@ ], "methods": [ "public void <init>(java.util.List)", - "public void invalid(com.yahoo.config.application.api.ValidationId, java.lang.String, java.time.Instant)", - "public boolean allows(java.lang.String, java.time.Instant)", + "public final void invalid(com.yahoo.config.application.api.ValidationId, java.lang.String, java.time.Instant)", + "public final boolean allows(java.lang.String, java.time.Instant)", "public boolean allows(com.yahoo.config.application.api.ValidationId, java.time.Instant)", - "public static java.lang.String toAllowMessage(com.yahoo.config.application.api.ValidationId)", "public java.lang.String xmlForm()", + "public static java.lang.String toAllowMessage(com.yahoo.config.application.api.ValidationId)", "public static com.yahoo.config.application.api.ValidationOverrides fromXml(java.io.Reader)", "public static com.yahoo.config.application.api.ValidationOverrides fromXml(java.lang.String)" ], "fields": [ - "public static final com.yahoo.config.application.api.ValidationOverrides empty" + "public static final com.yahoo.config.application.api.ValidationOverrides empty", + "public static final com.yahoo.config.application.api.ValidationOverrides all" ] } }
\ No newline at end of file diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java index 441ef273a6f..e076ffd0f10 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java @@ -16,6 +16,7 @@ import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.logging.Logger; /** * A set of allows which suppresses specific validations in limited time periods. @@ -27,7 +28,12 @@ import java.util.Optional; */ public class ValidationOverrides { - public static final ValidationOverrides empty = new ValidationOverrides(ImmutableList.of(), "<deployment version='1.0'/>"); + private static final Logger log = Logger.getLogger(ValidationOverrides.class.getName()); + + public static final ValidationOverrides empty = new ValidationOverrides(ImmutableList.of(), "<validation-overrides/>"); + + /** A special instance which behaves as if it contained a valid allow override for every (valid) validation id */ + public static final ValidationOverrides all = new AllowAllValidationOverrides(); private final List<Allow> overrides; @@ -44,12 +50,12 @@ public class ValidationOverrides { } /** Throws a ValidationException unless this validation is overridden at this time */ - public void invalid(ValidationId validationId, String message, Instant now) { + public final void invalid(ValidationId validationId, String message, Instant now) { if ( ! allows(validationId, now)) throw new ValidationException(validationId, message); } - public boolean allows(String validationIdString, Instant now) { + public final boolean allows(String validationIdString, Instant now) { Optional<ValidationId> validationId = ValidationId.from(validationIdString); if ( ! validationId.isPresent()) return false; // unknown id -> not allowed return allows(validationId.get(), now); @@ -66,14 +72,14 @@ public class ValidationOverrides { return false; } + /** Returns the XML form of this, or null if it was not created by fromXml, nor is empty */ + public String xmlForm() { return xmlForm; } + public static String toAllowMessage(ValidationId id) { return "To allow this add <allow until='yyyy-mm-dd'>" + id + "</allow> to validation-overrides.xml" + ", see https://docs.vespa.ai/documentation/reference/validation-overrides.html"; } - /** Returns the XML form of this, or null if it was not created by fromXml, nor is empty */ - public String xmlForm() { return xmlForm; } - /** * Returns a ValidationOverrides instance with the content of the given Reader. * @@ -166,4 +172,26 @@ public class ValidationOverrides { } + private static class AllowAllValidationOverrides extends ValidationOverrides { + + public AllowAllValidationOverrides() { + super(List.of()); + } + + /** Returns whether the given (assumed invalid) change is allowed by this at the moment */ + @Override + public boolean allows(ValidationId validationId, Instant now) { + log.warning("Possibly destructive change '" + validationId + "' allowed"); + return true; + } + + /** Returns the XML form of this, or null if it was not created by fromXml, nor is empty */ + @Override + public String xmlForm() { return null; } + + @Override + public String toString() { return "(A validation override which allows everything)"; } + + } + } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java index c17b9a6f220..2943b0bab34 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java @@ -8,10 +8,12 @@ import org.xml.sax.SAXException; import java.io.IOException; import java.io.StringReader; +import java.time.Clock; import java.time.Instant; import java.util.Optional; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * @author bratseth @@ -19,7 +21,7 @@ import static org.junit.Assert.assertEquals; public class ValidationOverrideTest { @Test - public void testValidationOverridesInIsolation() throws IOException, SAXException { + public void testValidationOverridesInIsolation() { String validationOverrides = "<validation-overrides>" + " <allow until='2000-01-01'>indexing-change</allow>" + @@ -55,7 +57,7 @@ public class ValidationOverrideTest { } @Test - public void testInvalidOverridePeriod() throws IOException, SAXException { + public void testInvalidOverridePeriod() { String validationOverrides = "<validation-overrides>" + " <allow until='2000-02-02'>indexing-change</allow>" + @@ -80,6 +82,15 @@ public class ValidationOverrideTest { assertEquals(empty.xmlForm(), emptyReserialized.xmlForm()); } + @Test + public void testAll() { + ValidationOverrides all = ValidationOverrides.all; + assertTrue(all.allows(ValidationId.deploymentRemoval, Clock.systemUTC().instant())); + assertTrue(all.allows(ValidationId.contentClusterRemoval, Clock.systemUTC().instant())); + assertTrue(all.allows(ValidationId.indexModeChange, Clock.systemUTC().instant())); + assertTrue(all.allows(ValidationId.fieldTypeChange, Clock.systemUTC().instant())); + } + private void assertOverridden(String validationId, ValidationOverrides overrides, Instant now) { overrides.invalid(ValidationId.from(validationId).get(), "message", now); // should not throw exception } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 910eff065d1..7a981cd6a53 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -121,7 +121,12 @@ public class DeployState implements ConfigDefinitionStore { this.importedModels = new ImportedMlModels(applicationPackage.getFileReference(ApplicationPackage.MODELS_DIR), modelImporters); - this.validationOverrides = applicationPackage.getValidationOverrides().map(ValidationOverrides::fromXml).orElse(ValidationOverrides.empty); + this.validationOverrides = + zone.environment().isManuallyDeployed() + ? ValidationOverrides.all // Don't protect manually deployed zones + : applicationPackage.getValidationOverrides().map(ValidationOverrides::fromXml) + .orElse(ValidationOverrides.empty); + this.wantedNodeVespaVersion = wantedNodeVespaVersion; this.now = now; } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java index a95ae1d1706..3f6b5dac2dc 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java @@ -9,6 +9,11 @@ import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.config.model.provision.InMemoryProvisioner; import com.yahoo.config.model.test.MockApplicationPackage; +import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.Zone; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; @@ -43,10 +48,14 @@ public class ValidationTester { * * @param previousModel the previous model, or null if no previous * @param services the services file content + * @param environment the environment this deploys to * @param validationOverrides the validation overrides file content, or null if none * @return the new model and any change actions */ - public Pair<VespaModel, List<ConfigChangeAction>> deploy(VespaModel previousModel, String services, String validationOverrides) { + public Pair<VespaModel, List<ConfigChangeAction>> deploy(VespaModel previousModel, + String services, + Environment environment, + String validationOverrides) { Instant now = LocalDate.parse("2000-01-01", DateTimeFormatter.ISO_DATE).atStartOfDay().atZone(ZoneOffset.UTC).toInstant(); ApplicationPackage newApp = new MockApplicationPackage.Builder() .withServices(services) @@ -55,6 +64,10 @@ public class ValidationTester { .build(); VespaModelCreatorWithMockPkg newModelCreator = new VespaModelCreatorWithMockPkg(newApp); DeployState.Builder deployStateBuilder = new DeployState.Builder() + .zone(new Zone(CloudName.defaultName(), + SystemName.defaultSystem(), + environment, + RegionName.defaultName())) .applicationPackage(newApp) .properties(new TestProperties().setHostedVespa(true)) .modelHostProvisioner(new InMemoryProvisioner(nodeCount)) diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidatorTest.java index 4c3583ba0ae..bce28dd9236 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ClusterSizeReductionValidatorTest.java @@ -5,6 +5,7 @@ import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ConfigChangeRefeedAction; +import com.yahoo.config.provision.Environment; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.search.AbstractSearchCluster; @@ -29,9 +30,9 @@ public class ClusterSizeReductionValidatorTest { public void testSizeReductionValidation() throws IOException, SAXException { ValidationTester tester = new ValidationTester(30); - VespaModel previous = tester.deploy(null, getServices(30), null).getFirst(); + VespaModel previous = tester.deploy(null, getServices(30), Environment.prod, null).getFirst(); try { - tester.deploy(previous, getServices(14), null); + tester.deploy(previous, getServices(14), Environment.prod, null); fail("Expected exception due to cluster size reduction"); } catch (IllegalArgumentException expected) { @@ -45,8 +46,8 @@ public class ClusterSizeReductionValidatorTest { public void testSizeReductionValidationMinimalDecreaseIsAllowed() throws IOException, SAXException { ValidationTester tester = new ValidationTester(30); - VespaModel previous = tester.deploy(null, getServices(3), null).getFirst(); - tester.deploy(previous, getServices(2), null); + VespaModel previous = tester.deploy(null, getServices(3), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(2), Environment.prod, null); } /* @@ -63,8 +64,8 @@ public class ClusterSizeReductionValidatorTest { public void testOverridingSizereductionValidation() throws IOException, SAXException { ValidationTester tester = new ValidationTester(30); - VespaModel previous = tester.deploy(null, getServices(30), null).getFirst(); - tester.deploy(previous, getServices(14), sizeReductionOverride); // Allowed due to override + VespaModel previous = tester.deploy(null, getServices(30), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices(14), Environment.prod, sizeReductionOverride); // Allowed due to override } private static String getServices(int size) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidatorTest.java index ee58ca67b02..84b50b37146 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidatorTest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; +import com.yahoo.config.provision.Environment; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.yolean.Exceptions; @@ -20,9 +21,9 @@ public class ContentClusterRemovalValidatorTest { public void testContentRemovalValidation() { ValidationTester tester = new ValidationTester(); - VespaModel previous = tester.deploy(null, getServices("contentClusterId"), null).getFirst(); + VespaModel previous = tester.deploy(null, getServices("contentClusterId"), Environment.prod, null).getFirst(); try { - tester.deploy(previous, getServices("newContentClusterId"), null); + tester.deploy(previous, getServices("newContentClusterId"), Environment.prod, null); fail("Expected exception due to content cluster id change"); } catch (IllegalArgumentException expected) { @@ -36,8 +37,8 @@ public class ContentClusterRemovalValidatorTest { public void testOverridingContentRemovalValidation() { ValidationTester tester = new ValidationTester(); - VespaModel previous = tester.deploy(null, getServices("contentClusterId"), null).getFirst(); - tester.deploy(previous, getServices("newContentClusterId"), removalOverride); // Allowed due to override + VespaModel previous = tester.deploy(null, getServices("contentClusterId"), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices("newContentClusterId"), Environment.prod, removalOverride); // Allowed due to override } private static String getServices(String contentClusterId) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidatorTest.java index ca45520711e..c8fdb8348c3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidatorTest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; +import com.yahoo.config.provision.Environment; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.yolean.Exceptions; @@ -23,9 +24,9 @@ public class ContentTypeRemovalValidatorTest { public void testContentTypeRemovalValidation() { ValidationTester tester = new ValidationTester(); - VespaModel previous = tester.deploy(null, getServices("music"), null).getFirst(); + VespaModel previous = tester.deploy(null, getServices("music"), Environment.prod, null).getFirst(); try { - tester.deploy(previous, getServices("book"), null); + tester.deploy(previous, getServices("book"), Environment.prod, null); fail("Expected exception due to removal of context type 'music"); } catch (IllegalArgumentException expected) { @@ -40,8 +41,16 @@ public class ContentTypeRemovalValidatorTest { public void testOverridingContentTypeRemovalValidation() { ValidationTester tester = new ValidationTester(); - VespaModel previous = tester.deploy(null, getServices("music"), null).getFirst(); - tester.deploy(previous, getServices("book"), removalOverride); // Allowed due to override + VespaModel previous = tester.deploy(null, getServices("music"), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices("book"), Environment.prod, removalOverride); // Allowed due to override + } + + @Test + public void testNoOverrideNeededinDev() { + ValidationTester tester = new ValidationTester(); + + VespaModel previous = tester.deploy(null, getServices("music"), Environment.prod, null).getFirst(); + tester.deploy(previous, getServices("book"), Environment.dev, null); } private static String getServices(String documentType) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java index 65423ad1333..bc6863c9a7c 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ConfigChangeRefeedAction; +import com.yahoo.config.provision.Environment; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ValidationTester; import org.junit.Test; @@ -33,9 +34,9 @@ public class GlobalDocumentChangeValidatorTest { private void testChangeGlobalAttribute(boolean allowed, boolean oldGlobal, boolean newGlobal, String validationOverrides) { ValidationTester tester = new ValidationTester(); - VespaModel oldModel = tester.deploy(null, getServices(oldGlobal), validationOverrides).getFirst(); + VespaModel oldModel = tester.deploy(null, getServices(oldGlobal), Environment.prod, validationOverrides).getFirst(); try { - tester.deploy(oldModel, getServices(newGlobal), validationOverrides).getSecond(); + tester.deploy(oldModel, getServices(newGlobal), Environment.prod, validationOverrides).getSecond(); assertTrue(allowed); } catch (IllegalStateException e) { assertFalse(allowed); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java index e9979119d84..cca112f3bd2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ConfigChangeRefeedAction; +import com.yahoo.config.provision.Environment; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.search.AbstractSearchCluster; @@ -26,9 +27,9 @@ public class IndexingModeChangeValidatorTest { ValidationTester tester = new ValidationTester(); VespaModel oldModel = - tester.deploy(null, getServices(AbstractSearchCluster.IndexingMode.REALTIME), validationOverrides).getFirst(); + tester.deploy(null, getServices(AbstractSearchCluster.IndexingMode.REALTIME), Environment.prod, validationOverrides).getFirst(); List<ConfigChangeAction> changeActions = - tester.deploy(oldModel, getServices(AbstractSearchCluster.IndexingMode.STREAMING), validationOverrides).getSecond(); + tester.deploy(oldModel, getServices(AbstractSearchCluster.IndexingMode.STREAMING), Environment.prod, validationOverrides).getSecond(); assertRefeedChange(true, // allowed=true due to validation override "Cluster 'default' changed indexing mode from 'indexed' to 'streaming'", diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java index 6b07448ee04..00f3d457d3d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/flags/FlagsHandler.java @@ -36,7 +36,7 @@ public class FlagsHandler extends HttpHandler { @Override protected HttpResponse handleGET(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/flags/v1")) return new V1Response(flagsV1Uri(request), "data", "defined"); if (path.matches("/flags/v1/data")) return getFlagDataList(request); if (path.matches("/flags/v1/data/{flagId}")) return getFlagData(findFlagId(request, path)); @@ -47,14 +47,14 @@ public class FlagsHandler extends HttpHandler { @Override protected HttpResponse handlePUT(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/flags/v1/data/{flagId}")) return putFlagData(request, findFlagId(request, path)); throw new NotFoundException("Nothing at path '" + path + "'"); } @Override protected HttpResponse handleDELETE(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/flags/v1/data/{flagId}")) return deleteFlagData(findFlagId(request, path)); throw new NotFoundException("Nothing at path '" + path + "'"); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java index 172d9c025d5..61f5e4f1230 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java @@ -90,11 +90,11 @@ public class RemoteSession extends Session { void makeActive(ReloadHandler reloadHandler) { Curator.CompletionWaiter waiter = zooKeeperClient.getActiveWaiter(); - log.log(LogLevel.DEBUG, logPre() + "Getting session from repo: " + getSessionId()); + log.log(LogLevel.DEBUG, () -> logPre() + "Getting session from repo: " + getSessionId()); ApplicationSet app = ensureApplicationLoaded(); - log.log(LogLevel.DEBUG, logPre() + "Reloading config for " + getSessionId()); + log.log(LogLevel.DEBUG, () -> logPre() + "Reloading config for " + getSessionId()); reloadHandler.reloadConfig(app); - log.log(LogLevel.DEBUG, logPre() + "Notifying " + waiter); + log.log(LogLevel.DEBUG, () -> logPre() + "Notifying " + waiter); notifyCompletion(waiter); log.log(LogLevel.INFO, logPre() + "Session activated: " + getSessionId()); } diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index b21a496cd01..81af58b6681 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -863,6 +863,8 @@ "methods": [ "public void <init>(java.lang.String)", "public void <init>(java.lang.String, java.lang.String)", + "public void <init>(java.net.URI)", + "public void <init>(java.net.URI, java.lang.String)", "public boolean matches(java.lang.String)", "public java.lang.String get(java.lang.String)", "public java.lang.String getRest()", @@ -871,4 +873,4 @@ ], "fields": [] } -}
\ No newline at end of file +} diff --git a/container-core/src/main/java/com/yahoo/restapi/Path.java b/container-core/src/main/java/com/yahoo/restapi/Path.java index 764fa64f954..fe65245fd15 100644 --- a/container-core/src/main/java/com/yahoo/restapi/Path.java +++ b/container-core/src/main/java/com/yahoo/restapi/Path.java @@ -1,17 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.restapi; +import java.net.URI; +import java.net.URLDecoder; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.stream.Stream; /** * A path which is able to match strings containing bracketed placeholders and return the - * values given at the placeholders. + * values given at the placeholders. The path is split on '/', and each part is then URL decoded. * - * E.g a path /a/1/bar/fuz - * will match /a/{foo}/bar/{b} - * and return foo=1 and b=fuz + * E.g a path /a/1/bar/fuz/baz/%62%2f + * will match /a/{foo}/bar/{b}/baz/{c} + * and return foo=1, b=fuz, and c=c/ * * Only full path elements may be placeholders, i.e /a{bar} is not interpreted as one. * @@ -21,7 +26,7 @@ import java.util.Map; * Note that for convenience in common use this has state which changes as a side effect of each matches * invocation. It is therefore for single thread use. * - * A optional prefix can be used to match the path spec against an alternative path. This + * An optional prefix can be used to match the path spec against an alternative path. This * is used when you have alternative paths mapped to the same resource. * * @author bratseth @@ -37,18 +42,36 @@ public class Path { private final Map<String, String> values = new HashMap<>(); private String rest = ""; + /** + * @deprecated use {@link #Path(URI)} for correct handling of URL encoded paths. + */ + @Deprecated public Path(String path) { - this.optionalPrefix = ""; - this.pathString = path; - this.elements = path.split("/"); + this(path, ""); } + /** + * @deprecated use {@link #Path(URI, String)} for correct handling of URL encoded paths. + */ + @Deprecated public Path(String path, String optionalPrefix) { this.optionalPrefix = optionalPrefix; this.pathString = path; this.elements = path.split("/"); } + public Path(URI uri) { + this(uri, ""); + } + + public Path(URI uri, String optionalPrefix) { + this.optionalPrefix = optionalPrefix; + this.pathString = uri.getRawPath(); + this.elements = Stream.of(this.pathString.split("/")) + .map(part -> URLDecoder.decode(part, StandardCharsets.UTF_8)) + .toArray(String[]::new); + } + private boolean matchesInner(String pathSpec) { values.clear(); String[] specElements = pathSpec.split("/"); diff --git a/container-core/src/test/java/com/yahoo/restapi/PathTest.java b/container-core/src/test/java/com/yahoo/restapi/PathTest.java index 886b3ba9c87..8d5a9bd6591 100644 --- a/container-core/src/test/java/com/yahoo/restapi/PathTest.java +++ b/container-core/src/test/java/com/yahoo/restapi/PathTest.java @@ -3,6 +3,8 @@ package com.yahoo.restapi; import org.junit.Test; +import java.net.URI; + import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; @@ -15,7 +17,7 @@ public class PathTest { @Test public void testWithPrefix() { // Test that a path with a prefix matches spec without the prefix - Path path = new Path("/ball/a/1/bar/fuz", "/ball"); + Path path = new Path(URI.create("/ball/a/1/bar/fuz"), "/ball"); assertTrue(path.matches("/a/{foo}/bar/{b}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); @@ -27,11 +29,11 @@ public class PathTest { @Test public void testPath() { - assertFalse(new Path("").matches("/a/{foo}/bar/{b}")); - assertFalse(new Path("///").matches("/a/{foo}/bar/{b}")); - assertFalse(new Path("///foo").matches("/a/{foo}/bar/{b}")); - assertFalse(new Path("///bar/").matches("/a/{foo}/bar/{b}")); - Path path = new Path("/a/1/bar/fuz"); + assertFalse(new Path(URI.create("")).matches("/a/{foo}/bar/{b}")); + assertFalse(new Path(URI.create("///")).matches("/a/{foo}/bar/{b}")); + assertFalse(new Path(URI.create("///foo")).matches("/a/{foo}/bar/{b}")); + assertFalse(new Path(URI.create("///bar/")).matches("/a/{foo}/bar/{b}")); + Path path = new Path(URI.create("/a/1/bar/fuz")); assertTrue(path.matches("/a/{foo}/bar/{b}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); @@ -40,7 +42,7 @@ public class PathTest { @Test public void testPathWithRest() { { - Path path = new Path("/a/1/bar/fuz/"); + Path path = new Path(URI.create("/a/1/bar/fuz/")); assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); @@ -48,7 +50,7 @@ public class PathTest { } { - Path path = new Path("/a/1/bar/fuz/kanoo"); + Path path = new Path(URI.create("/a/1/bar/fuz/kanoo")); assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); @@ -56,7 +58,7 @@ public class PathTest { } { - Path path = new Path("/a/1/bar/fuz/kanoo/trips"); + Path path = new Path(URI.create("/a/1/bar/fuz/kanoo/trips")); assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); @@ -64,7 +66,7 @@ public class PathTest { } { - Path path = new Path("/a/1/bar/fuz/kanoo/trips/"); + Path path = new Path(URI.create("/a/1/bar/fuz/kanoo/trips/")); assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); @@ -72,4 +74,16 @@ public class PathTest { } } + @Test + public void testUrlEncodedPath() { + assertTrue(new Path(URI.create("/a/%62/c")).matches("/a/b/c")); + assertTrue(new Path(URI.create("/a/%2e%2e/c")).matches("/a/../c")); + assertFalse(new Path(URI.create("/a/b%2fc")).matches("/a/b/c")); + + Path path = new Path(URI.create("/%61/%2f/%63")); + assertTrue(path.matches("/a/{slash}/{c}")); + assertEquals("/", path.get("slash")); + assertEquals("c", path.get("c")); + } + } diff --git a/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java b/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java index 1f47762477f..bafe1dbd43f 100644 --- a/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java +++ b/container-search-gui/src/main/java/com/yahoo/search/query/gui/GUIHandler.java @@ -9,6 +9,7 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.prelude.IndexModel; import com.yahoo.prelude.querytransform.RecallSearcher; +import com.yahoo.restapi.Path; import com.yahoo.search.Query; import com.yahoo.search.query.Model; import com.yahoo.search.query.Presentation; @@ -69,7 +70,7 @@ public class GUIHandler extends LoggingRequestHandler { } private HttpResponse handleGET(HttpRequest request) { - com.yahoo.restapi.Path path = new com.yahoo.restapi.Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/querybuilder/")) { return new FileResponse("_includes/index.html", null, null); } diff --git a/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java b/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java index e5ab00fb139..dd5a4fff584 100644 --- a/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java +++ b/container-search/src/main/java/com/yahoo/fs4/DocumentInfo.java @@ -17,14 +17,20 @@ public class DocumentInfo implements Cloneable { private final double metric; private final int partId; private final int distributionKey; + private final byte[] sortData; DocumentInfo(ByteBuffer buffer, QueryResultPacket owner) { + this(buffer, owner, null); + } + + DocumentInfo(ByteBuffer buffer, QueryResultPacket owner, byte[] sortData) { byte[] rawGid = new byte[GlobalId.LENGTH]; buffer.get(rawGid); globalId = new GlobalId(rawGid); metric = decodeMetric(buffer); partId = owner.getMldFeature() ? buffer.getInt() : 0; distributionKey = owner.getMldFeature() ? buffer.getInt() : 0; + this.sortData = sortData; } public DocumentInfo(GlobalId globalId, int metric, int partId, int distributionKey) { @@ -32,6 +38,7 @@ public class DocumentInfo implements Cloneable { this.metric = metric; this.partId = partId; this.distributionKey = distributionKey; + this.sortData = null; } private double decodeMetric(ByteBuffer buffer) { @@ -49,6 +56,10 @@ public class DocumentInfo implements Cloneable { /** Unique key for the node this document resides on */ public int getDistributionKey() { return distributionKey; } + public byte[] getSortData() { + return sortData; + } + public String toString() { return "document info [globalId=" + globalId + ", metric=" + metric + "]"; } diff --git a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java index 60bd5314cff..d1d8b9a0f77 100644 --- a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java +++ b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java @@ -5,6 +5,7 @@ import com.yahoo.compress.IntegerCompressor; import com.yahoo.io.GrowableByteBuffer; import com.yahoo.prelude.query.Item; import com.yahoo.search.Query; +import com.yahoo.search.dispatch.Dispatcher; import com.yahoo.search.grouping.vespa.GroupingExecutor; import com.yahoo.search.query.Ranking; import com.yahoo.searchlib.aggregation.Grouping; @@ -176,18 +177,7 @@ public class QueryPacket extends Packet { private int getFlagInt() { int flags = getQueryFlags(query); queryPacketData.setQueryFlags(flags); - - /* - * QFLAG_DROP_SORTDATA - * SORTDATA is a mangling of data from the attribute vectors - * which were used in the search which is byte comparable in - * such a way the comparing SORTDATA for two different hits - * will reproduce the order in which the data were returned when - * using sortspec. For now we simply drop these. If they - * become necessary, QueryResultPacket must be - * updated to be able to read the sort data. - */ - flags |= QFLAG_DROP_SORTDATA; + flags |= query.properties().getBoolean(Dispatcher.dispatchInternal, false) ? 0 : QFLAG_DROP_SORTDATA; return flags; } diff --git a/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java b/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java index 239101184ea..6a27beefb5e 100644 --- a/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java +++ b/container-search/src/main/java/com/yahoo/fs4/QueryResultPacket.java @@ -7,7 +7,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; - /** * A query result packet (code 217). This packet can be decoded only. * @@ -108,10 +107,9 @@ public class QueryResultPacket extends Packet { nodesReplied = buffer.getShort(); } - if (sortData && documentCount > 0) { // sort data is not needed - skip - buffer.position(buffer.position() + (documentCount -1) * 4); // one sortIndex int per document - int sortDataLengthInBytes = buffer.getInt(); - buffer.position(buffer.position() + sortDataLengthInBytes); + byte[][] documentSortData = null; + if (sortData && documentCount > 0) { + documentSortData = decodeSortData(buffer, documentCount); } if (groupDataFeature) { @@ -125,7 +123,7 @@ public class QueryResultPacket extends Packet { soonActiveDocs = buffer.getLong(); degradedReason = buffer.getInt(); - decodeDocuments(buffer, documentCount); + decodeDocuments(buffer, documentCount, documentSortData); if (propsFeature) { int numMaps = buffer.getInt(); propsArray = new FS4Properties[numMaps]; @@ -136,6 +134,25 @@ public class QueryResultPacket extends Packet { } } + private byte[][] decodeSortData(ByteBuffer buffer, int documentCount) { + int[] indexes = new int[documentCount]; + indexes[0] = 0; + for (int i = 1; i < documentCount; i++) { + indexes[i] = buffer.getInt(); + } + + int sortDataLengthInBytes = buffer.getInt(); + byte[][] ret = new byte[indexes.length][]; + + for (int i = 0; i < indexes.length; i++) { + int end = i + 1 >= indexes.length ? sortDataLengthInBytes : indexes[i + 1]; + int len = end - indexes[i]; + ret[i] = new byte[len]; + buffer.get(ret[i], 0, len); + } + return ret; + } + private Number decodeMaxRank(ByteBuffer buffer) { return Double.valueOf(buffer.getDouble()); } @@ -161,9 +178,10 @@ public class QueryResultPacket extends Packet { propsFeature = (QRF_PROPERTIES & features) != 0; } - private void decodeDocuments(ByteBuffer buffer, int documentCount) { + private void decodeDocuments(ByteBuffer buffer, int documentCount, byte[][] documentSortData) { for (int i = 0; i < documentCount; i++) { - documents.add(new DocumentInfo(buffer, this)); + byte[] sort = documentSortData == null ? null : documentSortData[i]; + documents.add(new DocumentInfo(buffer, this, sort)); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java index 24653db5671..2353054103d 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4SearchInvoker.java @@ -44,16 +44,11 @@ public class FS4SearchInvoker extends SearchInvoker implements ResponseMonitor<F } @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { log.finest("sending query packet"); - if (queryPacket == null) { - // query changed for subchannel - queryPacket = searcher.createQueryPacket(searcher.getServerId(), query); - } - this.query = query; - this.queryPacket = queryPacket; + this.queryPacket = searcher.createQueryPacket(searcher.getServerId(), query); try { boolean couldSend = channel.sendPacket(queryPacket); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java index ff719c3eadc..a06b1518a12 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java @@ -5,11 +5,13 @@ import com.yahoo.data.access.ObjectTraverser; import com.yahoo.document.GlobalId; import com.yahoo.fs4.QueryPacketData; import com.yahoo.net.URI; +import com.yahoo.search.query.Sorting; import com.yahoo.search.result.Hit; import com.yahoo.search.result.Relevance; import com.yahoo.data.access.Inspector; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -45,6 +47,9 @@ public class FastHit extends Hit { private transient QueryPacketData queryPacketData = null; + private transient byte[] sortData = null; + private transient Sorting sortDataSorting = null; + /** * Summaries added to this hit which are not yet decoded into fields. * Fields are resolved by returning the first non-null value found by @@ -148,6 +153,33 @@ public class FastHit extends Hit { /** Returns a serial encoding of the query which produced this hit, ot null if not available. */ public QueryPacketData getQueryPacketData() { return queryPacketData; } + public void setSortData(byte[] data, Sorting sorting) { + this.sortData = data; + this.sortDataSorting = sorting; + } + + public boolean hasSortData(Sorting sorting) { + return sortData != null && sortDataSorting != null && sortDataSorting.equals(sorting); + } + + public static int compareSortData(FastHit left, FastHit right, Sorting sorting) { + if (!left.hasSortData(sorting) || !right.hasSortData(sorting)) { + return 0; // cannot sort + } + int i = Arrays.mismatch(left.sortData, right.sortData); + if (i < 0) { + return 0; + } + int max = Integer.min(left.sortData.length, right.sortData.length); + if (i >= max) { + return left.sortData.length - right.sortData.length; + } + int vl = (int) left.sortData[i] & 0xFF; + int vr = (int) right.sortData[i] & 0xFF; + int diff = vl - vr; + return diff; + } + /** For internal use */ public void addSummary(DocsumDefinition docsumDef, Inspector value) { if (removedFields != null) diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java index e61ed1715b6..5f6d4220dd4 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java @@ -142,11 +142,11 @@ public class FastSearcher extends VespaBackEndSearcher { } @Override - public Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + public Result doSearch2(Query query, Execution execution) { if (dispatcher.searchCluster().groupSize() == 1) forceSinglePassGrouping(query); try(SearchInvoker invoker = getSearchInvoker(query)) { - Result result = invoker.search(query, queryPacket, execution); + Result result = invoker.search(query, execution); if (query.properties().getBoolean(Ranking.RANKFEATURES, false)) { // There is currently no correct choice for which diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java new file mode 100644 index 00000000000..e87c94fa3be --- /dev/null +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/SortDataHitSorter.java @@ -0,0 +1,64 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.prelude.fastsearch; + +import com.yahoo.search.query.Sorting; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; + +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +public class SortDataHitSorter { + public static void sort(HitGroup hitGroup, List<Hit> hits) { + var sorting = hitGroup.getQuery().getRanking().getSorting(); + var fallbackOrderer = hitGroup.getOrderer(); + if (sorting == null || fallbackOrderer == null) { + return; + } + var fallbackComparator = fallbackOrderer.getComparator(); + Collections.sort(hits, getComparator(sorting, fallbackComparator)); + } + + public static boolean isSortable(Hit hit, HitGroup hitGroup) { + if (hitGroup.getQuery() == null) { + return false; + } + if (hit instanceof FastHit) { + var fhit = (FastHit) hit; + return fhit.hasSortData(hitGroup.getQuery().getRanking().getSorting()); + } else { + return false; + } + } + + public static Comparator<Hit> getComparator(Sorting sorting, Comparator<Hit> fallback) { + if (fallback == null) { + return (left, right) -> compareTwo(left, right, sorting); + } else { + return (left, right) -> compareWithFallback(left, right, sorting, fallback); + } + } + + private static int compareTwo(Hit left, Hit right, Sorting sorting) { + if (left == null || right == null || !(left instanceof FastHit) || !(right instanceof FastHit)) { + return 0; + } + FastHit fl = (FastHit) left; + FastHit fr = (FastHit) right; + return FastHit.compareSortData(fl, fr, sorting); + } + + private static int compareWithFallback(Hit left, Hit right, Sorting sorting, Comparator<Hit> fallback) { + if (left == null || right == null || !(left instanceof FastHit) || !(right instanceof FastHit)) { + return fallback.compare(left, right); + } + FastHit fl = (FastHit) left; + FastHit fr = (FastHit) right; + if (fl.hasSortData(sorting) && fr.hasSortData(sorting)) { + return FastHit.compareSortData(fl, fr, sorting); + } else { + return fallback.compare(left, right); + } + } +} diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java index df72720a46c..f5e437c2410 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java @@ -24,6 +24,7 @@ import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.cluster.PingableSearcher; import com.yahoo.search.grouping.vespa.GroupingExecutor; +import com.yahoo.search.query.Sorting; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorHit; import com.yahoo.search.result.ErrorMessage; @@ -88,10 +89,9 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { * This is an endpoint - searchers will never propagate the search to any nested searcher. * * @param query the query to search - * @param queryPacket the serialized query representation to pass to the search cluster * @param execution the query execution context */ - protected abstract Result doSearch2(Query query, QueryPacket queryPacket, Execution execution); + protected abstract Result doSearch2(Query query, Execution execution); protected abstract void doPartialFill(Result result, String summaryClass); @@ -184,15 +184,10 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { if (root == null || root instanceof NullItem) // root can become null after resolving and transformation? return new Result(query); - QueryPacket queryPacket = createQueryPacket(serverId, query); - - if (isLoggingFine()) - getLogger().fine("made QueryPacket: " + queryPacket); - Result result = null; if (result == null) { - result = doSearch2(query, queryPacket, execution); + result = doSearch2(query, execution); if (isLoggingFine()) getLogger().fine("Result NOT retrieved from cache"); @@ -209,6 +204,10 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { queryPacket.setCompressionLimit(compressionLimit); if (compressionLimit != 0) queryPacket.setCompressionType(query.properties().getString(PACKET_COMPRESSION_TYPE, "lz4")); + + if (isLoggingFine()) + getLogger().fine("made QueryPacket: " + queryPacket); + return queryPacket; } @@ -452,7 +451,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { return new FillHitsResult(skippedHits, lastError); } - private void extractDocumentInfo(FastHit hit, DocumentInfo document) { + private void extractDocumentInfo(FastHit hit, DocumentInfo document, Sorting sorting) { hit.setSource(getName()); Number rank = document.getMetric(); @@ -462,6 +461,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { hit.setDistributionKey(document.getDistributionKey()); hit.setGlobalId(document.getGlobalId()); hit.setPartId(document.getPartId()); + hit.setSortData(document.getSortData(), sorting); } protected DocsumDefinitionSet getDocsumDefinitionSet(Query query) { @@ -498,6 +498,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { QueryPacketData queryPacketData, Optional<Integer> channelDistributionKey) { Query myQuery = result.getQuery(); + Sorting sorting = myQuery.getRanking().getSorting(); for (DocumentInfo document : documents) { @@ -510,7 +511,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { hit.setFillable(); hit.setCached(false); - extractDocumentInfo(hit, document); + extractDocumentInfo(hit, document, sorting); channelDistributionKey.ifPresent(hit::setDistributionKey); result.hits().add(hit); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index f2dbb1e8557..4f546201e76 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -45,7 +45,7 @@ public class Dispatcher extends AbstractComponent { private static final int MAX_GROUP_SELECTION_ATTEMPTS = 3; /** If enabled, this internal dispatcher will be preferred over fdispatch whenever possible */ - private static final CompoundName dispatchInternal = new CompoundName("dispatch.internal"); + public static final CompoundName dispatchInternal = new CompoundName("dispatch.internal"); /** If enabled, search queries will use protobuf rpc */ public static final CompoundName dispatchProtobuf = new CompoundName("dispatch.protobuf"); @@ -135,6 +135,8 @@ public class Dispatcher extends AbstractComponent { query.setOffset(0); } emitDispatchMetric(invoker); + query.properties().set(dispatchInternal, true); + return invoker; } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java index b8c9bb205e9..457e587da40 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java @@ -1,8 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; -import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.SearchCluster; @@ -37,7 +35,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private static final Logger log = Logger.getLogger(InterleavedSearchInvoker.class.getName()); private final Set<SearchInvoker> invokers; - private final VespaBackEndSearcher searcher; private final SearchCluster searchCluster; private final LinkedBlockingQueue<SearchInvoker> availableForProcessing; private final Set<Integer> alreadyFailedNodes; @@ -49,6 +46,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private long deadline = 0; private Result result = null; + private long answeredDocs = 0; private long answeredActiveDocs = 0; private long answeredSoonActiveDocs = 0; @@ -60,11 +58,10 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private boolean trimResult = false; - public InterleavedSearchInvoker(Collection<SearchInvoker> invokers, VespaBackEndSearcher searcher, SearchCluster searchCluster, Set<Integer> alreadyFailedNodes) { + public InterleavedSearchInvoker(Collection<SearchInvoker> invokers, SearchCluster searchCluster, Set<Integer> alreadyFailedNodes) { super(Optional.empty()); this.invokers = Collections.newSetFromMap(new IdentityHashMap<>()); this.invokers.addAll(invokers); - this.searcher = searcher; this.searchCluster = searchCluster; this.availableForProcessing = newQueue(); this.alreadyFailedNodes = alreadyFailedNodes; @@ -76,7 +73,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM * will be adjusted accordingly. */ @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; invokers.forEach(invoker -> invoker.setMonitor(this)); deadline = currentTime() + query.getTimeLeft(); @@ -88,7 +85,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM trimResult = originalHits != query.getHits() || originalOffset != query.getOffset(); for (SearchInvoker invoker : invokers) { - invoker.sendSearchRequest(query, null); + invoker.sendSearchRequest(query); askedNodes++; } @@ -128,10 +125,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private void trimResult(Execution execution) { if (trimResult) { - if (result.getHitOrderer() != null) { - searcher.fill(result, Execution.ATTRIBUTEPREFETCH, execution); - } - result.hits().trim(query.getOffset(), query.getHits()); } } @@ -209,7 +202,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM result = partialResult; return; } - result.mergeWith(partialResult); result.hits().addAll(partialResult.hits().asUnorderedHits()); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java index 8617c74ec41..0ce1b74c02d 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java @@ -93,7 +93,7 @@ public abstract class InvokerFactory { if (invokers.size() == 1 && failed == null) { return Optional.of(invokers.get(0)); } else { - return Optional.of(new InterleavedSearchInvoker(invokers, searcher, searchCluster, failed)); + return Optional.of(new InterleavedSearchInvoker(invokers, searchCluster, failed)); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java index 4dfdd65714d..b1d25dddbb8 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchErrorInvoker.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.result.Coverage; @@ -35,7 +34,7 @@ public class SearchErrorInvoker extends SearchInvoker { } @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; if(monitor != null) { monitor.responseAvailable(this); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java index 1650494db3a..3bfb9089457 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchInvoker.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.Node; @@ -31,14 +30,14 @@ public abstract class SearchInvoker extends CloseableInvoker { * nodes, the provided {@link Execution} may be used to retrieve document summaries required * for correct result windowing. */ - public Result search(Query query, QueryPacket queryPacket, Execution execution) throws IOException { - sendSearchRequest(query, queryPacket); + public Result search(Query query, Execution execution) throws IOException { + sendSearchRequest(query); Result result = getSearchResult(execution); setFinalStatus(result.hits().getError() == null); return result; } - protected abstract void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException; + protected abstract void sendSearchRequest(Query query) throws IOException; protected abstract Result getSearchResult(Execution execution) throws IOException; diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java index 9903aacdda0..5a8102ff5c7 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java @@ -181,13 +181,14 @@ public class ProtobufSerialization { result.hits().add(hit); } + var sorting = query.getRanking().getSorting(); for (var replyHit : protobuf.getHitsList()) { FastHit hit = new FastHit(); hit.setQuery(query); hit.setRelevance(new Relevance(replyHit.getRelevance())); hit.setGlobalId(new GlobalId(replyHit.getGlobalId().toByteArray())); - + hit.setSortData(replyHit.getSortData().toByteArray(), sorting); hit.setFillable(); hit.setCached(false); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java index e54ac00f821..d70a7d95b63 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java @@ -3,7 +3,6 @@ package com.yahoo.search.dispatch.rpc; import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; -import com.yahoo.fs4.QueryPacket; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; @@ -44,7 +43,7 @@ public class RpcSearchInvoker extends SearchInvoker implements Client.ResponseRe } @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; CompressionType compression = CompressionType diff --git a/container-search/src/main/java/com/yahoo/search/query/Model.java b/container-search/src/main/java/com/yahoo/search/query/Model.java index a874ed45e30..d2f59e0710e 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Model.java +++ b/container-search/src/main/java/com/yahoo/search/query/Model.java @@ -170,8 +170,8 @@ public class Model implements Cloneable { } /** - * <p>Explicitly sets the locale to be used during parsing. This method also calls {@link #setLanguage(Language)} - * with the corresponding {@link Language} instance.</p> + * Explicitly sets the locale to be used during parsing. This method also calls {@link #setLanguage(Language)} + * with the corresponding {@link Language} instance. * * @param locale the locale to set * @see #getLocale() diff --git a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java index ca776aef011..fdebe78c20b 100644 --- a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java +++ b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java @@ -6,6 +6,7 @@ import com.google.common.collect.Iterables; import com.google.common.util.concurrent.ListenableFuture; import com.yahoo.collections.ListenableArrayList; import com.yahoo.net.URI; +import com.yahoo.prelude.fastsearch.SortDataHitSorter; import com.yahoo.processing.response.ArrayDataList; import com.yahoo.processing.response.DataList; import com.yahoo.processing.response.DefaultIncomingData; @@ -19,8 +20,6 @@ import java.util.List; import java.util.Set; import java.util.stream.Collectors; -import static com.yahoo.collections.CollectionUtil.first; - /** * <p>A group of ordered hits. Since hitGroup is itself a kind of Hit, * this can compose hierarchies of grouped hits. @@ -63,6 +62,9 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< /** The current number of concrete (non-meta) hits in the result */ private int concreteHitCount = 0; + /** Number of hits known to have sort data */ + private int hitsWithSortData = 0; + /** The class used to determine the ordering of the hits of this */ transient private HitOrderer hitOrderer = null; @@ -398,7 +400,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< return errorHit; } - /** + /** * Removes the error hit of this. * This removes all error messages of this and the query producing it. * @@ -412,7 +414,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< errorHit = null; return removed; } - + /** * Returns the first error in this result, * or null if no searcher has produced an error AND the query doesn't contain an error @@ -451,9 +453,9 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< add(queryErrors); } - /** + /** * Consumes errors from the query and returns them in a new error hit - * + * * @return the error hit containing all query errors, or null if no query errors should be consumed */ private DefaultErrorHit consumeAnyQueryErrors() { @@ -469,9 +471,9 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< /** Compatibility */ private ErrorMessage toSearchError(com.yahoo.processing.request.ErrorMessage error) { - if (error instanceof ErrorMessage) + if (error instanceof ErrorMessage) return (ErrorMessage)error; - else + else return new ErrorMessage(error.getCode(), error.getMessage(), error.getDetailedMessage(), error.getCause()); } @@ -569,6 +571,9 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< if (hitOrderer == null) { Collections.sort(hits); hitsSorted = true; + } else if (sortableWithSortData()) { + SortDataHitSorter.sort(this, hits); + hitsSorted = true; } else { // This may or may not lead to a sorted result set, but it's a best effort hitOrderer.order(hits); @@ -578,6 +583,10 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< } } + private boolean sortableWithSortData() { + return hitsWithSortData > 0 && hitsWithSortData == concreteHitCount; + } + private boolean likelyHitsHaveCorrectValueForSortFields() { if (hitOrderer == null) { return true; @@ -620,7 +629,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< /** Called before hit lists or positions are used */ private void ensureSorted() { - if ( ! orderedHits && ! hitsSorted && likelyHitsHaveCorrectValueForSortFields()) { + if ( ! orderedHits && ! hitsSorted && (sortableWithSortData() || likelyHitsHaveCorrectValueForSortFields())) { sort(); } } @@ -678,6 +687,10 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< hit.setAddNumber(size()); } + if (SortDataHitSorter.isSortable(hit, this)) { + hitsWithSortData++; + } + hitsSorted = false; Set<String> hitFilled = hit.getFilled(); @@ -726,6 +739,10 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< errorHit = null; } + if (SortDataHitSorter.isSortable(hit, this)) { + hitsWithSortData--; + } + if (deletionBreaksOrdering) { hitsSorted = false; } @@ -740,6 +757,10 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< if (!hit.isCached()) notCachedCount++; + + if (SortDataHitSorter.isSortable(hit, this)) { + hitsWithSortData++; + } } /** @@ -747,9 +768,11 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable< * Recursively also update all subgroups. */ public void analyze() { - concreteHitCount=0; + concreteHitCount = 0; setFilledInternal(null); - notCachedCount=0; + notCachedCount = 0; + hitsWithSortData = 0; + Set<String> filled = getFilledInternal(); Iterator<Hit> i = unorderedIterator(); diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java index e41a582ea13..0fd3d5b5aa1 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java @@ -1,19 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.streamingvisitors; -import java.io.IOException; -import java.math.BigInteger; -import java.util.List; -import java.util.Map; -import java.util.logging.Logger; - import com.yahoo.document.DocumentId; import com.yahoo.document.idstring.IdString; import com.yahoo.document.select.parser.ParseException; import com.yahoo.document.select.parser.TokenMgrException; import com.yahoo.fs4.DocsumPacket; import com.yahoo.fs4.Packet; -import com.yahoo.fs4.QueryPacket; import com.yahoo.log.LogLevel; import com.yahoo.messagebus.routing.Route; import com.yahoo.prelude.Ping; @@ -22,9 +15,9 @@ import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.GroupingListHit; import com.yahoo.prelude.fastsearch.TimeoutException; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; +import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; -import com.yahoo.processing.request.CompoundName; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.result.Relevance; @@ -34,6 +27,12 @@ import com.yahoo.vdslib.DocumentSummary; import com.yahoo.vdslib.SearchResult; import com.yahoo.vdslib.VisitorStatistics; +import java.io.IOException; +import java.math.BigInteger; +import java.util.List; +import java.util.Map; +import java.util.logging.Logger; + /** * The searcher which forwards queries to storage nodes using visiting. * The searcher is a visitor client responsible for starting search @@ -92,7 +91,7 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { } @Override - public Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + public Result doSearch2(Query query, Execution execution) { // TODO refactor this method into smaller methods, it's hard to see the actual code lazyTrace(query, 7, "Routing to storage cluster ", getStorageClusterRouteSpec()); diff --git a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java index 1e19a1397e0..84f986d5371 100644 --- a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java @@ -201,7 +201,7 @@ public class ClusterSearcherTestCase { } @Override - protected com.yahoo.search.Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected com.yahoo.search.Result doSearch2(Query query, Execution execution) { return null; // search() is overriden, this should never be called } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java index 8fd3bc1f3eb..b9119528490 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/FS4SearchInvokerTestCase.java @@ -3,7 +3,6 @@ package com.yahoo.prelude.fastsearch; import com.yahoo.fs4.BasicPacket; -import com.yahoo.fs4.QueryPacket; import com.yahoo.fs4.mplex.FS4Channel; import com.yahoo.fs4.mplex.InvalidChannelException; import com.yahoo.search.Query; @@ -31,10 +30,10 @@ public class FS4SearchInvokerTestCase { var searcher = mockSearcher(); var cluster = new MockSearchCluster("?", 1, 1); var fs4invoker = new FS4SearchInvoker(searcher, query, mockFailingChannel(), Optional.empty()); - var interleave = new InterleavedSearchInvoker(Collections.singleton(fs4invoker), searcher, cluster, null); + var interleave = new InterleavedSearchInvoker(Collections.singleton(fs4invoker), cluster, null); long start = System.currentTimeMillis(); - interleave.search(query, QueryPacket.create(null, null), null); + interleave.search(query, null); long elapsed = System.currentTimeMillis() - start; assertThat("Connection error should fail fast", elapsed, Matchers.lessThan(500L)); @@ -43,7 +42,7 @@ public class FS4SearchInvokerTestCase { private static VespaBackEndSearcher mockSearcher() { return new VespaBackEndSearcher() { @Override - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { return null; } diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java index ed0441d8450..cb43f2e17e3 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/PartialFillTestCase.java @@ -3,7 +3,6 @@ package com.yahoo.prelude.fastsearch.test; import com.google.common.util.concurrent.MoreExecutors; import com.yahoo.component.chain.Chain; -import com.yahoo.fs4.QueryPacket; import com.yahoo.language.simple.SimpleLinguistics; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; @@ -32,7 +31,7 @@ public class PartialFillTestCase { public static class FS4 extends VespaBackEndSearcher { public List<Result> history = new ArrayList<>(); - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { return new Result(query); } protected void doPartialFill(Result result, String summaryClass) { @@ -41,7 +40,7 @@ public class PartialFillTestCase { } public static class BadFS4 extends VespaBackEndSearcher { - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { return new Result(query); } protected void doPartialFill(Result result, String summaryClass) { diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java index f84f35020d2..8686ddf229b 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java @@ -49,7 +49,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(4900, 100, 1)); expectedEvents.add(new Event(4800, 100, 2)); - invoker.search(query, null, null); + invoker.search(query, null); assertTrue("All test scenario events processed", expectedEvents.isEmpty()); } @@ -63,7 +63,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(4700, 300, 1)); expectedEvents.add(null); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); assertTrue("All test scenario events processed", expectedEvents.isEmpty()); assertNull("Result is not marked as an error", result.hits().getErrorHit()); @@ -82,7 +82,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(2400, 100, 2)); expectedEvents.add(new Event(0, 0, null)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); assertTrue("All test scenario events processed", expectedEvents.isEmpty()); assertNull("Result is not marked as an error", result.hits().getErrorHit()); @@ -101,7 +101,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(100000L)); @@ -122,7 +122,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(23420L)); @@ -144,7 +144,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(9900L)); @@ -166,7 +166,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(null); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(50155L)); @@ -191,7 +191,7 @@ public class InterleavedSearchInvokerTest { expectedEvents.add(new Event(null, 1, 1)); expectedEvents.add(new Event(null, 100, 0)); - Result result = invoker.search(query, null, null); + Result result = invoker.search(query, null); Coverage cov = result.getCoverage(true); assertThat(cov.getDocs(), is(50155L)); @@ -209,7 +209,7 @@ public class InterleavedSearchInvokerTest { invokers.add(new MockInvoker(i)); } - return new InterleavedSearchInvoker(invokers, null, searchCluster, null) { + return new InterleavedSearchInvoker(invokers, searchCluster, null) { @Override protected long currentTime() { return clock.millis(); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java index 00b1edec8b3..e347a884c17 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java @@ -1,7 +1,6 @@ // Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.fs4.QueryPacket; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.Node; @@ -25,7 +24,7 @@ class MockInvoker extends SearchInvoker { } @Override - protected void sendSearchRequest(Query query, QueryPacket queryPacket) throws IOException { + protected void sendSearchRequest(Query query) throws IOException { this.query = query; } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java index b9d894f9eb5..64863b9a8a6 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/rpc/RpcSearchInvokerTest.java @@ -5,7 +5,6 @@ package com.yahoo.search.dispatch.rpc; import ai.vespa.searchlib.searchprotocol.protobuf.SearchProtocol; import com.google.common.collect.ImmutableMap; import com.yahoo.compress.CompressionType; -import com.yahoo.fs4.QueryPacket; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; @@ -40,7 +39,7 @@ public class RpcSearchInvokerTest { var invoker = new RpcSearchInvoker(mockSearcher(), new Node(7, "seven", 77, 1), mockPool); Query q = new Query("search/?query=test&hits=10&offset=3"); - invoker.sendSearchRequest(q, null); + invoker.sendSearchRequest(q); var bytes = mockPool.compressor().decompress(payloadHolder.get(), compressionTypeHolder.get(), lengthHolder.get()); var request = SearchProtocol.SearchRequest.newBuilder().mergeFrom(bytes).build(); @@ -78,7 +77,7 @@ public class RpcSearchInvokerTest { private VespaBackEndSearcher mockSearcher() { return new VespaBackEndSearcher() { @Override - protected Result doSearch2(Query query, QueryPacket queryPacket, Execution execution) { + protected Result doSearch2(Query query, Execution execution) { fail("Unexpected call"); return null; } diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java index f4be9bb543b..b656a509683 100644 --- a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcherTestCase.java @@ -136,9 +136,8 @@ public class VdsStreamingSearcherTestCase { } private static Result executeQuery(VdsStreamingSearcher searcher, Query query) { - QueryPacket queryPacket = QueryPacket.create("container.0", query); Execution execution = new Execution(new Execution.Context(null, null, null, null, null)); - return searcher.doSearch2(query, queryPacket, execution); + return searcher.doSearch2(query, execution); } private static Query[] generateTestQueries(String queryString) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 402f91f1a14..fb27247c48a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -164,7 +164,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse handleGET(HttpRequest request) { - Path path = new Path(request.getUri().getPath(), OPTIONAL_PREFIX); + Path path = new Path(request.getUri(), OPTIONAL_PREFIX); if (path.matches("/application/v4/")) return root(request); if (path.matches("/application/v4/user")) return authenticatedUser(request); if (path.matches("/application/v4/tenant")) return tenants(request); @@ -187,7 +187,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse handlePUT(HttpRequest request) { - Path path = new Path(request.getUri().getPath(), OPTIONAL_PREFIX); + Path path = new Path(request.getUri(), OPTIONAL_PREFIX); if (path.matches("/application/v4/user")) return createUser(request); if (path.matches("/application/v4/tenant/{tenant}")) return updateTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override")) @@ -196,7 +196,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse handlePOST(HttpRequest request) { - Path path = new Path(request.getUri().getPath(), OPTIONAL_PREFIX); + Path path = new Path(request.getUri(), OPTIONAL_PREFIX); if (path.matches("/application/v4/tenant/{tenant}")) return createTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return createApplication(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/promote")) return promoteApplication(path.get("tenant"), path.get("application"), request); @@ -215,14 +215,14 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } private HttpResponse handlePATCH(HttpRequest request) { - Path path = new Path(request.getUri().getPath(), OPTIONAL_PREFIX); + Path path = new Path(request.getUri(), OPTIONAL_PREFIX); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return setMajorVersion(path.get("tenant"), path.get("application"), request); return ErrorResponse.notFoundError("Nothing at " + path); } private HttpResponse handleDELETE(HttpRequest request) { - Path path = new Path(request.getUri().getPath(), OPTIONAL_PREFIX); + Path path = new Path(request.getUri(), OPTIONAL_PREFIX); if (path.matches("/application/v4/tenant/{tenant}")) return deleteTenant(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return deleteApplication(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return cancelDeploy(path.get("tenant"), path.get("application"), "all"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/athenz/AthenzApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/athenz/AthenzApiHandler.java index 44164281411..e048c963641 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/athenz/AthenzApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/athenz/AthenzApiHandler.java @@ -60,7 +60,7 @@ public class AthenzApiHandler extends LoggingRequestHandler { } private HttpResponse get(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/athenz/v1")) return root(request); if (path.matches("/athenz/v1/domains")) return domainList(request); if (path.matches("/athenz/v1/properties")) return properties(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java index fbbd8724a8c..a59e0e9130f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java @@ -65,7 +65,7 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { } private HttpResponse get(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/controller/v1/")) return root(request); if (path.matches("/controller/v1/auditlog/")) return new AuditLogResponse(controller.auditLogger().readLog()); if (path.matches("/controller/v1/maintenance/")) return new JobsResponse(maintenance.jobControl()); @@ -74,21 +74,21 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { } private HttpResponse post(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/controller/v1/maintenance/inactive/{jobName}")) return setActive(path.get("jobName"), false); if (path.matches("/controller/v1/jobs/upgrader/confidence/{version}")) return overrideConfidence(request, path.get("version")); return notFound(path); } private HttpResponse delete(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/controller/v1/maintenance/inactive/{jobName}")) return setActive(path.get("jobName"), true); if (path.matches("/controller/v1/jobs/upgrader/confidence/{version}")) return removeConfidenceOverride(path.get("version")); return notFound(path); } private HttpResponse patch(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/controller/v1/jobs/upgrader")) return configureUpgrader(request); return notFound(path); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostApiHandler.java index a82d2f22e74..bae790a49ad 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostApiHandler.java @@ -35,7 +35,7 @@ public class CostApiHandler extends LoggingRequestHandler { return ErrorResponse.methodNotAllowed("Method '" + request.getMethod() + "' is not supported"); } - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/cost/v1/csv")) { Optional<String> cloudProperty = Optional.ofNullable(request.getProperty("cloud")); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiHandler.java index 35fa812851a..0c5cfc539f1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiHandler.java @@ -54,7 +54,7 @@ public class BadgeApiHandler extends LoggingRequestHandler { } private HttpResponse get(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/badge/v1/{tenant}/{application}/{instance}")) return badge(path.get("tenant"), path.get("application"), path.get("instance")); if (path.matches("/badge/v1/{tenant}/{application}/{instance}/{jobName}")) return badge(path.get("tenant"), path.get("application"), path.get("instance"), path.get("jobName"), request.getProperty("historyLength")); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java index 8a5013a9c16..978b7e4397d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java @@ -63,7 +63,7 @@ public class DeploymentApiHandler extends LoggingRequestHandler { } private HttpResponse handleGET(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/deployment/v1/")) return root(request); return ErrorResponse.notFoundError("Nothing at " + path); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java index 21c9875bb8b..a1dfdbeb245 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleResolver.java @@ -78,6 +78,7 @@ public class AthenzRoleResolver implements RoleMembership.Resolver { if ( ! (principal instanceof AthenzPrincipal)) throw new IllegalStateException("Expected an AthenzPrincipal to be set on the request."); + @SuppressWarnings("deprecation") // TODO: Use URI when refactoring this. Path path = new Path(uriPath.orElseThrow(() -> new IllegalArgumentException("This resolver needs the request path."))); path.matches("/application/v4/tenant/{tenant}/{*}"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java index 22747ab8510..dfcc5f732f8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java @@ -59,11 +59,11 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { Action action = Action.from(HttpRequest.Method.valueOf(request.getMethod())); // Avoid expensive lookups when request is always legal. - if (RoleMembership.everyoneIn(controller.system()).allows(action, request.getRequestURI())) + if (RoleMembership.everyoneIn(controller.system()).allows(action, request.getUri())) return Optional.empty(); RoleMembership roles = this.roleResolver.membership(principal, Optional.of(request.getRequestURI())); - if (roles.allows(action, request.getRequestURI())) + if (roles.allows(action, request.getUri())) return Optional.empty(); } catch (Exception e) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java index 3c0f515aa8a..73bbcb1c383 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java @@ -66,19 +66,19 @@ public class OsApiHandler extends AuditLoggingRequestHandler { } private HttpResponse patch(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/os/v1/")) return new SlimeJsonResponse(setOsVersion(request)); return ErrorResponse.notFoundError("Nothing at " + path); } private HttpResponse get(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/os/v1/")) return new SlimeJsonResponse(osVersions()); return ErrorResponse.notFoundError("Nothing at " + path); } private HttpResponse post(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/os/v1/firmware/")) return requestFirmwareCheckResponse(path); if (path.matches("/os/v1/firmware/{environment}/")) return requestFirmwareCheckResponse(path); if (path.matches("/os/v1/firmware/{environment}/{region}/")) return requestFirmwareCheckResponse(path); @@ -86,7 +86,7 @@ public class OsApiHandler extends AuditLoggingRequestHandler { } private HttpResponse delete(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/os/v1/firmware/")) return cancelFirmwareCheckResponse(path); if (path.matches("/os/v1/firmware/{environment}/")) return cancelFirmwareCheckResponse(path); if (path.matches("/os/v1/firmware/{environment}/{region}/")) return cancelFirmwareCheckResponse(path); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/statuspage/StatusPageProxyHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/statuspage/StatusPageProxyHandler.java index 9021de366cb..1aa883359ee 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/statuspage/StatusPageProxyHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/statuspage/StatusPageProxyHandler.java @@ -54,7 +54,7 @@ public class StatusPageProxyHandler extends LoggingRequestHandler { } private HttpResponse handleGET(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (!path.matches("/statuspage/v1/{page}")) { return ErrorResponse.notFoundError("Nothing at " + path); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index f7693968b43..18b124778d5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -48,7 +48,7 @@ public class UserApiHandler extends LoggingRequestHandler { } private HttpResponse handleGET(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); return ErrorResponse.notFoundError(String.format("No '%s' handler at '%s'", request.getMethod(), @@ -56,7 +56,7 @@ public class UserApiHandler extends LoggingRequestHandler { } private HttpResponse handlePUT(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); return ErrorResponse.notFoundError(String.format("No '%s' handler at '%s'", request.getMethod(), @@ -64,7 +64,7 @@ public class UserApiHandler extends LoggingRequestHandler { } private HttpResponse handlePOST(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); return ErrorResponse.notFoundError(String.format("No '%s' handler at '%s'", request.getMethod(), @@ -72,7 +72,7 @@ public class UserApiHandler extends LoggingRequestHandler { } private HttpResponse handleDELETE(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); return ErrorResponse.notFoundError(String.format("No '%s' handler at '%s'", request.getMethod(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java index 6f014e5661f..94a2004197a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java @@ -54,7 +54,7 @@ public class ZoneApiHandler extends LoggingRequestHandler { } private HttpResponse get(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/zone/v1")) { return root(request); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java index 377a57fbb91..65f0abb16c8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java @@ -66,7 +66,7 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { } private HttpResponse get(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/zone/v2")) { return root(request); } @@ -74,7 +74,7 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { } private HttpResponse proxy(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if ( ! path.matches("/zone/v2/{environment}/{region}/{*}")) { return notFound(path); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java index e3d81e04591..ef97421119f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.role; import com.yahoo.restapi.Path; +import java.net.URI; import java.util.EnumSet; import java.util.List; import java.util.Optional; @@ -58,8 +59,7 @@ public enum PathGroup { /** Path used to restart application nodes. */ // TODO move to the above when everyone is on new pipeline. applicationRestart(Matcher.tenant, Matcher.application, - "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{ignored}/restart"), - + "/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{ignored}/restart"), /** Paths used for development deployments. */ developmentDeployment(Matcher.tenant, Matcher.application, @@ -127,8 +127,9 @@ public enum PathGroup { } /** Returns path if it matches any spec in this group, with match groups set by the match. */ - private Optional<Path> get(String path) { - Path matcher = new Path(path); + @SuppressWarnings("deprecation") + private Optional<Path> get(URI uri) { + Path matcher = new Path(uri); // TODO Get URI down here. for (String spec : pathSpecs) // Iterate to be sure the Path's state is that of the match. if (matcher.matches(spec)) return Optional.of(matcher); return Optional.empty(); @@ -140,8 +141,8 @@ public enum PathGroup { } /** Returns whether this group matches path in given context */ - public boolean matches(String path, Context context) { - return get(path).map(p -> { + public boolean matches(URI uri, Context context) { + return get(uri).map(p -> { boolean match = true; String tenant = p.get(Matcher.tenant.name); if (tenant != null && context.tenant().isPresent()) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java index 85702ac1b89..6ae68f598f0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; +import java.net.URI; import java.util.Set; /** @@ -114,11 +115,11 @@ public enum Policy { } /** Returns whether action is allowed on path in given context */ - public boolean evaluate(Action action, String path, Context context) { + public boolean evaluate(Action action, URI uri, Context context) { return privileges.stream().anyMatch(privilege -> privilege.actions().contains(action) && privilege.systems().contains(context.system()) && privilege.pathGroups().stream() - .anyMatch(pg -> pg.matches(path, context))); + .anyMatch(pg -> pg.matches(uri, context))); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java index cae143a92a2..d82e4063391 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java @@ -1,6 +1,7 @@ // 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.role; +import java.net.URI; import java.util.EnumSet; import java.util.Set; @@ -85,8 +86,8 @@ public enum Role { * Returns whether this role is allowed to perform action in given role context. Action is allowed if at least one * policy evaluates to true. */ - public boolean allows(Action action, String path, Context context) { - return policies.stream().anyMatch(policy -> policy.evaluate(action, path, context)); + public boolean allows(Action action, URI uri, Context context) { + return policies.stream().anyMatch(policy -> policy.evaluate(action, uri, context)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java index 2ee59d90a63..09e66528913 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java @@ -5,6 +5,7 @@ import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; +import java.net.URI; import java.security.Principal; import java.util.Collections; import java.util.HashMap; @@ -39,11 +40,11 @@ public class RoleMembership { public static Builder in(SystemName system) { return new BuilderWithRole(system); } /** Returns whether any role in this allows action to take place in path */ - public boolean allows(Action action, String path) { + public boolean allows(Action action, URI uri) { return roles.entrySet().stream().anyMatch(kv -> { Role role = kv.getKey(); Set<Context> contexts = kv.getValue(); - return contexts.stream().anyMatch(context -> role.allows(action, path, context)); + return contexts.stream().anyMatch(context -> role.allows(action, uri, context)); }); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java index f2dc22c9c60..7cc00f1ad52 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ApplicationRequestToDiscFilterRequestWrapper.java @@ -48,7 +48,7 @@ public class ApplicationRequestToDiscFilterRequestWrapper extends DiscFilterRequ @Override public URI getUri() { - return URI.create(request.getUri()); + return URI.create(request.getUri()).normalize(); // Consistent with what JDisc does. } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 8c28c289889..40d39248cb5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -461,6 +461,21 @@ public class ApplicationApiTest extends ControllerContainerTest { .screwdriverIdentity(SCREWDRIVER_ID), "Requested restart of tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/default"); + // POST a 'restart application' in staging environment command + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-central-1/instance/default/restart", POST) + .screwdriverIdentity(SCREWDRIVER_ID), + "Requested restart of tenant/tenant1/application/application1/environment/staging/region/us-central-1/instance/default"); + + // POST a 'restart application' in staging test command + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/test/region/us-central-1/instance/default/restart", POST) + .screwdriverIdentity(SCREWDRIVER_ID), + "Requested restart of tenant/tenant1/application/application1/environment/test/region/us-central-1/instance/default"); + + // POST a 'restart application' in staging dev command + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-central-1/instance/default/restart", POST) + .userIdentity(USER_ID), + "Requested restart of tenant/tenant1/application/application1/environment/dev/region/us-central-1/instance/default"); + // POST a 'restart application' command with a host filter (other filters not supported yet) tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/default/restart?hostname=host1", POST) .screwdriverIdentity(SCREWDRIVER_ID), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java index 972ddefb1a5..1da5d3764f6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java @@ -6,6 +6,8 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import org.junit.Test; +import java.net.URI; + import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -21,11 +23,11 @@ public class RoleMembershipTest { .build(); // Operator actions - assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); - assertTrue(roles.allows(Action.create, "/controller/v1/foo")); - assertTrue(roles.allows(Action.update, "/os/v1/bar")); - assertTrue(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); - assertTrue(roles.allows(Action.update, "/application/v4/tenant/t2/application/a2")); + assertFalse(roles.allows(Action.create, URI.create("/not/explicitly/defined"))); + assertTrue(roles.allows(Action.create, URI.create("/controller/v1/foo"))); + assertTrue(roles.allows(Action.update, URI.create("/os/v1/bar"))); + assertTrue(roles.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); + assertTrue(roles.allows(Action.update, URI.create("/application/v4/tenant/t2/application/a2"))); } @Test @@ -33,35 +35,35 @@ public class RoleMembershipTest { RoleMembership roles = RoleMembership.in(SystemName.main) .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) .build(); - assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); - assertFalse("Deny access to operator API", roles.allows(Action.create, "/controller/v1/foo")); - assertFalse("Deny access to other tenant and app", roles.allows(Action.update, "/application/v4/tenant/t2/application/a2")); - assertFalse("Deny access to other app", roles.allows(Action.update, "/application/v4/tenant/t1/application/a2")); - assertTrue(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + assertFalse(roles.allows(Action.create, URI.create("/not/explicitly/defined"))); + assertFalse("Deny access to operator API", roles.allows(Action.create, URI.create("/controller/v1/foo"))); + assertFalse("Deny access to other tenant and app", roles.allows(Action.update, URI.create("/application/v4/tenant/t2/application/a2"))); + assertFalse("Deny access to other app", roles.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a2"))); + assertTrue(roles.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); RoleMembership multiContext = RoleMembership.in(SystemName.main) .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t2"), ApplicationName.from("a2")) .build(); - assertFalse("Deny access to other tenant and app", multiContext.allows(Action.update, "/application/v4/tenant/t3/application/a3")); - assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t2/application/a2")); - assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + assertFalse("Deny access to other tenant and app", multiContext.allows(Action.update, URI.create("/application/v4/tenant/t3/application/a3"))); + assertTrue(multiContext.allows(Action.update, URI.create("/application/v4/tenant/t2/application/a2"))); + assertTrue(multiContext.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); RoleMembership publicSystem = RoleMembership.in(SystemName.vaas) .add(Role.athenzTenantAdmin).limitedTo(TenantName.from("t1"), ApplicationName.from("a1")) .build(); - assertFalse(publicSystem.allows(Action.read, "/controller/v1/foo")); - assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + assertFalse(publicSystem.allows(Action.read, URI.create("/controller/v1/foo"))); + assertTrue(multiContext.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); } @Test public void build_service_membership() { RoleMembership roles = RoleMembership.in(SystemName.main) .add(Role.tenantPipeline).build(); - assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); - assertFalse(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); - assertTrue(roles.allows(Action.create, "/application/v4/tenant/t1/application/a1/jobreport")); - assertFalse("No global read access", roles.allows(Action.read, "/controller/v1/foo")); + assertFalse(roles.allows(Action.create, URI.create("/not/explicitly/defined"))); + assertFalse(roles.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); + assertTrue(roles.allows(Action.create, URI.create("/application/v4/tenant/t1/application/a1/jobreport"))); + assertFalse("No global read access", roles.allows(Action.read, URI.create("/controller/v1/foo"))); } @Test @@ -71,14 +73,14 @@ public class RoleMembershipTest { .add(Role.tenantPipeline) .add(Role.everyone) .build(); - assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); - assertFalse(roles.allows(Action.create, "/controller/v1/foo")); - assertTrue(roles.allows(Action.create, "/application/v4/tenant/t1/application/a1/jobreport")); - assertTrue(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); - assertTrue("Global read access", roles.allows(Action.read, "/controller/v1/foo")); - assertTrue("Dashboard read access", roles.allows(Action.read, "/")); - assertTrue("Dashboard read access", roles.allows(Action.read, "/d/nodes")); - assertTrue("Dashboard read access", roles.allows(Action.read, "/statuspage/v1/incidents")); + assertFalse(roles.allows(Action.create, URI.create("/not/explicitly/defined"))); + assertFalse(roles.allows(Action.create, URI.create("/controller/v1/foo"))); + assertTrue(roles.allows(Action.create, URI.create("/application/v4/tenant/t1/application/a1/jobreport"))); + assertTrue(roles.allows(Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); + assertTrue("Global read access", roles.allows(Action.read, URI.create("/controller/v1/foo"))); + assertTrue("Dashboard read access", roles.allows(Action.read, URI.create("/"))); + assertTrue("Dashboard read access", roles.allows(Action.read, URI.create("/d/nodes"))); + assertTrue("Dashboard read access", roles.allows(Action.read, URI.create("/statuspage/v1/incidents"))); } } diff --git a/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java b/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java index 39345399ea5..2760f9e673e 100644 --- a/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java +++ b/linguistics/src/main/java/com/yahoo/language/LocaleFactory.java @@ -18,7 +18,7 @@ public final class LocaleFactory { * Implements a simple parser for RFC5646 language tags. The language tag is parsed into a Locale. * * @param tag The language tag to parse. - * @return The corrseponding Locale. + * @return The corresponding Locale. */ @SuppressWarnings("ConstantConditions") public static Locale fromLanguageTag(String tag) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/Templar.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/Templar.java new file mode 100644 index 00000000000..113af76972b --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/Templar.java @@ -0,0 +1,75 @@ +// 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.node.admin.task.util.file; + +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; + +/** + * A very simple template engine when there's little complexity and lots of Velocity special characters $ and #, + * i.e. typically shell script. + * + * @author hakonhall + */ +public class Templar { + private final String template; + + private String prefix = "<%="; + private String suffix = "%>"; + + private final Map<String, String> settings = new HashMap<>(); + + public static Templar fromUtf8File(Path path) { + return new Templar(new UnixPath(path).readUtf8File()); + } + + public Templar(String template) { + this.template = template; + } + + public Templar set(String name, String value) { + settings.put(name, value); + return this; + } + + public String resolve() { + StringBuilder text = new StringBuilder(template.length() * 2); + + int start= 0; + int end; + + for (; start < template.length(); start = end) { + int prefixStart = template.indexOf(prefix, start); + + + if (prefixStart == -1) { + text.append(template, start, template.length()); + break; + } else { + text.append(template, start, prefixStart); + } + + int suffixStart = template.indexOf(suffix, prefixStart + prefix.length()); + if (suffixStart == -1) { + throw new IllegalArgumentException("Prefix at offset " + prefixStart + " is not terminated"); + } + + int prefixEnd = prefixStart + prefix.length(); + String name = template.substring(prefixEnd, suffixStart).trim(); + String value = settings.get(name); + if (value == null) { + throw new IllegalArgumentException("No value is set for name '" + name + "' at offset " + prefixEnd); + } + + text.append(value); + + end = suffixStart + suffix.length(); + } + + return text.toString(); + } + + public FileWriter getFileWriterTo(Path path) { + return new FileWriter(path, this::resolve); + } +} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/TemplarTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/TemplarTest.java new file mode 100644 index 00000000000..34b8312d426 --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/TemplarTest.java @@ -0,0 +1,21 @@ +// 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.node.admin.task.util.file; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +/** + * @author hakonhall + */ +public class TemplarTest { + @Test + public void test() { + Templar templar = new Templar("x y <%= foo %>, some other <%=bar%> text"); + templar.set("foo", "fidelity") + .set("bar", "halimov") + .set("not", "used"); + + assertEquals("x y fidelity, some other halimov text", templar.resolve()); + } +}
\ No newline at end of file diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java index 9c3e83b9f5a..58125304d6f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesApiHandler.java @@ -155,7 +155,7 @@ public class NodesApiHandler extends LoggingRequestHandler { } private HttpResponse handlePOST(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/nodes/v2/command/restart")) { int restartCount = nodeRepository.restart(toNodeFilter(request)).size(); return new MessageResponse("Scheduled restart of " + restartCount + " matching nodes"); @@ -177,7 +177,7 @@ public class NodesApiHandler extends LoggingRequestHandler { } private HttpResponse handleDELETE(HttpRequest request) { - Path path = new Path(request.getUri().getPath()); + Path path = new Path(request.getUri()); if (path.matches("/nodes/v2/node/{hostname}")) { String hostname = path.get("hostname"); List<Node> removedNodes = nodeRepository.removeRecursively(hostname); |