Handle timeout units in doc/v1, and catch parse errors in handler

2 files changed, 26 insertions, 12 deletions

diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
index caf1403bdf8..f9bb94b3ccb 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/DocumentV1ApiHandler.java
@@ -100,6 +100,7 @@ import static com.yahoo.jdisc.http.HttpRequest.Method.PUT;
 import static java.util.Objects.requireNonNull;
 import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.WARNING;
+import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toMap;
 import static java.util.stream.Collectors.toUnmodifiableMap;
@@ -113,7 +114,11 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
 
     private static final Logger log = Logger.getLogger(DocumentV1ApiHandler.class.getName());
     private static final Parser<Integer> integerParser = Integer::parseInt;
-    private static final Parser<Double> doubleParser = Double::parseDouble;
+    private static final Parser<Double> timeoutParser = value -> {
+        int suffixLength = value.endsWith("ms") ? 2 : value.endsWith("s") ? 1 : 0;
+        double factor = suffixLength == 2 ? 1e-3 : 1;
+        return Double.parseDouble(value.substring(0, value.length() - suffixLength)) * factor;
+    };
     private static final Parser<Boolean> booleanParser = Boolean::parseBoolean;
 
     private static final CompletionHandler logException = new CompletionHandler() {
@@ -193,19 +198,19 @@ public class DocumentV1ApiHandler extends AbstractRequestHandler {
 
     @Override
     public ContentChannel handleRequest(Request rawRequest, ResponseHandler rawResponseHandler) {
-        HttpRequest request = (HttpRequest) rawRequest;
-        rawRequest.setTimeout(getProperty(request, TIMEOUT, doubleParser)
-                                      .map(timeoutSeconds -> (long) (timeoutSeconds * 1000))
-                                      .orElse(defaultTimeout.toMillis()),
-                              TimeUnit.MILLISECONDS);
-
-        HandlerMetricContextUtil.onHandle(request, metric, getClass());
+        HandlerMetricContextUtil.onHandle(rawRequest, metric, getClass());
         ResponseHandler responseHandler = response -> {
-            HandlerMetricContextUtil.onHandled(request, metric, getClass());
+            HandlerMetricContextUtil.onHandled(rawRequest, metric, getClass());
             return rawResponseHandler.handleResponse(response);
         };
 
+        HttpRequest request = (HttpRequest) rawRequest;
         try {
+            request.setTimeout(getProperty(request, TIMEOUT, timeoutParser)
+                                       .map(timeoutSeconds -> (long) (timeoutSeconds * 1000))
+                                       .orElse(defaultTimeout.toMillis()),
+                               TimeUnit.MILLISECONDS);
+
             Path requestPath = new Path(request.getUri());
             for (String path : handlers.keySet())
                 if (requestPath.matches(path)) {
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
index 723c84935f1..989f8c6a387 100644
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/DocumentV1ApiTest.java
@@ -279,7 +279,7 @@ public class DocumentV1ApiTest {
             parameters.responseHandler().get().handleResponse(new DocumentResponse(0, null));
             return new Result(Result.ResultType.SUCCESS, null);
         });
-        response = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?cluster=content&fieldSet=go");
+        response = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?cluster=content&fieldSet=go&timeout=123");
         assertSameJson("{" +
                        "  \"pathId\": \"/document/v1/space/music/docid/one\"," +
                        "  \"id\": \"id:space:music::one\"" +
@@ -368,7 +368,7 @@ public class DocumentV1ApiTest {
             parameters.responseHandler().get().handleResponse(new UpdateResponse(0, true));
             return new Result(Result.ResultType.SUCCESS, null);
         });
-        response = driver.sendRequest("http://localhost/document/v1/space/music/group/a/three?create=true", PUT,
+        response = driver.sendRequest("http://localhost/document/v1/space/music/group/a/three?create=true&timeout=1e1s", PUT,
                                       "{" +
                                       "  \"fields\": {" +
                                       "    \"artist\": { \"assign\": \"Lisa Ekdahl\" }" +
@@ -449,6 +449,15 @@ public class DocumentV1ApiTest {
                        "}", response.readAll());
         assertEquals(400, response.getStatus());
 
+        // GET with wrong timeout fails
+        access.session.expect((__, ___) -> { throw new AssertionError("Not supposed to happen"); });
+        response = driver.sendRequest("http://localhost/document/v1/space/music/number/1/two?timeout=30us");
+        assertSameJson("{" +
+                       "  \"pathId\": \"/document/v1/space/music/number/1/two\"," +
+                       "  \"message\": \"Failed parsing '30us': For input string: \\\"30u\\\"\"" +
+                       "}", response.readAll());
+        assertEquals(400, response.getStatus());
+
         // INSUFFICIENT_STORAGE is a 507
         access.session.expect((id, parameters) -> {
             parameters.responseHandler().get().handleResponse(new Response(0, "disk full", Response.Outcome.INSUFFICIENT_STORAGE));
@@ -526,7 +535,7 @@ public class DocumentV1ApiTest {
                 }
                 return new Result(Result.ResultType.SUCCESS, null);
             });
-            var response4 = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?cluster=content&fieldSet=go&timeout=0.001");
+            var response4 = driver.sendRequest("http://localhost/document/v1/space/music/docid/one?cluster=content&fieldSet=go&timeout=1ms");
             assertSameJson("{" +
                            "  \"pathId\": \"/document/v1/space/music/docid/one\"," +
                            "  \"message\": \"Request timeout after 1ms\"" +