Modernize MIME negotiation and serialization for Spring Boot 4.1 - #16237
codeconsole wants to merge 82 commits into
Conversation
Deprecation migration examplesThis comment maps every API deprecated by this PR to its modern replacement. Overloads are grouped where their migration is identical.
1.
|
Selecting a named configuration returned before the projection was considered, so respond with both a jsonConfiguration and includes or excludes silently dropped the projection the legacy converter applied. Give NamedJsonRenderer an overload carrying the projection, derive the writer with the include and exclude attributes the domain serializer reads, and pass the response's projection through.
Two problems with resolving domain serializers lazily. The catch treated any runtime failure as "GORM is not ready" and fell back to ordinary bean serialization, so a genuine mapping defect would silently serialize a domain object as a plain bean, potentially exposing properties the domain serializer would not write. Catch only the GORM-not-initialized failure. The window in which that happens exists because JsonDataBindingSourceCreator injected the JsonMapper, pulling Jackson's auto-configuration into a graph MimeTypesConfiguration depends on, so Boot's mapper was built before GORM. Resolve it when a request body is first parsed instead, which is after startup.
Removing the render(Map, Object) overload took the named-configuration
render form with it, leaving respond as the only way to select one.
Restore it as a render argument instead:
render json: book, jsonConfiguration: 'deep'
Keying off the argument map means no other two-argument render call can
be captured by it, which is what made the overload unsafe. Projections
are passed through, matching respond.
Returning null while GORM's metadata was unreadable let Jackson select and cache its ordinary bean serializer for a domain class. That choice survived GORM starting, so the class kept serializing with the wrong shape for the life of the mapper. Deferring the data-binding mapper lookup made that less likely but any early component using Boot's JsonMapper can still reach it. A class is recognisable as a domain artefact from the artefact registry, which does not need GORM. When one is asked for too early, hand back a serializer of ours that binds to the persistent metadata on first write, so Jackson caches that rather than a bean serializer. Writing before the metadata exists now fails with a message instead of emitting a different shape. Covers the sequence: build the mapper before GORM, write the class, initialize GORM, write it again, and assert the domain serializer is used.
The projection overload was a default method delegating to the projection-free one, so any other implementation of this new public interface would drop includes and excludes with nothing to show a projection had been asked for. The interface is unreleased, so nothing is gained by tolerating that; make the method abstract. SpringMessageConverters returned the list Spring itself configures, which a caller could mutate. Wrap it unmodifiable: the wrapper still observes later configurers, without offering a way to alter Spring MVC's converters through Grails.
Adds status, the default JSON content type, an explicit content type, excludes reaching the renderer, and view rendering staying enabled when writing fails. The spec removed with the old overload covered status and content type; that coverage is restored here.
The readiness probe asked whether the mapping context was null, but DefaultGrailsApplication.getMappingContext never returns null: it hands out a proxy that fails only when one of its methods is called. The probe therefore reported ready, no deferred serializer was installed, and Jackson still cached a bean serializer for a domain class written too early -- the very case the previous change set out to fix. Let the failed lookup out of persistentEntity instead, so the caller can tell "GORM is not initialized" from "this type is not mapped", and decide domain-ness from DomainClassArtefactHandler.isDomainClass, which needs neither GORM nor a registered artefact handler. The earlier regression test passed only because it overrode getMappingContext to throw, which no real application does. It now uses a real DefaultGrailsApplication and its proxy, and unit tests cover the selection directly for each of the four cases.
The remaining test in this spec still overrode getMappingContext to throw, which no application does -- the real one returns a proxy that fails on use. Overriding the method under test can only confirm the assumption being made about it, which is how the ordering bug survived a passing test once already. Use a real DefaultGrailsApplication and set its mapping context when GORM would.
The Javadoc still described the behaviour from before the fix, saying null covered both an unmapped type and GORM not being initialized. The second case now propagates GrailsConfigurationException, and the whole correction turns on the caller being able to tell them apart -- DeferredDomainSerializer catches that exception, which the old wording made look like dead code.
|
This is an extremely large change that I think needs deferred to 8.1 or possibly 9 with feedback from multiple committers to merge. |
|
@jdaugherty I think it is too much for 8.1. I am fine with 9 if we can get it reviewed in a timely manner and released as a 9.0 milestone prior to any 8.1 milestone |
Conflict resolutions:
* SpringIOUtils: upstream replaced the inline setFeature block with the
cached factories and the XmlParserFeature enum. That rewrite already
carries the http:// feature identifiers this branch was fixing, so
upstream's version is taken whole.
* upgrading80x.adoc: both sides appended sections. Upstream's 53-60 are
kept in place and this branch's five sections are renumbered 61-65.
Semantic conflicts git did not flag:
* XmlDataBindingSourceCreatorSpec, added upstream in grails-web-databinding,
moved to grails-xml, where this branch moved the creator it exercises.
grails-web-databinding cannot depend on grails-xml.
* The XML NonPublicClassMarshallingSpec stays in grails-converters beside
its JSON twin and the bean fixtures it shares, reached through the
existing testImplementation project(':grails-xml').
* XmlCompatibilitySpec expected XML.parse to accept a DOCTYPE and resolve
internal entities. Upstream's parser hardening refuses any declaration,
internal or not, as upgrading.adoc and threat model P10 both state. The
two affected features now assert refusal on the converter and binding
paths.
…ia-type-negotiation-8.0.x
Review: PR #16237 — Modernize MIME negotiation and serialization for Spring Boot 4.1I agree on targeting Grails 9 if this merges Head: What I ran locally (all green):
Verdict: request changes. The negotiation cleanup (removal of the static state in Blocking1. GString values become bean garbage on the new default
|
|
@matrei Addressed the review in e8bb2eb0a5.
Validation: The final scoped run completed successfully, including aggregate violations, affected-module checks, GSP, web/uber suites, fields, the guide build, and combined test report ( |
🚨 TestLens detected 1 failed test 🚨Here is what you can do:
Test SummaryCI / Functional Tests (Java 21, indy=false) > :grails-test-examples-scaffolding:integrationTest
🏷️ Commit: e8bb2eb Test FailuresUserControllerSpec > User list (:grails-test-examples-scaffolding:integrationTest in CI / Functional Tests (Java 21, indy=false))Rerun ControlsNote Checks are currently running using the configuration below. Select tests to mute in this pull request: 🔲 UserControllerSpec > User list Reuse successful test results: 🔲 ♻️ Only rerun the tests that failed or were muted before Click the checkbox to trigger a rerun: 🔲 Rerun jobs Learn more about TestLens at testlens.app/docs. |
3 participants
Summary
Modernize Grails MIME negotiation and provide an opt-in Spring/Jackson JSON rendering path on
8.0.x, while preserving legacy JSON rendering as the default.MediaType, remove static negotiation state, and resolve strategy bean ambiguity.*/*.NamedJsonConfigurationRegistryshared by controller rendering and direct serialization.grails-xml, keep plugin descriptors unchanged, and make new REST artefacts JSON-only by default.Compatibility and migration
grails.web.rendering.json.springdefaults tofalseon8.0.x. Set it totrueto opt into Spring JSON rendering. Legacy marshallers and ordinary legacy response shapes remain the default.render json: valueuses the default Jackson writer.render json: value, jsonConfiguration: 'deep'andrespond value, jsonConfiguration: 'deep'select the same registered configuration.Errorsserializers globally; the latter's error representation is documented.grails-xmland opt into XML. XML plugin descriptors are unaffected.respond. Use an explicitly registered Grails HAL renderer or Spring MVC controller handling. This PR does not add agrails-spring-hateoasmodule.?format=values.Verification
The final scoped verification completed successfully: aggregate violations, checks for
grails-web-common,grails-converters,grails-rest-transforms,grails-controllers,grails-xml, the three testing-support modules,grails-gsp,grails-test-suite-web,grails-test-suite-uber, andgrails-fields, plus the guide build and combined Markdown test report.The run used
-PonlyCoreTests --max-workers=2 --no-daemon --continue -x aggregateGroovydoc. PMD and SpotBugs were not enabled. An earlier clean repository-wide attempt ended when the Gradle daemon disappeared; this is not a claim that the full repository suite passed.