Merge pull request #10075 from vespa-engine/vekterli/support-cluster-parameter-for-document-v1-get-operations

Support cluster parameter for document v1 get operations

6 files changed, 57 insertions, 8 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 a6fdcb10a00..848fe4b5726 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
@@ -101,6 +101,10 @@ public interface OperationHandler {
     default Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
         return get(restUri);
     }
+
+    default Optional<String> get(RestUri restUri, Optional<String> fieldSet, Optional<String> cluster) throws RestApiException {
+        return get(restUri, fieldSet);
+    }
     
     /** Called just before this is disposed of */
     default void shutdown() {}
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 4ca43f5fde2..333f1d2f8c9 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
@@ -279,9 +279,13 @@ public class OperationHandlerImpl implements OperationHandler {
     }
 
     @Override
-    public Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
+    public Optional<String> get(RestUri restUri, Optional<String> fieldSet, Optional<String> cluster) throws RestApiException {
         SyncSession syncSession = syncSessions.alloc();
-        setRoute(syncSession, Optional.empty());
+        // Explicit unary used instead of map() due to unhandled exceptions, blargh.
+        Optional<String> route = cluster.isPresent()
+                ? Optional.of(clusterDefToRoute(resolveClusterDef(cluster, clusterEnumerator.enumerateClusters())))
+                : Optional.empty();
+        setRoute(syncSession, route);
         try {
             DocumentId id = new DocumentId(restUri.generateFullId());
             final Document document = syncSession.get(id, fieldSet.orElse(restUri.getDocumentType() + ":[document]"), DocumentProtocol.Priority.NORMAL_1);
@@ -301,6 +305,11 @@ public class OperationHandlerImpl implements OperationHandler {
     }
 
     @Override
+    public Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
+        return get(restUri, fieldSet, Optional.empty());
+    }
+
+    @Override
     public Optional<String> get(RestUri restUri) throws RestApiException {
         return get(restUri, Optional.empty());
     }
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 d2fd8d92495..7e572dae941 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
@@ -262,8 +262,9 @@ public class RestApi extends LoggingRequestHandler {
     }
 
     private HttpResponse handleGet(RestUri restUri, HttpRequest request) throws RestApiException {
-        final Optional<String> fieldSet = requestProperty("fieldSet", request);
-        final Optional<String> getDocument = operationHandler.get(restUri, fieldSet);
+        final Optional<String> fieldSet = requestProperty(FIELD_SET, request);
+        final Optional<String> cluster  = requestProperty(CLUSTER, request);
+        final Optional<String> getDocument = operationHandler.get(restUri, fieldSet, cluster);
         final ObjectNode resultNode = mapper.createObjectNode();
         if (getDocument.isPresent()) {
             final JsonNode parseNode;
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 374c91c13d3..e98e6babf84 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
@@ -107,7 +107,7 @@ public class OperationHandlerImplTest {
         VisitorControlHandler.CompletionCode completionCode = VisitorControlHandler.CompletionCode.SUCCESS;
         int bucketsVisited = 0;
         Map<String, String> bucketSpaces = new HashMap<>();
-        SyncSession mockSyncSession = mock(MessageBusSyncSession.class); // MBus session needed to avoid setRoute throwing.
+        MessageBusSyncSession mockSyncSession = mock(MessageBusSyncSession.class); // MBus session needed to avoid setRoute throwing.
 
         OperationHandlerImplFixture() {
             bucketSpaces.put("foo", "global");
@@ -317,6 +317,25 @@ public class OperationHandlerImplTest {
     }
 
     @Test
+    public void get_route_has_default_value_if_no_cluster_is_provided() throws Exception {
+        OperationHandlerImplFixture fixture = new OperationHandlerImplFixture();
+        OperationHandlerImpl handler = fixture.createHandler();
+        handler.get(dummyGetUri(), Optional.empty(), Optional.empty());
+
+        // TODO shouldn't this be default-get?
+        verify(fixture.mockSyncSession).setRoute(eq("default"));
+    }
+
+    @Test
+    public void provided_get_cluster_is_propagated_as_route_to_sync_session() throws Exception {
+        OperationHandlerImplFixture fixture = new OperationHandlerImplFixture();
+        OperationHandlerImpl handler = fixture.createHandler();
+        handler.get(dummyGetUri(), Optional.empty(), Optional.of("foo"));
+
+        verify(fixture.mockSyncSession).setRoute(eq("[Storage:cluster=foo;clusterconfigid=configId]"));
+    }
+
+    @Test
     public void api_root_visit_uri_requires_cluster_set() throws Exception {
         OperationHandlerImplFixture fixture = new OperationHandlerImplFixture();
         OperationHandlerImpl handler = fixture.createHandler();
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 895c34436ce..0b3ee6d0792 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
@@ -58,11 +58,20 @@ public class MockedOperationHandler implements OperationHandler {
     }
 
     @Override
-    public Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
+    public Optional<String> get(RestUri restUri, Optional<String> fieldSet, Optional<String> cluster) throws RestApiException {
         log.append("GET: " + restUri.generateFullId());
         // This is _not_ an elegant way to return data back to the test.
         // An alternative is removing this entire class in favor of explicit mock expectations.
-        return fieldSet.map(fs -> String.format("{\"fields\": {\"fieldset\": \"%s\"}}", fs));
+        if (!fieldSet.isPresent() && !cluster.isPresent()) {
+            return Optional.empty();
+        }
+        return Optional.of(String.format("{\"fields\": {\"fieldset\": \"%s\",\"cluster\":\"%s\"}}",
+                                         fieldSet.orElse(""), cluster.orElse("")));
+    }
+
+    @Override
+    public Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
+        return get(restUri, fieldSet, Optional.empty());
     }
 
     @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 773d45b6b18..aeddc762586 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
@@ -288,12 +288,19 @@ public class RestApiTest {
     }
 
     @Test
-    public void get_fieldset_parameter_is_propagated() throws IOException {
+    public void get_fieldset_parameter_is_propagated() {
         Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/docid/bar?fieldSet=foo,baz", getFirstListenPort()));
         HttpGet get = new HttpGet(request.getUri());
         assertHttp200ResponseContains(doRest(get), "\"fieldset\":\"foo,baz\"");
     }
 
+    @Test
+    public void get_cluster_parameter_is_propagated() {
+        Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/docid/bar?cluster=my_cool_cluster", getFirstListenPort()));
+        HttpGet get = new HttpGet(request.getUri());
+        assertHttp200ResponseContains(doRest(get), "\"cluster\":\"my_cool_cluster\"");
+    }
+
     String visit_test_uri = "/document/v1/namespace/document-type/docid/?continuation=abc";
     String visit_response_part1 = "\"documents\":[List of json docs, cont token abc, doc selection: '']";
     String visit_response_part2 = "\"continuation\":\"token\"";