Skip to content

Commit

Permalink
useInsensitiveEnums feature flag fully restores old behaviour (#239)
Browse files Browse the repository at this point in the history
## Before this PR

@jcvitanovic reported problems on an internal product where empty strings were persisted to a database, but trying to opt in to the old behaviour using this feature flag would still break.

## After this PR

The feature flag brings back exactly the old pre-2.8.0 behavour, where strings are leniently deserialized.


_A full revert is still being considered here: https://github.com/palantir/conjure-java/pull/238_
  • Loading branch information
iamdanfox authored and bulldozer-bot[bot] committed Feb 18, 2019
1 parent 1c0fb58 commit 8bf6a57
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

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;
Expand Down Expand Up @@ -75,7 +74,6 @@ public static InsensitiveEnum valueOf(String value) {
case "ONE_HUNDRED":
return ONE_HUNDRED;
default:
ConjureEnums.validate(value);
return new InsensitiveEnum(Value.UNKNOWN, value);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,12 @@ private static MethodSpec createValueOf(
.addStatement("return $L", value.getValue())
.unindent();
}
parser.add("default:\n")
.indent()
// Only validate unknown values, matches are validated at build time.
.addStatement("$T.validate($N)", ConjureEnums.class, param)
.addStatement("return new $T(Value.UNKNOWN, $N)", thisClass, param)
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)
.unindent()
.endControlFlow();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,18 @@ public void testInsensitiveEnum_lowerCaseUnknown() {
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");
Expand Down

0 comments on commit 8bf6a57

Please sign in to comment.