diff options
75 files changed, 681 insertions, 377 deletions
diff --git a/build_settings.cmake b/build_settings.cmake index 272242ecc5c..1dfd55e7d0e 100644 --- a/build_settings.cmake +++ b/build_settings.cmake @@ -138,6 +138,14 @@ if(VALGRIND_EXECUTABLE) set(VALGRIND_OPTIONS "--leak-check=yes --error-exitcode=1 --run-libc-freeres=no --track-origins=yes --suppressions=${VALGRIND_SUPPRESSIONS_FILE}") set(VALGRIND_COMMAND "${VALGRIND_EXECUTABLE} ${VALGRIND_OPTIONS}") endif() +# Automatically set sanitizer suppressions file and arguments for unit tests +if(VESPA_USE_SANITIZER) + if(VESPA_USE_SANITIZER STREQUAL "thread") + set(VESPA_SANITIZER_SUPPRESSIONS_FILE "${PROJECT_SOURCE_DIR}/tsan-suppressions.txt") + # Maximize the amount of history we can track, including mutex order inversion histories + set(VESPA_SANITIZER_ENV "TSAN_OPTIONS=suppressions=${VESPA_SANITIZER_SUPPRESSIONS_FILE} history_size=7 detect_deadlocks=1 second_deadlock_stack=1") + endif() +endif() if(VESPA_LLVM_VERSION) else() diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java index 23b5195486b..cdcd13619e8 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java @@ -12,6 +12,7 @@ import com.yahoo.yolean.Exceptions; import java.io.File; import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -39,7 +40,7 @@ public class IntermediateCollection { this.modelProperties = properties; } - public Map<String, ParsedSchema> getParsedSchemas() { return Map.copyOf(parsedSchemas); } + public Map<String, ParsedSchema> getParsedSchemas() { return Collections.unmodifiableMap(parsedSchemas); } public ParsedSchema getParsedSchema(String name) { return parsedSchemas.get(name); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java index ebb6acbc54c..fcbb62b1229 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedField.java @@ -4,6 +4,7 @@ package com.yahoo.searchdefinition.parser; import com.yahoo.searchdefinition.document.Stemming; import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -64,7 +65,7 @@ class ParsedField extends ParsedBlock { Optional<String> getNormalizing() { return Optional.ofNullable(normalizing); } Optional<ParsedIndexingOp> getIndexing() { return Optional.ofNullable(indexingOp); } Optional<ParsedSorting> getSorting() { return Optional.ofNullable(sortSettings); } - Map<String, String> getRankTypes() { return Map.copyOf(rankTypes); } + Map<String, String> getRankTypes() { return Collections.unmodifiableMap(rankTypes); } /** get an existing summary field for modification, or create it */ ParsedSummaryField summaryFieldFor(String name) { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java index c0a9e2df73f..1918f31749d 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedRankProfile.java @@ -73,12 +73,14 @@ class ParsedRankProfile extends ParsedBlock { List<ParsedRankFunction> getFunctions() { return List.copyOf(functions.values()); } List<MutateOperation> getMutateOperations() { return List.copyOf(mutateOperations); } List<String> getInherited() { return List.copyOf(inherited); } - Map<String, Boolean> getFieldsWithRankFilter() { return Map.copyOf(fieldsRankFilter); } - Map<String, Integer> getFieldsWithRankWeight() { return Map.copyOf(fieldsRankWeight); } - Map<String, String> getFieldsWithRankType() { return Map.copyOf(fieldsRankType); } - Map<String, List<String>> getRankProperties() { return Map.copyOf(rankProperties); } - Map<String, Value> getConstants() { return Map.copyOf(constants); } + + Map<String, Boolean> getFieldsWithRankFilter() { return Collections.unmodifiableMap(fieldsRankFilter); } + Map<String, Integer> getFieldsWithRankWeight() { return Collections.unmodifiableMap(fieldsRankWeight); } + Map<String, String> getFieldsWithRankType() { return Collections.unmodifiableMap(fieldsRankType); } + Map<String, List<String>> getRankProperties() { return Collections.unmodifiableMap(rankProperties); } + Map<String, Value> getConstants() { return Collections.unmodifiableMap(constants); } Map<Reference, TensorType> getInputs() { return Collections.unmodifiableMap(inputs); } + Optional<String> getInheritedSummaryFeatures() { return Optional.ofNullable(this.inheritedSummaryFeatures); } Optional<String> getSecondPhaseExpression() { return Optional.ofNullable(this.secondPhaseExpression); } Optional<Boolean> isStrict() { return Optional.ofNullable(this.strict); } diff --git a/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java b/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java index b96d41648da..3422d96e1d3 100644 --- a/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java +++ b/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java @@ -1,6 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.client; +import ai.vespa.hosted.client.ConfigServerClient.RequestBuilder; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.core5.http.ClassicHttpRequest; @@ -12,14 +16,11 @@ import org.apache.hc.core5.http.ParseException; import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.http.io.entity.HttpEntities; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; -import org.apache.hc.core5.net.URIBuilder; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; -import java.net.URISyntaxException; import java.time.Duration; -import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; @@ -50,8 +51,11 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { Throwable thrown = null; for (URI host : builder.hosts) { ClassicHttpRequest request = ClassicRequestBuilder.create(builder.method.name()) - .setUri(concat(host, builder.uriBuilder)) - .build(); + .setUri(HttpURL.from(host) + .appendPath(builder.path) + .appendQuery(builder.query) + .asURI()) + .build(); request.setEntity(builder.entity); try { try { @@ -87,23 +91,6 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { throw new IllegalStateException("No hosts to perform the request against"); } - /** Append path to the given host, which may already contain a root path. */ - static URI concat(URI host, URIBuilder pathAndQuery) { - URIBuilder builder = new URIBuilder(host); - List<String> pathSegments = new ArrayList<>(builder.getPathSegments()); - if ( ! pathSegments.isEmpty() && pathSegments.get(pathSegments.size() - 1).isEmpty()) - pathSegments.remove(pathSegments.size() - 1); - pathSegments.addAll(pathAndQuery.getPathSegments()); - try { - return builder.setPathSegments(pathSegments) - .setParameters(pathAndQuery.getQueryParams()) - .build(); - } - catch (URISyntaxException e) { - throw new IllegalArgumentException("URISyntaxException should not be possible here", e); - } - } - @Override public ConfigServerClient.RequestBuilder send(HostStrategy hosts, Method method) { return new RequestBuilder(hosts, method); @@ -114,8 +101,8 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { private final Method method; private final HostStrategy hosts; - private final URIBuilder uriBuilder = new URIBuilder(); - private final List<String> pathSegments = new ArrayList<>(); + private HttpURL.Path path = Path.empty(); + private HttpURL.Query query = Query.empty(); private HttpEntity entity; private RequestConfig config = ConfigServerClient.defaultRequestConfig; private ResponseVerifier verifier = ConfigServerClient.throwOnError; @@ -130,8 +117,8 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { } @Override - public RequestBuilder at(List<String> pathSegments) { - this.pathSegments.addAll(pathSegments); + public RequestBuilder at(Path subPath) { + path = path.append(subPath); return this; } @@ -149,7 +136,7 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { @Override public ConfigServerClient.RequestBuilder emptyParameters(List<String> keys) { for (String key : keys) - uriBuilder.setParameter(key, null); + query = query.add(key); return this; } @@ -162,13 +149,19 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { for (int i = 0; i < pairs.size(); ) { String key = pairs.get(i++), value = pairs.get(i++); if (value != null) - uriBuilder.setParameter(key, value); + query = query.add(key, value); } return this; } @Override + public ConfigServerClient.RequestBuilder parameters(Query query) { + this.query = this.query.add(query.entries()); + return this; + } + + @Override public RequestBuilder timeout(Duration timeout) { return config(RequestConfig.copy(config) .setResponseTimeout(timeout.toMillis(), TimeUnit.MILLISECONDS) @@ -230,7 +223,6 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { @Override public <T> T handle(ResponseHandler<T> handler) { - uriBuilder.setPathSegments(pathSegments); return execute(this, (response, request) -> { try { diff --git a/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java b/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java index bfed26779ec..47e062766c6 100644 --- a/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java +++ b/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java @@ -1,6 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.client; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; @@ -60,11 +63,14 @@ public interface ConfigServerClient extends Closeable { /** Builder for a request against a given set of hosts, using this config server client. */ interface RequestBuilder { - /** Appends to the request path. */ + /** Appends to the request path, with no trailing slash. */ default RequestBuilder at(String... pathSegments) { return at(List.of(pathSegments)); } + /** Appends to the request path, with no trailing slash. */ + default RequestBuilder at(List<String> pathSegments) { return at(Path.empty().append(pathSegments).withoutTrailingSlash()); } + /** Appends to the request path. */ - RequestBuilder at(List<String> pathSegments); + RequestBuilder at(HttpURL.Path path); /** Sets the request body as UTF-8 application/json. */ RequestBuilder body(byte[] json); @@ -85,9 +91,12 @@ public interface ConfigServerClient extends Closeable { return parameters(Arrays.asList(pairs)); } - /** Sets the parameter key/values for the request. Number of arguments must be even. Null values are omitted. */ + /** Sets the parameter key/values for the request. Number of arguments must be even. Pairs with {@code null} values are omitted. */ RequestBuilder parameters(List<String> pairs); + /** Appends all parameters from the given query. */ + RequestBuilder parameters(Query query); + /** Overrides the default socket read timeout of the request. {@code Duration.ZERO} gives infinite timeout. */ RequestBuilder timeout(Duration timeout); diff --git a/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java b/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java index bcdfaa9efa9..e969201605e 100644 --- a/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java +++ b/configserver-client/src/test/java/ai/vespa/hosted/client/HttpConfigServerClientTest.java @@ -55,7 +55,7 @@ class HttpConfigServerClientTest { server.resetRequests(); // Two attempts on a different IOException. - server.stubFor(post("/prefix/%2Froot")) + server.stubFor(post("/prefix/root")) .setResponse(okJson("{}").withFault(Fault.EMPTY_RESPONSE) .build()); assertThrows(UncheckedIOException.class, @@ -63,9 +63,9 @@ class HttpConfigServerClientTest { URI.create("http://localhost:" + server.port() + "/prefix/"))), Method.POST) .body("hello".getBytes(UTF_8)) - .at("/root") + .at("root") .stream()); - server.verify(2, postRequestedFor(urlEqualTo("/prefix/%2Froot")).withRequestBody(equalTo("hello"))); + server.verify(2, postRequestedFor(urlEqualTo("/prefix/root")).withRequestBody(equalTo("hello"))); server.verify(2, anyRequestedFor(anyUrl())); server.resetRequests(); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 76d7ff2fc7f..c4731ef0860 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -1,7 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server; -import com.yahoo.net.DomainName; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Query; import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; @@ -31,7 +33,6 @@ import com.yahoo.docproc.jdisc.metric.NullMetric; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.Metric; import com.yahoo.path.Path; -import com.yahoo.restapi.HttpURL; import com.yahoo.slime.Slime; import com.yahoo.transaction.NestedTransaction; import com.yahoo.transaction.Transaction; @@ -559,24 +560,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } } - public HttpResponse serviceStatusPage(ApplicationId applicationId, String hostName, String serviceName, HttpURL.Path pathSuffix) { - // WARNING: pathSuffix may be given by the external user. Make sure no security issues arise... - // We should be OK here, because at most, pathSuffix may change the parent path, but cannot otherwise - // change the hostname and port. Exposing other paths on the cluster controller should be fine. - // TODO: It would be nice to have a simple check to verify pathSuffix doesn't contain /../ components. - HttpURL.Path pathPrefix = HttpURL.Path.empty(); - switch (serviceName) { - case "container-clustercontroller": - pathPrefix = pathPrefix.append("clustercontroller-status").append("v1"); - break; - case "distributor": - case "storagenode": - break; - default: - throw new NotFoundException("No status page for service: " + serviceName); - } - - return httpProxy.get(getApplication(applicationId), hostName, serviceName, pathPrefix.append(pathSuffix)); + public HttpResponse proxyServiceHostnameRequest(ApplicationId applicationId, String hostName, String serviceName, HttpURL.Path path, Query query) { + return httpProxy.get(getApplication(applicationId), hostName, serviceName, path, query); } public Map<String, ClusterReindexing> getClusterReindexingStatus(ApplicationId applicationId) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java index 14acd1b5630..cc022c93278 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java @@ -1,20 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.application; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; +import ai.vespa.http.HttpURL.Scheme; import com.google.inject.Inject; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.PortInfo; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.container.jdisc.HttpResponse; - -import java.util.HashSet; -import java.util.List; -import java.util.logging.Level; - -import com.yahoo.net.DomainName; -import com.yahoo.restapi.HttpURL; -import com.yahoo.restapi.HttpURL.Path; -import com.yahoo.restapi.HttpURL.Scheme; import com.yahoo.vespa.config.server.http.HttpErrorResponse; import com.yahoo.vespa.config.server.http.HttpFetcher; import com.yahoo.vespa.config.server.http.HttpFetcher.Params; @@ -22,10 +18,9 @@ import com.yahoo.vespa.config.server.http.NotFoundException; import com.yahoo.vespa.config.server.http.SimpleHttpFetcher; import java.net.MalformedURLException; -import java.net.URL; +import java.util.List; +import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; -import java.util.stream.Stream; public class HttpProxy { @@ -39,7 +34,7 @@ public class HttpProxy { this.fetcher = fetcher; } - public HttpResponse get(Application application, String hostName, String serviceType, Path relativePath) { + public HttpResponse get(Application application, String hostName, String serviceType, Path path, Query query) { HostInfo host = application.getModel().getHosts().stream() .filter(hostInfo -> hostInfo.getHostname().equals(hostName)) .findFirst() @@ -57,11 +52,11 @@ public class HttpProxy { .findFirst() .orElseThrow(() -> new NotFoundException("Failed to find HTTP state port")); - return internalGet(host.getHostname(), port.getPort(), relativePath); + return internalGet(host.getHostname(), port.getPort(), path, query); } - private HttpResponse internalGet(String hostname, int port, Path relativePath) { - HttpURL url = HttpURL.create(Scheme.http, DomainName.of(hostname), port, relativePath); + private HttpResponse internalGet(String hostname, int port, Path path, Query query) { + HttpURL url = HttpURL.create(Scheme.http, DomainName.of(hostname), port, path, query); try { return fetcher.get(new Params(2000), // 2_000 ms read timeout url.asURI().toURL()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java index 0ceb459233b..2989eee0b55 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentHandler.java @@ -3,13 +3,12 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; import java.io.InputStreamReader; -import java.util.Arrays; import java.util.Collections; import java.util.List; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java index f06e1dabf8c..4536bbad9f6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/ContentRequest.java @@ -3,7 +3,7 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import java.io.InputStream; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java index 4fb14dacd9a..6add1f7a9fc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java @@ -8,16 +8,13 @@ import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; import org.apache.hc.core5.http.HttpEntity; -import org.apache.hc.core5.http.ParseException; -import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.util.Timeout; -import java.util.logging.Level; - import java.io.IOException; import java.net.SocketTimeoutException; import java.net.URISyntaxException; import java.net.URL; +import java.util.logging.Level; import java.util.logging.Logger; public class SimpleHttpFetcher implements HttpFetcher { @@ -40,16 +37,12 @@ public class SimpleHttpFetcher implements HttpFetcher { return new StaticResponse( response.getCode(), entity.getContentType(), - EntityUtils.toString(entity)); + entity.getContent().readAllBytes()); } } catch (SocketTimeoutException e) { String message = "Timed out after " + params.readTimeoutMs + " ms reading response from " + url; logger.log(Level.WARNING, message, e); throw new RequestTimeoutException(message); - } catch (ParseException e) { - String message = "Parse error in response from " + url; - logger.log(Level.WARNING, message, e); - throw new InternalServerException(message); } catch (IOException e) { String message = "Failed to get response from " + url; logger.log(Level.WARNING, message, e); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/StaticResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/StaticResponse.java index 3b0f29be765..a64a5551095 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/StaticResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/StaticResponse.java @@ -3,33 +3,27 @@ package com.yahoo.vespa.config.server.http; import com.yahoo.container.jdisc.HttpResponse; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; public class StaticResponse extends HttpResponse { private final String contentType; - private final InputStream body; + private final byte[] body; - /** - * @param body Ownership is passed to StaticResponse (is responsible for closing it) - */ - public StaticResponse(int status, String contentType, InputStream body) { + public StaticResponse(int status, String contentType, byte[] body) { super(status); this.contentType = contentType; this.body = body; } public StaticResponse(int status, String contentType, String body) { - this(status, contentType, new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8))); + this(status, contentType, body.getBytes(StandardCharsets.UTF_8)); } @Override public void render(OutputStream outputStream) throws IOException { - body.transferTo(outputStream); - body.close(); + outputStream.write(body); } @Override diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java index 885456ff69c..813933a5d9b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java @@ -1,7 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http.v2; -import com.yahoo.net.DomainName; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Query; import com.google.inject.Inject; import com.yahoo.component.Version; import com.yahoo.config.application.api.ApplicationFile; @@ -16,7 +18,6 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.io.IOUtils; import com.yahoo.jdisc.Response; import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.HttpURL; import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.Path; import com.yahoo.slime.Cursor; @@ -85,7 +86,8 @@ public class ApplicationHandler extends HttpHandler { if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/metrics/deployment")) return deploymentMetrics(applicationId(path)); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/metrics/proton")) return protonMetrics(applicationId(path)); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/reindexing")) return getReindexingStatus(applicationId(path)); - if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/service/{service}/{hostname}/status/{*}")) return serviceStatusPage(applicationId(path), path.get("service"), path.get("hostname"), path.getRest()); + if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/service/{service}/{hostname}/status/{*}")) return serviceStatusPage(applicationId(path), path.get("service"), path.get("hostname"), path.getRest(), request); + if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/service/{service}/{hostname}/state/v1/metrics")) return serviceStateV1metrics(applicationId(path), path.get("service"), path.get("hostname")); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/serviceconverge")) return listServiceConverge(applicationId(path), request); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/serviceconverge/{hostAndPort}")) return checkServiceConverge(applicationId(path), path.get("hostAndPort"), request); if (path.matches("/application/v2/tenant/{tenant}/application/{application}/environment/{ignore}/region/{ignore}/instance/{instance}/suspended")) return isSuspended(applicationId(path)); @@ -133,8 +135,23 @@ public class ApplicationHandler extends HttpHandler { return HttpServiceResponse.createResponse(response, hostAndPort, request.getUri()); } - private HttpResponse serviceStatusPage(ApplicationId applicationId, String service, String hostname, HttpURL.Path pathSuffix) { - return applicationRepository.serviceStatusPage(applicationId, hostname, service, pathSuffix); + private HttpResponse serviceStatusPage(ApplicationId applicationId, String service, String hostname, HttpURL.Path pathSuffix, HttpRequest request) { + HttpURL.Path pathPrefix = HttpURL.Path.empty(); + switch (service) { + case "container-clustercontroller": + pathPrefix = pathPrefix.append("clustercontroller-status").append("v1"); + break; + case "distributor": + case "storagenode": + break; + default: + throw new com.yahoo.vespa.config.server.NotFoundException("No status page for service: " + service); + } + return applicationRepository.proxyServiceHostnameRequest(applicationId, hostname, service, pathPrefix.append(pathSuffix), Query.empty().add(request.getJDiscRequest().parameters())); + } + + private HttpResponse serviceStateV1metrics(ApplicationId applicationId, String service, String hostname) { + return applicationRepository.proxyServiceHostnameRequest(applicationId, hostname, service, HttpURL.Path.parse("/state/v1/metrics"), Query.empty()); } private HttpResponse content(ApplicationId applicationId, HttpURL.Path contentPath, HttpRequest request) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java index f6af9e616a9..8c69c52e17d 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandler.java @@ -6,7 +6,7 @@ import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.http.ContentRequest; import com.yahoo.vespa.config.server.http.ContentHandler; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java index 15d6c5c18ff..fe59119a088 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/ApplicationContentRequest.java @@ -5,7 +5,7 @@ import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Zone; import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import com.yahoo.vespa.config.server.http.ContentRequest; /** diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java index 449058eb911..ec402f263db 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/request/SessionContentRequestV2.java @@ -4,11 +4,9 @@ package com.yahoo.vespa.config.server.http.v2.request; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.jdisc.application.BindingMatch; -import com.yahoo.restapi.HttpURL; +import ai.vespa.http.HttpURL; import com.yahoo.restapi.Path; import com.yahoo.vespa.config.server.http.ContentRequest; -import com.yahoo.vespa.config.server.http.Utils; /** * Requests for content and content status (v2) diff --git a/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java index 8b9714c3bfb..59b91a52791 100644 --- a/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/serviceview/StateRequestHandler.java @@ -5,14 +5,13 @@ import com.fasterxml.jackson.jaxrs.json.JacksonJsonProvider; import com.google.inject.Inject; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; -import com.yahoo.net.DomainName; -import com.yahoo.restapi.HttpURL; -import com.yahoo.restapi.HttpURL.Path; -import com.yahoo.restapi.HttpURL.Query; -import com.yahoo.restapi.HttpURL.Scheme; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; +import ai.vespa.http.HttpURL.Scheme; import com.yahoo.restapi.RestApi; import com.yahoo.restapi.RestApiRequestHandler; -import com.yahoo.restapi.UriBuilder; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import com.yahoo.vespa.serviceview.bindings.ConfigClient; import com.yahoo.vespa.serviceview.bindings.HealthClient; @@ -92,7 +91,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl } private ApplicationView getDefaultUserInfo(RestApi.RequestContext context) { - return getUserInfo(context.uriBuilder(), "default", "default", "default", "default", "default"); + return getUserInfo(context.baseRequestURL(), "default", "default", "default", "default", "default"); } private ApplicationView getUserInfo(RestApi.RequestContext context) { @@ -101,7 +100,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl String environmentName = context.pathParameters().getStringOrThrow("environmentName"); String regionName = context.pathParameters().getStringOrThrow("regionName"); String instanceName = context.pathParameters().getStringOrThrow("instanceName"); - return getUserInfo(context.uriBuilder(), tenantName, applicationName, environmentName, regionName, instanceName); + return getUserInfo(context.baseRequestURL(), tenantName, applicationName, environmentName, regionName, instanceName); } public HashMap<?, ?> singleService(RestApi.RequestContext context) { @@ -112,14 +111,15 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl String instanceName = context.pathParameters().getStringOrThrow("instanceName"); String identifier = context.pathParameters().getStringOrThrow("serviceIdentifier"); Path apiParams = context.pathParameters().getRest().orElse(Path.empty()); - return singleService(context.uriBuilder(), context.request().getUri(), tenantName, applicationName, environmentName, regionName, instanceName, identifier, apiParams); + Query apiQuery = context.queryParameters().getFullQuery(); + return singleService(context.baseRequestURL(), tenantName, applicationName, environmentName, regionName, instanceName, identifier, apiParams, apiQuery); } - protected ApplicationView getUserInfo(UriBuilder uriBuilder, String tenantName, String applicationName, String environmentName, String regionName, String instanceName) { + protected ApplicationView getUserInfo(HttpURL url, String tenantName, String applicationName, String environmentName, String regionName, String instanceName) { ServiceModel model = new ServiceModel( getModelConfig(tenantName, applicationName, environmentName, regionName, instanceName)); return model.showAllClusters( - baseUri(uriBuilder).toString(), + baseUri(url).toString(), applicationIdentifier(tenantName, applicationName, environmentName, regionName, instanceName)); } @@ -130,18 +130,18 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl } protected HashMap<?, ?> singleService( - UriBuilder uriBuilder, URI requestUri, String tenantName, String applicationName, String environmentName, String regionName, String instanceName, String identifier, Path apiParams) { + HttpURL url, String tenantName, String applicationName, String environmentName, String regionName, String instanceName, String identifier, Path path, Query query) { ServiceModel model = new ServiceModel(getModelConfig(tenantName, applicationName, environmentName, regionName, instanceName)); Service s = model.getService(identifier); int requestedPort = s.matchIdentifierWithPort(identifier); - HealthClient resource = getHealthClient(apiParams, s, requestedPort, requestUri.getRawQuery(), client); + HealthClient resource = getHealthClient(path, s, requestedPort, query, client); HashMap<?, ?> apiResult = resource.getHealthInfo(); - rewriteResourceLinks(uriBuilder, apiResult, model, s, applicationIdentifier(tenantName, applicationName, environmentName, regionName, instanceName), identifier); + rewriteResourceLinks(url, apiResult, model, s, applicationIdentifier(tenantName, applicationName, environmentName, regionName, instanceName), identifier); return apiResult; } - protected HealthClient getHealthClient(Path apiParams, Service s, int requestedPort, String uriQuery, Client client) { - URI uri = HttpURL.create(Scheme.http, DomainName.of(s.host), requestedPort, apiParams, Query.parse(uriQuery)).asURI(); + protected HealthClient getHealthClient(Path apiParams, Service s, int requestedPort, Query query, Client client) { + URI uri = HttpURL.create(Scheme.http, DomainName.of(s.host), requestedPort, apiParams, query).asURI(); WebTarget target = client.target(uri); return WebResourceFactory.newResource(HealthClient.class, target); } @@ -154,7 +154,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl + "/instance/" + instance; } - private void rewriteResourceLinks(UriBuilder uriBuilder, + private void rewriteResourceLinks(HttpURL url, Object apiResult, ServiceModel model, Service self, @@ -165,7 +165,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl Object resource = i.next(); if (resource instanceof String) { try { - StringBuilder buffer = linkBuffer(uriBuilder, applicationIdentifier); + StringBuilder buffer = linkBuffer(url, applicationIdentifier); // if it points to a port and host not part of the application, rewriting will not occur, so this is kind of safe retarget(model, self, buffer, (String) resource); i.set(buffer.toString()); @@ -173,7 +173,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl break; // assume relatively homogenous lists when doing rewrites to avoid freezing up on scanning long lists } } else { - rewriteResourceLinks(uriBuilder, resource, model, self, applicationIdentifier, incomingIdentifier); + rewriteResourceLinks(url, resource, model, self, applicationIdentifier, incomingIdentifier); } } } else if (apiResult instanceof Map) { @@ -182,14 +182,14 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl for (Map.Entry<Object, Object> entry : api.entrySet()) { if (SINGLE_API_LINK.equals(entry.getKey()) && entry.getValue() instanceof String) { try { - rewriteSingleLink(entry, model, self, linkBuffer(uriBuilder, applicationIdentifier)); + rewriteSingleLink(entry, model, self, linkBuffer(url, applicationIdentifier)); } catch (GiveUpLinkRetargetingException e) { // NOP } } else if ("link".equals(entry.getKey()) && entry.getValue() instanceof String) { - buildSingleLink(entry, linkBuffer(uriBuilder, applicationIdentifier), incomingIdentifier); + buildSingleLink(entry, linkBuffer(url, applicationIdentifier), incomingIdentifier); } else { - rewriteResourceLinks(uriBuilder, entry.getValue(), model, self, applicationIdentifier, incomingIdentifier); + rewriteResourceLinks(url, entry.getValue(), model, self, applicationIdentifier, incomingIdentifier); } } } @@ -210,8 +210,8 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl } } - private StringBuilder linkBuffer(UriBuilder uriBuilder, String applicationIdentifier) { - return baseUri(uriBuilder).append(applicationIdentifier); + private StringBuilder linkBuffer(HttpURL url, String applicationIdentifier) { + return new StringBuilder(baseUri(url).appendPath(Path.parse(applicationIdentifier)).toString()); } private void rewriteSingleLink(Map.Entry<Object, Object> entry, @@ -247,7 +247,7 @@ public class StateRequestHandler extends RestApiRequestHandler<StateRequestHandl newUri.append(link.getRawPath()); } - private static StringBuilder baseUri(UriBuilder uriBuilder) { - return new StringBuilder(uriBuilder.withPath("/serviceview/v1/").toString()); + private static HttpURL baseUri(HttpURL url) { + return url.withPath(Path.parse("/serviceview/v1/")); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index e3fc2345c68..ce44fade2f0 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server; -import com.yahoo.net.DomainName; +import ai.vespa.http.DomainName; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; import com.yahoo.config.ConfigInstance; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java index 83cae04cbfd..3e934e5e19e 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java @@ -1,12 +1,13 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.application; +import ai.vespa.http.HttpURL.Query; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import com.yahoo.vespa.config.server.http.HttpFetcher; import com.yahoo.vespa.config.server.http.RequestTimeoutException; import com.yahoo.vespa.config.server.http.StaticResponse; @@ -49,13 +50,14 @@ public class HttpProxyTest { when(fetcher.get(actualParams.capture(), actualUrl.capture())).thenReturn(response); HttpResponse actualResponse = proxy.get(applicationMock, hostname, CLUSTERCONTROLLER_CONTAINER.serviceName, - Path.parse("clustercontroller-status/v1/clusterName")); + Path.parse("clustercontroller-status/v1/clusterName"), + Query.parse("foo=bar")); assertEquals(1, actualParams.getAllValues().size()); assertEquals(2000, actualParams.getValue().readTimeoutMs); assertEquals(1, actualUrl.getAllValues().size()); - assertEquals(new URL("http://" + hostname + ":" + port + "/clustercontroller-status/v1/clusterName"), + assertEquals(new URL("http://" + hostname + ":" + port + "/clustercontroller-status/v1/clusterName?foo=bar"), actualUrl.getValue()); // The HttpResponse returned by the fetcher IS the same object as the one returned by the proxy, @@ -68,7 +70,8 @@ public class HttpProxyTest { when(fetcher.get(any(), any())).thenThrow(new RequestTimeoutException("timed out")); proxy.get(applicationMock, hostname, CLUSTERCONTROLLER_CONTAINER.serviceName, - Path.parse("clustercontroller-status/v1/clusterName")); + Path.parse("clustercontroller-status/v1/clusterName"), + Query.empty()); } private static MockModel createClusterController() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java index 9f7e539a2e3..856e778942b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java @@ -15,7 +15,7 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.HttpRequest.Method; -import com.yahoo.restapi.HttpURL; +import ai.vespa.http.HttpURL; import com.yahoo.test.ManualClock; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.MockLogRetriever; @@ -362,17 +362,18 @@ public class ApplicationHandlerTest { "<html>" + "host=" + invoc.getArgument(1, String.class) + "," + "service=" + invoc.getArgument(2, String.class) + "," + - "path=" + invoc.getArgument(3, HttpURL.Path.class) + + "path=" + invoc.getArgument(3, HttpURL.Path.class) + "," + + "query=" + invoc.getArgument(4, HttpURL.Query.class) + "</html>")) - .when(mockHttpProxy).get(any(), any(), any(), any()); + .when(mockHttpProxy).get(any(), any(), any(), any(), any()); - HttpResponse response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/container-clustercontroller/" + host + "/status/some/path/clusterName1", GET)); - assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=container-clustercontroller,path=path '/clustercontroller-status/v1/some/path/clusterName1'</html>"); + HttpResponse response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/container-clustercontroller/" + host + "/status/some/path/clusterName1?foo=bar", GET)); + assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=container-clustercontroller,path=path '/clustercontroller-status/v1/some/path/clusterName1',query=query 'foo=bar'</html>"); - response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/distributor/" + host + "/status/something", GET)); - assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=distributor,path=path '/something'</html>"); + response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/distributor/" + host + "/status/something?foo=bar", GET)); + assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>host=foo.yahoo.com,service=distributor,path=path '/something',query=query 'foo=bar'</html>"); - response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/fake-service/" + host + "/status/something", GET)); + response = mockHandler.handle(createTestRequest(toUrlPath(applicationId, Zone.defaultZone(), true) + "/service/fake-service/" + host + "/status/something?foo=bar", GET)); assertHttpStatusCodeAndMessage(response, 404, "{\"error-code\":\"NOT_FOUND\",\"message\":\"No status page for service: fake-service\"}"); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java index 58a3593ae14..2e86f5e0538 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java @@ -7,7 +7,7 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.MockProvisioner; import com.yahoo.vespa.config.server.application.CompressedApplicationInputStreamTest; diff --git a/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java index 35e63c46bf8..79ed38b2025 100644 --- a/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/serviceview/StateRequestHandlerTest.java @@ -1,10 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.serviceview; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Query; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.jdisc.test.MockMetric; -import com.yahoo.restapi.HttpURL.Path; -import com.yahoo.restapi.UriBuilder; +import ai.vespa.http.HttpURL.Path; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import com.yahoo.vespa.serviceview.bindings.HealthClient; import com.yahoo.vespa.serviceview.bindings.ModelResponse; @@ -46,7 +47,7 @@ public class StateRequestHandlerTest { } @Override - protected HealthClient getHealthClient(Path apiParams, Service s, int requestedPort, String uriQuery, Client client) { + protected HealthClient getHealthClient(Path apiParams, Service s, int requestedPort, Query query, Client client) { HealthClient healthClient = Mockito.mock(HealthClient.class); HashMap<Object, Object> dummyHealthData = new HashMap<>(); HashMap<String, String> dummyLink = new HashMap<>(); @@ -76,14 +77,14 @@ public class StateRequestHandlerTest { public final void test() { Service s = correspondingModel.resolve("vespa.yahoo.com", 8080, null); String api = "/state/v1"; - HashMap<?, ?> boom = testHandler.singleService(new UriBuilder("http://someserver:8080"), URI.create(EXTERNAL_BASE_URI), "default", "default", "default", "default", "default", s.getIdentifier(8080), Path.parse(api)); + HashMap<?, ?> boom = testHandler.singleService(HttpURL.from(URI.create("http://someserver:8080")), "default", "default", "default", "default", "default", s.getIdentifier(8080), Path.parse(api), Query.empty().add("foo", "bar")); assertEquals(EXTERNAL_BASE_URI + "tenant/default/application/default/environment/default/region/default/instance/default/service/" + s.getIdentifier(8080) + api, ((Map<?, ?>) ((List<?>) boom.get("resources")).get(0)).get("url")); } @Test public final void testLinkEquality() { - ApplicationView explicitParameters = testHandler.getUserInfo(new UriBuilder("http://someserver:8080"), "default", "default", "default", "default", "default"); + ApplicationView explicitParameters = testHandler.getUserInfo(HttpURL.from(URI.create("http://someserver:8080")), "default", "default", "default", "default", "default"); assertEquals(EXTERNAL_BASE_URI + "tenant/default/application/default/environment/default/region/default/instance" + "/default/service/container-clustercontroller-2ul67p8psr451t3w8kdd0qwgg/state/v1/", explicitParameters.clusters.get(0).services.get(0).url); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java index 2580b4a6ac0..a5d133b9eb4 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java @@ -52,7 +52,7 @@ public class DiscFilterRequest { untreatedHeaders = new HeaderFields(); parent.copyHeaders(untreatedHeaders); - untreatedParams = new HashMap<>(parent.parameters()); + untreatedParams = new HashMap<>(parent.parameters()); // TODO jonmv: probably a bug that this is not deep-copied } public String getMethod() { return parent.getMethod().name(); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java index 84d343c0c8e..72057563e36 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java @@ -37,7 +37,6 @@ import static com.yahoo.jdisc.http.server.jetty.CompletionHandlerUtils.NOOP_COMP * parsed the form parameters and merged them into the request's parameters. * * @author bakksjo - * $Id$ */ class FormPostRequestHandler extends AbstractRequestHandler implements ContentChannel, DelegatedRequestHandler { @@ -84,7 +83,6 @@ class FormPostRequestHandler extends AbstractRequestHandler implements ContentCh completionHandler.completed(); } - @SuppressWarnings("try") @Override public void close(final CompletionHandler completionHandler) { try (final ResourceReference ref = requestReference) { 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 c639432db89..40f281a948e 100644 --- a/container-core/src/main/java/com/yahoo/restapi/Path.java +++ b/container-core/src/main/java/com/yahoo/restapi/Path.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.restapi; +import ai.vespa.http.HttpURL; + import java.net.URI; import java.util.HashMap; import java.util.List; @@ -103,6 +105,13 @@ public class Path { return rest; } + /** + * The path this holds. + */ + public HttpURL.Path getPath() { + return path; + } + @Override public String toString() { return path.toString(); diff --git a/container-core/src/main/java/com/yahoo/restapi/RestApi.java b/container-core/src/main/java/com/yahoo/restapi/RestApi.java index 353ac3eb5cc..5dfb19029cc 100644 --- a/container-core/src/main/java/com/yahoo/restapi/RestApi.java +++ b/container-core/src/main/java/com/yahoo/restapi/RestApi.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.restapi; +import ai.vespa.http.HttpURL; import com.fasterxml.jackson.databind.ObjectMapper; import com.yahoo.container.jdisc.AclMapping; import com.yahoo.container.jdisc.HttpRequest; @@ -128,12 +129,9 @@ public interface RestApi { Optional<RequestContent> requestContent(); RequestContent requestContentOrThrow(); ObjectMapper jacksonJsonMapper(); - /** - * Creates a URI builder pre-initialized with scheme, host and port. - * Intended for response generation (e.g for interactive REST APIs). - * DO NOT USE FOR CUSTOM ROUTING. - */ - UriBuilder uriBuilder(); + /** Scheme, domain and port, for the original request. <em>Use this only for generating resources links, not for custom routing!</em> */ + // TODO: this needs to include path and query as well, to be useful for generating resource links that need not be rewritten. + HttpURL baseRequestURL(); AclMapping.Action aclAction(); Optional<Principal> userPrincipal(); Principal userPrincipalOrThrow(); @@ -154,9 +152,11 @@ public interface RestApi { } interface PathParameters extends Parameters { + HttpURL.Path getFullPath(); Optional<HttpURL.Path> getRest(); } interface QueryParameters extends Parameters { + HttpURL.Query getFullQuery(); List<String> getStringList(String name); } interface Headers extends Parameters {} diff --git a/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java b/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java index cc243a3e92b..1bde8d635a5 100644 --- a/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java +++ b/container-core/src/main/java/com/yahoo/restapi/RestApiImpl.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.restapi; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Query; import com.fasterxml.jackson.databind.ObjectMapper; import com.yahoo.container.jdisc.AclMapping; import com.yahoo.container.jdisc.HttpRequest; @@ -398,7 +400,7 @@ class RestApiImpl implements RestApi { return requestContent().orElseThrow(() -> new RestApiException.BadRequest("Request content missing")); } @Override public ObjectMapper jacksonJsonMapper() { return jacksonJsonMapper; } - @Override public UriBuilder uriBuilder() { + @Override public HttpURL baseRequestURL() { URI uri = request.getUri(); // Reconstruct the URI used by the client to access the API. // This is needed for producing URIs in the response that links to other parts of the Rest API. @@ -408,7 +410,7 @@ class RestApiImpl implements RestApi { if (hostHeader == null || hostHeader.isBlank()) { hostHeader = request.getHeader("Host"); } - if (hostHeader != null && !hostHeader.isBlank()) { + if (hostHeader != null && ! hostHeader.isBlank()) { sb.append(hostHeader); } else { sb.append(uri.getHost()); @@ -416,7 +418,7 @@ class RestApiImpl implements RestApi { sb.append(":").append(uri.getPort()); } } - return new UriBuilder(sb.toString()); + return HttpURL.from(URI.create(sb.toString())); } @Override public AclMapping.Action aclAction() { return aclAction; } @Override public Optional<Principal> userPrincipal() { @@ -435,10 +437,12 @@ class RestApiImpl implements RestApi { return getString(name) .orElseThrow(() -> new RestApiException.BadRequest("Path parameter '" + name + "' is missing")); } + @Override public HttpURL.Path getFullPath() { + return pathMatcher.getPath(); + } @Override public Optional<HttpURL.Path> getRest() { return Optional.ofNullable(pathMatcher.getRest()); } - } private class QueryParametersImpl implements RestApi.RequestContext.QueryParameters { @@ -452,6 +456,7 @@ class RestApiImpl implements RestApi { if (result == null) return List.of(); return List.copyOf(result); } + @Override public HttpURL.Query getFullQuery() { return Query.empty().add(request.getJDiscRequest().parameters()); } } private class HeadersImpl implements RestApi.RequestContext.Headers { 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 4786eb9775c..17b35a6343c 100644 --- a/container-core/src/test/java/com/yahoo/restapi/PathTest.java +++ b/container-core/src/test/java/com/yahoo/restapi/PathTest.java @@ -4,6 +4,7 @@ package com.yahoo.restapi; import org.junit.Test; import java.net.URI; +import java.util.List; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; @@ -35,7 +36,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("/", path.getRest().raw()); + assertEquals(List.of(), path.getRest().segments()); } { @@ -43,7 +44,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("/kanoo", path.getRest().raw()); + assertEquals(List.of("kanoo"), path.getRest().segments()); } { @@ -51,7 +52,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("/kanoo/trips", path.getRest().raw()); + assertEquals(List.of("kanoo", "trips"), path.getRest().segments()); } { @@ -59,7 +60,7 @@ public class PathTest { assertTrue(path.matches("/a/{foo}/bar/{b}/{*}")); assertEquals("1", path.get("foo")); assertEquals("fuz", path.get("b")); - assertEquals("/kanoo/trips/", path.getRest().raw()); + assertEquals(List.of("kanoo", "trips"), path.getRest().segments()); } } diff --git a/container-core/src/test/java/com/yahoo/restapi/RestApiImplTest.java b/container-core/src/test/java/com/yahoo/restapi/RestApiImplTest.java index 1b03d87f405..d4a53fb5d85 100644 --- a/container-core/src/test/java/com/yahoo/restapi/RestApiImplTest.java +++ b/container-core/src/test/java/com/yahoo/restapi/RestApiImplTest.java @@ -111,7 +111,7 @@ class RestApiImplTest { @Test void uri_builder_creates_valid_uri_prefix() { RestApi restApi = RestApi.builder() - .addRoute(route("/test").get(ctx -> new MessageResponse(ctx.uriBuilder().toString()))) + .addRoute(route("/test").get(ctx -> new MessageResponse(ctx.baseRequestURL().toString()))) .build(); verifyJsonResponse(restApi, Method.GET, "/test", null, 200, "{\"message\":\"http://localhost\"}"); verifyJsonResponse(restApi, Method.GET, "/test", null, 200, "{\"message\":\"http://mydomain:81\"}", Map.of("Host", "mydomain:81")); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java index a35ad91acbd..54a6e5b6e90 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java @@ -160,8 +160,8 @@ abstract class StructuredParser extends AbstractParser { firstWord.add(tokens.next()); } - if (tokens.currentIsNoIgnore(DOT)) { - tokens.skip(); + while (tokens.currentIsNoIgnore(DOT)) { + secondWord.add(tokens.next()); if (tokens.currentIsNoIgnore(WORD) || tokens.currentIsNoIgnore(NUMBER)) { secondWord.add(tokens.next()); } else { @@ -177,11 +177,7 @@ abstract class StructuredParser extends AbstractParser { if ( ! tokens.skipNoIgnore(COLON)) return null; - if (secondWord.size() == 0) { - item = concatenate(firstWord); - } else { - item = concatenate(firstWord) + "." + concatenate(secondWord); - } + item = concatenate(firstWord) + concatenate(secondWord); item = indexFacts.getCanonicName(item); @@ -395,7 +391,7 @@ abstract class StructuredParser extends AbstractParser { if ( ! tokens.currentIs(NUMBER)) return null; item = new IntItem(">" + (negative ? "-" : "") + tokens.next() + decimalPart(), true); - item.setOrigin(new Substring(initial.substring.start, tokens.currentNoIgnore().substring.start, + item.setOrigin(new Substring(initial.substring.start, tokens.currentNoIgnore().substring.start, initial.getSubstring().getSuperstring())); // XXX: Unsafe end? return item; } finally { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java index 42368fa358d..6dccf493d92 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/ConfigServer.java @@ -1,11 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.configserver; +import ai.vespa.http.HttpURL.Query; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.net.DomainName; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL.Path; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeploymentData; @@ -56,7 +57,8 @@ public interface ConfigServer { Map<?,?> getServiceApiResponse(DeploymentId deployment, String serviceName, Path restPath); - String getServiceStatusPage(DeploymentId deployment, String serviceName, DomainName node, Path subPath); + /** Returns a proxied response from a given path running on a given service and node */ + ProxyResponse getServiceNodePage(DeploymentId deployment, String serviceName, DomainName node, Path subPath, Query query); /** * Gets the Vespa logs of the given deployment. diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java index e186541c85c..594908a3bc1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java @@ -38,6 +38,9 @@ public class Notifier { if (notifications.isEmpty()) { return; } + if (skipSource(source)) { + return; + } var tenant = curatorDb.readTenant(source.tenant()); tenant.stream().forEach(t -> { if (t instanceof CloudTenant) { @@ -51,6 +54,11 @@ public class Notifier { }); } + private boolean skipSource(NotificationSource source) { + // Limit sources to production systems only. Dev and test systems cause too much noise at the moment. + return source.jobType().map(t -> !t.isProduction()).orElse(false); + } + public void dispatch(Notification notification) { dispatch(List.of(notification), notification.source()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 6eaad63251c..84ff3d5d8c3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -14,6 +14,10 @@ import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.curator.MultiplePathsLock; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.StringFlag; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.archive.ArchiveBucket; @@ -111,6 +115,7 @@ public class CuratorDb { private final Curator curator; private final Duration tryLockTimeout; + private final StringFlag lockScheme; // For each application id (path), store the ZK node version and its deserialised data - update when version changes. // This will grow to keep all applications in memory, but this should be OK @@ -120,13 +125,14 @@ public class CuratorDb { private final Map<Path, Pair<Integer, NavigableMap<RunId, Run>>> cachedHistoricRuns = new ConcurrentHashMap<>(); @Inject - public CuratorDb(Curator curator) { - this(curator, defaultTryLockTimeout); + public CuratorDb(Curator curator, FlagSource flagSource) { + this(curator, defaultTryLockTimeout, flagSource); } - CuratorDb(Curator curator, Duration tryLockTimeout) { + CuratorDb(Curator curator, Duration tryLockTimeout, FlagSource flagSource) { this.curator = curator; this.tryLockTimeout = tryLockTimeout; + this.lockScheme = Flags.CONTROLLER_LOCK_SCHEME.bindTo(flagSource); } /** Returns all hostnames configured to be part of this ZooKeeper cluster */ @@ -144,19 +150,55 @@ public class CuratorDb { } public Lock lock(TenantAndApplicationId id) { - return curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); + switch (lockScheme.value()) { + case "BOTH": + return new MultiplePathsLock(lockPath(id), legacyLockPath(id), defaultLockTimeout.multipliedBy(2), curator); + case "OLD": + return curator.lock(legacyLockPath(id), defaultLockTimeout.multipliedBy(2)); + case "NEW": + return curator.lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); + default: + throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value()); + } } public Lock lockForDeployment(ApplicationId id, ZoneId zone) { - return curator.lock(lockPath(id, zone), deployLockTimeout); + switch (lockScheme.value()) { + case "BOTH": + return new MultiplePathsLock(lockPath(id, zone), legacyLockPath(id, zone), deployLockTimeout, curator); + case "OLD": + return curator.lock(legacyLockPath(id, zone), deployLockTimeout); + case "NEW": + return curator.lock(lockPath(id, zone), deployLockTimeout); + default: + throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value()); + } } public Lock lock(ApplicationId id, JobType type) { - return curator.lock(lockPath(id, type), defaultLockTimeout); + switch (lockScheme.value()) { + case "BOTH": + return new MultiplePathsLock(lockPath(id, type), legacyLockPath(id, type), defaultLockTimeout, curator); + case "OLD": + return curator.lock(legacyLockPath(id, type), defaultLockTimeout); + case "NEW": + return curator.lock(lockPath(id, type), defaultLockTimeout); + default: + throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value()); + } } public Lock lock(ApplicationId id, JobType type, Step step) throws TimeoutException { - return tryLock(lockPath(id, type, step)); + switch (lockScheme.value()) { + case "BOTH": + return tryLock(lockPath(id, type, step), legacyLockPath(id, type, step)); + case "OLD": + return tryLock(legacyLockPath(id, type, step)); + case "NEW": + return tryLock(lockPath(id, type, step)); + default: + throw new IllegalArgumentException("Unknown lock scheme " + lockScheme.value()); + } } public Lock lockRotations() { @@ -239,6 +281,19 @@ public class CuratorDb { } } + /** Try locking with a low timeout, meaning it is OK to fail lock acquisition. + * + * Useful for maintenance jobs, where there is no point in running the jobs back to back. + */ + private Lock tryLock(Path path, Path path2) throws TimeoutException { + try { + return new MultiplePathsLock(path, path2, tryLockTimeout, curator); + } + catch (UncheckedTimeoutException e) { + throw new TimeoutException(e.getMessage()); + } + } + private <T> Optional<T> read(Path path, Function<byte[], T> mapper) { return curator.getData(path).filter(data -> data.length > 0).map(mapper); } @@ -667,32 +722,48 @@ public class CuratorDb { .append(tenant.value()); } - private Path lockPath(TenantAndApplicationId application) { + private Path legacyLockPath(TenantAndApplicationId application) { return lockPath(application.tenant()) .append(application.application().value()); } - private Path lockPath(ApplicationId instance) { - return lockPath(TenantAndApplicationId.from(instance)) + private Path legacyLockPath(ApplicationId instance) { + return legacyLockPath(TenantAndApplicationId.from(instance)) .append(instance.instance().value()); } - private Path lockPath(ApplicationId instance, ZoneId zone) { - return lockPath(instance) + private Path legacyLockPath(ApplicationId instance, ZoneId zone) { + return legacyLockPath(instance) .append(zone.environment().value()) .append(zone.region().value()); } - private Path lockPath(ApplicationId instance, JobType type) { - return lockPath(instance) + private Path legacyLockPath(ApplicationId instance, JobType type) { + return legacyLockPath(instance) .append(type.jobName()); } - private Path lockPath(ApplicationId instance, JobType type, Step step) { - return lockPath(instance, type) + private Path legacyLockPath(ApplicationId instance, JobType type, Step step) { + return legacyLockPath(instance, type) .append(step.name()); } + private Path lockPath(TenantAndApplicationId application) { + return lockRoot.append(application.tenant().value() + ":" + application.application().value()); + } + + private Path lockPath(ApplicationId instance, ZoneId zone) { + return lockRoot.append(instance.serializedForm() + ":" + zone.environment().value() + ":" + zone.region().value()); + } + + private Path lockPath(ApplicationId instance, JobType type) { + return lockRoot.append(instance.serializedForm() + ":" + type.jobName()); + } + + private Path lockPath(ApplicationId instance, JobType type, Step step) { + return lockRoot.append(instance.serializedForm() + ":" + type.jobName() + ":" + step.name()); + } + private Path lockPath(String provisionId) { return lockRoot .append(provisionStatePath()) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java index f98ecb80dd5..65c9859ad72 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MockCuratorDb.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.persistence; import com.google.inject.Inject; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; import java.time.Duration; @@ -31,7 +32,7 @@ public class MockCuratorDb extends CuratorDb { } public MockCuratorDb(MockCurator curator) { - super(curator, Duration.ofMillis(100)); + super(curator, Duration.ofMillis(100), new InMemoryFlagSource()); this.curator = curator; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java index 7e1c9c8884f..8b89c2300e4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java @@ -3,13 +3,12 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.container.jdisc.HttpRequest; -import com.yahoo.restapi.HttpURL; -import com.yahoo.restapi.HttpURL.Path; -import com.yahoo.restapi.HttpURL.Query; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; import com.yahoo.text.Text; import java.io.InputStream; import java.net.URI; -import java.net.URISyntaxException; import java.util.List; import java.util.Map; import java.util.Objects; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java index 9ac30898f8b..05cdc0d0565 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponse.java @@ -2,14 +2,12 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.restapi.HttpURL; -import com.yahoo.restapi.HttpURL.Path; -import org.apache.http.client.utils.URIBuilder; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; import java.io.IOException; import java.io.OutputStream; import java.net.URI; -import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; /** 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 0f5322af176..924f7c0b0a5 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 @@ -2,7 +2,7 @@ package com.yahoo.vespa.hosted.controller.restapi.application; import ai.vespa.hosted.api.Signatures; -import ai.vespa.validation.Validation; +import ai.vespa.http.HttpURL.Query; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Joiner; @@ -26,10 +26,10 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; import com.yahoo.io.IOUtils; -import com.yahoo.net.DomainName; +import ai.vespa.http.DomainName; import com.yahoo.restapi.ByteArrayResponse; import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.HttpURL; +import ai.vespa.http.HttpURL; import com.yahoo.restapi.MessageResponse; import com.yahoo.restapi.Path; import com.yahoo.restapi.ResourceResponse; @@ -70,9 +70,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.api.integration.noderepository.RestartFilter; -import com.yahoo.vespa.hosted.controller.api.integration.resource.MeteringData; -import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceAllocation; -import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshot; import com.yahoo.vespa.hosted.controller.api.integration.secrets.TenantSecretStore; import com.yahoo.vespa.hosted.controller.api.integration.user.User; import com.yahoo.vespa.hosted.controller.api.role.Role; @@ -158,7 +155,6 @@ import java.util.logging.Level; import java.util.stream.Collectors; import java.util.stream.Stream; -import static ai.vespa.validation.Validation.requireAtLeast; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.CONFLICT; import static com.yahoo.yolean.Exceptions.uncheck; @@ -275,6 +271,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service")) return services(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/state/v1/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/{host}/status/{*}")) return status(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.get("host"), path.getRest(), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/service/{service}/{host}/state/v1/metrics")) return stateV1Metrics(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.get("host")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/nodes")) return nodes(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/clusters")) return clusters(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/{environment}/region/{region}/content/{*}")) return content(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.getRest(), request); @@ -1802,11 +1799,17 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { private HttpResponse status(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String host, HttpURL.Path restPath, HttpRequest request) { DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); - String result = controller.serviceRegistry().configServer().getServiceStatusPage(deploymentId, - serviceName, - DomainName.of(host), - restPath); // TODO: add query - return new HtmlResponse(result); + return controller.serviceRegistry().configServer().getServiceNodePage(deploymentId, + serviceName, + DomainName.of(host), + HttpURL.Path.parse("/status").append(restPath), + Query.empty().add(request.getJDiscRequest().parameters())); + } + + private HttpResponse stateV1Metrics(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, String host) { + DeploymentId deploymentId = new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName), requireZone(environment, region)); + return controller.serviceRegistry().configServer().getServiceNodePage( + deploymentId, serviceName, DomainName.of(host), HttpURL.Path.parse("/state/v1/metrics"), Query.empty()); } private HttpResponse service(String tenantName, String applicationName, String instanceName, String environment, String region, String serviceName, HttpURL.Path restPath, HttpRequest request) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java index a21f93bdaea..41d891a0987 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ServiceApiResponse.java @@ -4,13 +4,12 @@ package com.yahoo.vespa.hosted.controller.restapi.application; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.restapi.HttpURL; -import com.yahoo.restapi.HttpURL.Path; -import com.yahoo.restapi.HttpURL.Query; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; import com.yahoo.slime.Cursor; import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; -import com.yahoo.restapi.UriBuilder; import com.yahoo.vespa.serviceview.bindings.ApplicationView; import com.yahoo.vespa.serviceview.bindings.ClusterView; import com.yahoo.vespa.serviceview.bindings.ServiceView; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java index 27a8cbeaf3e..99632203645 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java @@ -6,7 +6,7 @@ import com.yahoo.config.provision.zone.ZoneList; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.HttpURL; +import ai.vespa.http.HttpURL; import com.yahoo.restapi.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; @@ -25,7 +25,7 @@ import java.util.logging.Level; import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.restapi.HttpURL.Path.parse; +import static ai.vespa.http.HttpURL.Path.parse; /** * REST API for proxying operator APIs to config servers in a given zone. 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 40a402f209b..800588fdf8c 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 @@ -7,7 +7,7 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.HttpURL; +import ai.vespa.http.HttpURL; import com.yahoo.restapi.Path; import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java index 1643f1f818b..e98ff71884d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ConfigServerMock.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.integration; +import ai.vespa.http.HttpURL.Query; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.component.Version; @@ -13,8 +14,8 @@ import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.net.DomainName; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL.Path; import com.yahoo.text.Text; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; @@ -534,8 +535,8 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer } @Override - public String getServiceStatusPage(DeploymentId deployment, String serviceName, DomainName node, Path subPath) { - return "<h1>OK</h1>"; + public ProxyResponse getServiceNodePage(DeploymentId deployment, String serviceName, DomainName node, Path subPath, Query query) { + return new ProxyResponse("<h1>OK</h1>".getBytes(StandardCharsets.UTF_8), "text/html", 200); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java index e10a41cbae3..dd43f419624 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImplTest.java @@ -6,7 +6,7 @@ import com.github.tomakehurst.wiremock.stubbing.Scenario; import com.yahoo.config.provision.SystemName; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.yolean.concurrent.Sleeper; import org.apache.http.protocol.HttpContext; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java index e8b5df7efa1..5827ef676d7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java @@ -2,10 +2,8 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.jdisc.http.HttpRequest; -import com.yahoo.restapi.HttpURL.Path; -import org.junit.Rule; +import ai.vespa.http.HttpURL.Path; import org.junit.Test; -import org.junit.rules.ExpectedException; import java.net.URI; import java.util.List; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java index 1ba0200eec3..599827f2d03 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.hosted.controller.proxy; import com.yahoo.jdisc.http.HttpRequest; -import com.yahoo.restapi.HttpURL.Path; +import ai.vespa.http.HttpURL.Path; import org.junit.Test; import java.io.ByteArrayOutputStream; diff --git a/document/src/vespa/document/fieldvalue/iteratorhandler.h b/document/src/vespa/document/fieldvalue/iteratorhandler.h index 2f7b5d522e9..bbf24b77fb2 100644 --- a/document/src/vespa/document/fieldvalue/iteratorhandler.h +++ b/document/src/vespa/document/fieldvalue/iteratorhandler.h @@ -77,6 +77,7 @@ public: void setArrayIndex(uint32_t index) { _arrayIndexStack.back() = index; } ModificationStatus modify(FieldValue &fv) { return doModify(fv); } fieldvalue::VariableMap &getVariables() { return _variables; } + fieldvalue::VariableMap && stealVariables() { return std::move(_variables); } void setVariables(fieldvalue::VariableMap vars) { _variables = std::move(vars); } virtual bool createMissingPath() const { return false; } private: diff --git a/document/src/vespa/document/select/resultlist.cpp b/document/src/vespa/document/select/resultlist.cpp index 5bb2f510e0d..e6ce79fe117 100644 --- a/document/src/vespa/document/select/resultlist.cpp +++ b/document/src/vespa/document/select/resultlist.cpp @@ -153,11 +153,11 @@ ResultList::operator||(const ResultList& other) const } ResultList -ResultList::operator!() const { +ResultList::operator!() && { ResultList result; - for (const auto & it : _results) { - result.add(it.first, !*it.second); + for (auto & it : _results) { + result.add(std::move(it.first), !*it.second); } return result; diff --git a/document/src/vespa/document/select/resultlist.h b/document/src/vespa/document/select/resultlist.h index 3f810004168..4099c1e39ba 100644 --- a/document/src/vespa/document/select/resultlist.h +++ b/document/src/vespa/document/select/resultlist.h @@ -14,7 +14,7 @@ public: typedef std::vector<ResultPair> Results; typedef Results::iterator iterator; typedef Results::const_iterator const_iterator; - using const_reverse_iterator = Results::const_reverse_iterator; + using reverse_iterator = Results::reverse_iterator; ResultList(); ResultList(ResultList &&) noexcept; @@ -32,7 +32,7 @@ public: ResultList operator&&(const ResultList& other) const; ResultList operator||(const ResultList& other) const; - ResultList operator!() const; + ResultList operator!() &&; void print(std::ostream& out, bool verbose, const std::string& indent) const override; @@ -45,8 +45,8 @@ public: const Results& getResults() const { return _results; } const_iterator begin() const { return _results.begin(); } const_iterator end() const { return _results.end(); } - const_reverse_iterator rbegin() const { return _results.rbegin(); } - const_reverse_iterator rend() const { return _results.rend(); } + reverse_iterator rbegin() { return _results.rbegin(); } + reverse_iterator rend() { return _results.rend(); } private: Results _results; diff --git a/document/src/vespa/document/select/value.cpp b/document/src/vespa/document/select/value.cpp index aabd09b2558..a36a25103da 100644 --- a/document/src/vespa/document/select/value.cpp +++ b/document/src/vespa/document/select/value.cpp @@ -380,13 +380,29 @@ StructValue::operator==(const Value& value) const } void -StructValue::print(std::ostream& out, bool verbose, - const std::string& indent) const +StructValue::print(std::ostream& out, bool verbose, const std::string& indent) const { (void) verbose; (void) indent; out << "<no struct representation in language yet>"; } +namespace { + +fieldvalue::VariableMap +cloneMap(const fieldvalue::VariableMap &map) { + fieldvalue::VariableMap m; + for (const auto & item : map) { + if (item.second.key) { + m.emplace(item.first, fieldvalue::IndexValue(*item.second.key)); + } else { + m.emplace(item.first, fieldvalue::IndexValue(item.second.index)); + } + } + return m; +} + +} + template <typename Predicate> ResultList ArrayValue::doCompare(const Value& value, const Predicate& cmp) const @@ -415,7 +431,7 @@ ArrayValue::doCompare(const Value& value, const Predicate& cmp) const if (item.first.empty()) { resultForNoVariables.set(result.toEnum()); } else { - results.add(item.first, result); + results.add(cloneMap(item.first), result); } } for (uint32_t i(0); i < resultForNoVariables.size(); i++) { diff --git a/document/src/vespa/document/select/valuenodes.cpp b/document/src/vespa/document/select/valuenodes.cpp index c770974adfe..1c7d47d0591 100644 --- a/document/src/vespa/document/select/valuenodes.cpp +++ b/document/src/vespa/document/select/valuenodes.cpp @@ -263,7 +263,7 @@ IteratorHandler::onPrimitive(uint32_t fid, const Content& fv) { if (!_firstValue && getVariables().empty()) { _firstValue = getInternalValue(fv.getValue()); } else { - _values.emplace_back(std::move(getVariables()), Value::SP(getInternalValue(fv.getValue()).release())); + _values.emplace_back(stealVariables(), Value::SP(getInternalValue(fv.getValue()).release())); } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index d915b18ebdf..46ddacf7a2e 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -404,6 +404,13 @@ public class Flags { "Takes effect immediately", ZONE_ID); + public static final UnboundBooleanFlag USE_ZSTD_IN_FILE_DISTRIBUTION = defineFeatureFlag( + "use-zstd-in-file-distribution", false, + List.of("hmusum"), "2022-04-07", "2022-05-07", + "Whether to use zstd compression for data sent with file distribution", + "Takes effect immediately", + ZONE_ID, APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/functions.cmake b/functions.cmake index 65c02885ddb..b8ac3497ff2 100644 --- a/functions.cmake +++ b/functions.cmake @@ -428,6 +428,9 @@ function(vespa_add_test) if(NOT VALGRIND_EXECUTABLE) message(FATAL_ERROR "Requested valgrind tests, but could not find valgrind executable.") endif() + if(VESPA_USE_SANITIZER) + message(FATAL_ERROR "Cannot run sanitizer-instrumented unit tests under Valgrind") + endif() if(IS_SCRIPT) # For shell scripts, export a VALGRIND environment variable @@ -445,6 +448,9 @@ function(vespa_add_test) set(ARG_COMMAND "${VALGRIND_COMMAND} ${COMMAND_FIRST} ${COMMAND_REST}") endif() endif() + if(VESPA_USE_SANITIZER AND VESPA_SANITIZER_ENV) + list(APPEND ARG_ENVIRONMENT "${VESPA_SANITIZER_ENV}") + endif() separate_arguments(ARG_COMMAND) add_test(NAME ${ARG_NAME} COMMAND ${ARG_COMMAND} WORKING_DIRECTORY ${ARG_WORKING_DIRECTORY}) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index fc0da672e6f..d544ea76983 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -323,8 +323,9 @@ public class Nodes { return park(node.hostname(), false, agent, reason, transaction); } else { Node.State toState = Node.State.dirty; - if (node.state() == Node.State.parked && node.status().wantToDeprovision()) { - throw new IllegalArgumentException("Cannot move " + node + " to " + toState + ": It's being deprovisioned"); + if (node.state() == Node.State.parked) { + if (node.status().wantToDeprovision()) throw new IllegalArgumentException("Cannot move " + node + " to " + toState + ": It's being deprovisioned"); + if (node.status().wantToRebuild()) throw new IllegalArgumentException("Cannot move " + node + " to " + toState + ": It's being rebuilt"); } return db.writeTo(toState, List.of(node), agent, Optional.of(reason), transaction).get(0); } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HealthRequestHandler.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HealthRequestHandler.java index 09311e6f3e0..f54b9e319d2 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HealthRequestHandler.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HealthRequestHandler.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.resources; +import ai.vespa.http.HttpURL.Path; import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationId; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; @@ -51,8 +52,8 @@ public class HealthRequestHandler extends RestApiRequestHandler<HealthRequestHan ApplicationReferenceList list = new ApplicationReferenceList(); list.applicationList = applications.stream().map(applicationId -> { UrlReference reference = new UrlReference(); - reference.url = context.uriBuilder() - .withPath("/orchestrator/v1/health/" + applicationId.serializedForm()) + reference.url = context.baseRequestURL() + .withPath(Path.parse("/orchestrator/v1/health/" + applicationId.serializedForm())) .toString(); return reference; }).collect(Collectors.toList()); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandler.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandler.java index f90258d97d7..8e292d1bd6e 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandler.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandler.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.resources; +import ai.vespa.http.HttpURL.Path; import com.google.inject.Inject; import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.container.jdisc.ThreadedHttpRequestHandler; @@ -71,9 +72,9 @@ public class HostRequestHandler extends RestApiRequestHandler<HostRequestHandler try { Host host = orchestrator.getHost(hostName); - URI applicationUri = context.uriBuilder() - .withPath("/orchestrator/v1/instances/" + host.getApplicationInstanceReference().asString()) - .toURI(); + URI applicationUri = context.baseRequestURL() + .withPath(Path.parse( "/orchestrator/v1/instances/" + host.getApplicationInstanceReference().asString())) + .asURI(); List<HostService> hostServices = host.getServiceInstances().stream() .map(serviceInstance -> new HostService( diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandlerTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandlerTest.java index 03f2c0b5e3b..242c8e5b495 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandlerTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostRequestHandlerTest.java @@ -293,7 +293,7 @@ class HostRequestHandlerTest { HttpResponse httpResponse = executeRequest(testDriver, Method.GET, "/orchestrator/v1/hosts/hostname", null); GetHostResponse response = parseResponseContent(testDriver, httpResponse, GetHostResponse.class); - assertEquals("http://localhost/orchestrator/v1/instances/tenantId:applicationId", response.applicationUrl()); + assertEquals("http://localhost/orchestrator/v1/instances/tenantId%3AapplicationId", response.applicationUrl()); assertEquals("hostname", response.hostname()); assertEquals("ALLOWED_TO_BE_DOWN", response.state()); assertEquals("1970-01-01T00:00:00Z", response.suspendedSince()); diff --git a/searchcore/src/tests/proton/common/cachedselect_test.cpp b/searchcore/src/tests/proton/common/cachedselect_test.cpp index d197d94fda4..2fd798cd8fd 100644 --- a/searchcore/src/tests/proton/common/cachedselect_test.cpp +++ b/searchcore/src/tests/proton/common/cachedselect_test.cpp @@ -61,8 +61,6 @@ using search::attribute::IAttributeContext; using search::attribute::test::MockAttributeManager; using vespalib::string; -using namespace search::index; - using IATint32 = IntegerAttributeTemplate<int32_t>; using IntEnumAttribute = EnumAttribute<IATint32>; using NodeUP = Node::UP; diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp index 605a59b8a6a..192e137ddff 100644 --- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp @@ -152,6 +152,16 @@ struct StringAttributeFiller { } }; +struct WsetStringAttributeFiller { + using ValueType = vespalib::string; + static void add(AttributeVector& attr, const vespalib::string& value) { + auto& real = downcast<StringAttribute>(attr); + uint32_t docid = attr.getNumDocs() - 1; + real.append(docid, value, 1); + real.commit(); + } +}; + struct IntegerAttributeFiller { using ValueType = int64_t; static void add(AttributeVector& attr, int64_t value) { @@ -183,6 +193,17 @@ make_string_attribute(const std::string& value) } AttributeVector::SP +make_wset_string_attribute(const std::string& value) +{ + Config cfg(BasicType::STRING, CollectionType::WSET); + // fast-search is needed to trigger use of DirectAttributeBlueprint. + cfg.setFastSearch(true); + auto attr = AttributeFactory::createAttribute(field, cfg); + fill<WsetStringAttributeFiller>(*attr, value); + return attr; +} + +AttributeVector::SP make_int_attribute(int64_t value) { Config cfg(BasicType::INT32, CollectionType::SINGLE); @@ -413,4 +434,17 @@ TEST(AttributeBlueprintTest, attribute_field_blueprint_wraps_filter_search_itera EXPECT_TRUE(wrapper.seek(2)); } +TEST(AttributeBlueprintTest, direct_attribute_blueprint_wraps_filter_search_iterator) +{ + BlueprintFactoryFixture f(make_wset_string_attribute("foo")); + SimpleStringTerm term("foo", field, 0, Weight(0)); + auto blueprint = f.create_blueprint(term); + + auto itr = blueprint->createFilterSearch(true, Blueprint::FilterConstraint::UPPER_BOUND); + auto& wrapper = downcast<FilterWrapper>(*itr); + wrapper.initRange(1, 3); + EXPECT_FALSE(wrapper.seek(1)); + EXPECT_TRUE(wrapper.seek(2)); +} + GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index f14966dbfc8..bde73faf466 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -562,6 +562,13 @@ public: return std::make_unique<queryeval::DocumentWeightSearchIterator>(*tfmda[0], _attr, _dict_entry); } + SearchIteratorUP createFilterSearch(bool strict, FilterConstraint constraint) const override { + (void) constraint; // We provide an iterator with exact results, so no need to take constraint into consideration. + auto wrapper = std::make_unique<FilterWrapper>(getState().numFields()); + wrapper->wrap(createLeafSearch(wrapper->tfmda(), strict)); + return wrapper; + } + void visitMembers(vespalib::ObjectVisitor &visitor) const override { LeafBlueprint::visitMembers(visitor); visit(visitor, "attribute", _attrName); diff --git a/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp b/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp index 346c238f0cc..17a898765d3 100644 --- a/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp @@ -4,6 +4,7 @@ #include "load_utils.hpp" #include "multinumericattribute.hpp" #include "multi_numeric_flag_search_context.h" +#include <vespa/searchlib/common/bitvector.h> #include <vespa/log/log.h> LOG_SETUP(".searchlib.attribute.flag_attribute"); diff --git a/searchlib/src/vespa/searchlib/attribute/multinumericattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multinumericattribute.hpp index 99963094366..39e4f6866fb 100644 --- a/searchlib/src/vespa/searchlib/attribute/multinumericattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multinumericattribute.hpp @@ -4,13 +4,11 @@ #include "multinumericattribute.h" #include "multivalueattribute.hpp" #include "attributevector.hpp" -#include "attributeiterators.hpp" #include "multinumericattributesaver.h" #include "multi_numeric_search_context.h" #include "load_utils.h" #include "primitivereader.h" #include <vespa/searchlib/query/query_term_simple.h> -#include <vespa/searchlib/queryeval/emptysearch.h> #include <vespa/searchlib/util/fileutil.h> namespace search { diff --git a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp index c35a2e55ec3..3323440dd0d 100644 --- a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp @@ -2,13 +2,11 @@ #pragma once -#include "attributeiterators.hpp" #include "load_utils.h" #include "loadednumericvalue.h" #include "multinumericenumattribute.h" #include "multi_numeric_enum_search_context.h" #include <vespa/searchlib/query/query_term_simple.h> -#include <vespa/searchlib/queryeval/emptysearch.h> #include <vespa/searchlib/util/fileutil.hpp> namespace search { diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp index cf60183e2ca..917f0f55894 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include "attributeiterators.hpp" #include "attributevector.hpp" #include "load_utils.h" #include "numeric_matcher.h" @@ -11,7 +10,6 @@ #include "singlenumericattributesaver.h" #include "single_numeric_search_context.h" #include <vespa/searchlib/query/query_term_simple.h> -#include <vespa/searchlib/queryeval/emptysearch.h> namespace search { diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp index f5219e2e8c7..aefc3c1cba3 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp @@ -2,7 +2,6 @@ #pragma once -#include "attributeiterators.hpp" #include "load_utils.h" #include "loadednumericvalue.h" #include "primitivereader.h" @@ -10,7 +9,6 @@ #include "singlenumericenumattribute.h" #include "single_numeric_enum_search_context.h" #include <vespa/searchlib/query/query_term_simple.h> -#include <vespa/searchlib/queryeval/emptysearch.h> #include <vespa/searchlib/util/fileutil.hpp> namespace search { diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 642fdd5c16f..ae69564b671 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -349,20 +349,6 @@ ], "fields": [] }, - "com.yahoo.net.DomainName": { - "superClass": "ai.vespa.validation.PatternedStringWrapper", - "interfaces": [], - "attributes": [ - "public" - ], - "methods": [ - "public static com.yahoo.net.DomainName of(java.lang.String)", - "public static java.lang.String requireLabel(java.lang.String)" - ], - "fields": [ - "public static final com.yahoo.net.DomainName localhost" - ] - }, "com.yahoo.net.HostName": { "superClass": "ai.vespa.validation.PatternedStringWrapper", "interfaces": [], diff --git a/vespajlib/src/main/java/com/yahoo/net/DomainName.java b/vespajlib/src/main/java/ai/vespa/http/DomainName.java index ff8ba204674..a566f5b95be 100644 --- a/vespajlib/src/main/java/com/yahoo/net/DomainName.java +++ b/vespajlib/src/main/java/ai/vespa/http/DomainName.java @@ -1,5 +1,5 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.net; +package ai.vespa.http; import ai.vespa.validation.PatternedStringWrapper; diff --git a/container-core/src/main/java/com/yahoo/restapi/HttpURL.java b/vespajlib/src/main/java/ai/vespa/http/HttpURL.java index e890b0fe71a..916bbab9fe0 100644 --- a/container-core/src/main/java/com/yahoo/restapi/HttpURL.java +++ b/vespajlib/src/main/java/ai/vespa/http/HttpURL.java @@ -1,13 +1,14 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.restapi; +package ai.vespa.http; import ai.vespa.validation.StringWrapper; -import com.yahoo.net.DomainName; import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; +import java.util.Deque; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -22,7 +23,6 @@ import static ai.vespa.validation.Validation.requireInRange; import static java.net.URLDecoder.decode; import static java.net.URLEncoder.encode; import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Collections.unmodifiableMap; import static java.util.Objects.requireNonNull; /** @@ -96,30 +96,46 @@ public class HttpURL { Query.parse(uri.getRawQuery(), queryValidator)); } + /** Returns a copy of this with the given scheme. */ public HttpURL withScheme(Scheme scheme) { return create(scheme, domain, port, path, query); } + /** Returns a copy of this with the given domain. */ public HttpURL withDomain(DomainName domain) { return create(scheme, domain, port, path, query); } + /** Returns a copy of this with the given non-negative port. */ public HttpURL withPort(int port) { return create(scheme, domain, port, path, query); } + /** Returns a copy of this with no port specified. */ public HttpURL withoutPort() { return create(scheme, domain, -1, path, query); } + /** Returns a copy of this with only the given path. */ public HttpURL withPath(Path path) { return create(scheme, domain, port, path, query); } + /** Returns a copy of this with the given path appended. */ + public HttpURL appendPath(Path path) { + return create(scheme, domain, port, this.path.append(path), query); + } + + /** Returns a copy of this with only the given query. */ public HttpURL withQuery(Query query) { return create(scheme, domain, port, path, query); } + /** Returns a copy of this with all entries of the query appended. */ + public HttpURL appendQuery(Query query) { + return create(scheme, domain, port, path, this.query.add(query.entries())); + } + public Scheme scheme() { return scheme; } @@ -150,6 +166,24 @@ public class HttpURL { } } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + HttpURL httpURL = (HttpURL) o; + return port == httpURL.port && scheme == httpURL.scheme && domain.equals(httpURL.domain) && path.equals(httpURL.path) && query.equals(httpURL.query); + } + + @Override + public int hashCode() { + return Objects.hash(scheme, domain, port, path, query); + } + + @Override + public String toString() { + return asURI().toString(); + } + /** Require that the given string (possibly decoded multiple times) contains none of {@code '/', '?', '#'}, and isn't either of {@code "", ".", ".."}. */ public static String requirePathSegment(String value) { while ( ! value.equals(value = decode(value, UTF_8))); @@ -163,6 +197,8 @@ public class HttpURL { public static class Path { + private static final Path empty = empty(HttpURL::requirePathSegment); + private final List<String> segments; private final boolean trailingSlash; private final UnaryOperator<String> validator; @@ -175,7 +211,7 @@ public class HttpURL { /** Creates a new, empty path, with a trailing slash, using {@link HttpURL#requirePathSegment} for segment validation. */ public static Path empty() { - return empty(HttpURL::requirePathSegment); + return empty; } /** Creates a new, empty path, with a trailing slash, using the indicated validator for segments. */ @@ -183,16 +219,6 @@ public class HttpURL { return new Path(List.of(), true, segmentValidator(validator)); } - /** Creates a new path with the given <em>decoded</em> segments. */ - public static Path from(List<String> segments) { - return from(segments, __ -> { }); - } - - /** Creates a new path with the given <em>decoded</em> segments, and the validator applied to each segment. */ - public static Path from(List<String> segments, Consumer<String> validator) { - return empty(validator).append(segments, true); - } - /** Parses the given raw, normalized path string; this ignores whether the path is absolute or relative. */ public static Path parse(String raw) { return parse(raw, HttpURL::requirePathSegment); @@ -285,7 +311,7 @@ public class HttpURL { } /** A raw path string which parses to this, by splitting on {@code "/"}, and then URL decoding. */ - String raw() { + private String raw() { StringJoiner joiner = new StringJoiner("/", "/", trailingSlash ? "/" : "").setEmptyValue(trailingSlash ? "/" : ""); for (String segment : segments) joiner.add(encode(segment, UTF_8)); return joiner.toString(); @@ -315,50 +341,57 @@ public class HttpURL { public static class Query { - private final Map<String, String> values; + private static final Query empty = empty(__ -> { }); + + private static class Node { + + final Node next; + final String key; + final String value; + + public Node(Node next, String key, String value) { + this.next = next; + this.key = key; + this.value = value; + } + + } + + private final Node head; private final UnaryOperator<String> validator; - private Query(Map<String, String> values, UnaryOperator<String> validator) { - this.values = requireNonNull(values); + private Query(Node head, UnaryOperator<String> validator) { + this.head = head; this.validator = requireNonNull(validator); } /** Creates a new, empty query part. */ public static Query empty() { - return empty(__ -> { }); + return empty; } /** Creates a new, empty query part, using the indicated string wrapper for keys and non-null values. */ public static Query empty(Consumer<String> validator) { - return new Query(Map.of(), entryValidator(validator)); - } - - /** Creates a new query part with the given <em>decoded</em> values. */ - public static Query from(Map<String, String> values) { - return from(values, __ -> { }); - } - - /** Creates a new query part with the given <em>decoded</em> values, and the validator applied to each pair. */ - public static Query from(Map<String, String> values, Consumer<String> validator) { - return empty(validator).merge(values); + return new Query(null, entryValidator(validator)); } /** Parses the given raw query string. */ public static Query parse(String raw) { + if (raw == null) return empty(); return parse(raw, __-> { }); } - /** Parses the given raw query string, using the indicated string wrapper to hold keys and non-null values. */ + + /** Parses the given raw query string, using the validator on all keys and non-null values. */ public static Query parse(String raw, Consumer<String> validator) { if (raw == null) return empty(validator); - Map<String, String> values = new LinkedHashMap<>(); + Query query = empty(validator); for (String pair : raw.split("&")) { int split = pair.indexOf("="); - String key, value; - if (split == -1) { key = pair; value = null; } - else { key = pair.substring(0, split); value = pair.substring(split + 1); } - values.put(decode(key, UTF_8), value == null ? null : decode(value, UTF_8)); + if (split == -1) query = query.add(decode(pair, UTF_8)); + else query = query.add(decode(pair.substring(0, split), UTF_8), + decode(pair.substring(split + 1), UTF_8)); // any additional '=' become part of the value } - return empty(validator).merge(values); + return query; } private static UnaryOperator<String> entryValidator(Consumer<String> validator) { @@ -370,56 +403,107 @@ public class HttpURL { } /** Returns a copy of this with the <em>decoded</em> non-null key pointing to the <em>decoded</em> non-null value. */ - public Query put(String key, String value) { - Map<String, String> copy = new LinkedHashMap<>(values); - copy.put(validator.apply(requireNonNull(key)), validator.apply(requireNonNull(value))); - return new Query(copy, validator); + public Query add(String key, String value) { + return new Query(new Node(head, validator.apply(requireNonNull(key)), validator.apply(requireNonNull(value))), validator); } /** Returns a copy of this with the <em>decoded</em> non-null key pointing to "nothing". */ public Query add(String key) { - Map<String, String> copy = new LinkedHashMap<>(values); - copy.put(validator.apply(requireNonNull(key)), null); - return new Query(copy, validator); + return new Query(new Node(head, validator.apply(requireNonNull(key)), null), validator); + } + + /** Returns a copy of this with the <em>decoded</em> non-null key pointing <em>only</em> to the <em>decoded</em> non-null value. */ + public Query set(String key, String value) { + return remove(key).add(key, value); + } + + /** Returns a copy of this with the <em>decoded</em> non-null key <em>only</em> pointing to "nothing". */ + public Query set(String key) { + return remove(key).add(key); } /** Returns a copy of this without any key-value pair with the <em>decoded</em> key. */ public Query remove(String key) { - Map<String, String> copy = new LinkedHashMap<>(values); - copy.remove(validator.apply(requireNonNull(key))); - return new Query(copy, validator); + return new Query(without(validator.apply(requireNonNull(key)), head), validator); } - /** Returns a copy of this with all mappings from the other query added to this, possibly overwriting existing mappings. */ - public Query merge(Query other) { - return merge(other.values); + private static Node without(String key, Node node) { + if (node == null) return node; // null does not contain the key + Node child = without(key, node.next); // get a child that does not contain the key + if (node.key.equals(key)) return child; // if we have the key, unlink us + if (child == node.next) return node; // if our next didn't have the key, return unchanged + return new Node(child, node.key, node.value); // if our next has changed, we must change too } - /** Returns a copy of this with all given mappings added to this, possibly overwriting existing mappings. */ - public Query merge(Map<String, String> values) { - Map<String, String> copy = new LinkedHashMap<>(this.values); - values.forEach((key, value) -> copy.put(validator.apply(requireNonNull(key, "keys cannot be null")), - value == null ? null : validator.apply(value))); - return new Query(copy, validator); + /** Returns a copy of this with all given mappings appended to this. {@code null} values, but not lists of values, are allowed. */ + public Query add(Map<String, ? extends Iterable<String>> values) { + Query query = this; + for (Map.Entry<String, ? extends Iterable<String>> entry : values.entrySet()) + for (String value : entry.getValue()) + query = value == null ? query.add(entry.getKey()) + : query.add(entry.getKey(), value); + + return query; } - /** The <em>URL decoded</em> key-value pairs that make up this query; keys and values may be {@code ""}, and values are {@code null} when only key was specified. */ - public Map<String, String> entries() { - return unmodifiableMap(values); + /** Returns a copy of this with all given mappings added to this, possibly replacing existing mappings. */ + public Query set(Map<String, String> values) { + Query query = this; + for (Map.Entry<String, String> entry : values.entrySet()) + query = entry.getValue() == null ? query.set(entry.getKey()) + : query.set(entry.getKey(), entry.getValue()); + + return query; + } + + /** + * The <em>URL decoded</em> key-value pairs that make up this query; + * keys and values may be {@code ""}, and values are {@code null} when only key was specified. + * When a key was used multiple times, this map contains only the last value associated with the key. + */ + public Map<String, String> lastEntries() { + Map<String, String> entries = new LinkedHashMap<>(); + for (Node node : nodes()) + entries.put(node.key, node.value); + + return entries; + } + + /** + * The <em>URL decoded</em> key-value pairs that make up this query; + * keys and values may be {@code ""}, and values (not lists of values) are {@code null} when only key was specified. + * When a key was used multiple times, this map lists the values in the same order as they were given. + */ + public Map<String, List<String>> entries() { + Map<String, List<String>> entries = new LinkedHashMap<>(); + for (Node node : nodes()) + entries.computeIfAbsent(node.key, __ -> new ArrayList<>(2)).add(node.value); + + return entries; } /** A raw query string, with {@code '?'} prepended, that parses to this, by splitting on {@code "&"}, then on {@code "="}, and then URL decoding; or the empty string if this is empty. */ private String raw() { StringJoiner joiner = new StringJoiner("&", "?", "").setEmptyValue(""); - values.forEach((key, value) -> joiner.add(encode(key, UTF_8) + - (value == null ? "" : "=" + encode(value, UTF_8)))); + for (Node node : nodes()) + joiner.add(encode(node.key, UTF_8) + + (node.value == null ? "" : "=" + encode(node.value, UTF_8))); + return joiner.toString(); } + /** Nodes in insertion order. */ + private Iterable<Node> nodes() { + Deque<Node> nodes = new ArrayDeque<>(); + for (Node node = head; node != null; node = node.next) + nodes.push(node); + + return nodes; + } /** Intentionally not usable for constructing new URIs. Use {@link HttpURL} for that instead. */ @Override public String toString() { - return "query '" + raw() + "'"; + return head == null ? "no query" : "query '" + raw().substring(1) + "'"; } @Override @@ -427,12 +511,12 @@ public class HttpURL { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Query query = (Query) o; - return values.equals(query.values); + return entries().equals(query.entries()); } @Override public int hashCode() { - return Objects.hash(values); + return Objects.hash(entries()); } } diff --git a/vespajlib/src/main/java/ai/vespa/http/package-info.java b/vespajlib/src/main/java/ai/vespa/http/package-info.java new file mode 100644 index 00000000000..e5600c6f82d --- /dev/null +++ b/vespajlib/src/main/java/ai/vespa/http/package-info.java @@ -0,0 +1,5 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package ai.vespa.http; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/vespajlib/src/main/java/ai/vespa/validation/StringWrapper.java b/vespajlib/src/main/java/ai/vespa/validation/StringWrapper.java index 258be824ef5..c3c44c0ff4c 100644 --- a/vespajlib/src/main/java/ai/vespa/validation/StringWrapper.java +++ b/vespajlib/src/main/java/ai/vespa/validation/StringWrapper.java @@ -5,7 +5,19 @@ import static java.util.Objects.requireNonNull; /** * Abstract wrapper for glorified strings, to ease adding new such wrappers. - * + * <p> + * <br> + * What's in a name?<br> + * That which we call a String<br> + * by any other name would smell as foul.<br> + * No? 'Tis not soft?<br> + * No ... I see it now!<br> + * Baptiz'd a-new, the String—<br> + * no more a String,<br> + * no less a {@link #value}—<br> + * it bringeth counsel<br> + * and is proof against their enmity.<br> + * </p> * @param <T> child type * * @author jonmv diff --git a/vespajlib/src/main/java/com/yahoo/net/HostName.java b/vespajlib/src/main/java/com/yahoo/net/HostName.java index 47bd8246bb3..20f1008055e 100644 --- a/vespajlib/src/main/java/com/yahoo/net/HostName.java +++ b/vespajlib/src/main/java/com/yahoo/net/HostName.java @@ -1,14 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.net; +import ai.vespa.http.DomainName; import ai.vespa.validation.PatternedStringWrapper; import java.util.Optional; +import java.util.regex.Pattern; import static ai.vespa.validation.Validation.requireLength; /** - * Hostnames match {@link DomainName#domainNamePattern}, but are restricted to 64 characters in length. + * Hostnames match {@link #hostNamePattern}, and are restricted to 64 characters in length. * * This class also has utilities for getting the hostname of the system running the JVM. * Detection of the hostname is now done before starting any Vespa @@ -20,10 +22,13 @@ import static ai.vespa.validation.Validation.requireLength; */ public class HostName extends PatternedStringWrapper<HostName> { + static final Pattern labelPattern = Pattern.compile("([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])"); + static final Pattern hostNamePattern = Pattern.compile("(" + labelPattern + "\\.)*" + labelPattern); + private static HostName preferredHostName = null; private HostName(String value) { - super(requireLength(value, "hostname length", 1, 64), DomainName.domainNamePattern, "hostname"); + super(requireLength(value, "hostname length", 1, 64), hostNamePattern, "hostname"); } public static HostName of(String value) { diff --git a/vespajlib/src/main/java/com/yahoo/net/UriTools.java b/vespajlib/src/main/java/com/yahoo/net/UriTools.java index 55a94884a57..58fa2825a14 100644 --- a/vespajlib/src/main/java/com/yahoo/net/UriTools.java +++ b/vespajlib/src/main/java/com/yahoo/net/UriTools.java @@ -6,7 +6,7 @@ import java.net.URI; /** * Utility methods for working with URIs. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public final class UriTools { private UriTools() { diff --git a/vespajlib/src/test/java/com/yahoo/net/DomainNameTest.java b/vespajlib/src/test/java/ai/vespa/http/DomainNameTest.java index d8e76b71d7e..aa19e8667f6 100644 --- a/vespajlib/src/test/java/com/yahoo/net/DomainNameTest.java +++ b/vespajlib/src/test/java/ai/vespa/http/DomainNameTest.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.net; +package ai.vespa.http; -import com.yahoo.net.DomainName; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertThrows; diff --git a/container-core/src/test/java/com/yahoo/restapi/HttpURLTest.java b/vespajlib/src/test/java/ai/vespa/http/HttpURLTest.java index 858513c2a69..7f3fce27074 100644 --- a/container-core/src/test/java/com/yahoo/restapi/HttpURLTest.java +++ b/vespajlib/src/test/java/ai/vespa/http/HttpURLTest.java @@ -1,9 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.restapi; +package ai.vespa.http; +import ai.vespa.http.HttpURL.Path; import ai.vespa.validation.Name; -import com.yahoo.net.DomainName; -import com.yahoo.restapi.HttpURL.Query; +import ai.vespa.http.HttpURL.Query; import org.junit.jupiter.api.Test; import java.net.URI; @@ -14,9 +14,9 @@ import java.util.Map; import java.util.OptionalInt; import java.util.function.Consumer; -import static com.yahoo.net.DomainName.localhost; -import static com.yahoo.restapi.HttpURL.Scheme.http; -import static com.yahoo.restapi.HttpURL.Scheme.https; +import static ai.vespa.http.DomainName.localhost; +import static ai.vespa.http.HttpURL.Scheme.http; +import static ai.vespa.http.HttpURL.Scheme.https; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -43,22 +43,22 @@ class HttpURLTest { @Test void testModification() { - HttpURL url = HttpURL.create(http, localhost).withPath(HttpURL.Path.empty(Name::of)); + HttpURL url = HttpURL.create(http, localhost).withPath(Path.empty(Name::of)); assertEquals(http, url.scheme()); assertEquals(localhost, url.domain()); assertEquals(OptionalInt.empty(), url.port()); - assertEquals(HttpURL.Path.empty(Name::of), url.path()); + assertEquals(Path.empty(Name::of), url.path()); assertEquals(HttpURL.Query.empty(Name::of), url.query()); url = url.withScheme(https) .withDomain(DomainName.of("domain")) .withPort(0) .withPath(url.path().append("foo").withoutTrailingSlash()) - .withQuery(url.query().put("boo", "bar").add("baz")); + .withQuery(url.query().add("boo", "bar").add("baz")); assertEquals(https, url.scheme()); assertEquals(DomainName.of("domain"), url.domain()); assertEquals(OptionalInt.of(0), url.port()); - assertEquals(HttpURL.Path.parse("/foo", Name::of), url.path()); + assertEquals(Path.parse("/foo", Name::of), url.path()); assertEquals(HttpURL.Query.parse("boo=bar&baz", Name::of), url.query()); } @@ -107,7 +107,7 @@ class HttpURLTest { @Test void testPath() { - HttpURL.Path path = HttpURL.Path.parse("foo/bar/baz", Name::of); + Path path = Path.parse("foo/bar/baz", Name::of); List<String> expected = List.of("foo", "bar", "baz"); assertEquals(expected, path.segments()); @@ -119,7 +119,7 @@ class HttpURLTest { assertEquals(path, path.withoutTrailingSlash().withoutTrailingSlash()); assertEquals(List.of("one", "foo", "bar", "baz", "two"), - HttpURL.Path.from(List.of("one")).append(path).append("two").segments()); + Path.empty().append(List.of("one")).append(path).append("two").segments()); assertEquals(List.of(expected.get(2), expected.get(0)), path.append(path).cut(2).skip(2).segments()); @@ -147,19 +147,19 @@ class HttpURLTest { assertEquals("path segment decoded cannot contain '/', but got: '/'", assertThrows(IllegalArgumentException.class, - () -> HttpURL.Path.empty().append("%2525252525252525%2525252525253%25252532%252525%252534%36")).getMessage()); + () -> Path.empty().append("%2525252525252525%2525252525253%25252532%252525%252534%36")).getMessage()); assertEquals("path segment decoded cannot contain '?', but got: '?'", assertThrows(IllegalArgumentException.class, - () -> HttpURL.Path.empty().append("?")).getMessage()); + () -> Path.empty().append("?")).getMessage()); assertEquals("path segment decoded cannot contain '#', but got: '#'", assertThrows(IllegalArgumentException.class, - () -> HttpURL.Path.empty().append("#")).getMessage()); + () -> Path.empty().append("#")).getMessage()); assertEquals("path segments cannot be \"\", \".\", or \"..\", but got: '..'", assertThrows(IllegalArgumentException.class, - () -> HttpURL.Path.empty().append("%2E%25252E")).getMessage()); + () -> Path.empty().append("%2E%25252E")).getMessage()); } @Test @@ -168,35 +168,51 @@ class HttpURLTest { Map<String, String> expected = new LinkedHashMap<>(); expected.put("foo", "bar"); expected.put("baz", null); - assertEquals(expected, query.entries()); + assertEquals(expected, query.lastEntries()); expected.remove("baz"); - assertEquals(expected, query.remove("baz").entries()); + assertEquals(expected, query.remove("baz").lastEntries()); expected.put("baz", null); expected.remove("foo"); - assertEquals(expected, query.remove("foo").entries()); - assertEquals(expected, Query.empty(Name::of).add("baz").entries()); + assertEquals(expected, query.remove("foo").lastEntries()); + assertEquals(expected, Query.empty(Name::of).set("baz").lastEntries()); - assertEquals("query '?foo=bar&baz=bax&quu=fez&moo'", - query.put("baz", "bax").merge(Query.from(Map.of("quu", "fez"))).add("moo").toString()); + assertEquals("query 'foo=bar&baz=bax&quu=fez&moo'", + query.set("baz", "bax").set(Map.of("quu", "fez")).set("moo").toString()); + + Query bloated = query.add("baz", "bax").add(Map.of("quu", List.of("fez", "pop"))).add("moo").add("moo").add("foo", "bar"); + assertEquals("query 'foo=bar&baz&baz=bax&quu=fez&quu=pop&moo&moo&foo=bar'", + bloated.toString()); + + assertEquals("query 'foo=bar&quu=fez&quu=pop&moo&moo&foo=bar'", + bloated.remove("baz").toString()); + + assertEquals("query 'baz&baz=bax&quu=fez&quu=pop&moo&moo'", + bloated.remove("foo").toString()); + + assertEquals("query 'foo=bar&baz&baz=bax&quu=fez&quu=pop&foo=bar&moo'", + bloated.set("moo").toString()); + + assertEquals("no query", + bloated.remove("foo").remove("baz").remove("quu").remove("moo").toString()); assertThrows(NullPointerException.class, () -> query.remove(null)); assertThrows(NullPointerException.class, - () -> query.add(null)); + () -> query.add((String) null)); assertThrows(NullPointerException.class, - () -> query.put(null, "hax")); + () -> query.add(null, "hax")); assertThrows(NullPointerException.class, - () -> query.put("hax", null)); + () -> query.add("hax", null)); Map<String, String> names = new LinkedHashMap<>(); names.put(null, "hax"); assertThrows(NullPointerException.class, - () -> query.merge(names)); + () -> query.set(names)); } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java index 56ae65bb317..3f9c52594a3 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java @@ -69,6 +69,9 @@ public class Lock implements Mutex { throw new RuntimeException("Exception releasing lock '" + lockPath + "'"); } } + + protected String lockPath() { return lockPath; } + } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/MultiplePathsLock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/MultiplePathsLock.java new file mode 100644 index 00000000000..9576f5c0385 --- /dev/null +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/MultiplePathsLock.java @@ -0,0 +1,41 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.curator; + +import com.yahoo.path.Path; + +import java.time.Duration; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Class that holds two locks, originally used for transitioning from one lock to + * another, where you need to hold both the old lock and the new lock in the + * transition period. Locks are acquired in constructor. + * + * @author hmusum + */ +public class MultiplePathsLock extends Lock { + + private static final Logger log = Logger.getLogger(MultiplePathsLock.class.getName()); + + private final Lock oldLock; + + public MultiplePathsLock(Path newLockPath, Path oldLockPath, Duration timeout, Curator curator) { + super(newLockPath.getAbsolute(), curator); + log.log(Level.INFO, "Acquiring lock " + oldLockPath); + this.oldLock = curator.lock(oldLockPath, timeout);; + log.log(Level.INFO, "Acquiring lock " + lockPath()); + super.acquire(timeout); + } + + @Override + public void close() { + log.log(Level.INFO, "Closing lock " + oldLock.lockPath()); + oldLock.close(); + log.log(Level.INFO, "Closing lock " + lockPath()); + super.close(); + } + +} + + |