Merge pull request #4979 from vespa-engine/vekterli/support-fieldset-for-v1-api-gets

Also support fieldSet for Document V1 get operations

6 files changed, 68 insertions, 15 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 6a811eb5afd..35999df5a6b 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
@@ -99,6 +99,10 @@ public interface OperationHandler {
     void delete(RestUri restUri, String condition, Optional<String> route) throws RestApiException;
 
     Optional<String> get(RestUri restUri) throws RestApiException;
+
+    default Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
+        return get(restUri);
+    }
     
     /** 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 af8650a8e7c..cb4d1b9feed 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
@@ -295,12 +295,12 @@ public class OperationHandlerImpl implements OperationHandler {
     }
 
     @Override
-    public Optional<String> get(RestUri restUri) throws RestApiException {
+    public Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
         SyncSession syncSession = syncSessions.alloc();
         setRoute(syncSession, Optional.empty());
         try {
             DocumentId id = new DocumentId(restUri.generateFullId());
-            final Document document = syncSession.get(id, restUri.getDocumentType() + ":[document]", DocumentProtocol.Priority.NORMAL_1);
+            final Document document = syncSession.get(id, fieldSet.orElse(restUri.getDocumentType() + ":[document]"), DocumentProtocol.Priority.NORMAL_1);
             if (document == null) {
                 return Optional.empty();
             }
@@ -316,6 +316,11 @@ public class OperationHandlerImpl implements OperationHandler {
         }
     }
 
+    @Override
+    public Optional<String> get(RestUri restUri) throws RestApiException {
+        return get(restUri, Optional.empty());
+    }
+
     protected BucketSpaceRoute resolveBucketSpaceRoute(Optional<String> wantedCluster, String docType) throws RestApiException {
         final List<ClusterDef> clusters = clusterEnumerator.enumerateClusters();
         ClusterDef clusterDef = resolveClusterDef(wantedCluster, clusters);
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 a45557859ad..bb9387dc6e0 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
@@ -170,7 +170,7 @@ public class RestApi extends LoggingRequestHandler {
         try {
             switch (request.getMethod()) {
                 case GET:    // Vespa Visit/Get
-                    return restUri.getDocId().isEmpty() ? handleVisit(restUri, request) : handleGet(restUri);
+                    return restUri.getDocId().isEmpty() ? handleVisit(restUri, request) : handleGet(restUri, request);
                 case POST:   // Vespa Put
                     operationHandler.put(restUri, createPutOperation(request, restUri.generateFullId(), condition), route);
                     break;
@@ -212,8 +212,9 @@ public class RestApi extends LoggingRequestHandler {
         return operationUpdate;
     }
 
-    private HttpResponse handleGet(RestUri restUri) throws RestApiException {
-        final Optional<String> getDocument = operationHandler.get(restUri);
+    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 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 06054bd2dbb..5021899e30b 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
@@ -6,6 +6,9 @@ import com.yahoo.documentapi.ProgressToken;
 import com.yahoo.documentapi.VisitorControlHandler;
 import com.yahoo.documentapi.VisitorParameters;
 import com.yahoo.documentapi.VisitorSession;
+import com.yahoo.documentapi.SyncParameters;
+import com.yahoo.documentapi.SyncSession;
+import com.yahoo.documentapi.messagebus.MessageBusSyncSession;
 import com.yahoo.messagebus.StaticThrottlePolicy;
 import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.vdslib.VisitorStatistics;
@@ -24,7 +27,9 @@ import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsEqual.equalTo;
 import static org.junit.Assert.*;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 
@@ -101,6 +106,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.
 
         OperationHandlerImplFixture() {
             bucketSpaces.put("foo", "global");
@@ -124,6 +130,7 @@ public class OperationHandlerImplTest {
                 params.getControlHandler().onDone(completionCode, "bork bork");
                 return visitorSession;
             });
+            when(documentAccess.createSyncSession(any(SyncParameters.class))).thenReturn(mockSyncSession);
             OperationHandlerImpl.ClusterEnumerator clusterEnumerator = () -> Arrays.asList(new ClusterDef("foo", "configId"));
             OperationHandlerImpl.BucketSpaceResolver bucketSpaceResolver = (configId, docType) -> Optional.ofNullable(bucketSpaces.get(docType));
             return new OperationHandlerImpl(documentAccess, clusterEnumerator, bucketSpaceResolver, MetricReceiver.nullImplementation);
@@ -138,6 +145,10 @@ public class OperationHandlerImplTest {
         return new RestUri(new URI("http://localhost/document/v1/namespace/document-type/docid/"));
     }
 
+    private static RestUri dummyGetUri() throws Exception {
+        return new RestUri(new URI("http://localhost/document/v1/namespace/document-type/docid/foo"));
+    }
+
     private static OperationHandler.VisitOptions visitOptionsWithWantedDocumentCount(int wantedDocumentCount) {
         return optionsBuilder().wantedDocumentCount(wantedDocumentCount).build();
     }
@@ -250,29 +261,47 @@ public class OperationHandlerImplTest {
     }
 
     @Test
-    public void field_set_covers_all_fields_by_default() throws Exception {
+    public void visit_field_set_covers_all_fields_by_default() throws Exception {
         VisitorParameters params = generatedParametersFromVisitOptions(emptyVisitOptions());
         assertThat(params.fieldSet(), equalTo("document-type:[document]"));
     }
 
     @Test
-    public void provided_fieldset_is_propagated_to_visitor_parameters() throws Exception {
+    public void provided_visit_fieldset_is_propagated_to_visitor_parameters() throws Exception {
         VisitorParameters params = generatedParametersFromVisitOptions(optionsBuilder().fieldSet("document-type:bjarne").build());
         assertThat(params.fieldSet(), equalTo("document-type:bjarne"));
     }
 
     @Test
-    public void concurrency_is_1_by_default() throws Exception {
+    public void visit_concurrency_is_1_by_default() throws Exception {
         VisitorParameters params = generatedParametersFromVisitOptions(emptyVisitOptions());
         assertThat(params.getThrottlePolicy(), instanceOf(StaticThrottlePolicy.class));
         assertThat(((StaticThrottlePolicy)params.getThrottlePolicy()).getMaxPendingCount(), is((int)1));
     }
 
     @Test
-    public void concurrency_is_propagated_to_visitor_parameters() throws Exception {
+    public void visit_concurrency_is_propagated_to_visitor_parameters() throws Exception {
         VisitorParameters params = generatedParametersFromVisitOptions(optionsBuilder().concurrency(3).build());
         assertThat(params.getThrottlePolicy(), instanceOf(StaticThrottlePolicy.class));
         assertThat(((StaticThrottlePolicy)params.getThrottlePolicy()).getMaxPendingCount(), is((int)3));
     }
 
+    @Test
+    public void get_field_covers_all_fields_by_default() throws Exception {
+        OperationHandlerImplFixture fixture = new OperationHandlerImplFixture();
+        OperationHandlerImpl handler = fixture.createHandler();
+        handler.get(dummyGetUri(), Optional.empty());
+
+        verify(fixture.mockSyncSession).get(any(), eq("document-type:[document]"), any());
+    }
+
+    @Test
+    public void provided_get_fieldset_is_propagated_to_sync_session() throws Exception {
+        OperationHandlerImplFixture fixture = new OperationHandlerImplFixture();
+        OperationHandlerImpl handler = fixture.createHandler();
+        handler.get(dummyGetUri(), Optional.of("donald,duck"));
+
+        verify(fixture.mockSyncSession).get(any(), eq("donald,duck"), any());
+    }
+
 }
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 3b16d9a378c..297576f1bef 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
@@ -55,9 +55,16 @@ public class MockedOperationHandler implements OperationHandler {
     }
 
     @Override
-    public Optional<String> get(RestUri restUri) throws RestApiException {
+    public Optional<String> get(RestUri restUri, Optional<String> fieldSet) throws RestApiException {
         log.append("GET: " + restUri.generateFullId());
-        return Optional.empty();
+        // 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));
+    }
+
+    @Override
+    public Optional<String> get(RestUri restUri) throws RestApiException {
+        return get(restUri, Optional.empty());
     }
 
 }
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 faee5947794..eb06af41348 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
@@ -254,12 +254,19 @@ public class RestApiTest {
         assertThat(rest, containsString(get_enc_response_part2));
     }
 
+    @Test
+    public void get_fieldset_parameter_is_propagated() throws IOException {
+        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());
+        String rest = doRest(get);
+        assertThat(rest, containsString("\"fieldset\":\"foo,baz\""));
+    }
+
     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\"";
     String visit_response_part3 = "\"pathId\":\"/document/v1/namespace/document-type/docid/\"";
 
-
     @Test
     public void testbasicVisit() throws Exception {
         Request request = new Request("http://localhost:" + getFirstListenPort() + visit_test_uri);
@@ -320,7 +327,7 @@ public class RestApiTest {
     }
 
     @Test
-    public void fieldset_parameter_is_propagated() throws IOException {
+    public void visit_fieldset_parameter_is_propagated() throws IOException {
         Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/docid/?fieldSet=foo,baz", getFirstListenPort()));
         HttpGet get = new HttpGet(request.getUri());
         String rest = doRest(get);
@@ -328,7 +335,7 @@ public class RestApiTest {
     }
 
     @Test
-    public void concurrency_parameter_is_propagated() throws IOException {
+    public void visit_concurrency_parameter_is_propagated() throws IOException {
         Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/docid/?concurrency=42", getFirstListenPort()));
         HttpGet get = new HttpGet(request.getUri());
         String rest = doRest(get);
@@ -336,7 +343,7 @@ public class RestApiTest {
     }
 
     @Test
-    public void invalid_concurrency_parameter_returns_error_response() throws IOException {
+    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());
         String rest = doRest(get);