fix: Re-implemented ContainerRenderer on AbstractJsonViewContainerRenderer #15234
Conversation
…derer * Re-enabled gson view resolver * `ContainerRenderer` extends `Renderer` fix to generics parameter * - consequence fix, that `Renderer` calls `render(T render, ...` changed to `render(Object render, ...` and thus contract changed back to pre Grails 7.0.x * New test project to verify outcome.
sbglasius
force-pushed
the
fix/issue_15228-respond-errors
branch
from
5ef8b65 to
1c7fa75
Compare
matrei
left a comment
matrei
left a comment
There was a problem hiding this comment.
Looks solid! Good find and fix. Thanks for adding functional test.
jdaugherty
left a comment
jdaugherty
left a comment
There was a problem hiding this comment.
My only concern is the API change and putting this in 7.0.x. I understand that this is a critical bug fix, but given that we were wanting to do 7.1.x in November anyhow, should we instead promote 7.1.x sooner?
| * @param context The {@link RenderContext} | ||
| */ | ||
| void render(T object, RenderContext context) | ||
| void render(Object object, RenderContext context) |
There was a problem hiding this comment.
Since this is part of the rest transforms, that means this is an API breaking change. I think we have to do this, but I'm wondering if it should be a 7.1.x bug fix and we just ship 7.1.x sooner.
There was a problem hiding this comment.
I disagree, that it's a breaking change, since that's how it was in Grails 6. It was changed in Grails 7.
| ] | ||
| } | ||
| ''') | ||
| "message": "Property [title] of class [class functional.tests.Book] cannot be null", |
There was a problem hiding this comment.
I assume if there isn't an error.gson, then it does render errors like before?
There was a problem hiding this comment.
Yes, I found it because my client had a custom _errors.gson file that was not rendered.
jamesfredley
left a comment
jamesfredley
left a comment
There was a problem hiding this comment.
Thank you @sbglasius for finding a solution. My changes in September 2024 allowed grails-views to compile on Groovy 4, but I was worried we would run into some issue, at some point and these issues tracked that, for many months:
apache/grails-views#582
#14199
4 participants
ContainerRendererextendsRendererfix to generics parameterRenderercallsrender(T render, ...changed torender(Object render, ...and thus contract changed back to pre Grails 7.0.xFixes Restore implements ContainerRenderer<C, T> on AbstractJsonViewContainerRenderer<C,T> #14199
Fixes respond with Errors does not render view errors/_errors.gson #15228