diff options
author | Tor Brede Vekterli <vekterli@oath.com> | 2018-05-30 17:00:57 +0200 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@oath.com> | 2018-10-09 15:30:48 +0200 |
commit | d5a6b5ebf36c89c12def0296c4497008e6372523 (patch) | |
tree | 4ab6e6e0293ebbe42bf51954e29144f09e5d9a91 /vespaclient-container-plugin/src | |
parent | 242ca38ad85396eceaa5248aea34368dcabf556f (diff) |
Support cross-document type visiting via /document/v1/ root
Requires `cluster` to be set since we don't have a document type to
auto-infer the target cluster from. Can use `bucketSpace` parameter
to explicitly state the target bucket space to visit (if not given,
implicitly visits the 'default' space).
Note: since we are not bound to a single document type, the field set
used is `[all]`, not `doctype:[document]`. This means that this does
_not_ have parity with non-root Document V1 visitor requests, though
it _does_ have parity with legacy `/visit` and `vespa-visit`. To
have same behavior for a single document type, use an explicit document
`selection=mydoctype` parameter combined with a `fieldSet` parameter
of `mydoctype:[document]`.
This fixes #5794
Diffstat (limited to 'vespaclient-container-plugin/src')
7 files changed, 274 insertions, 40 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandler.java index 35999df5a6b..775207d8629 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandler.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandler.java @@ -29,6 +29,7 @@ public interface OperationHandler { public final Optional<Integer> wantedDocumentCount; public final Optional<String> fieldSet; public final Optional<Integer> concurrency; + public final Optional<String> bucketSpace; /** @deprecated Use a VisitOptions.Builder instead */ @Deprecated @@ -38,6 +39,7 @@ public interface OperationHandler { this.wantedDocumentCount = wantedDocumentCount; this.fieldSet = Optional.empty(); this.concurrency = Optional.empty(); + this.bucketSpace = Optional.empty(); } private VisitOptions(Builder builder) { @@ -46,6 +48,7 @@ public interface OperationHandler { this.wantedDocumentCount = Optional.ofNullable(builder.wantedDocumentCount); this.fieldSet = Optional.ofNullable(builder.fieldSet); this.concurrency = Optional.ofNullable(builder.concurrency); + this.bucketSpace = Optional.ofNullable(builder.bucketSpace); } public static class Builder { @@ -54,6 +57,7 @@ public interface OperationHandler { Integer wantedDocumentCount; String fieldSet; Integer concurrency; + String bucketSpace; public Builder cluster(String cluster) { this.cluster = cluster; @@ -80,6 +84,11 @@ public interface OperationHandler { return this; } + public Builder bucketSpace(String bucketSpace) { + this.bucketSpace = bucketSpace; + return this; + } + public VisitOptions build() { return new VisitOptions(this); } diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java index c0ce3e84232..c3844e398fe 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java @@ -4,10 +4,10 @@ package com.yahoo.document.restapi; import com.yahoo.document.Document; import com.yahoo.document.DocumentId; import com.yahoo.document.DocumentRemove; +import com.yahoo.document.FixedBucketSpaces; import com.yahoo.document.TestAndSetCondition; import com.yahoo.document.json.JsonWriter; import com.yahoo.document.DocumentPut; -import com.yahoo.document.restapi.resource.RestApi; import com.yahoo.documentapi.DocumentAccess; import com.yahoo.documentapi.DocumentAccessException; import com.yahoo.documentapi.SyncParameters; @@ -142,7 +142,7 @@ public class OperationHandlerImpl implements OperationHandler { restUri, RestUri.apiErrorCodes.DOCUMENT_CONDITION_NOT_MET); } return Response.createErrorResponse(getHTTPStatusCode(documentException.getErrorCodes()), documentException.getMessage(), restUri, - RestUri.apiErrorCodes.DOCUMENT_EXCPETION); + RestUri.apiErrorCodes.DOCUMENT_EXCEPTION); } @Override @@ -282,7 +282,7 @@ public class OperationHandlerImpl implements OperationHandler { response = Response.createErrorResponse(412, "Condition not met: " + documentException.getMessage(), restUri, RestUri.apiErrorCodes.DOCUMENT_CONDITION_NOT_MET); } else { - response = Response.createErrorResponse(400, documentException.getMessage(), restUri, RestUri.apiErrorCodes.DOCUMENT_EXCPETION); + response = Response.createErrorResponse(400, documentException.getMessage(), restUri, RestUri.apiErrorCodes.DOCUMENT_EXCEPTION); } } catch (Exception e) { response = Response.createErrorResponse(500, ExceptionUtils.getStackTrace(e), restUri, RestUri.apiErrorCodes.UNSPECIFIED); @@ -321,16 +321,40 @@ public class OperationHandlerImpl implements OperationHandler { return get(restUri, Optional.empty()); } - protected BucketSpaceRoute resolveBucketSpaceRoute(Optional<String> wantedCluster, String docType) throws RestApiException { + private static boolean isValidBucketSpace(String spaceName) { + // TODO need bucket space repo in Java as well + return (FixedBucketSpaces.defaultSpace().equals(spaceName) + || FixedBucketSpaces.globalSpace().equals(spaceName)); + } + + protected BucketSpaceRoute resolveBucketSpaceRoute(Optional<String> wantedCluster, + Optional<String> wantedBucketSpace, + RestUri restUri) throws RestApiException { final List<ClusterDef> clusters = clusterEnumerator.enumerateClusters(); ClusterDef clusterDef = resolveClusterDef(wantedCluster, clusters); - Optional<String> targetBucketSpace = bucketSpaceResolver.clusterBucketSpaceFromDocumentType(clusterDef.getConfigId(), docType); - if (!targetBucketSpace.isPresent()) { - throw new RestApiException(Response.createErrorResponse(400, String.format( - "Document type '%s' in cluster '%s' is not mapped to a known bucket space", docType, clusterDef.getName()), - RestUri.apiErrorCodes.UNKNOWN_BUCKET_SPACE)); + + String targetBucketSpace; + if (!restUri.isRootOnly()) { + String docType = restUri.getDocumentType(); + Optional<String> resolvedSpace = bucketSpaceResolver.clusterBucketSpaceFromDocumentType(clusterDef.getConfigId(), docType); + if (!resolvedSpace.isPresent()) { + throw new RestApiException(Response.createErrorResponse(400, String.format( + "Document type '%s' in cluster '%s' is not mapped to a known bucket space", docType, clusterDef.getName()), + RestUri.apiErrorCodes.UNKNOWN_BUCKET_SPACE)); + } + targetBucketSpace = resolvedSpace.get(); + } else { + if (wantedBucketSpace.isPresent() && !isValidBucketSpace(wantedBucketSpace.get())) { + // TODO enumerate known bucket spaces from a repo instead of having a fixed set + throw new RestApiException(Response.createErrorResponse(400, String.format( + "Bucket space '%s' is not a known bucket space (expected '%s' or '%s')", + wantedBucketSpace.get(), FixedBucketSpaces.defaultSpace(), FixedBucketSpaces.globalSpace()), + RestUri.apiErrorCodes.UNKNOWN_BUCKET_SPACE)); + } + targetBucketSpace = wantedBucketSpace.orElse(FixedBucketSpaces.defaultSpace()); } - return new BucketSpaceRoute(clusterDefToRoute(clusterDef), targetBucketSpace.get()); + + return new BucketSpaceRoute(clusterDefToRoute(clusterDef), targetBucketSpace); } protected static ClusterDef resolveClusterDef(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException { @@ -365,26 +389,41 @@ public class OperationHandlerImpl implements OperationHandler { return clusterListString.toString(); } - private VisitorParameters createVisitorParameters( - RestUri restUri, - String documentSelection, - VisitOptions options) - throws RestApiException { - + private static String buildAugmentedDocumentSelection(RestUri restUri, String documentSelection) { + if (restUri.isRootOnly()) { + return documentSelection; // May be empty, that's fine. + } StringBuilder selection = new StringBuilder(); - if (! documentSelection.isEmpty()) { - // TODO shouldn't selection be wrapped in () itself ? - selection.append("(").append(documentSelection).append(" and "); + selection.append("((").append(documentSelection).append(") and "); } selection.append(restUri.getDocumentType()).append(" and (id.namespace=='").append(restUri.getNamespace()).append("')"); if (! documentSelection.isEmpty()) { selection.append(")"); } + return selection.toString(); + } + + private VisitorParameters createVisitorParameters( + RestUri restUri, + String documentSelection, + VisitOptions options) + throws RestApiException { + + if (restUri.isRootOnly() && !options.cluster.isPresent()) { + throw new RestApiException(Response.createErrorResponse(400, + "Must set 'cluster' parameter to a valid content cluster id when visiting at a root /document/v1/ level", + RestUri.apiErrorCodes.MISSING_CLUSTER)); + } + + String augmentedSelection = buildAugmentedDocumentSelection(restUri, documentSelection); - VisitorParameters params = new VisitorParameters(selection.toString()); - // Only return fieldset that is part of the document. - params.fieldSet(options.fieldSet.orElse(restUri.getDocumentType() + ":[document]")); + VisitorParameters params = new VisitorParameters(augmentedSelection); + // Only return fieldset that is part of the document, unless we're visiting across all + // document types in which case we can't explicitly state a single document type. + // This matches legacy /visit API and vespa-visit tool behavior. + params.fieldSet(options.fieldSet.orElse( + restUri.isRootOnly() ? "[all]" : restUri.getDocumentType() + ":[document]")); params.setMaxBucketsPerVisitor(1); params.setMaxPending(32); params.setMaxFirstPassHits(1); @@ -399,7 +438,7 @@ public class OperationHandlerImpl implements OperationHandler { params.visitInconsistentBuckets(true); // TODO document this as part of consistency doc params.setVisitorOrdering(VisitorOrdering.ASCENDING); - BucketSpaceRoute bucketSpaceRoute = resolveBucketSpaceRoute(options.cluster, restUri.getDocumentType()); + BucketSpaceRoute bucketSpaceRoute = resolveBucketSpaceRoute(options.cluster, options.bucketSpace, restUri); params.setRoute(bucketSpaceRoute.getClusterRoute()); params.setBucketSpace(bucketSpaceRoute.getBucketSpace()); diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java index e3423eec2c8..975075fd2fa 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/RestUri.java @@ -36,7 +36,7 @@ public class RestUri { TOO_MANY_PARALLEL_REQUESTS(-8), MISSING_CLUSTER(-9), INTERNAL_EXCEPTION(-9), DOCUMENT_CONDITION_NOT_MET(-10), - DOCUMENT_EXCPETION(-11), + DOCUMENT_EXCEPTION(-11), PARSER_ERROR(-11), GROUP_AND_EXPRESSION_ERROR(-12), TIME_OUT(-13), @@ -67,6 +67,10 @@ public class RestUri { private Optional<Group> group = Optional.empty(); private final String rawPath; + public boolean isRootOnly() { + return namespace == null; + } + public String getRawPath() { return rawPath; } @@ -89,8 +93,8 @@ public class RestUri { public String generateFullId() { return ID + namespace + ":" + documentType + ":" - + (getGroup().isPresent() ? group.get().name + "=" + group.get().value : "") - + ":" + docId; + + group.map(g -> String.format("%s=%s", g.name, g.value)).orElse("") + + ":" + docId; } static class PathParser { @@ -102,6 +106,11 @@ public class RestUri { this.originalPath = path; this.rawParts = Splitter.on('/').splitToList(path); } + + boolean hasNextToken() { + return readPos < rawParts.size(); + } + String nextTokenOrException() throws RestApiException { if (readPos >= rawParts.size()) { throwUsage(originalPath); @@ -132,7 +141,15 @@ public class RestUri { ! pathParser.nextTokenOrException().equals(V_1)) { throwUsage(uri.getRawPath()); } - namespace = pathParser.nextTokenOrException(); + // If /document/v1 root request, there's an empty token at the end. + String maybeNamespace = pathParser.nextTokenOrException(); + if (maybeNamespace.isEmpty()) { + namespace = null; + documentType = null; + docId = null; + return; + } + namespace = maybeNamespace; documentType = pathParser.nextTokenOrException(); switch (pathParser.nextTokenOrException()) { case "number": diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java index 49b27eba613..1bd4cf535c9 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java @@ -38,6 +38,8 @@ import java.util.Optional; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; +import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; + /** * API for handling single operation on a document and visiting. * @@ -58,6 +60,7 @@ public class RestApi extends LoggingRequestHandler { private static final String WANTED_DOCUMENT_COUNT = "wantedDocumentCount"; private static final String FIELD_SET = "fieldSet"; private static final String CONCURRENCY = "concurrency"; + private static final String BUCKET_SPACE = "bucketSpace"; private static final String APPLICATION_JSON = "application/json"; private final OperationHandler operationHandler; private SingleDocumentParser singleDocumentParser; @@ -148,11 +151,24 @@ public class RestApi extends LoggingRequestHandler { } } + private static void validateUriStructureForRequestMethod(RestUri uri, com.yahoo.jdisc.http.HttpRequest.Method method) throws RestApiException { + if ((method != com.yahoo.jdisc.http.HttpRequest.Method.GET) && uri.isRootOnly()) { + throw new RestApiException(Response.createErrorResponse(BAD_REQUEST, + "Root /document/v1/ requests only supported for HTTP GET", + RestUri.apiErrorCodes.ERROR_ID_BASIC_USAGE)); + } + } + + private static boolean isVisitRequestUri(RestUri uri) { + return (uri.isRootOnly() || uri.getDocId().isEmpty()); + } + // protected for testing protected HttpResponse handleInternal(HttpRequest request) { final RestUri restUri; try { restUri = new RestUri(request.getUri()); + validateUriStructureForRequestMethod(restUri, request.getMethod()); } catch (RestApiException e) { return e.getResponse(); } catch (Exception e2) { @@ -173,7 +189,7 @@ public class RestApi extends LoggingRequestHandler { try { switch (request.getMethod()) { case GET: // Vespa Visit/Get - return restUri.getDocId().isEmpty() ? handleVisit(restUri, request) : handleGet(restUri, request); + return isVisitRequestUri(restUri) ? handleVisit(restUri, request) : handleGet(restUri, request); case POST: // Vespa Put operationHandler.put(restUri, createPutOperation(request, restUri.generateFullId(), condition), route); break; @@ -276,6 +292,7 @@ public class RestApi extends LoggingRequestHandler { Optional.ofNullable(request.getProperty(CLUSTER)).ifPresent(c -> optionsBuilder.cluster(c)); Optional.ofNullable(request.getProperty(CONTINUATION)).ifPresent(c -> optionsBuilder.continuation(c)); Optional.ofNullable(request.getProperty(FIELD_SET)).ifPresent(fs -> optionsBuilder.fieldSet(fs)); + Optional.ofNullable(request.getProperty(BUCKET_SPACE)).ifPresent(s -> optionsBuilder.bucketSpace(s)); parsePositiveIntegerRequestParameter(WANTED_DOCUMENT_COUNT, request).ifPresent(c -> optionsBuilder.wantedDocumentCount(c)); parsePositiveIntegerRequestParameter(CONCURRENCY, request).ifPresent(c -> optionsBuilder.concurrency(c)); diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java index 5021899e30b..4c04070cec4 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/OperationHandlerImplTest.java @@ -145,6 +145,10 @@ public class OperationHandlerImplTest { return new RestUri(new URI("http://localhost/document/v1/namespace/document-type/docid/")); } + private static RestUri apiRootVisitUri() throws Exception { + return new RestUri(new URI("http://localhost/document/v1/")); + } + private static RestUri dummyGetUri() throws Exception { return new RestUri(new URI("http://localhost/document/v1/namespace/document-type/docid/foo")); } @@ -166,6 +170,7 @@ public class OperationHandlerImplTest { try { OperationHandlerImpl handler = fixture.createHandler(); handler.visit(dummyVisitUri(), "", emptyVisitOptions()); + fail("Exception expected"); } catch (RestApiException e) { assertThat(e.getResponse().getStatus(), is(500)); assertThat(renderRestApiExceptionAsString(e), containsString("Timed out")); @@ -192,14 +197,19 @@ public class OperationHandlerImplTest { assertThat(fixture.assignedParameters.get().getSessionTimeoutMs(), is((long)OperationHandlerImpl.VISIT_TIMEOUT_MS)); } - private static VisitorParameters generatedParametersFromVisitOptions(OperationHandler.VisitOptions options) throws Exception { + private static VisitorParameters generatedVisitParametersFrom(RestUri restUri, String documentSelection, + OperationHandler.VisitOptions options) throws Exception { OperationHandlerImplFixture fixture = new OperationHandlerImplFixture(); OperationHandlerImpl handler = fixture.createHandler(); - handler.visit(dummyVisitUri(), "", options); + handler.visit(restUri, documentSelection, options); return fixture.assignedParameters.get(); } + private static VisitorParameters generatedParametersFromVisitOptions(OperationHandler.VisitOptions options) throws Exception { + return generatedVisitParametersFrom(dummyVisitUri(), "", options); + } + @Test public void document_type_is_mapped_to_correct_bucket_space() throws Exception { OperationHandlerImplFixture fixture = new OperationHandlerImplFixture(); @@ -218,6 +228,7 @@ public class OperationHandlerImplTest { try { OperationHandlerImpl handler = fixture.createHandler(); handler.visit(dummyVisitUri(), "", emptyVisitOptions()); + fail("Exception expected"); } catch (RestApiException e) { assertThat(e.getResponse().getStatus(), is(400)); String errorMsg = renderRestApiExceptionAsString(e); @@ -304,4 +315,89 @@ public class OperationHandlerImplTest { verify(fixture.mockSyncSession).get(any(), eq("donald,duck"), any()); } + @Test + public void api_root_visit_uri_requires_cluster_set() throws Exception { + OperationHandlerImplFixture fixture = new OperationHandlerImplFixture(); + OperationHandlerImpl handler = fixture.createHandler(); + try { + handler.visit(apiRootVisitUri(), "", emptyVisitOptions()); + fail("Exception expected"); + } catch (RestApiException e) { + assertThat(e.getResponse().getStatus(), is(400)); + assertThat(renderRestApiExceptionAsString(e), containsString( + "MISSING_CLUSTER Must set 'cluster' parameter to a valid content cluster id " + + "when visiting at a root /document/v1/ level")); + } + } + + @Test + public void api_root_visiting_propagates_request_route() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "", optionsBuilder().cluster("foo").build()); + assertEquals("[Storage:cluster=foo;clusterconfigid=configId]", parameters.getRoute().toString()); + } + + @Test + public void api_root_visiting_targets_default_bucket_space_by_default() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "", optionsBuilder().cluster("foo").build()); + assertEquals("default", parameters.getBucketSpace()); + } + + @Test + public void api_root_visiting_can_explicitly_specify_bucket_space() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "", + optionsBuilder().cluster("foo").bucketSpace("global").build()); + assertEquals("global", parameters.getBucketSpace()); + } + + @Test + public void api_root_visiting_throws_exception_on_unknown_bucket_space_name() throws Exception { + try { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "", + optionsBuilder().cluster("foo").bucketSpace("langbein").build()); + } catch (RestApiException e) { + assertThat(e.getResponse().getStatus(), is(400)); + assertThat(renderRestApiExceptionAsString(e), containsString( + "UNKNOWN_BUCKET_SPACE Bucket space 'langbein' is not a known bucket space " + + "(expected 'default' or 'global')")); + } + } + + @Test + public void api_root_visiting_has_empty_document_selection_by_default() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "", optionsBuilder().cluster("foo").build()); + assertEquals("", parameters.getDocumentSelection()); + } + + @Test + public void api_root_visiting_propagates_provided_document_selection() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "baz.blarg", optionsBuilder().cluster("foo").build()); + // Note: syntax correctness of selection is checked and enforced by RestApi + assertEquals("baz.blarg", parameters.getDocumentSelection()); + } + + @Test + public void api_root_visiting_uses_all_fieldset_by_default() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "", optionsBuilder().cluster("foo").build()); + assertEquals("[all]", parameters.getFieldSet()); + } + + @Test + public void api_root_visiting_propagates_provided_fieldset() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(apiRootVisitUri(), "", + optionsBuilder().cluster("foo").fieldSet("zoidberg:[document]").build()); + assertEquals("zoidberg:[document]", parameters.getFieldSet()); + } + + @Test + public void namespace_and_doctype_augmented_selection_has_parenthesized_selection_sub_expression() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(dummyVisitUri(), "1 != 2", optionsBuilder().cluster("foo").build()); + assertEquals("((1 != 2) and document-type and (id.namespace=='namespace'))", parameters.getDocumentSelection()); + } + + @Test + public void namespace_and_doctype_visit_without_selection_does_not_contain_selection_sub_expression() throws Exception { + VisitorParameters parameters = generatedVisitParametersFrom(dummyVisitUri(), "", optionsBuilder().cluster("foo").build()); + assertEquals("document-type and (id.namespace=='namespace')", parameters.getDocumentSelection()); + } + } diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/MockedOperationHandler.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/MockedOperationHandler.java index 297576f1bef..5f44b06ab77 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/MockedOperationHandler.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/MockedOperationHandler.java @@ -24,7 +24,9 @@ public class MockedOperationHandler implements OperationHandler { + documentSelection + "'" + options.wantedDocumentCount.map(n -> String.format(", min docs returned: %d", n)).orElse("") + options.fieldSet.map(s -> String.format(", field set: '%s'", s)).orElse("") - + options.concurrency.map(n -> String.format(", concurrency: %d", n)).orElse("")); + + options.concurrency.map(n -> String.format(", concurrency: %d", n)).orElse("") + + options.bucketSpace.map(s -> String.format(", bucket space: '%s'", s)).orElse("") + + options.cluster.map(s -> String.format(", cluster: '%s'", s)).orElse("")); } @Override diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java index 6b7f708e090..5e3aa05130d 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java @@ -28,6 +28,8 @@ import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.nio.file.Paths; +import java.util.function.Function; +import java.util.function.Supplier; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsNot.not; @@ -288,19 +290,47 @@ public class RestApiTest { } } - private String performV1RestCall(String pathSuffix) { + private static String defaultPathPrefix() { + return "namespace/document-type/"; + } + + private String performV1RestCall(String pathPrefix, String pathSuffix, Function<Request, HttpRequestBase> methodOp) { try { - Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/%s", - getFirstListenPort(), pathSuffix)); - HttpGet get = new HttpGet(request.getUri()); - return doRest(get); + Request request = new Request(String.format("http://localhost:%s/document/v1/%s%s", + getFirstListenPort(), pathPrefix, pathSuffix)); + HttpRequestBase restOp = methodOp.apply(request); + return doRest(restOp); } catch (Exception e) { throw new RuntimeException(e); } } + private String performV1GetRestCall(String pathSuffix) { + return performV1RestCall(defaultPathPrefix(), pathSuffix, (request) -> new HttpGet(request.getUri())); + } + + private void doTestRootPathNotAccepted(Function<Request, HttpRequestBase> methodOpFactory) { + String output = performV1RestCall("", "", methodOpFactory); + assertThat(output, containsString("Root /document/v1/ requests only supported for HTTP GET")); + } + + @Test + public void root_api_path_not_accepted_for_http_put() { + doTestRootPathNotAccepted((request) -> new HttpPut(request.getUri())); + } + + @Test + public void root_api_path_not_accepted_for_http_post() { + doTestRootPathNotAccepted((request) -> new HttpPost(request.getUri())); + } + + @Test + public void root_api_path_not_accepted_for_http_delete() { + doTestRootPathNotAccepted((request) -> new HttpDelete(request.getUri())); + } + private void assertResultingDocumentSelection(String suffix, String expected) { - String output = performV1RestCall(suffix); + String output = performV1GetRestCall(suffix); assertThat(output, containsString(String.format("doc selection: '%s'", expected))); } @@ -321,7 +351,7 @@ public class RestApiTest { } private void assertNumericIdFailsParsing(String id) { - String output = performV1RestCall(String.format("number/%s", encoded(id))); + String output = performV1GetRestCall(String.format("number/%s", encoded(id))); assertThat(output, containsString("Failed to parse numeric part of selection URI")); } @@ -335,7 +365,7 @@ public class RestApiTest { @Test public void non_text_group_string_character_returns_error() { - String output = performV1RestCall(String.format("group/%s", encoded("\u001f"))); + String output = performV1GetRestCall(String.format("group/%s", encoded("\u001f"))); assertThat(output, containsString("Failed to parse group part of selection URI; contains invalid text code point U001F")); } @@ -362,7 +392,7 @@ public class RestApiTest { } private void assertDocumentSelectionFailsParsing(String expression) { - String output = performV1RestCall(String.format("number/1234?selection=%s", encoded(expression))); + String output = performV1GetRestCall(String.format("number/1234?selection=%s", encoded(expression))); assertThat(output, containsString("Failed to parse expression given in 'selection' parameter. Must be a complete and valid sub-expression.")); } @@ -416,6 +446,30 @@ public class RestApiTest { } @Test + public void root_api_visit_cluster_parameter_is_propagated() throws IOException { + Request request = new Request(String.format("http://localhost:%s/document/v1/?cluster=vaffel", getFirstListenPort())); + HttpGet get = new HttpGet(request.getUri()); + String rest = doRest(get); + assertThat(rest, containsString("cluster: 'vaffel'")); + } + + @Test + public void root_api_visit_selection_parameter_is_propagated() throws IOException { + Request request = new Request(String.format("http://localhost:%s/document/v1/?cluster=foo&selection=yoshi", getFirstListenPort())); + HttpGet get = new HttpGet(request.getUri()); + String rest = doRest(get); + assertThat(rest, containsString("doc selection: 'yoshi'")); + } + + @Test + public void root_api_visit_bucket_space_parameter_is_propagated() throws IOException { + Request request = new Request(String.format("http://localhost:%s/document/v1/?cluster=foo&bucketSpace=global", getFirstListenPort())); + HttpGet get = new HttpGet(request.getUri()); + String rest = doRest(get); + assertThat(rest, containsString("bucket space: 'global'")); + } + + @Test public void invalid_visit_concurrency_parameter_returns_error_response() throws IOException { Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/docid/?concurrency=badgers", getFirstListenPort())); HttpGet get = new HttpGet(request.getUri()); |