jamesfredley commented on code in PR #15557:
URL: https://github.com/apache/grails-core/pull/15557#discussion_r3343109565
##########
grails-rest-transforms/src/main/groovy/grails/rest/RestfulController.groovy:
##########
@@ -66,8 +66,7 @@ class RestfulController<T> {
* @return A list of resources
*/
def index(Integer max) {
- if (max < 0) { max = null }
- params.max = Math.min(max ?: 10, 100)
+ params.max = Math.min(params.int('max', 10), 100)
Review Comment:
Reverted to the original `8.0.x` body (`if (max < 0) { max = null };
params.max = Math.min(max ?: 10, 100)`). The reason it had been rewritten was
the `indy=false` parameterized-action scope bug; that is now fixed
framework-side by the `OptimizingStatementWriter.ClassNodeSkip` tag in
`ControllerActionTransformer`, so the behaviour-changing rewrite is no longer
needed.
##########
grails-rest-transforms/src/main/groovy/org/grails/plugins/web/rest/transform/ResourceTransform.groovy:
##########
@@ -232,6 +233,8 @@ class ResourceTransform implements ASTTransformation,
CompilationUnitAware, Tran
final resourcesUrlMapping = new
MethodCallExpression(buildThisExpression(), uri, new MapExpression([ new
MapEntryExpression(new ConstantExpression('resources'), new
ConstantExpression(domainPropertyName))]))
final urlMappingsClosure = new ClosureExpression(null, new
ExpressionStatement(resourcesUrlMapping))
+ // Groovy 5/6 ClosureWriter NPE on null VariableScope -
same family as AbstractMethodDecoratingTransformation.makeDelegatingClosureCall.
Review Comment:
You're right - it is an API requirement, not a Groovy-5 workaround (a
synthesised `ClosureExpression` must carry a `VariableScope` or `ClosureWriter`
NPEs, on Groovy 4 too). Reworded the comment to say exactly that.
##########
grails-shell-cli/src/main/groovy/org/grails/cli/profile/commands/templates/TemplateRendererImpl.groovy:
##########
@@ -74,7 +74,7 @@ class TemplateRendererImpl implements TemplateRenderer,
ProfileRepositoryAware {
@Override
@CompileDynamic
void render(Map<String, Object> namedArguments) {
- if (namedArguments?.template && namedArguments?.destination) {
+ if (namedArguments?.template != null && namedArguments?.destination !=
null) {
Review Comment:
Restored Groovy truth on `template` - `render(Map)` and
`render(CharSequence, ...)` now use `if (template && destination != null)`, so
`template: ''` is falsy and skips as before. Same fix as the grails-core
`TemplateRendererImpl`.
##########
grails-test-examples/app3/build.gradle:
##########
@@ -67,5 +67,6 @@ grails {
apply {
from
rootProject.layout.projectDirectory.file('gradle/functional-test-config.gradle')
+ from
rootProject.layout.projectDirectory.file('gradle/boot4-disabled-integration-test-config.gradle')
Review Comment:
Same as the app1 thread - resolved on the current branch. `integrationTest`
is no longer disabled; the `indy=false` blocker is fixed via
`OptimizingStatementWriter.ClassNodeSkip` in `ControllerActionTransformer`, and
app3 does not use SiteMesh 3.
##########
grails-test-examples/cache/grails-app/controllers/com/demo/DemoController.groovy:
##########
@@ -69,47 +69,47 @@ class DemoController {
render "Value From Service Is
\"${basicCachingService.resetData()}\""
}
- def cachePut(String key, String value) {
- def result = basicCachingService.getData(key, value)
+ def cachePut() {
Review Comment:
Reverted - restored the original action signatures (`def cachePut(String
key, String value)`, etc.). The `params.*` rewrite was the manual workaround
for the `indy=false` parameterized-action bug, which is now handled
framework-side by `OptimizingStatementWriter.ClassNodeSkip`, so this
out-of-scope change is gone.
##########
grails-test-examples/cache/grails-app/controllers/com/demo/AdvancedCachingController.groovy:
##########
@@ -33,9 +33,9 @@ class AdvancedCachingController {
// ========== null value endpoints ==========
- def dataOrNull(String input) {
+ def dataOrNull() {
Review Comment:
Reverted - restored the original action signatures (`def dataOrNull(String
input)`, etc.). The `params.*` rewrite was the manual `indy=false` workaround,
now handled framework-side by `OptimizingStatementWriter.ClassNodeSkip`, so
this out-of-scope change is gone.
##########
grails-test-examples/mongodb/database-per-tenant/grails-app/controllers/examples/mongo/tenant/BookController.groovy:
##########
@@ -35,8 +35,8 @@ class BookController {
*/
def bindParams = ['title']
- def index(Integer max) {
- params.max = Math.min(max ?: 10, 100)
+ def index() {
Review Comment:
Reverted - restored `def index(Integer max)`. The `params.int('max', 10)`
rewrite was the manual `indy=false` workaround, now handled framework-side by
`OptimizingStatementWriter.ClassNodeSkip`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
