Add the '--useInsensitiveEnums' flag to allow case-insensitive enum values #232
Conversation
|
should this be the default so that there's no surprise breaks? |
|
why would we not want this to just be the only one? |
|
thankfully it's not a backcompat break to allow more input |
@markelliot This flag enables a feature which does not align with the specification, nor should it, the goal of conjure is to provide a strict set of standards across our ecosystem. Based on the features that have been removed despite existing usage in favor of a cleaner standard, I do not think it's reasonable to update the spec based on this detail either.
@j-baker I'm sorry but I'm not sure what you're trying to say. |
|
@j-baker you might have missed all the back and forth on this, more context on: palantir/conjure#193 |
|
idk, feels like we're having a circular argument at this point. This unbreaks the thing I'd like to unbreak, and likely unbreaks a lot of services. I think the spec should be changed along with other implementations because case-insensitive interpretation and single-case production is as reasonable a semantic as case-sensitive interpretation and no less precise, while helping a number of existing clients. When can this be merged and released? |
There was a problem hiding this comment.
OK let's get this merged as a short term fix. I'm still going to discourage anyone from using this flag because it reduces the guarantees we can provide about interop with other languages - would prefer repos to make a decent effort to switch to uppercase enums.
## 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](palantir/conjure#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`](#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
No description provided.