Skip to content

Conversation

@codeconsole
Copy link
Contributor

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

  • JSON.java: Handle enums as primitives (like strings, numbers, booleans)
  • XML.java: Handle enums as primitives (like strings, numbers, booleans)
  • Removed dead code: EnumMarshaller classes (no longer needed)
  • Updated tests: Verify symmetric serialization/deserialization

# See the License for the specific language governing permissions and
# limitations under the License.

projectVersion=7.0.1-SNAPSHOT
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7.1.x

@jdaugherty
Copy link
Contributor

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.

@codeconsole codeconsole changed the base branch from 7.0.x to 7.1.x November 2, 2025 02:17
@codeconsole
Copy link
Contributor Author

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.

@matrei
Copy link
Contributor

matrei commented Nov 3, 2025

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.

@codeconsole
Copy link
Contributor Author

codeconsole commented Nov 4, 2025

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?

curl -X POST 'https://localhost:8080/test' \
  -H 'Content-Type: application/json' \
  -d '{"stage":{"enumType":"com.example.ChallengeStage","name":"SUBMIT"}}'

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:

curl -X POST 'https://localhost:8080/test' \
  -H 'Content-Type: application/json' \
  -d '{"stage":"SUBMIT"}'

even form submission sends enum String values

curl -X POST 'https://localhost:8080/test'\
  -H 'Content-Type: application/x-www-form-urlencoded' \
  --data 'stage=SUBMIT'

@matrei
Copy link
Contributor

matrei commented Nov 4, 2025

I think 8 is too far off

With a feature flag, the new serialization format would be available in 7.1.

something that IMO is completely broken

I don't think it's broken, it just does not work like you would like.
And I agree with you.

But it is a breaking change, and we should avoid that in a minor version update.

@jdaugherty
Copy link
Contributor

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.

@codeconsole
Copy link
Contributor Author

Well the trade off is doing it with a marshaller or not. Marshaller seems overkill.

if (o == null) {
writer.valueNull();
}
else if (o instanceof CharSequence) {
writer.value(o);
}
else if (o instanceof Class<?>) {
writer.value(((Class<?>) o).getName());
}
else if (o instanceof Enum) {
writer.value(((Enum<?>) o).name());
}
else if (o instanceof Number) {
writer.value((Number) o);
} else if (o instanceof Boolean) {
writer.value((Boolean) o);
} else if (o.getClass().isPrimitive() && !o.getClass().equals(byte[].class)) {
writer.value(o);
}

@matrei define feature flag. A config setting in application.yml?

@matrei
Copy link
Contributor

matrei commented Nov 5, 2025

define feature flag. A config setting in application.yml?

@codeconsole Yes, a config value. However it is set.

@codeconsole codeconsole changed the title Fix Enum JSON/XML Serialization for Round-Trip Compatibility 7.1.x Fix Enum JSON/XML Serialization for Round-Trip Compatibility Nov 5, 2025
@codeconsole
Copy link
Contributor Author

merged in 7.0.x #15212

@sbglasius
Copy link
Contributor

@codeconsole Good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants