jamesfredley commented on code in PR #15557:
URL: https://github.com/apache/grails-core/pull/15557#discussion_r3178549260
##########
grails-test-suite-web/src/test/groovy/org/grails/web/binding/DataBindingTests.groovy:
##########
@@ -404,16 +404,10 @@ class DataBindingTests extends Specification implements
ControllerUnitTest<TestC
}
void testAssociationsBinding() {
- when:
- def authorClass = new Author()
-
- Author.metaClass.static.get = { Serializable id ->
- def result = new Author()
- result.id = id as long
- result.name = "Mocked ${id}"
- result
- }
+ given:
+ GroovySpy(Author, global: true)
Review Comment:
You're right - `GroovySpy(_, global: true)` does need explicit cleanup.
Spock auto-cleans the per-method spy state at the end of the feature method,
but the `global: true` flag specifically opts into the per-class metaclass
replacement that does NOT auto-revert in Spock 2.4 if the test fails before
reaching the implicit cleanup point. The cherry-picked commit 153e14c5ad06
didn't add the cleanup block.
Two options I'm considering:
1. Add an explicit `cleanup:` block that calls
`GroovySystem.metaClassRegistry.removeMetaClass(Author)` to force re-resolution.
2. Drop `global: true` and use the default per-method scope, since this test
only needs the spy active during the `b.properties = params` data-binding pass
- which is single-threaded inside the feature method.
Leaning toward option 2 (smaller blast radius, no inter-test leakage
possible). I'll push that as a follow-up commit and leave this thread open
until the cleanup is in place and verified by running
`:grails-test-suite-web:test --tests org.grails.web.binding.DataBindingTests`
twice in a row to confirm no leakage.
##########
grails-views-gson/src/main/groovy/grails/plugin/json/builder/DefaultJsonGenerator.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package grails.plugin.json.builder;
+
+/**
+ * Temporary fork of DefaultJsonGenerator until Groovy 2.5.0 is out.
+ * <p>A JsonGenerator that can be configured with various {@link
JsonGenerator.Options}.
+ * If the default options are sufficient consider using the static {@code
JsonOutput.toJson}
+ * methods.
+ *
+ * @see JsonGenerator.Options#build()
+ * @since 2.5
+ * @deprecated Use {@link groovy.json.DefaultJsonGenerator} instead.
+ */
+@Deprecated(since = "7.1", forRemoval = true)
+public class DefaultJsonGenerator extends groovy.json.DefaultJsonGenerator {
Review Comment:
Confirmed - on origin/8.0.x these three files (`StreamingJsonBuilder.java`,
`JsonGenerator.java`, `DefaultJsonGenerator.java`) do not exist. They were
deleted by Mattias's deprecation commit 23d0ae568f9 on 2026-03-12. The PR
re-introduces them via commit ff5d97284cf (`Restore grails.plugin.json.builder
deprecation shims for Groovy 5 build`).
The shims aren't there to be the entry point - they exist solely so that
`.gson` template AST output (which still references
`grails.plugin.json.builder.StreamingJsonBuilder` from the
JsonViewWritableScript code-gen path) compiles without an `unresolved class`
error during the Grails-views-gson test suite. The actual
`StreamingJsonBuilder` we want at runtime is
`groovy.json.StreamingJsonBuilder`, but Groovy 5 made the inner
`StreamingJsonDelegate` package-private, so naked `StreamingJsonDelegate`
references from compiled `.gson` templates fail to resolve.
The correct cleanup direction here is to update
`JsonViewWritableScript.groovy` (the template-AST emitter) to qualify all
`StreamingJsonBuilder` references at the FQN `groovy.json.StreamingJsonBuilder`
and stop synthesising the Grails inner-delegate alias. That removes the need
for these shims entirely and matches what was done on `8.0.x`. I'll do that in
a follow-up commit on this PR and resolve this thread once the shims are
deleted again. Leaving open until the JsonViewWritableScript change lands.
--
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]
