Revert "Geirst/add bucket space to document api"

5 files changed, 24 insertions, 142 deletions

diff --git a/vespaclient-container-plugin/pom.xml b/vespaclient-container-plugin/pom.xml
index 150a3241153..3472f3fcdc5 100644
--- a/vespaclient-container-plugin/pom.xml
+++ b/vespaclient-container-plugin/pom.xml
@@ -37,6 +37,7 @@
       <version>${project.version}</version>
       <scope>provided</scope>
     </dependency>
+
     <dependency>
       <groupId>org.apache.httpcomponents</groupId>
       <artifactId>httpclient</artifactId>
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/BucketSpaceEnumerator.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/BucketSpaceEnumerator.java
deleted file mode 100644
index 24692859266..00000000000
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/BucketSpaceEnumerator.java
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.document.restapi;
-
-import com.yahoo.config.subscription.ConfigGetter;
-import com.yahoo.vespa.config.content.core.BucketspacesConfig;
-
-import java.util.Collections;
-import java.util.Map;
-import java.util.stream.Collectors;
-
-/**
- * TODO description
- */
-class BucketSpaceEnumerator {
-
-    private final Map<String, String> doctypeToSpace;
-
-    private BucketSpaceEnumerator(String configId) {
-        doctypeToSpace = Collections.unmodifiableMap(buildMappingFromConfig(configId));
-    }
-
-    public static BucketSpaceEnumerator fromConfig(String configId) {
-        return new BucketSpaceEnumerator(configId);
-    }
-
-    public Map<String, String> getDoctypeToSpaceMapping() {
-        return doctypeToSpace;
-    }
-
-    private static Map<String, String> buildMappingFromConfig(String configId) {
-        BucketspacesConfig config = new ConfigGetter<>(BucketspacesConfig.class).getConfig(configId);
-        return config.documenttype().stream().collect(Collectors.toMap(dt -> dt.name(), dt -> dt.bucketspace()));
-    }
-
-}
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 8fe2007e88a..46678ea67e3 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
@@ -7,7 +7,6 @@ import com.yahoo.document.DocumentRemove;
 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;
@@ -49,34 +48,11 @@ public class OperationHandlerImpl implements OperationHandler {
         List<ClusterDef> enumerateClusters();
     }
 
-    public interface BucketSpaceResolver {
-        Optional<String> clusterBucketSpaceFromDocumentType(String clusterConfigId, String docType);
-    }
-
-    public static class BucketSpaceRoute {
-        private final String clusterRoute;
-        private final String bucketSpace;
-
-        public BucketSpaceRoute(String clusterRoute, String bucketSpace) {
-            this.clusterRoute = clusterRoute;
-            this.bucketSpace = bucketSpace;
-        }
-
-        public String getClusterRoute() {
-            return clusterRoute;
-        }
-
-        public String getBucketSpace() {
-            return bucketSpace;
-        }
-    }
-
     public static final int VISIT_TIMEOUT_MS = 120000;
     public static final int WANTED_DOCUMENT_COUNT_UPPER_BOUND = 1000; // Approximates the max default size of a bucket
     private final DocumentAccess documentAccess;
     private final DocumentApiMetrics metricsHelper;
     private final ClusterEnumerator clusterEnumerator;
-    private final BucketSpaceResolver bucketSpaceResolver;
 
     private static final class SyncSessionFactory extends ResourceFactory<SyncSession> {
         private final DocumentAccess documentAccess;
@@ -91,25 +67,13 @@ public class OperationHandlerImpl implements OperationHandler {
 
     private final ConcurrentResourcePool<SyncSession> syncSessions;
 
-    private static ClusterEnumerator defaultClusterEnumerator() {
-        return () -> new ClusterList("client").getStorageClusters();
-    }
-
-    private static BucketSpaceResolver defaultBucketResolver() {
-        return (clusterConfigId, docType) -> Optional.ofNullable(BucketSpaceEnumerator
-                .fromConfig(clusterConfigId).getDoctypeToSpaceMapping()
-                .get(docType));
-    }
-
     public OperationHandlerImpl(DocumentAccess documentAccess, MetricReceiver metricReceiver) {
-        this(documentAccess, defaultClusterEnumerator(), defaultBucketResolver(), metricReceiver);
+        this(documentAccess, () -> new ClusterList("client").getStorageClusters(), metricReceiver);
     }
 
-    public OperationHandlerImpl(DocumentAccess documentAccess, ClusterEnumerator clusterEnumerator,
-                                BucketSpaceResolver bucketSpaceResolver, MetricReceiver metricReceiver) {
+    public OperationHandlerImpl(DocumentAccess documentAccess, ClusterEnumerator clusterEnumerator, MetricReceiver metricReceiver) {
         this.documentAccess = documentAccess;
         this.clusterEnumerator = clusterEnumerator;
-        this.bucketSpaceResolver = bucketSpaceResolver;
         syncSessions = new ConcurrentResourcePool<>(new SyncSessionFactory(documentAccess));
         metricsHelper = new DocumentApiMetrics(metricReceiver, "documentV1");
     }
@@ -316,19 +280,13 @@ public class OperationHandlerImpl implements OperationHandler {
         }
     }
 
-    protected BucketSpaceRoute resolveBucketSpaceRoute(Optional<String> wantedCluster, String docType) throws RestApiException {
+    private String resolveClusterRoute(Optional<String> wantedCluster) 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)); // TODO own code
-        }
-        return new BucketSpaceRoute(clusterDefToRoute(clusterDef), targetBucketSpace.get());
+        return resolveClusterRoute(wantedCluster, clusters);
     }
 
