Move timeout query param parsing out of JsonReader

3 files changed, 33 insertions, 44 deletions

diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java
index 3c62d87098b..d871a8ed6bc 100644
--- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java
+++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReader.java
@@ -8,17 +8,10 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState;
 import org.codehaus.jettison.json.JSONArray;
 import org.codehaus.jettison.json.JSONObject;
 
-import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Optional;
-import java.util.concurrent.TimeUnit;
 
 public class JsonReader {
-    public static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(30);
-    public static final Duration MAX_TIMEOUT = Duration.ofHours(1);
-    private static final long MICROS_IN_SECOND = TimeUnit.SECONDS.toMillis(1);
-
     private static class UnitStateImpl implements UnitState {
         private final String id;
         private final String reason;
@@ -43,16 +36,13 @@ public class JsonReader {
         final Map<String, UnitState> stateMap;
         final SetUnitStateRequest.Condition condition;
         final SetUnitStateRequest.ResponseWait responseWait;
-        final Optional<Duration> timeout;
 
         public SetRequestData(Map<String, UnitState> stateMap,
                               SetUnitStateRequest.Condition condition,
-                              SetUnitStateRequest.ResponseWait responseWait,
-                              Optional<Duration> timeout) {
+                              SetUnitStateRequest.ResponseWait responseWait) {
             this.stateMap = stateMap;
             this.condition = condition;
             this.responseWait = responseWait;
-            this.timeout = timeout;
         }
     }
 
@@ -110,32 +100,6 @@ public class JsonReader {
             stateMap.put(type, new UnitStateImpl(code, reason));
         }
 
-        final Optional<Duration> timeout = parseTimeout(request.getOption("timeout", null));
-
-        return new SetRequestData(stateMap, condition, responseWait, timeout);
-    }
-
-    public static Optional<Duration> parseTimeout(String timeoutOption) throws InvalidContentException {
-        if (timeoutOption == null) {
-            return Optional.empty();
-        } else {
-            float timeoutSeconds;
-            try {
-                timeoutSeconds = Float.parseFloat(timeoutOption);
-            } catch (NumberFormatException e) {
-                throw new InvalidContentException("value of timeout->" + timeoutOption + " is not a float");
-            }
-
-            if (timeoutSeconds <= 0.0) {
-                return Optional.of(Duration.ZERO);
-            } else if (timeoutSeconds <= MAX_TIMEOUT.getSeconds()) {
-                long micros = Math.round(timeoutSeconds * MICROS_IN_SECOND);
-                long nanoAdjustment = TimeUnit.MILLISECONDS.toNanos(micros % MICROS_IN_SECOND);
-                return Optional.of(Duration.ofSeconds(micros / MICROS_IN_SECOND, nanoAdjustment));
-            } else {
-                throw new InvalidContentException("value of timeout->" + timeoutOption + " exceeds max timeout " + MAX_TIMEOUT);
-            }
-        }
+        return new SetRequestData(stateMap, condition, responseWait);
     }
-
 }
diff --git a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java
index 2ccfecf3d17..d3cab74c66f 100644
--- a/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java
+++ b/clustercontroller-utils/src/main/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandler.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.clustercontroller.utils.communication.http.HttpResult;
 import com.yahoo.vespa.clustercontroller.utils.communication.http.JsonHttpResult;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.StateRestAPI;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.DeadlineExceededException;
+import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InvalidContentException;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.InvalidOptionValueException;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.OtherMasterException;
 import com.yahoo.vespa.clustercontroller.utils.staterestapi.errors.StateRestApiException;
@@ -22,15 +23,18 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.response.UnitState;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Clock;
+import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 public class RestApiHandler implements HttpRequestHandler {
+    public static final Duration MAX_TIMEOUT = Duration.ofHours(1);
 
     private final static Logger log = Logger.getLogger(RestApiHandler.class.getName());
 
@@ -77,6 +81,7 @@ public class RestApiHandler implements HttpRequestHandler {
                 return new JsonHttpResult().setJson(jsonWriter.createJson(data));
             } else {
                 final JsonReader.SetRequestData setRequestData = jsonReader.getStateRequestData(request);
+                final Optional<Duration> timeout = parseTimeout(request.getOption("timeout", null));
                 SetResponse setResponse = restApi.setUnitState(new SetUnitStateRequest() {
                     @Override
                     public Map<String, UnitState> getNewState() {
@@ -91,7 +96,7 @@ public class RestApiHandler implements HttpRequestHandler {
                     @Override
                     public ResponseWait getResponseWait() { return setRequestData.responseWait; }
                     @Override
-                    public TimeBudget timeBudget() { return TimeBudget.from(clock, start, setRequestData.timeout); }
+                    public TimeBudget timeBudget() { return TimeBudget.from(clock, start, timeout); }
                 });
                 return new JsonHttpResult().setJson(jsonWriter.createJson(setResponse));
             }
@@ -191,4 +196,24 @@ public class RestApiHandler implements HttpRequestHandler {
         return value;
     }
 
+    static Optional<Duration> parseTimeout(String timeoutOption) throws InvalidContentException {
+        if (timeoutOption == null) {
+            return Optional.empty();
+        }
+
+        float timeoutSeconds;
+        try {
+            timeoutSeconds = Float.parseFloat(timeoutOption);
+        } catch (NumberFormatException e) {
+            throw new InvalidContentException("value of timeout->" + timeoutOption + " is not a float");
+        }
+
+        if (timeoutSeconds <= 0.0) {
+            return Optional.of(Duration.ZERO);
+        } else if (timeoutSeconds <= MAX_TIMEOUT.getSeconds()) {
+            return Optional.of(Duration.ofMillis(Math.round(timeoutSeconds * 1000)));
+        } else {
+            throw new InvalidContentException("value of timeout->" + timeoutOption + " exceeds max timeout " + MAX_TIMEOUT);
+        }
+    }
 }
diff --git a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReaderTest.java b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandlerTest.java
index bf47a6605a2..f7f109fffc8 100644
--- a/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/JsonReaderTest.java
+++ b/clustercontroller-utils/src/test/java/com/yahoo/vespa/clustercontroller/utils/staterestapi/server/RestApiHandlerTest.java
@@ -9,12 +9,12 @@ import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 
-public class JsonReaderTest {
+public class RestApiHandlerTest {
     @Test
     public void testParsingOfTimeout() throws InvalidContentException {
-        assertEquals(Optional.empty(), JsonReader.parseTimeout(null));
-        assertEquals(Optional.of(Duration.ofMillis(12500)), JsonReader.parseTimeout("12.5"));
-        assertEquals(Optional.of(Duration.ofMillis(0)), JsonReader.parseTimeout("-1"));
-        assertEquals(Optional.of(Duration.ofMillis(0)), JsonReader.parseTimeout("0.0001"));
+        assertEquals(Optional.empty(), RestApiHandler.parseTimeout(null));
+        assertEquals(Optional.of(Duration.ofMillis(12500)), RestApiHandler.parseTimeout("12.5"));
+        assertEquals(Optional.of(Duration.ofMillis(0)), RestApiHandler.parseTimeout("-1"));
+        assertEquals(Optional.of(Duration.ofMillis(0)), RestApiHandler.parseTimeout("0.0001"));
     }
 }
\ No newline at end of file