-
-
Notifications
You must be signed in to change notification settings - Fork 966
7.1.x Fix Enum JSON/XML Serialization for Round-Trip Compatibility #15196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
gradle.properties
Outdated
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| projectVersion=7.0.1-SNAPSHOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are asking to merge this into 7.0.x, but this is on the 7.1.x code line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7.1.x
|
So this is one of the first things I do in a Grails app. I 100% agree it needs fixed, but unfortunately this is a breaking change since we're registering the marshaller by default. Our support policy says we won't break except on major releases. I think to accept this change in 7.x, we have to not register these by default & then in 8.x we can register them by default. |
@jdaugherty I am not registering anything. I am deleting the marshallers which could be added back in by someone if they really wanted them. |
|
As this could break someones parsing of data returned from an endpoint, we could keep backwards compatibility by defaulting to the old behavior and add a feature flag to enable the new behavior in 7.1 and then replace and remove the old behavior in 8. |
@matrei I think 8 is too far off for something that IMO is completely broken. Does this POST request work? if the answer is no, then this is clearly a bug and needs to be fixed. There is a reason why you have to submit json like this: even form submission sends enum String values |
With a feature flag, the new serialization format would be available in 7.1.
I don't think it's broken, it just does not work like you would like. But it is a breaking change, and we should avoid that in a minor version update. |
|
I'm in agreement with @matrei . I originally looked at this and assumed we were registering new marshallers, but my point was that we can't break behavior - even bad behavior in a point release. We've stated we would only do it in a major release. Everyone seems to agree this is really bad behavior though. I'm willing to discuss in the weekly and if there is consensus we can deviate from our policy. |
|
Well the trade off is doing it with a marshaller or not. Marshaller seems overkill. grails-core/grails-converters/src/main/groovy/grails/converters/JSON.java Lines 179 to 197 in c899909
@matrei define feature flag. A config setting in application.yml? |
@codeconsole Yes, a config value. However it is set. |
|
merged in 7.0.x #15212 |
|
@codeconsole Good work! |
The Problem
Grails had asymmetric enum handling that broke round-trips:
POST/PUT (What you send):
{"stage":"SUBMIT"}GET (What you received back):
{"stage":{"enumType":"com.example.ChallengeStage","name":"SUBMIT"}}Result: You couldn't POST back what you GET!
The Fix
Enums now serialize the same way they deserialize - as simple strings:
POST/PUT (What you send):
{"stage":"SUBMIT"}GET (What you get back):
{"stage":"SUBMIT"}Result: What you POST is what you GET.
Technical Changes