From 1a12a046b627ad935a802718a43f3c9439930250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Denis?= Date: Fri, 1 Mar 2019 19:50:33 +0100 Subject: [PATCH] Fix inconsistent enum constant names between schema and serialization: - param and resource de/serialization uses Jackson configuration - schema generation used Enum.name(), should use Jackson instead --- .../spi/config/model/SchemaRepository.java | 12 +++---- .../api/server/spi/config/model/Types.java | 33 +++++++++++++++++++ .../spi/discovery/DiscoveryGenerator.java | 13 ++++---- .../server/spi/swagger/SwaggerGenerator.java | 7 ++-- .../config/model/SchemaRepositoryTest.java | 22 +++++++++++-- .../server/spi/discovery/enum_endpoint.json | 4 +-- .../server/spi/swagger/enum_endpoint.swagger | 4 +-- .../api/server/spi/testing/TestEnum.java | 6 +++- 8 files changed, 74 insertions(+), 27 deletions(-) diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/SchemaRepository.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/SchemaRepository.java index 56dc21cd..dc3efd92 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/SchemaRepository.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/SchemaRepository.java @@ -2,7 +2,6 @@ import com.google.api.client.util.Maps; import com.google.api.server.spi.TypeLoader; -import com.google.api.server.spi.config.Description; import com.google.api.server.spi.config.ResourcePropertySchema; import com.google.api.server.spi.config.ResourceSchema; import com.google.api.server.spi.config.annotationreader.ApiAnnotationIntrospector; @@ -18,7 +17,6 @@ import com.google.common.reflect.TypeToken; import java.util.EnumSet; -import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -172,15 +170,13 @@ private Schema getOrCreateTypeForConfig( schemaByApiKeys.put(key, schema); return schema; } else if (Types.isEnumType(type)) { + Map valuesAndDescriptions = Types.getEnumValuesAndDescriptions(type); Schema.Builder builder = Schema.builder() .setName(Types.getSimpleName(type, config.getSerializationConfig())) .setType("string"); - for (java.lang.reflect.Field field : type.getRawType().getFields()) { - if (field.isEnumConstant()) { - builder.addEnumValue(field.getName()); - Description description = field.getAnnotation(Description.class); - builder.addEnumDescription(description == null ? "" : description.value()); - } + for (Entry entry : valuesAndDescriptions.entrySet()) { + builder.addEnumValue(entry.getKey()); + builder.addEnumDescription(entry.getValue()); } schema = builder.build(); typesForConfig.put(type, schema); diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/Types.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/Types.java index 1a3d80d2..2f8e3320 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/Types.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/Types.java @@ -15,8 +15,12 @@ */ package com.google.api.server.spi.config.model; +import com.fasterxml.jackson.databind.SerializationConfig; +import com.fasterxml.jackson.databind.util.EnumValues; import com.google.api.client.util.GenericData; import com.google.api.client.util.Preconditions; +import com.google.api.server.spi.ObjectMapperUtil; +import com.google.api.server.spi.config.Description; import com.google.api.server.spi.config.ResourceSchema; import com.google.api.server.spi.config.ResourceTransformer; import com.google.api.server.spi.config.Transformer; @@ -32,6 +36,8 @@ import java.lang.reflect.WildcardType; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import javax.annotation.Nullable; @@ -56,6 +62,33 @@ public static boolean isEnumType(TypeToken type) { return type.isSubtypeOf(Enum.class); } + /** + * Determines enum constant names for serialization, and their description for API schema. + * + * @param type an enum type + * @return a map containing the enum field names as keys, + * and schema description as values (possibly empty but not null). + */ + public static Map getEnumValuesAndDescriptions(TypeToken> type) { + Class> enumType = (Class>) type.getRawType(); + Map descriptions = new HashMap<>(); + for (java.lang.reflect.Field field : enumType.getFields()) { + if (field.isEnumConstant()) { + Description description = field.getAnnotation(Description.class); + descriptions.put(field.getName(), description != null ? description.value() : ""); + } + } + SerializationConfig serializationConfig = ObjectMapperUtil.createStandardObjectMapper() + .getSerializationConfig(); + EnumValues enumValues = EnumValues.construct(serializationConfig, enumType); + Map valueAndDescription = new LinkedHashMap<>(); + for (Enum enumConstant : enumType.getEnumConstants()) { + valueAndDescription.put(enumValues.serializedValueFor(enumConstant).toString(), + descriptions.get(enumConstant.name())); + } + return valueAndDescription; + } + /** * Returns whether or not this type is a {@link Map}. This excludes {@link GenericData}, which is * used by the Google Java client library as a supertype of resource types with concrete fields. diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/DiscoveryGenerator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/DiscoveryGenerator.java index 4e741acf..5664e52c 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/DiscoveryGenerator.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/discovery/DiscoveryGenerator.java @@ -20,7 +20,6 @@ import com.google.api.server.spi.ObjectMapperUtil; import com.google.api.server.spi.Strings; import com.google.api.server.spi.TypeLoader; -import com.google.api.server.spi.config.Description; import com.google.api.server.spi.config.annotationreader.ApiAnnotationIntrospector; import com.google.api.server.spi.config.model.ApiConfig; import com.google.api.server.spi.config.model.ApiKey; @@ -34,6 +33,7 @@ import com.google.api.server.spi.config.model.SchemaRepository; import com.google.api.server.spi.config.model.AuthScopeRepository; import com.google.api.server.spi.config.model.StandardParameters; +import com.google.api.server.spi.config.model.Types; import com.google.api.server.spi.config.scope.AuthScopeExpression; import com.google.api.server.spi.config.scope.AuthScopeExpressions; import com.google.api.services.discovery.model.DirectoryList; @@ -363,14 +363,13 @@ private JsonSchema convertMethodParameter( } if (parameterConfig.isEnum()) { + Map enumValuesAndDescriptions = Types + .getEnumValuesAndDescriptions((TypeToken>) type); List enumValues = Lists.newArrayList(); List enumDescriptions = Lists.newArrayList(); - for (java.lang.reflect.Field field : type.getRawType().getFields()) { - if (field.isEnumConstant()) { - enumValues.add(field.getName()); - Description description = field.getAnnotation(Description.class); - enumDescriptions.add(description == null ? "" : description.value()); - } + for (Entry entry : enumValuesAndDescriptions.entrySet()) { + enumValues.add(entry.getKey()); + enumDescriptions.add(entry.getValue()); } schema.setEnum(enumValues); schema.setEnumDescriptions(enumDescriptions); diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/SwaggerGenerator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/SwaggerGenerator.java index 948fdba4..96ef0bef 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/SwaggerGenerator.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/swagger/SwaggerGenerator.java @@ -33,6 +33,7 @@ import com.google.api.server.spi.config.model.Schema; import com.google.api.server.spi.config.model.Schema.Field; import com.google.api.server.spi.config.model.SchemaRepository; +import com.google.api.server.spi.config.model.Types; import com.google.api.server.spi.config.validation.ApiConfigValidator; import com.google.api.server.spi.types.DateAndTime; import com.google.api.server.spi.types.SimpleDate; @@ -487,11 +488,7 @@ private Path getOrCreatePath(Swagger swagger, ApiMethodConfig methodConfig) { } private static List getEnumValues(TypeToken t) { - List values = Lists.newArrayList(); - for (Object value : t.getRawType().getEnumConstants()) { - values.add(value.toString()); - } - return values; + return new ArrayList<>(Types.getEnumValuesAndDescriptions((TypeToken>) t).keySet()); } private static SecuritySchemeDefinition toScheme( diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/SchemaRepositoryTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/SchemaRepositoryTest.java index f62d6d6a..68663c62 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/SchemaRepositoryTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/SchemaRepositoryTest.java @@ -226,12 +226,30 @@ public void getOrAdd_enum() throws Exception { .setName("TestEnum") .setType("string") .addEnumValue("VALUE1") - .addEnumValue("VALUE2") + .addEnumValue("value_2") .addEnumDescription("") .addEnumDescription("") .build()); } + @Test + public void getOrAdd_enum_disableJacksonAnnotations() throws Exception { + System.setProperty(EndpointsFlag.JSON_USE_JACKSON_ANNOTATIONS.systemPropertyName, "false"); + try { + assertThat(repo.getOrAdd(TypeToken.of(TestEnum.class), config)) + .isEqualTo(Schema.builder() + .setName("TestEnum") + .setType("string") + .addEnumValue("VALUE1") + .addEnumValue("VALUE2") + .addEnumDescription("") + .addEnumDescription("") + .build()); + } finally { + System.clearProperty(EndpointsFlag.JSON_USE_JACKSON_ANNOTATIONS.systemPropertyName); + } + } + @Test public void getOrAdd_recursiveSchema() throws Exception { TypeToken type = TypeToken.of(SelfReferencingObject.class); @@ -270,7 +288,7 @@ public void getOrAdd_multipleApis() throws Exception { .setName("TestEnum") .setType("string") .addEnumValue("VALUE1") - .addEnumValue("VALUE2") + .addEnumValue("value_2") .addEnumDescription("") .addEnumDescription("") .build(), diff --git a/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/enum_endpoint.json b/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/enum_endpoint.json index 5ef91688..f9dcbd9d 100644 --- a/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/enum_endpoint.json +++ b/endpoints-framework/src/test/resources/com/google/api/server/spi/discovery/enum_endpoint.json @@ -30,7 +30,7 @@ "value": { "enum": [ "VALUE1", - "VALUE2" + "value_2" ], "enumDescriptions": [ "", @@ -110,7 +110,7 @@ }, "TestEnum": { "id": "TestEnum", - "enum" : [ "VALUE1", "VALUE2" ], + "enum" : [ "VALUE1", "value_2" ], "enumDescriptions" : [ "", "" ], "type": "string" } diff --git a/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/enum_endpoint.swagger b/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/enum_endpoint.swagger index 98bf9c95..2d7ff181 100644 --- a/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/enum_endpoint.swagger +++ b/endpoints-framework/src/test/resources/com/google/api/server/spi/swagger/enum_endpoint.swagger @@ -27,7 +27,7 @@ "type": "string", "enum": [ "VALUE1", - "VALUE2" + "value_2" ] } ], @@ -50,7 +50,7 @@ "type": "string", "enum": [ "VALUE1", - "VALUE2" + "value_2" ] } } diff --git a/test-utils/src/main/java/com/google/api/server/spi/testing/TestEnum.java b/test-utils/src/main/java/com/google/api/server/spi/testing/TestEnum.java index 40a66fea..8af65b5a 100644 --- a/test-utils/src/main/java/com/google/api/server/spi/testing/TestEnum.java +++ b/test-utils/src/main/java/com/google/api/server/spi/testing/TestEnum.java @@ -15,6 +15,10 @@ */ package com.google.api.server.spi.testing; +import com.fasterxml.jackson.annotation.JsonProperty; + public enum TestEnum { - VALUE1, VALUE2 + VALUE1, + @JsonProperty("value_2") + VALUE2; }