From ee749aea3a5a058e7f845eb7e716ce4e5c9836e8 Mon Sep 17 00:00:00 2001 From: iamdanfox Date: Mon, 18 Feb 2019 15:23:18 +0000 Subject: [PATCH] [fix] bring back old enum behaviour (#238) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Before this PR Pre-2.8.0, conjure-java generated very lenient code: an enum defined as `[BLUE, GREEN]` would accept values `blue`, `bLuE` and `bLUE` as BLUE. It would also tolerate unknown values, such as "RED", "", "!@%^£". When looking into tightening up the [Conjure spec](https://github.com/palantir/conjure/issues/193), we decided that all conjure languages should not be required to implement this behaviour. In 2.8.0, we tightened up the validation of conjure-generated enums, I expected this to be safe because conjure-typescript only ever emitted uppercase values. Unfortunately, this caused P0s at some products where bad values had already been persisted into databases, from non-typescript origins. To alleviate some of this pain, we added a [`--useInsensitiveEnums`](https://github.com/palantir/conjure-java/pull/232) flag to bring back the old behaviour. Unfortunately, this would still throw when deserializing strange values like `""`, which have already been persisted to databases. Also, existing products do not know whether they have nonsensical values in their database already, and realistically do not have serialization tests that exercise every enum edge case, so they didn't know whether they needed to opt-in. ## After this PR To avoid causing lots more P0s and keeping feature flags forever, this PR reverts conjure-java's codegen to the old pre-2.8.0 behaviour. The Conjure spec still says only deserialize and serialize UPPERCASE values, but this PR intentionally diverges from the spec in order to avoid causing pain for existing products. We think this will be safe for inter-language communication because a string `"bLuE"` is deserialized as `MyEnum.BLUE` and re-serialized as `"BLUE"` so typescript clients keep working. In the future, we may want to implement a feature where servers can reject all 'unknown' values in incoming requests. cc @diogoholanda @markelliot @cakofony --- .../palantir/insensitive/InsensitiveEnum.java | 117 ------------------ .../com/palantir/product/EnumExample.java | 8 +- .../java/com/palantir/product/SimpleEnum.java | 18 +-- .../palantir/conjure/java/FeatureFlags.java | 6 - .../conjure/java/types/EnumGenerator.java | 57 +++++---- .../conjure/java/types/ObjectGenerator.java | 3 +- .../conjure/java/types/EnumTests.java | 24 ---- .../java/types/ObjectGeneratorTests.java | 11 -- .../conjure/java/types/WireFormatTests.java | 42 ++----- .../src/test/resources/ete-service.yml | 1 - .../test/resources/example-compat-enum.yml | 12 -- .../src/test/resources/example-types.yml | 1 - .../src/test/resources/ignored-test-cases.yml | 3 - .../conjure/java/cli/CliConfiguration.java | 4 - .../conjure/java/cli/ConjureJavaCli.java | 7 -- .../conjure/java/cli/ConjureJavaCliTest.java | 6 +- .../java/lib/internal/ConjureEnums.java | 6 + 17 files changed, 56 insertions(+), 270 deletions(-) delete mode 100644 conjure-java-core/src/integrationInput/java/com/palantir/insensitive/InsensitiveEnum.java delete mode 100644 conjure-java-core/src/test/resources/example-compat-enum.yml diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/insensitive/InsensitiveEnum.java b/conjure-java-core/src/integrationInput/java/com/palantir/insensitive/InsensitiveEnum.java deleted file mode 100644 index 44e40d602..000000000 --- a/conjure-java-core/src/integrationInput/java/com/palantir/insensitive/InsensitiveEnum.java +++ /dev/null @@ -1,117 +0,0 @@ -package com.palantir.insensitive; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonValue; -import com.palantir.logsafe.Preconditions; -import java.util.Locale; -import javax.annotation.Generated; - -/** - * This enumerates the numbers 1:2 also 100. - * - *

This class is used instead of a native enum to support unknown values. Rather than throw an - * exception, the {@link InsensitiveEnum#valueOf} method defaults to a new instantiation of {@link - * InsensitiveEnum} where {@link InsensitiveEnum#get} will return {@link - * InsensitiveEnum.Value#UNKNOWN}. - * - *

For example, {@code InsensitiveEnum.valueOf("corrupted value").get()} will return {@link - * InsensitiveEnum.Value#UNKNOWN}, but {@link InsensitiveEnum#toString} will return "corrupted - * value". - * - *

There is no method to access all instantiations of this class, since they cannot be known at - * compile time. - */ -@Generated("com.palantir.conjure.java.types.EnumGenerator") -public final class InsensitiveEnum { - public static final InsensitiveEnum ONE = new InsensitiveEnum(Value.ONE, "ONE"); - - public static final InsensitiveEnum TWO = new InsensitiveEnum(Value.TWO, "TWO"); - - /** Value of 100. */ - public static final InsensitiveEnum ONE_HUNDRED = - new InsensitiveEnum(Value.ONE_HUNDRED, "ONE_HUNDRED"); - - private final Value value; - - private final String string; - - private InsensitiveEnum(Value value, String string) { - this.value = value; - this.string = string; - } - - public Value get() { - return this.value; - } - - @Override - @JsonValue - public String toString() { - return this.string; - } - - @Override - public boolean equals(Object other) { - return (this == other) - || (other instanceof InsensitiveEnum - && this.string.equals(((InsensitiveEnum) other).string)); - } - - @Override - public int hashCode() { - return this.string.hashCode(); - } - - @JsonCreator - public static InsensitiveEnum valueOf(String value) { - Preconditions.checkNotNull(value, "value cannot be null"); - value = value.toUpperCase(Locale.ENGLISH); - switch (value) { - case "ONE": - return ONE; - case "TWO": - return TWO; - case "ONE_HUNDRED": - return ONE_HUNDRED; - default: - return new InsensitiveEnum(Value.UNKNOWN, value); - } - } - - public T accept(Visitor visitor) { - switch (value) { - case ONE: - return visitor.visitOne(); - case TWO: - return visitor.visitTwo(); - case ONE_HUNDRED: - return visitor.visitOneHundred(); - default: - return visitor.visitUnknown(string); - } - } - - @Generated("com.palantir.conjure.java.types.EnumGenerator") - public enum Value { - ONE, - - TWO, - - /** Value of 100. */ - ONE_HUNDRED, - - UNKNOWN - } - - @Generated("com.palantir.conjure.java.types.EnumGenerator") - public interface Visitor { - T visitOne(); - - T visitTwo(); - - /** Value of 100. */ - T visitOneHundred(); - - T visitUnknown(String unknownValue); - } -} diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java index 087212385..8d76fa873 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EnumExample.java @@ -2,8 +2,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; -import com.palantir.conjure.java.lib.internal.ConjureEnums; import com.palantir.logsafe.Preconditions; +import java.util.Locale; import javax.annotation.Generated; /** @@ -62,7 +62,8 @@ public int hashCode() { @JsonCreator public static EnumExample valueOf(String value) { Preconditions.checkNotNull(value, "value cannot be null"); - switch (value) { + String upperCasedValue = value.toUpperCase(Locale.ROOT); + switch (upperCasedValue) { case "ONE": return ONE; case "TWO": @@ -70,8 +71,7 @@ public static EnumExample valueOf(String value) { case "ONE_HUNDRED": return ONE_HUNDRED; default: - ConjureEnums.validate(value); - return new EnumExample(Value.UNKNOWN, value); + return new EnumExample(Value.UNKNOWN, upperCasedValue); } } diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/SimpleEnum.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/SimpleEnum.java index 2fdb73487..3fd0ed5b3 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/SimpleEnum.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/SimpleEnum.java @@ -2,8 +2,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; -import com.palantir.conjure.java.lib.internal.ConjureEnums; import com.palantir.logsafe.Preconditions; +import java.util.Locale; import javax.annotation.Generated; /** @@ -21,8 +21,6 @@ public final class SimpleEnum { public static final SimpleEnum VALUE = new SimpleEnum(Value.VALUE, "VALUE"); - public static final SimpleEnum VALUE2 = new SimpleEnum(Value.VALUE2, "VALUE2"); - private final Value value; private final String string; @@ -56,14 +54,12 @@ public int hashCode() { @JsonCreator public static SimpleEnum valueOf(String value) { Preconditions.checkNotNull(value, "value cannot be null"); - switch (value) { + String upperCasedValue = value.toUpperCase(Locale.ROOT); + switch (upperCasedValue) { case "VALUE": return VALUE; - case "VALUE2": - return VALUE2; default: - ConjureEnums.validate(value); - return new SimpleEnum(Value.UNKNOWN, value); + return new SimpleEnum(Value.UNKNOWN, upperCasedValue); } } @@ -71,8 +67,6 @@ public T accept(Visitor visitor) { switch (value) { case VALUE: return visitor.visitValue(); - case VALUE2: - return visitor.visitValue2(); default: return visitor.visitUnknown(string); } @@ -82,8 +76,6 @@ public T accept(Visitor visitor) { public enum Value { VALUE, - VALUE2, - UNKNOWN } @@ -91,8 +83,6 @@ public enum Value { public interface Visitor { T visitValue(); - T visitValue2(); - T visitUnknown(String unknownValue); } } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/FeatureFlags.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/FeatureFlags.java index 9947903df..042656fe9 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/FeatureFlags.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/FeatureFlags.java @@ -59,10 +59,4 @@ public enum FeatureFlags { * Use the conjure immutable "Bytes" class over ByteBuffer. */ UseImmutableBytes, - - /** - * Enums valueOf function will use a case-insensitive lookup. Note that this is not allowed by the conjure - * specification, however may be enabled for backwards compatibility. - */ - CaseInsensitiveEnums, } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java index ddfea145f..ede773391 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/EnumGenerator.java @@ -22,8 +22,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.palantir.conjure.java.ConjureAnnotations; -import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.lib.internal.ConjureEnums; import com.palantir.conjure.spec.EnumDefinition; import com.palantir.conjure.spec.EnumValueDefinition; import com.squareup.javapoet.ClassName; @@ -38,7 +36,6 @@ import com.squareup.javapoet.TypeVariableName; import java.util.List; import java.util.Locale; -import java.util.Set; import javax.lang.model.element.Modifier; import org.apache.commons.lang3.StringUtils; @@ -52,28 +49,24 @@ public final class EnumGenerator { private EnumGenerator() {} - public static JavaFile generateEnumType(EnumDefinition typeDef, Set featureFlags) { + public static JavaFile generateEnumType(EnumDefinition typeDef) { String typePackage = typeDef.getTypeName().getPackage(); ClassName thisClass = ClassName.get(typePackage, typeDef.getTypeName().getName()); ClassName enumClass = ClassName.get(typePackage, typeDef.getTypeName().getName(), "Value"); ClassName visitorClass = ClassName.get(typePackage, typeDef.getTypeName().getName(), "Visitor"); - return JavaFile.builder(typePackage, createSafeEnum(typeDef, thisClass, enumClass, visitorClass, featureFlags)) + return JavaFile.builder(typePackage, createSafeEnum(typeDef, thisClass, enumClass, visitorClass)) .skipJavaLangImports(true) .indent(" ") .build(); } private static TypeSpec createSafeEnum( - EnumDefinition typeDef, - ClassName thisClass, - ClassName enumClass, - ClassName visitorClass, - Set featureFlags) { + EnumDefinition typeDef, ClassName thisClass, ClassName enumClass, ClassName visitorClass) { TypeSpec.Builder wrapper = TypeSpec.classBuilder(typeDef.getTypeName().getName()) .addAnnotation(ConjureAnnotations.getConjureGeneratedAnnotation(EnumGenerator.class)) .addModifiers(Modifier.PUBLIC, Modifier.FINAL) - .addType(createEnum(enumClass, typeDef.getValues())) + .addType(createEnum(enumClass, typeDef.getValues(), true)) .addType(createVisitor(visitorClass, typeDef.getValues())) .addField(enumClass, VALUE_PARAMETER, Modifier.PRIVATE, Modifier.FINAL) .addField(ClassName.get(String.class), STRING_PARAMETER, Modifier.PRIVATE, Modifier.FINAL) @@ -93,7 +86,7 @@ private static TypeSpec createSafeEnum( .build()) .addMethod(createEquals(thisClass)) .addMethod(createHashCode()) - .addMethod(createValueOf(thisClass, typeDef.getValues(), featureFlags)) + .addMethod(createValueOf(thisClass, typeDef.getValues())) .addMethod(generateAcceptVisitMethod(visitorClass, typeDef.getValues())); typeDef.getDocs().ifPresent( @@ -130,7 +123,7 @@ private static Iterable createConstants(Iterable }); } - private static TypeSpec createEnum(ClassName enumClass, Iterable values) { + private static TypeSpec createEnum(ClassName enumClass, Iterable values, boolean withUnknown) { TypeSpec.Builder enumBuilder = TypeSpec.enumBuilder(enumClass.simpleName()) .addAnnotation(ConjureAnnotations.getConjureGeneratedAnnotation(EnumGenerator.class)) .addModifiers(Modifier.PUBLIC); @@ -140,7 +133,19 @@ private static TypeSpec createEnum(ClassName enumClass, Iterable values) { @@ -208,28 +213,20 @@ private static MethodSpec createConstructor(ClassName enumClass) { .build(); } - private static MethodSpec createValueOf( - ClassName thisClass, - Iterable values, - Set featureFlags) { + private static MethodSpec createValueOf(ClassName thisClass, Iterable values) { ParameterSpec param = ParameterSpec.builder(ClassName.get(String.class), "value").build(); - CodeBlock.Builder parser = CodeBlock.builder(); - if (featureFlags.contains(FeatureFlags.CaseInsensitiveEnums)) { - parser.addStatement("value = value.toUpperCase($T.ENGLISH)", Locale.class); - } - parser.beginControlFlow("switch ($N)", param); + + CodeBlock.Builder parser = CodeBlock.builder() + .beginControlFlow("switch (upperCasedValue)"); for (EnumValueDefinition value : values) { parser.add("case $S:\n", value.getValue()) .indent() .addStatement("return $L", value.getValue()) .unindent(); } - parser.add("default:\n").indent(); - if (!featureFlags.contains(FeatureFlags.CaseInsensitiveEnums)) { - // Only validate unknown values, matches are validated at build time. - parser.addStatement("$T.validate($N)", ConjureEnums.class, param); - } - parser.addStatement("return new $T(Value.UNKNOWN, $N)", thisClass, param) + parser.add("default:\n") + .indent() + .addStatement("return new $T(Value.UNKNOWN, upperCasedValue)", thisClass) .unindent() .endControlFlow(); @@ -239,6 +236,8 @@ private static MethodSpec createValueOf( .addAnnotation(JsonCreator.class) .addParameter(param) .addStatement("$L", Expressions.requireNonNull(param.name, param.name + " cannot be null")) + // uppercase param for backwards compatibility + .addStatement("String upperCasedValue = $N.toUpperCase($T.ROOT)", param, Locale.class) .addCode(parser.build()) .build(); } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ObjectGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ObjectGenerator.java index 41592b765..ae1875b36 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ObjectGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ObjectGenerator.java @@ -46,7 +46,8 @@ public Set generateTypes(List types) { return UnionGenerator.generateUnionType( typeMapper, typeDef.accept(TypeDefinitionVisitor.UNION)); } else if (typeDef.accept(TypeDefinitionVisitor.IS_ENUM)) { - return EnumGenerator.generateEnumType(typeDef.accept(TypeDefinitionVisitor.ENUM), featureFlags); + return EnumGenerator.generateEnumType( + typeDef.accept(TypeDefinitionVisitor.ENUM)); } else if (typeDef.accept(TypeDefinitionVisitor.IS_ALIAS)) { return AliasGenerator.generateAliasType( typeMapper, typeDef.accept(TypeDefinitionVisitor.ALIAS)); diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java index e3a906afc..48732bcec 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/EnumTests.java @@ -19,7 +19,6 @@ import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy; import static org.assertj.core.api.Assertions.assertThat; -import com.palantir.insensitive.InsensitiveEnum; import com.palantir.product.EnumExample; import org.junit.Test; @@ -49,29 +48,6 @@ public void testVisitUnknown() { assertThat(enumExample.accept(Visitor.INSTANCE)).isEqualTo("SOME_VALUE"); } - @Test - public void testInsensitiveEnum_lowerCase() { - assertThat(InsensitiveEnum.valueOf("one")).isEqualTo(InsensitiveEnum.ONE); - } - - @Test - public void testInsensitiveEnum_lowerCaseUnknown() { - InsensitiveEnum value = InsensitiveEnum.valueOf("notknown"); - assertThat(value.get()).isEqualTo(InsensitiveEnum.Value.UNKNOWN); - } - - @Test - public void testInsensitiveEnum_unknownToleratesEmptyStringsForHistoricalReasons() { - InsensitiveEnum value = InsensitiveEnum.valueOf(""); - assertThat(value.get()).isEqualTo(InsensitiveEnum.Value.UNKNOWN); - } - - @Test - public void testInsensitiveEnum_unknownToleratesStrangeCharactersForHistoricalReasons() { - InsensitiveEnum value = InsensitiveEnum.valueOf("!@£$%^&*()"); - assertThat(value.get()).isEqualTo(InsensitiveEnum.Value.UNKNOWN); - } - @Test public void testNullValidationUsesSafeLoggable() { assertThatLoggableExceptionThrownBy(() -> EnumExample.valueOf(null)).hasLogMessage("value cannot be null"); diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java index a525616d5..45b33ab8a 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/ObjectGeneratorTests.java @@ -49,17 +49,6 @@ public void testObjectGenerator_byteBufferCompatibility() throws IOException { assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER); } - - @Test - public void testObjectGenerator_insensitiveEnum() throws IOException { - ConjureDefinition def = Conjure.parse( - ImmutableList.of(new File("src/test/resources/example-compat-enum.yml"))); - List files = new ObjectGenerator(Collections.singleton(FeatureFlags.CaseInsensitiveEnums)) - .emit(def, folder.getRoot()); - - assertThatFilesAreTheSame(files, REFERENCE_FILES_FOLDER); - } - @Test public void testConjureImports() throws IOException { ConjureDefinition conjure = Conjure.parse( diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/WireFormatTests.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/WireFormatTests.java index 56680b575..f94845e84 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/WireFormatTests.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/WireFormatTests.java @@ -4,19 +4,13 @@ package com.palantir.conjure.java.types; -import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy; import static org.assertj.core.api.Assertions.assertThat; import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableSet; import com.palantir.conjure.java.lib.Bytes; import com.palantir.conjure.java.serialization.ObjectMappers; -import com.palantir.logsafe.SafeLoggable; -import com.palantir.logsafe.UnsafeArg; -import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; -import com.palantir.logsafe.testing.LoggableExceptionAssert; import com.palantir.product.BinaryAliasExample; import com.palantir.product.BinaryExample; import com.palantir.product.DateTimeExample; @@ -39,7 +33,6 @@ import java.time.ZoneOffset; import java.util.Set; import java.util.UUID; -import org.assertj.core.api.ThrowableAssert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -186,34 +179,19 @@ public void testEmptyObjectsDeserialize() throws Exception { assertThat(mapper.readValue("{}", EmptyObjectExample.class)).isEqualTo(EmptyObjectExample.of()); } + @Test + public void enumSerializationDoesntPreserveCase() throws Exception { + assertThat(mapper.writeValueAsString(mapper.readValue("\"one\"", EnumExample.class))).isEqualTo("\"ONE\""); + assertThat(mapper.writeValueAsString(mapper.readValue("\"ONE\"", EnumExample.class))).isEqualTo("\"ONE\""); + assertThat(mapper.writeValueAsString(mapper.readValue("\"onE\"", EnumExample.class))).isEqualTo("\"ONE\""); + } + @Test public void testEnumCasingDeserializationInvariantToInputCase() throws Exception { assertThat(mapper.readValue("\"ONE\"", EnumExample.class)).isEqualTo(EnumExample.ONE); - assertThat(mapper.readValue("\"ONE_HUNDRED\"", EnumExample.class)).isEqualTo(EnumExample.ONE_HUNDRED); - assertThatExceptionThrownRootCause(() -> mapper.readValue("\"one\"", EnumExample.class)) - .isInstanceOf(SafeIllegalArgumentException.class) - .hasLogMessage("Enum values must use UPPER_SNAKE_CASE") - .hasExactlyArgs(UnsafeArg.of("value", "one")); - assertThatExceptionThrownRootCause(() -> mapper.readValue("\"onE\"", EnumExample.class)) - .isInstanceOf(SafeIllegalArgumentException.class) - .hasLogMessage("Enum values must use UPPER_SNAKE_CASE") - .hasExactlyArgs(UnsafeArg.of("value", "onE")); - assertThatExceptionThrownRootCause(() -> mapper.readValue("\"oNE\"", EnumExample.class)) - .isInstanceOf(SafeIllegalArgumentException.class) - .hasLogMessage("Enum values must use UPPER_SNAKE_CASE") - .hasExactlyArgs(UnsafeArg.of("value", "oNE")); - assertThat(mapper.readValue("\"100\"", EnumExample.class)).isEqualTo(EnumExample.valueOf("100")); - } - - private LoggableExceptionAssert assertThatExceptionThrownRootCause( - ThrowableAssert.ThrowingCallable task) { - return assertThatLoggableExceptionThrownBy(() -> { - try { - task.call(); - } catch (Throwable t) { - throw Throwables.getRootCause(t); - } - }); + assertThat(mapper.readValue("\"one\"", EnumExample.class)).isEqualTo(EnumExample.ONE); + assertThat(mapper.readValue("\"onE\"", EnumExample.class)).isEqualTo(EnumExample.ONE); + assertThat(mapper.readValue("\"oNE\"", EnumExample.class)).isEqualTo(EnumExample.ONE); } @Test diff --git a/conjure-java-core/src/test/resources/ete-service.yml b/conjure-java-core/src/test/resources/ete-service.yml index e3bf4e4da..a32451144 100644 --- a/conjure-java-core/src/test/resources/ete-service.yml +++ b/conjure-java-core/src/test/resources/ete-service.yml @@ -13,7 +13,6 @@ types: SimpleEnum: values: - VALUE - - VALUE2 services: EmptyPathService: diff --git a/conjure-java-core/src/test/resources/example-compat-enum.yml b/conjure-java-core/src/test/resources/example-compat-enum.yml deleted file mode 100644 index 1b4454bbc..000000000 --- a/conjure-java-core/src/test/resources/example-compat-enum.yml +++ /dev/null @@ -1,12 +0,0 @@ -types: - definitions: - default-package: com.palantir.insensitive - objects: - InsensitiveEnum: - docs: | - This enumerates the numbers 1:2 also 100. - values: - - ONE - - TWO - - value: ONE_HUNDRED - docs: Value of 100. diff --git a/conjure-java-core/src/test/resources/example-types.yml b/conjure-java-core/src/test/resources/example-types.yml index b7b635c4e..b8c2fde0d 100644 --- a/conjure-java-core/src/test/resources/example-types.yml +++ b/conjure-java-core/src/test/resources/example-types.yml @@ -184,4 +184,3 @@ types: SimpleEnum: values: - VALUE - - VALUE2 diff --git a/conjure-java-server-verifier/src/test/resources/ignored-test-cases.yml b/conjure-java-server-verifier/src/test/resources/ignored-test-cases.yml index fc4c653c0..8ffea15e8 100644 --- a/conjure-java-server-verifier/src/test/resources/ignored-test-cases.yml +++ b/conjure-java-server-verifier/src/test/resources/ignored-test-cases.yml @@ -3,9 +3,6 @@ server: getMapBinaryAliasExample: - '{}' - '{"SGVsbG8sIFdvcmxk": true}' - # 'BAD VARIANT' is not allowed, this test should use 'BAD_VARIANT' which is a valid unknown enum string. - getMapEnumExampleAlias: - - '{"ONE": "", "TWO": "", "BAD VARIANT": ""}' getRawOptionalExample: - 'null' getOptionalBearerTokenAliasExample: diff --git a/conjure-java/src/main/java/com/palantir/conjure/java/cli/CliConfiguration.java b/conjure-java/src/main/java/com/palantir/conjure/java/cli/CliConfiguration.java index fcc34af6d..08a9f1008 100644 --- a/conjure-java/src/main/java/com/palantir/conjure/java/cli/CliConfiguration.java +++ b/conjure-java/src/main/java/com/palantir/conjure/java/cli/CliConfiguration.java @@ -96,9 +96,5 @@ Builder undertowServicePrefix(boolean flag) { Builder useImmutableBytes(boolean flag) { return flag ? addFeatureFlags(FeatureFlags.UseImmutableBytes) : this; } - - Builder useInsensitiveEnums(boolean flag) { - return flag ? addFeatureFlags(FeatureFlags.CaseInsensitiveEnums) : this; - } } } diff --git a/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java b/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java index 6898ede35..e5da51c3e 100644 --- a/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java +++ b/conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java @@ -119,12 +119,6 @@ public static final class GenerateCommand implements Runnable { description = "Generate binary fields using the immutable 'Bytes' type instead of 'ByteBuffer'") private boolean useImmutableBytes; - @CommandLine.Option(names = "--useInsensitiveEnums", - defaultValue = "false", - description = "Enums valueOf function will use a case-insensitive lookup. Note that this is not " - + "allowed by the conjure specification, however may be enabled for backwards compatibility.") - private boolean useInsensitiveEnums; - @CommandLine.Unmatched private List unmatchedOptions; @@ -175,7 +169,6 @@ CliConfiguration getConfiguration() { .notNullAuthAndBody(notNullAuthAndBody) .undertowServicePrefix(undertowServicePrefix) .useImmutableBytes(useImmutableBytes) - .useInsensitiveEnums(useInsensitiveEnums) .build(); } diff --git a/conjure-java/src/test/java/com/palantir/conjure/java/cli/ConjureJavaCliTest.java b/conjure-java/src/test/java/com/palantir/conjure/java/cli/ConjureJavaCliTest.java index ff13aa456..06584da93 100644 --- a/conjure-java/src/test/java/com/palantir/conjure/java/cli/ConjureJavaCliTest.java +++ b/conjure-java/src/test/java/com/palantir/conjure/java/cli/ConjureJavaCliTest.java @@ -72,8 +72,7 @@ public void parseFeatureFlags() { "--retrofitCompletableFutures", "--jerseyBinaryAsResponse", "--requireNotNullAuthAndBodyParams", - "--useImmutableBytes", - "--useInsensitiveEnums" + "--useImmutableBytes" }; CliConfiguration expectedConfiguration = CliConfiguration.builder() .input(targetFile) @@ -83,8 +82,7 @@ public void parseFeatureFlags() { FeatureFlags.RetrofitCompletableFutures, FeatureFlags.JerseyBinaryAsResponse, FeatureFlags.RequireNotNullAuthAndBodyParams, - FeatureFlags.UseImmutableBytes, - FeatureFlags.CaseInsensitiveEnums)) + FeatureFlags.UseImmutableBytes)) .build(); ConjureJavaCli.GenerateCommand cmd = new CommandLine(new ConjureJavaCli()).parse(args).get(1).getCommand(); assertThat(cmd.getConfiguration()).isEqualTo(expectedConfiguration); diff --git a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureEnums.java b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureEnums.java index 2803c16d0..fa0ec5d33 100644 --- a/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureEnums.java +++ b/conjure-lib/src/main/java/com/palantir/conjure/java/lib/internal/ConjureEnums.java @@ -28,6 +28,12 @@ private ConjureEnums() { // cannot instantiate } + /** + * Ensures a string could be a legitimate Enum value. + * @deprecated No longer used by conjure-java generated code. Preserving this function to ensure that enums + * generated by conjure-java 2.8.0 - 2.11.0 continue to work with newer conjure-lib. + */ + @Deprecated public static void validate(String value) { if (value.isEmpty()) { throw new SafeIllegalArgumentException("Enum values must not be empty");