-    protected static ClusterDef resolveClusterDef(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException {
+    // Based on resolveClusterRoute in VdsVisit, protected for testability
+    protected static String resolveClusterRoute(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException {
         if (clusters.size() == 0) {
             throw new IllegalArgumentException("Your Vespa cluster does not have any content clusters " +
                     "declared. Visiting feature is not available.");
@@ -338,25 +296,20 @@ public class OperationHandlerImpl implements OperationHandler {
                 throw new RestApiException(Response.createErrorResponse(400, "Several clusters exist: " +
                         clusterListToString(clusters) + " you must specify one. ", RestUri.apiErrorCodes.SEVERAL_CLUSTERS));
             }
-            return clusters.get(0);
+            return clusterDefToRoute(clusters.get(0));
         }
 
         for (ClusterDef clusterDef : clusters) {
             if (clusterDef.getName().equals(wantedCluster.get())) {
-                return clusterDef;
+                return clusterDefToRoute(clusterDef);
             }
         }
         throw new RestApiException(Response.createErrorResponse(400, "Your vespa cluster contains the content clusters " +
                 clusterListToString(clusters) + " not " + wantedCluster.get() + ". Please select a valid vespa cluster.", RestUri.apiErrorCodes.MISSING_CLUSTER));
-    }
 
-    // Based on resolveClusterRoute in VdsVisit, protected for testability
-    // TODO remove in favor of resolveClusterDef
-    protected static String resolveClusterRoute(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException {
-        return clusterDefToRoute(resolveClusterDef(wantedCluster, clusters));
     }
 
-    protected static String clusterDefToRoute(ClusterDef clusterDef) {
+    private static String clusterDefToRoute(ClusterDef clusterDef) {
         return "[Storage:cluster=" + clusterDef.getName() + ";clusterconfigid=" + clusterDef.getConfigId() + "]";
     }
 
@@ -400,9 +353,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());
-        params.setRoute(bucketSpaceRoute.getClusterRoute());
-        params.setBucketSpace(bucketSpaceRoute.getBucketSpace());
+        params.setRoute(resolveClusterRoute(options.cluster));
 
         params.setTraceLevel(0);
         params.setPriority(DocumentProtocol.Priority.NORMAL_4);
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 15d1b54adbe..4eaac48a511 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
@@ -34,7 +34,7 @@ public class RestUri {
         URL_PARSING(-6),
         INVALID_CREATE_VALUE(-7),
         TOO_MANY_PARALLEL_REQUESTS(-8),
-        MISSING_CLUSTER(-9), UNKNOWN_BUCKET_SPACE(-9), INTERNAL_EXCEPTION(-9),
+        MISSING_CLUSTER(-9), INTERNAL_EXCEPTION(-9),
         DOCUMENT_CONDITION_NOT_MET(-10),
         DOCUMENT_EXCPETION(-11),
         PARSER_ERROR(-11),
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 61bbe1e7030..5735e84f3fe 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
@@ -14,7 +14,10 @@ import org.junit.Test;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.net.URI;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 
 import static org.hamcrest.CoreMatchers.containsString;
@@ -30,31 +33,27 @@ public class OperationHandlerImplTest {
     @Test(expected = IllegalArgumentException.class)
     public void missingClusterDef() throws RestApiException {
         List<ClusterDef> clusterDef = new ArrayList<>();
-        OperationHandlerImpl.resolveClusterDef(Optional.empty(), clusterDef);
+        OperationHandlerImpl.resolveClusterRoute(Optional.empty(), clusterDef);
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void missingClusterDefSpecifiedCluster() throws RestApiException {
         List<ClusterDef> clusterDef = new ArrayList<>();
-        OperationHandlerImpl.resolveClusterDef(Optional.of("cluster"), clusterDef);
+        OperationHandlerImpl.resolveClusterRoute(Optional.of("cluster"), clusterDef);
     }
 
     @Test(expected = RestApiException.class)
     public void oneClusterPresentNotMatching() throws RestApiException {
         List<ClusterDef> clusterDef = new ArrayList<>();
         clusterDef.add(new ClusterDef("foo", "configId"));
-        OperationHandlerImpl.resolveClusterDef(Optional.of("cluster"), clusterDef);
-    }
-
-    private static String toRoute(ClusterDef clusterDef) {
-        return OperationHandlerImpl.clusterDefToRoute(clusterDef);
+        OperationHandlerImpl.resolveClusterRoute(Optional.of("cluster"), clusterDef);
     }
 
     @Test()
     public void oneClusterMatching() throws RestApiException {
         List<ClusterDef> clusterDef = new ArrayList<>();
         clusterDef.add(new ClusterDef("foo", "configId"));
-        assertThat(toRoute(OperationHandlerImpl.resolveClusterDef(Optional.of("foo"), clusterDef)),
+        assertThat(OperationHandlerImpl.resolveClusterRoute(Optional.of("foo"), clusterDef),
                 is("[Storage:cluster=foo;clusterconfigid=configId]"));
     }
 
@@ -64,18 +63,18 @@ public class OperationHandlerImplTest {
         clusterDef.add(new ClusterDef("foo2", "configId2"));
         clusterDef.add(new ClusterDef("foo", "configId"));
         clusterDef.add(new ClusterDef("foo3", "configId2"));
-        assertThat(toRoute(OperationHandlerImpl.resolveClusterDef(Optional.of("foo"), clusterDef)),
+        assertThat(OperationHandlerImpl.resolveClusterRoute(Optional.of("foo"), clusterDef),
                 is("[Storage:cluster=foo;clusterconfigid=configId]"));
     }
 
     @Test()
-    public void unknown_target_cluster_throws_exception() throws RestApiException, IOException {
+    public void checkErrorMessage() throws RestApiException, IOException {
         List<ClusterDef> clusterDef = new ArrayList<>();
         clusterDef.add(new ClusterDef("foo2", "configId2"));
         clusterDef.add(new ClusterDef("foo", "configId"));
         clusterDef.add(new ClusterDef("foo3", "configId2"));
         try {
-            OperationHandlerImpl.resolveClusterDef(Optional.of("wrong"), clusterDef);
+            OperationHandlerImpl.resolveClusterRoute(Optional.of("wrong"), clusterDef);
         } catch(RestApiException e) {
             String errorMsg = renderRestApiExceptionAsString(e);
             assertThat(errorMsg, is("{\"errors\":[{\"description\":" +
@@ -97,12 +96,6 @@ public class OperationHandlerImplTest {
         AtomicReference<VisitorParameters> assignedParameters = new AtomicReference<>();
         VisitorControlHandler.CompletionCode completionCode = VisitorControlHandler.CompletionCode.SUCCESS;
         int bucketsVisited = 0;
-        Map<String, String> bucketSpaces = new HashMap<>();
-
-        OperationHandlerImplFixture() {
-            bucketSpaces.put("foo", "global");
-            bucketSpaces.put("document-type", "default");
-        }
 
         OperationHandlerImpl createHandler() throws Exception {
             VisitorSession visitorSession = mock(VisitorSession.class);
@@ -122,8 +115,7 @@ public class OperationHandlerImplTest {
                 return visitorSession;
             });
             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);
+            return new OperationHandlerImpl(documentAccess, clusterEnumerator, MetricReceiver.nullImplementation);
         }
     }
 
@@ -183,33 +175,6 @@ public class OperationHandlerImplTest {
     }
 
     @Test
-    public void document_type_is_mapped_to_correct_bucket_space() throws Exception {
-        OperationHandlerImplFixture fixture = new OperationHandlerImplFixture();
-        fixture.bucketSpaces.put("document-type", "langbein");
-        OperationHandlerImpl handler = fixture.createHandler();
-        handler.visit(dummyVisitUri(), "", emptyVisitOptions());
-
-        VisitorParameters parameters = fixture.assignedParameters.get();
-        assertEquals("langbein", parameters.getBucketSpace());
-    }
-
-    @Test
-    public void unknown_bucket_space_mapping_throws_exception() throws Exception {
-        OperationHandlerImplFixture fixture = new OperationHandlerImplFixture();
-        fixture.bucketSpaces.remove("document-type");
-        try {
-            OperationHandlerImpl handler = fixture.createHandler();
-            handler.visit(dummyVisitUri(), "", emptyVisitOptions());
-        } catch (RestApiException e) {
-            assertThat(e.getResponse().getStatus(), is(400));
-            String errorMsg = renderRestApiExceptionAsString(e);
-            // FIXME isn't this really more of a case of unknown document type..?
-            assertThat(errorMsg, is("{\"errors\":[{\"description\":" +
-                    "\"UNKNOWN_BUCKET_SPACE Document type 'document-type' in cluster 'foo' is not mapped to a known bucket space\",\"id\":-9}]}"));
-        }
-    }
-
-    @Test
     public void provided_wanted_document_count_is_propagated_to_visitor_parameters() throws Exception {
         VisitorParameters params = generatedParametersFromVisitOptions(visitOptionsWithWantedDocumentCount(123));
         assertThat(params.getMaxTotalHits(), is((long)123));