Minor cleanup

2 files changed, 49 insertions, 47 deletions

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 6fed0dff36d..b4992fb519b 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
@@ -80,16 +80,14 @@ public class RestApi extends LoggingRequestHandler {
     public RestApi(LoggingRequestHandler.Context parentCtx, DocumentmanagerConfig documentManagerConfig,
                    LoadTypeConfig loadTypeConfig, ThreadpoolConfig threadpoolConfig,
                    AllClustersBucketSpacesConfig bucketSpacesConfig,
-                   ClusterListConfig clusterListConfig, MetricReceiver metricReceiver)
-    {
+                   ClusterListConfig clusterListConfig, MetricReceiver metricReceiver) {
         super(parentCtx);
         MessageBusParams params = new MessageBusParams(new LoadTypeSet(loadTypeConfig));
         params.setDocumentmanagerConfig(documentManagerConfig);
-        this.operationHandler = new OperationHandlerImpl(
-                new MessageBusDocumentAccess(params),
-                fixedClusterEnumeratorFromConfig(clusterListConfig),
-                fixedBucketSpaceResolverFromConfig(bucketSpacesConfig),
-                metricReceiver);
+        this.operationHandler = new OperationHandlerImpl(new MessageBusDocumentAccess(params),
+                                                        fixedClusterEnumeratorFromConfig(clusterListConfig),
+                                                        fixedBucketSpaceResolverFromConfig(bucketSpacesConfig),
+                                                        metricReceiver);
         this.singleDocumentParser = new SingleDocumentParser(new DocumentTypeManager(documentManagerConfig));
         // 40% of the threads can be blocked before we deny requests.
         if (threadpoolConfig != null) {
@@ -101,11 +99,10 @@ public class RestApi extends LoggingRequestHandler {
     }
 
     // For testing and development
-    public RestApi(
-            Executor executor,
-            AccessLog accessLog,
-            OperationHandler operationHandler,
-            int threadsAvailable) {
+    public RestApi(Executor executor,
+                   AccessLog accessLog,
+                   OperationHandler operationHandler,
+                   int threadsAvailable) {
         super(executor, accessLog, null);
         this.operationHandler = operationHandler;
         this.threadsAvailableForApi = new AtomicInteger(threadsAvailable);
@@ -122,12 +119,11 @@ public class RestApi extends LoggingRequestHandler {
     }
 
     private static OperationHandlerImpl.ClusterEnumerator fixedClusterEnumeratorFromConfig(ClusterListConfig config) {
-        final List<ClusterDef> clusters = Collections.unmodifiableList(new ClusterList(config).getStorageClusters());
+        List<ClusterDef> clusters = Collections.unmodifiableList(new ClusterList(config).getStorageClusters());
         return () -> clusters;
     }
 
-    private static OperationHandlerImpl.BucketSpaceResolver fixedBucketSpaceResolverFromConfig(
-            AllClustersBucketSpacesConfig bucketSpacesConfig) {
+    private static OperationHandlerImpl.BucketSpaceResolver fixedBucketSpaceResolverFromConfig(AllClustersBucketSpacesConfig bucketSpacesConfig) {
         return (clusterId, docType) ->
             Optional.ofNullable(bucketSpacesConfig.cluster(clusterId))
                     .map(cluster -> cluster.documentType(docType))
@@ -135,7 +131,7 @@ public class RestApi extends LoggingRequestHandler {
     }
 
     private static Optional<String> requestProperty(String parameter, HttpRequest request) {
-        final String property = request.getProperty(parameter);
+        String property = request.getProperty(parameter);
         if (property != null && ! property.isEmpty()) {
             return Optional.of(property);
         }
@@ -152,8 +148,15 @@ public class RestApi extends LoggingRequestHandler {
     }
 
     private static Optional<Boolean> parseBoolean(String parameter, HttpRequest request) {
-        Optional<String> property = requestProperty(parameter, request);
-        return property.map(RestApi::parseBooleanStrict);
+        try {
+            Optional<String> property = requestProperty(parameter, request);
+            return property.map(RestApi::parseBooleanStrict);
+        }
+        catch (IllegalArgumentException e) {
+            throw new IllegalArgumentException("Invalid value for '" + parameter + "' parameter: " +
+                                               "Must be empty, true, or false but was '" +
+                                               request.getProperty(parameter) + "'");
+        }
     }
 
     private static int parsePositiveInt(String str) throws NumberFormatException {
@@ -168,9 +171,9 @@ public class RestApi extends LoggingRequestHandler {
     public HttpResponse handle(HttpRequest request) {
         try {
             if (threadsAvailableForApi.decrementAndGet() < 1) {
-                return Response.createErrorResponse(
-                        429 /* Too Many Requests */,
-                        "Too many parallel requests, consider using http-vespa-java-client. Please try again later.", RestUri.apiErrorCodes.TOO_MANY_PARALLEL_REQUESTS);
+                return Response.createErrorResponse(429 /* Too Many Requests */,
+                                                    "Too many parallel requests, consider using http-vespa-java-client. Please try again later.",
+                                                    RestUri.apiErrorCodes.TOO_MANY_PARALLEL_REQUESTS);
             }
             return handleInternal(request);
         } finally {
@@ -178,11 +181,12 @@ public class RestApi extends LoggingRequestHandler {
         }
     }
 
-    private static void validateUriStructureForRequestMethod(RestUri uri, com.yahoo.jdisc.http.HttpRequest.Method method) throws RestApiException {
+    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));
+                                       "Root /document/v1/ requests only supported for HTTP GET",
+                                       RestUri.apiErrorCodes.ERROR_ID_BASIC_USAGE));
         }
     }
 
@@ -192,28 +196,23 @@ public class RestApi extends LoggingRequestHandler {
 
     // protected for testing
     protected HttpResponse handleInternal(HttpRequest request) {
-        final RestUri restUri;
+        RestUri restUri = null;
         try {
             restUri = new RestUri(request.getUri());
             validateUriStructureForRequestMethod(restUri, request.getMethod());
-        } catch (RestApiException e) {
-            return e.getResponse();
-        } catch (Exception e2) {
-            return Response.createErrorResponse(500, "Exception while parsing URI: " + e2.getMessage(), RestUri.apiErrorCodes.URL_PARSING);
-        }
 
-        final Optional<Boolean> create;
-        try {
-            create = parseBoolean(CREATE_PARAMETER_NAME, request);
-        } catch (IllegalArgumentException e) {
-            return Response.createErrorResponse(400, "Non valid value for 'create' parameter, must be empty, true, or " +
-                    "false: " + request.getProperty(CREATE_PARAMETER_NAME), RestUri.apiErrorCodes.INVALID_CREATE_VALUE);
-        }
-        String condition = request.getProperty(CONDITION_PARAMETER_NAME);
-        Optional<String> route = Optional.ofNullable(request.getProperty(ROUTE_PARAMETER_NAME));
+            Optional<Boolean> create;
+            try {
+                create = parseBoolean(CREATE_PARAMETER_NAME, request);
+            }
+            catch (IllegalArgumentException e) {
+                return Response.createErrorResponse(400, e.getMessage(), RestUri.apiErrorCodes.INVALID_CREATE_VALUE);
+            }
 
-        Optional<ObjectNode> resultJson = Optional.empty();
-        try {
+            String condition = request.getProperty(CONDITION_PARAMETER_NAME);
+            Optional<String> route = Optional.ofNullable(request.getProperty(ROUTE_PARAMETER_NAME));
+
+            Optional<ObjectNode> resultJson = Optional.empty();
             switch (request.getMethod()) {
                 case GET:    // Vespa Visit/Get
                     return isVisitRequestUri(restUri) ? handleVisit(restUri, request) : handleGet(restUri, request);
@@ -229,18 +228,21 @@ public class RestApi extends LoggingRequestHandler {
                 default:
                     return new Response(405, Optional.empty(), Optional.of(restUri));
             }
-        } catch (RestApiException e) {
+            return new Response(200, resultJson, Optional.of(restUri));
+        }
+        catch (RestApiException e) {
             return e.getResponse();
-        } catch (IllegalArgumentException userException) {
+        }
+        catch (IllegalArgumentException userException) {
             return Response.createErrorResponse(400, Exceptions.toMessageString(userException),
                                                 restUri,
                                                 RestUri.apiErrorCodes.PARSER_ERROR);
-        } catch (RuntimeException systemException) {
+        }
+        catch (RuntimeException systemException) {
             return Response.createErrorResponse(500, Exceptions.toMessageString(systemException),
                                                 restUri,
-                                                RestUri.apiErrorCodes.PARSER_ERROR);
+                                                RestUri.apiErrorCodes.UNSPECIFIED);
         }
-        return new Response(200, resultJson, Optional.of(restUri));
     }
 
     private FeedOperation createPutOperation(HttpRequest request, String id, String condition) {
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 94bc8deb47a..773d45b6b18 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
@@ -197,7 +197,7 @@ public class RestApiTest {
         HttpPut httpPut = new HttpPut(request.getUri());
         StringEntity entity = new StringEntity(update_test_doc, ContentType.create("application/json"));
         httpPut.setEntity(entity);
-        assertHttp400ResponseContains(doRest(httpPut), "Non valid value for 'create' parameter, must be empty, true, or false: batman");
+        assertHttp400ResponseContains(doRest(httpPut), "Invalid value for 'create' parameter: Must be empty, true, or false but was 'batman'");
     }
 
     // Get logs through some hackish fetch method. Logs is something the mocked backend write.