Skip to content

Commit

Permalink
Addressed PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
millems committed Oct 13, 2017
1 parent 7ee7b13 commit 5db9cde
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private Stream<MethodSpec> memberGetters(MemberModel member) {
result.add(enumMemberGetter(member));
}

result.add(nonEnumMemberGetter(member));
result.add(memberGetter(member));

return result.stream();
}
Expand All @@ -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();
}

Expand Down Expand Up @@ -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");
}
Expand All @@ -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())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public enum TestEnumClass {

PERMANENT_FAILURE("permanent-failure"),

UNKNOWN(null);
UNKNOWN_TO_SDK_VERSION(null);

private final String value;

Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public Map<String, SimpleStruct> mapOfStringToSimpleStruct() {
*/
public Map<EnumType, EnumType> 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));
}

/**
Expand All @@ -292,7 +292,7 @@ public Map<String, String> mapOfEnumToEnumStrings() {
*/
public Map<EnumType, String> 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));
}

/**
Expand Down Expand Up @@ -341,7 +341,7 @@ public Map<String, String> mapOfStringToEnumStrings() {
*/
public Map<EnumType, SimpleStruct> 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));
}

/**
Expand Down Expand Up @@ -450,7 +450,7 @@ public SubTypeOne polymorphicTypeWithoutSubTypes() {
* Returns the value of the EnumType property for this object.
* <p>
* 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}.
* </p>
*
* @return The value of the EnumType property for this object.
Expand All @@ -464,7 +464,7 @@ public EnumType enumType() {
* Returns the value of the EnumType property for this object.
* <p>
* 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}.
* </p>
*
* @return The value of the EnumType property for this object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public Map<String, SimpleStruct> mapOfStringToSimpleStruct() {
*/
public Map<EnumType, EnumType> 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));
}

/**
Expand All @@ -293,7 +293,7 @@ public Map<String, String> mapOfEnumToEnumStrings() {
*/
public Map<EnumType, String> 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));
}

/**
Expand Down Expand Up @@ -342,7 +342,7 @@ public Map<String, String> mapOfStringToEnumStrings() {
*/
public Map<EnumType, SimpleStruct> 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));
}

/**
Expand Down Expand Up @@ -451,7 +451,7 @@ public SubTypeOne polymorphicTypeWithoutSubTypes() {
* Returns the value of the EnumType property for this object.
* <p>
* 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}.
* </p>
*
* @return The value of the EnumType property for this object.
Expand All @@ -465,7 +465,7 @@ public EnumType enumType() {
* Returns the value of the EnumType property for this object.
* <p>
* 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}.
* </p>
*
* @return The value of the EnumType property for this object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<EnumType> simulateUnmarshallingEnumList(String... values) {
private List<EnumType> convertToListEnumWithBuilder(String... values) {
return AllTypesResponse.builder().listOfEnums(values).build().listOfEnums();
}

@SafeVarargs
private final Map<EnumType, EnumType> simulateUnmarshallingEnumMap(Entry<String, String>... values) {
private final Map<EnumType, EnumType> convertToMapEnumWithBuilder(Entry<String, String>... values) {
Map<String, String> enumMap = Stream.of(values).collect(toMap(Entry::getKey, Entry::getValue));
return AllTypesResponse.builder().mapOfEnumToEnum(enumMap).build().mapOfEnumToEnum();
}
Expand Down

0 comments on commit 5db9cde

Please sign in to comment.