timeout should propagate to OkHttp (#2014)

* test if timeout propagates to OkHttp * remove maxUnzippedLength * make test a bit shorter via ValueSource
graphhopper · Apr 23, 2020 · d20bd00 · d20bd00
1 parent 82b6f8b
commit d20bd00
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 50 deletions.
diff --git a/client-hc/src/main/java/com/graphhopper/api/GraphHopperWeb.java b/client-hc/src/main/java/com/graphhopper/api/GraphHopperWeb.java
@@ -223,7 +223,7 @@ public GHResponse route(GHRequest ghRequest) {
         }
     }
 
-    private OkHttpClient getClientForRequest(GHRequest request) {
+    OkHttpClient getClientForRequest(GHRequest request) {
         OkHttpClient client = this.downloader;
         if (request.getHints().has(TIMEOUT)) {
             long timeout = request.getHints().getLong(TIMEOUT, DEFAULT_TIMEOUT);

diff --git a/client-hc/src/test/java/com/graphhopper/api/GraphHopperWebTest.java b/client-hc/src/test/java/com/graphhopper/api/GraphHopperWebTest.java
@@ -0,0 +1,59 @@
+package com.graphhopper.api;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.graphhopper.GHRequest;
+import com.graphhopper.PathWrapper;
+import com.graphhopper.jackson.Jackson;
+import com.graphhopper.jackson.PathWrapperDeserializer;
+import com.graphhopper.util.shapes.GHPoint;
+import org.junit.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.io.IOException;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * This class unit tests the class. For integration tests against a real server see RouteResourceClientHCTest.
+ */
+public class GraphHopperWebTest {
+
+    @ParameterizedTest(name = "POST={0}")
+    @ValueSource(booleans = {true, false})
+    public void testGetClientForRequest(boolean usePost) {
+        GraphHopperWeb gh = new GraphHopperWeb(null).setPostRequest(usePost);
+        GHRequest req = new GHRequest().
+                addPoint(new GHPoint(42.509225, 1.534728)).
+                addPoint(new GHPoint(42.512602, 1.551558)).
+                putHint("vehicle", "car");
+        req.putHint(GraphHopperWeb.TIMEOUT, 5);
+
+        assertEquals(5, gh.getClientForRequest(req).connectTimeoutMillis());
+    }
+
+    @Test
+    public void testPutPOJO() {
+        ObjectNode requestJson = new ObjectMapper().createObjectNode();
+        requestJson.putPOJO("double", 1.0);
+        requestJson.putPOJO("int", 1);
+        requestJson.putPOJO("boolean", true);
+        // does not work requestJson.putPOJO("string", "test");
+        assertEquals("{\"double\":1.0,\"int\":1,\"boolean\":true}", requestJson.toString());
+    }
+
+    @Test
+    public void testUnknownInstructionSign() throws IOException {
+        // Modified the sign though
+        ObjectMapper objectMapper = Jackson.newObjectMapper();
+        JsonNode json = objectMapper.readTree("{\"instructions\":[{\"distance\":1.073,\"sign\":741,\"interval\":[0,1],\"text\":\"Continue onto A 81\",\"time\":32,\"street_name\":\"A 81\"},{\"distance\":0,\"sign\":4,\"interval\":[1,1],\"text\":\"Finish!\",\"time\":0,\"street_name\":\"\"}],\"descend\":0,\"ascend\":0,\"distance\":1.073,\"bbox\":[8.676286,48.354446,8.676297,48.354453],\"weight\":0.032179,\"time\":32,\"points_encoded\":true,\"points\":\"gfcfHwq}s@}c~AAA?\",\"snapped_waypoints\":\"gfcfHwq}s@}c~AAA?\"}");
+        PathWrapper wrapper = PathWrapperDeserializer.createPathWrapper(objectMapper, json, true, true);
+
+        assertEquals(741, wrapper.getInstructions().get(0).getSign());
+        assertEquals("Continue onto A 81", wrapper.getInstructions().get(0).getName());
+    }
+}
diff --git a/web/src/test/java/com/graphhopper/http/resources/RouteResourceClientHCTest.java b/web/src/test/java/com/graphhopper/http/resources/RouteResourceClientHCTest.java
@@ -17,9 +17,6 @@
  */
 package com.graphhopper.http.resources;
 
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.graphhopper.GHRequest;
 import com.graphhopper.GHResponse;
 import com.graphhopper.PathWrapper;
@@ -30,8 +27,6 @@
 import com.graphhopper.http.GraphHopperServerConfiguration;
 import com.graphhopper.http.util.GraphHopperServerTestConfiguration;
 import com.graphhopper.http.util.TestUtils;
-import com.graphhopper.jackson.Jackson;
-import com.graphhopper.jackson.PathWrapperDeserializer;
 import com.graphhopper.util.Helper;
 import com.graphhopper.util.Instruction;
 import com.graphhopper.util.InstructionList;
@@ -44,14 +39,11 @@
 import io.dropwizard.testing.junit5.DropwizardExtensionsSupport;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
 
 import java.io.File;
-import java.io.IOException;
-import java.net.SocketTimeoutException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -140,16 +132,6 @@ public void testSimpleRoute(TestParam p) {
         assertEquals("[0, 1]", alt.getPointsOrder().toString());
     }
 
-    @Test
-    public void testPutPOJO() {
-        ObjectNode requestJson = new ObjectMapper().createObjectNode();
-        requestJson.putPOJO("double", 1.0);
-        requestJson.putPOJO("int", 1);
-        requestJson.putPOJO("boolean", true);
-        // does not work requestJson.putPOJO("string", "test");
-        assertEquals("{\"double\":1.0,\"int\":1,\"boolean\":true}", requestJson.toString());
-    }
-
     @ParameterizedTest
     @EnumSource(value = TestParam.class)
     public void testAlternativeRoute(TestParam p) {
@@ -178,26 +160,6 @@ public void testAlternativeRoute(TestParam p) {
         assertTrue("Carrer Doctor Vilanova".contains(path.getDescription().get(0)), "expected: " + path.getDescription().get(0));
     }
 
-    @ParameterizedTest
-    @EnumSource(value = TestParam.class)
-    public void testTimeout(TestParam p) {
-        GraphHopperWeb gh = createGH(p);
-        GHRequest req = new GHRequest().
-                addPoint(new GHPoint(42.509225, 1.534728)).
-                addPoint(new GHPoint(42.512602, 1.551558)).
-                putHint("vehicle", "car");
-        GHResponse res = gh.route(req);
-        assertFalse(res.hasErrors(), "errors:" + res.getErrors().toString());
-
-        req.putHint(GraphHopperWeb.TIMEOUT, 1);
-        try {
-            gh.route(req);
-            fail();
-        } catch (RuntimeException e) {
-            assertEquals(SocketTimeoutException.class, e.getCause().getClass());
-        }
-    }
-
     @ParameterizedTest
     @EnumSource(value = TestParam.class)
     public void testNoPoints(TestParam p) {
@@ -334,17 +296,6 @@ void isBetween(double from, double to, double expected) {
         assertTrue(expected <= to, "expected value " + expected + " was bigger than limit " + to);
     }
 
-    @Test
-    public void testUnknownInstructionSign() throws IOException {
-        // Modified the sign though
-        ObjectMapper objectMapper = Jackson.newObjectMapper();
-        JsonNode json = objectMapper.readTree("{\"instructions\":[{\"distance\":1.073,\"sign\":741,\"interval\":[0,1],\"text\":\"Continue onto A 81\",\"time\":32,\"street_name\":\"A 81\"},{\"distance\":0,\"sign\":4,\"interval\":[1,1],\"text\":\"Finish!\",\"time\":0,\"street_name\":\"\"}],\"descend\":0,\"ascend\":0,\"distance\":1.073,\"bbox\":[8.676286,48.354446,8.676297,48.354453],\"weight\":0.032179,\"time\":32,\"points_encoded\":true,\"points\":\"gfcfHwq}s@}c~AAA?\",\"snapped_waypoints\":\"gfcfHwq}s@}c~AAA?\"}");
-        PathWrapper wrapper = PathWrapperDeserializer.createPathWrapper(objectMapper, json, true, true);
-
-        assertEquals(741, wrapper.getInstructions().get(0).getSign());
-        assertEquals("Continue onto A 81", wrapper.getInstructions().get(0).getName());
-    }
-
     @ParameterizedTest
     @EnumSource(value = TestParam.class)
     public void testPathDetails(TestParam p) {