fix empty servicePath in discovery

tangiel · tangiel · commit d58ca550ccfc · 2017-03-10T13:17:31.000-08:00
The discovery test was not catching this issue in part because it wasn't
using the exact same JSON serialization as it does at runtime. That has
also been fixed. This change will likely reintroduce empty maps in
serialization, but this is currently the best workaround to allow empty
strings inside maps, which we need to serialize the discovery document
correctly.
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/ObjectMapperUtil.java b/endpoints-framework/src/main/java/com/google/api/server/spi/ObjectMapperUtil.java
@@ -18,7 +18,6 @@
 import com.google.api.server.spi.config.annotationreader.ApiAnnotationIntrospector;
 import com.google.api.server.spi.config.model.ApiSerializationConfig;
 
-import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.AnnotationIntrospector;
@@ -107,10 +106,9 @@ public JsonSerializer<?> modifyCollectionSerializer(SerializationConfig config,
     public JsonSerializer<?> modifyMapSerializer(SerializationConfig config, MapType valueType,
         BeanDescription beanDesc, JsonSerializer<?> serializer) {
       if (serializer instanceof MapSerializer) {
-        // For some reason, NON_EMPTY is not being propagated to MapSerializer, so we replace it
-        // with one that has it set.
-        return new DeepEmptyCheckingSerializer<>(
-            ((MapSerializer) serializer).withContentInclusion(JsonInclude.Include.NON_EMPTY));
+        // TODO: We should probably be propagating the NON_EMPTY inclusion here, but it's breaking
+        // discovery.
+        return new DeepEmptyCheckingSerializer<>(serializer);
       }
       return serializer;
     }
diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/discovery/DiscoveryGeneratorTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/discovery/DiscoveryGeneratorTest.java
@@ -19,6 +19,7 @@
 
 import com.google.api.client.json.jackson2.JacksonFactory;
 import com.google.api.server.spi.IoUtil;
+import com.google.api.server.spi.ObjectMapperUtil;
 import com.google.api.server.spi.ServiceContext;
 import com.google.api.server.spi.TypeLoader;
 import com.google.api.server.spi.config.ApiConfigLoader;
@@ -47,16 +48,14 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-import io.swagger.util.Json;
-
 /**
  * Tests for {@link DiscoveryGenerator}.
  */
 @RunWith(JUnit4.class)
 public class DiscoveryGeneratorTest {
   private final DiscoveryContext context = new DiscoveryContext()
       .setApiRoot("https://discovery-test.appspot.com/api");
-  private final ObjectMapper mapper = Json.mapper();
+  private final ObjectMapper mapper = ObjectMapperUtil.createStandardObjectMapper();
   private DiscoveryGenerator generator;
   private ApiConfigLoader configLoader;
   private SchemaRepository schemaRepository;
diff --git a/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/absolute_path_endpoint.json b/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/absolute_path_endpoint.json
@@ -23,7 +23,15 @@
     "absolutepath": {
       "httpMethod": "POST",
       "id": "absolutepath.absolutepath",
-      "path": "absolutepathmethod",
+      "path": "absolutepathmethod/v1",
+      "scopes": [
+        "https://www.googleapis.com/auth/userinfo.email"
+      ]
+    },
+    "absolutepath2": {
+      "httpMethod": "POST",
+      "id": "absolutepath.absolutepath2",
+      "path": "absolutepathmethod2/v1",
       "scopes": [
         "https://www.googleapis.com/auth/userinfo.email"
       ]
diff --git a/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/absolute_path_endpoint.swagger b/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/absolute_path_endpoint.swagger
@@ -30,7 +30,7 @@
         }
       }
     },
-    "/absolutepathmethod": {
+    "/absolutepathmethod/v1": {
       "post": {
         "operationId": "AbsolutepathAbsolutePath",
         "parameters": [],
@@ -40,6 +40,17 @@
           }
         }
       }
+    },
+    "/absolutepathmethod2/v1": {
+      "post": {
+        "operationId": "AbsolutepathAbsolutePath2",
+        "parameters": [],
+        "responses": {
+          "200": {
+            "description": "A successful response"
+          }
+        }
+      }
     }
   },
   "definitions": {
diff --git a/test-utils/src/main/java/com/google/api/server/spi/testing/AbsolutePathEndpoint.java b/test-utils/src/main/java/com/google/api/server/spi/testing/AbsolutePathEndpoint.java
@@ -13,6 +13,9 @@ public Foo createFoo() {
     return null;
   }
 
-  @ApiMethod(name = "absolutepath", path = "/absolutepathmethod")
+  @ApiMethod(name = "absolutepath", path = "/absolutepathmethod/v1")
   public void absolutePath() { }
+
+  @ApiMethod(name = "absolutepath2", path = "/absolutepathmethod2/v1")
+  public void absolutePath2() { }
 }

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@`
`30`	`30`	`}`
`31`	`31`	`}`
`32`	`32`	`},`
`33`		`- "/absolutepathmethod": {`
	`33`	`+ "/absolutepathmethod/v1": {`
`34`	`34`	`"post": {`
`35`	`35`	`"operationId": "AbsolutepathAbsolutePath",`
`36`	`36`	`"parameters": [],`
`@@ -40,6 +40,17 @@`
`40`	`40`	`}`
`41`	`41`	`}`
`42`	`42`	`}`
	`43`	`+ },`
	`44`	`+ "/absolutepathmethod2/v1": {`
	`45`	`+ "post": {`
	`46`	`+ "operationId": "AbsolutepathAbsolutePath2",`
	`47`	`+ "parameters": [],`
	`48`	`+ "responses": {`
	`49`	`+ "200": {`
	`50`	`+ "description": "A successful response"`
	`51`	`+ }`
	`52`	`+ }`
	`53`	`+ }`
`43`	`54`	`}`
`44`	`55`	`},`
`45`	`56`	`"definitions": {`
Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,9 @@ public Foo createFoo() {`
`13`	`13`	`return null;`
`14`	`14`	`}`
`15`	`15`
`16`		`- @ApiMethod(name = "absolutepath", path = "/absolutepathmethod")`
	`16`	`+ @ApiMethod(name = "absolutepath", path = "/absolutepathmethod/v1")`
`17`	`17`	`public void absolutePath() { }`
	`18`	`+`
	`19`	`+ @ApiMethod(name = "absolutepath2", path = "/absolutepathmethod2/v1")`
	`20`	`+ public void absolutePath2() { }`
`18`	`21`	`}`