From 5db9cde8ff3e81ce46a982c6ff4347d755de62da Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 13 Oct 2017 12:08:59 -0700 Subject: [PATCH] Addressed PR comments --- .../model/intermediate/MemberModel.java | 8 ++-- .../codegen/naming/DefaultNamingStrategy.java | 5 +-- .../awssdk/codegen/poet/common/EnumClass.java | 4 +- .../codegen/poet/model/AwsServiceModel.java | 12 +++--- .../codegen/poet/common/test-enum-class.java | 4 +- .../codegen/poet/model/alltypesrequest.java | 10 ++--- .../codegen/poet/model/alltypesresponse.java | 10 ++--- .../awssdk/protocol/tests/EnumTest.java | 40 +++++++++---------- 8 files changed, 46 insertions(+), 47 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java b/codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java index e6f00f94f3be..bb473993035b 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java @@ -344,8 +344,8 @@ public String getGetterDocumentation() { if (returnTypeIs(List.class)) { appendParagraph(docBuilder, "If the list returned by the service includes enum values that are not available in the " - + "current SDK version, {@link #%s} will use {@link %s#UNKNOWN} in place of those values in the " - + "list. The raw values returned by the service are available from {@link #%s}.", + + "current SDK version, {@link #%s} will use {@link %s#UNKNOWN_TO_SDK_VERSION} in place of those " + + "values in the list. The raw values returned by the service are available from {@link #%s}.", getFluentEnumGetterMethodName(), getEnumType(), getFluentGetterMethodName()); } else if (returnTypeIs(Map.class)) { appendParagraph(docBuilder, @@ -356,8 +356,8 @@ public String getGetterDocumentation() { } else { appendParagraph(docBuilder, "If the service returns an enum value that is not available in the current SDK version, " - + "{@link #%s} will return {@link %s#UNKNOWN}. The raw value returned by the service is " - + "available from {@link #%s}.", + + "{@link #%s} will return {@link %s#UNKNOWN_TO_SDK_VERSION}. The raw value returned by the " + + "service is available from {@link #%s}.", getFluentEnumGetterMethodName(), getEnumType(), getFluentGetterMethodName()); } } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/naming/DefaultNamingStrategy.java b/codegen/src/main/java/software/amazon/awssdk/codegen/naming/DefaultNamingStrategy.java index c28f08304b90..b46d06f8b49b 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/naming/DefaultNamingStrategy.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/naming/DefaultNamingStrategy.java @@ -132,8 +132,7 @@ public String getEnumValueName(String enumValue) { String result = enumValue; // Special cases - result = result.replaceAll("IoT", "IOT") - .replaceAll("textORcsv", "TEXT_OR_CSV"); + result = result.replaceAll("textORcsv", "TEXT_OR_CSV"); // Convert non-underscore word boundaries into underscore word boundaries result = result.replaceAll("[:/()-. ]+", "_"); // acm-success -> acm_success @@ -143,7 +142,7 @@ public String getEnumValueName(String enumValue) { .replaceAll("([^A-Z]{2,})V([0-9]+)", "$1_V$2_"); // TestV4 -> Test_V4_ // Add an underscore between camelCased words - result = result.replaceAll("([a-z])([A-Z])", "$1_$2"); // AcmSuccess -> Acm_Success + result = result.replaceAll("([a-z])([A-Z][a-zA-Z])", "$1_$2"); // AcmSuccess -> Acm_Success // Add an underscore after acronyms result = result.replaceAll("([A-Z]+)([A-Z][a-z])", "$1_$2"); // ACMSuccess -> ACM_Success diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/common/EnumClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/common/EnumClass.java index cc957a204f12..728b42f9c00c 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/common/EnumClass.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/common/EnumClass.java @@ -54,7 +54,7 @@ public TypeSpec poetSpec() { shape.getEnums().forEach( e -> enumBuilder.addEnumConstant(e.getName(), TypeSpec.anonymousClassBuilder("$S", e.getValue()).build()) ); - enumBuilder.addEnumConstant("UNKNOWN", TypeSpec.anonymousClassBuilder("null").build()); + enumBuilder.addEnumConstant("UNKNOWN_TO_SDK_VERSION", TypeSpec.anonymousClassBuilder("null").build()); return enumBuilder.build(); } @@ -87,7 +87,7 @@ private MethodSpec fromValueSpec() { .addStatement("return $1T.of($2T.values())\n" + ".filter(e -> e.toString().equals($3N))\n" + ".findFirst()\n" + - ".orElse(UNKNOWN)", + ".orElse(UNKNOWN_TO_SDK_VERSION)", Stream.class, className, VALUE) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AwsServiceModel.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AwsServiceModel.java index e1da6d99025b..5bf01ba45104 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AwsServiceModel.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AwsServiceModel.java @@ -180,7 +180,7 @@ private Stream memberGetters(MemberModel member) { result.add(enumMemberGetter(member)); } - result.add(nonEnumMemberGetter(member)); + result.add(memberGetter(member)); return result.stream(); } @@ -198,12 +198,12 @@ private MethodSpec enumMemberGetter(MemberModel member) { .build(); } - private MethodSpec nonEnumMemberGetter(MemberModel member) { + private MethodSpec memberGetter(MemberModel member) { return MethodSpec.methodBuilder(member.getFluentGetterMethodName()) .addJavadoc("$L", member.getGetterDocumentation()) .addModifiers(Modifier.PUBLIC) .returns(typeProvider.returnType(member)) - .addCode(nonEnumGetterStatement(member)) + .addCode(getterStatement(member)) .build(); } @@ -238,8 +238,8 @@ private CodeBlock enumGetterStatement(MemberModel member) { } private CodeBlock mapEntryFilter(String keyEnumType) { - // Don't include UNKNOWN keys in the enum map. Customers should use the string version to get at that data. - return keyEnumType != null ? CodeBlock.of("(k, v) -> !$T.equals(k, $T.UNKNOWN)", + // Don't include UNKNOWN_TO_SDK_VERSION keys in the enum map. Customers should use the string version to get at that data. + return keyEnumType != null ? CodeBlock.of("(k, v) -> !$T.equals(k, $T.UNKNOWN_TO_SDK_VERSION)", Objects.class, poetExtensions.getModelClass(keyEnumType)) : CodeBlock.of("(k, v) -> true"); } @@ -252,7 +252,7 @@ private CodeBlock identityFunction() { return CodeBlock.of("$T.identity()", Function.class); } - private CodeBlock nonEnumGetterStatement(MemberModel model) { + private CodeBlock getterStatement(MemberModel model) { VariableModel modelVariable = model.getVariable(); if ("java.nio.ByteBuffer".equals(modelVariable.getVariableType())) { diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/common/test-enum-class.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/common/test-enum-class.java index b22cfd24bcb5..07d836b3bf0e 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/common/test-enum-class.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/common/test-enum-class.java @@ -12,7 +12,7 @@ public enum TestEnumClass { PERMANENT_FAILURE("permanent-failure"), - UNKNOWN(null); + UNKNOWN_TO_SDK_VERSION(null); private final String value; @@ -36,6 +36,6 @@ public static TestEnumClass fromValue(String value) { if (value == null) { return null; } - return Stream.of(TestEnumClass.values()).filter(e -> e.toString().equals(value)).findFirst().orElse(UNKNOWN); + return Stream.of(TestEnumClass.values()).filter(e -> e.toString().equals(value)).findFirst().orElse(UNKNOWN_TO_SDK_VERSION); } } diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesrequest.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesrequest.java index 7f0a48705567..3bffe8077116 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesrequest.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesrequest.java @@ -267,7 +267,7 @@ public Map mapOfStringToSimpleStruct() { */ public Map mapOfEnumToEnum() { return TypeConverter.convert(mapOfEnumToEnum, EnumType::fromValue, EnumType::fromValue, - (k, v) -> !Objects.equals(k, EnumType.UNKNOWN)); + (k, v) -> !Objects.equals(k, EnumType.UNKNOWN_TO_SDK_VERSION)); } /** @@ -292,7 +292,7 @@ public Map mapOfEnumToEnumStrings() { */ public Map mapOfEnumToString() { return TypeConverter.convert(mapOfEnumToString, EnumType::fromValue, Function.identity(), - (k, v) -> !Objects.equals(k, EnumType.UNKNOWN)); + (k, v) -> !Objects.equals(k, EnumType.UNKNOWN_TO_SDK_VERSION)); } /** @@ -341,7 +341,7 @@ public Map mapOfStringToEnumStrings() { */ public Map mapOfEnumToSimpleStruct() { return TypeConverter.convert(mapOfEnumToSimpleStruct, EnumType::fromValue, Function.identity(), - (k, v) -> !Objects.equals(k, EnumType.UNKNOWN)); + (k, v) -> !Objects.equals(k, EnumType.UNKNOWN_TO_SDK_VERSION)); } /** @@ -450,7 +450,7 @@ public SubTypeOne polymorphicTypeWithoutSubTypes() { * Returns the value of the EnumType property for this object. *

* If the service returns an enum value that is not available in the current SDK version, {@link #enumType} will - * return {@link EnumType#UNKNOWN}. The raw value returned by the service is available from {@link #enumTypeString}. + * return {@link EnumType#UNKNOWN_TO_SDK_VERSION}. The raw value returned by the service is available from {@link #enumTypeString}. *

* * @return The value of the EnumType property for this object. @@ -464,7 +464,7 @@ public EnumType enumType() { * Returns the value of the EnumType property for this object. *

* If the service returns an enum value that is not available in the current SDK version, {@link #enumType} will - * return {@link EnumType#UNKNOWN}. The raw value returned by the service is available from {@link #enumTypeString}. + * return {@link EnumType#UNKNOWN_TO_SDK_VERSION}. The raw value returned by the service is available from {@link #enumTypeString}. *

* * @return The value of the EnumType property for this object. diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesresponse.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesresponse.java index 3d652103a43a..9e49416923e7 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesresponse.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/alltypesresponse.java @@ -268,7 +268,7 @@ public Map mapOfStringToSimpleStruct() { */ public Map mapOfEnumToEnum() { return TypeConverter.convert(mapOfEnumToEnum, EnumType::fromValue, EnumType::fromValue, - (k, v) -> !Objects.equals(k, EnumType.UNKNOWN)); + (k, v) -> !Objects.equals(k, EnumType.UNKNOWN_TO_SDK_VERSION)); } /** @@ -293,7 +293,7 @@ public Map mapOfEnumToEnumStrings() { */ public Map mapOfEnumToString() { return TypeConverter.convert(mapOfEnumToString, EnumType::fromValue, Function.identity(), - (k, v) -> !Objects.equals(k, EnumType.UNKNOWN)); + (k, v) -> !Objects.equals(k, EnumType.UNKNOWN_TO_SDK_VERSION)); } /** @@ -342,7 +342,7 @@ public Map mapOfStringToEnumStrings() { */ public Map mapOfEnumToSimpleStruct() { return TypeConverter.convert(mapOfEnumToSimpleStruct, EnumType::fromValue, Function.identity(), - (k, v) -> !Objects.equals(k, EnumType.UNKNOWN)); + (k, v) -> !Objects.equals(k, EnumType.UNKNOWN_TO_SDK_VERSION)); } /** @@ -451,7 +451,7 @@ public SubTypeOne polymorphicTypeWithoutSubTypes() { * Returns the value of the EnumType property for this object. *

* If the service returns an enum value that is not available in the current SDK version, {@link #enumType} will - * return {@link EnumType#UNKNOWN}. The raw value returned by the service is available from {@link #enumTypeString}. + * return {@link EnumType#UNKNOWN_TO_SDK_VERSION}. The raw value returned by the service is available from {@link #enumTypeString}. *

* * @return The value of the EnumType property for this object. @@ -465,7 +465,7 @@ public EnumType enumType() { * Returns the value of the EnumType property for this object. *

* If the service returns an enum value that is not available in the current SDK version, {@link #enumType} will - * return {@link EnumType#UNKNOWN}. The raw value returned by the service is available from {@link #enumTypeString}. + * return {@link EnumType#UNKNOWN_TO_SDK_VERSION}. The raw value returned by the service is available from {@link #enumTypeString}. *

* * @return The value of the EnumType property for this object. diff --git a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/EnumTest.java b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/EnumTest.java index e8e3ca0de47c..8f6933a3ab91 100644 --- a/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/EnumTest.java +++ b/test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/EnumTest.java @@ -33,63 +33,63 @@ public class EnumTest { @Test public void knownEnumFieldsBehaveCorrectly() { - assertThat(simulateUnmarshallingEnum("EnumValue1")).isEqualTo(EnumType.ENUM_VALUE1); - assertThat(simulateUnmarshallingEnum((String) null)).isEqualTo(null); - assertThat(simulateUnmarshallingEnum(EnumType.ENUM_VALUE1)).isEqualTo(EnumType.ENUM_VALUE1); + assertThat(convertToEnumWithBuilder("EnumValue1")).isEqualTo(EnumType.ENUM_VALUE1); + assertThat(convertToEnumWithBuilder((String) null)).isEqualTo(null); + assertThat(convertToEnumWithBuilder(EnumType.ENUM_VALUE1)).isEqualTo(EnumType.ENUM_VALUE1); } @Test public void unknownEnumFieldsBehaveCorrectly() { - assertThat(simulateUnmarshallingEnum("Foo")).isEqualTo(EnumType.UNKNOWN); + assertThat(convertToEnumWithBuilder("Foo")).isEqualTo(EnumType.UNKNOWN_TO_SDK_VERSION); } @Test public void knownEnumListFieldsBehaveCorrectly() { - assertThat(simulateUnmarshallingEnumList("EnumValue1", "EnumValue2")) + assertThat(convertToListEnumWithBuilder("EnumValue1", "EnumValue2")) .containsExactly(EnumType.ENUM_VALUE1, EnumType.ENUM_VALUE2); - assertThat(simulateUnmarshallingEnumList(new String[] { null })).containsExactly(new EnumType[] { null }); - assertThat(simulateUnmarshallingEnumList()).isEmpty(); + assertThat(convertToListEnumWithBuilder(new String[] {null })).containsExactly(new EnumType[] {null }); + assertThat(convertToMapEnumWithBuilder()).isEmpty(); } @Test public void unknownEnumListFieldsBehaveCorrectly() { - assertThat(simulateUnmarshallingEnumList("Foo", "EnumValue2")).containsExactly(EnumType.UNKNOWN, EnumType.ENUM_VALUE2); + assertThat(convertToListEnumWithBuilder("Foo", "EnumValue2")).containsExactly(EnumType.UNKNOWN_TO_SDK_VERSION, EnumType.ENUM_VALUE2); } @Test public void knownEnumMapFieldsBehaveCorrectly() { - assertThat(simulateUnmarshallingEnumMap(new SimpleImmutableEntry<>("EnumValue1", "EnumValue2"))) + assertThat(convertToMapEnumWithBuilder(new SimpleImmutableEntry<>("EnumValue1", "EnumValue2"))) .containsExactly(new SimpleImmutableEntry<>(EnumType.ENUM_VALUE1, EnumType.ENUM_VALUE2)); } @Test public void unknownEnumMapFieldsBehaveCorrectly() { - assertThat(simulateUnmarshallingEnumMap(new SimpleImmutableEntry<>("Foo", "EnumValue2"))) + assertThat(convertToMapEnumWithBuilder(new SimpleImmutableEntry<>("Foo", "EnumValue2"))) .isEmpty(); - assertThat(simulateUnmarshallingEnumMap(new SimpleImmutableEntry<>("Foo", "Foo"))) + assertThat(convertToMapEnumWithBuilder(new SimpleImmutableEntry<>("Foo", "Foo"))) .isEmpty(); - assertThat(simulateUnmarshallingEnumMap(new SimpleImmutableEntry<>("Foo", ""))) + assertThat(convertToMapEnumWithBuilder(new SimpleImmutableEntry<>("Foo", ""))) .isEmpty(); - assertThat(simulateUnmarshallingEnumMap(new SimpleImmutableEntry<>("EnumValue1", "Foo"))) - .containsExactly(new SimpleImmutableEntry<>(EnumType.ENUM_VALUE1, EnumType.UNKNOWN)); - assertThat(simulateUnmarshallingEnumMap(new SimpleImmutableEntry<>("EnumValue1", ""))) - .containsExactly(new SimpleImmutableEntry<>(EnumType.ENUM_VALUE1, EnumType.UNKNOWN)); + assertThat(convertToMapEnumWithBuilder(new SimpleImmutableEntry<>("EnumValue1", "Foo"))) + .containsExactly(new SimpleImmutableEntry<>(EnumType.ENUM_VALUE1, EnumType.UNKNOWN_TO_SDK_VERSION)); + assertThat(convertToMapEnumWithBuilder(new SimpleImmutableEntry<>("EnumValue1", ""))) + .containsExactly(new SimpleImmutableEntry<>(EnumType.ENUM_VALUE1, EnumType.UNKNOWN_TO_SDK_VERSION)); } - private EnumType simulateUnmarshallingEnum(String value) { + private EnumType convertToEnumWithBuilder(String value) { return AllTypesResponse.builder().enumMember(value).build().enumMember(); } - private EnumType simulateUnmarshallingEnum(EnumType value) { + private EnumType convertToEnumWithBuilder(EnumType value) { return AllTypesResponse.builder().enumMember(value).build().enumMember(); } - private List simulateUnmarshallingEnumList(String... values) { + private List convertToListEnumWithBuilder(String... values) { return AllTypesResponse.builder().listOfEnums(values).build().listOfEnums(); } @SafeVarargs - private final Map simulateUnmarshallingEnumMap(Entry... values) { + private final Map convertToMapEnumWithBuilder(Entry... values) { Map enumMap = Stream.of(values).collect(toMap(Entry::getKey, Entry::getValue)); return AllTypesResponse.builder().mapOfEnumToEnum(enumMap).build().mapOfEnumToEnum(); }