You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/07/04 17:49:05 UTC

[PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

jamesfredley opened a new pull request, #15824:
URL: https://github.com/apache/grails-core/pull/15824

   Fixes #14915
   
   ## Summary
   
   - Lazily initialize BeanBuilder XML support so ordinary `resources.groovy` DSL evaluation does not create Spring XML reader/context infrastructure.
   - Preserve XML compatibility for `importBeans` with XML resources and Spring XML namespace DSL support through `xmlns`.
   - Enable `grails-spring` module test configuration and add regressions for plain DSL, XML import, and namespace support.
   - Document the Grails 8 compatibility behavior in the upgrade notes and Spring DSL guide.
   
   ## Verification
   
   - `JAVA_HOME=/home/james/.jdks/corretto-21 PATH=/home/james/.jdks/corretto-21/bin:$PATH ./gradlew :grails-spring:test --rerun-tasks`
   - `JAVA_HOME=/home/james/.jdks/corretto-21 PATH=/home/james/.jdks/corretto-21/bin:$PATH ./gradlew :grails-spring:codeStyle`
   - External BeanBuilder driver against built `grails-spring` runtime classpath: `PASS plain BeanBuilder DSL did not initialize XML support`
   - Review gate: Oracle and Codex reviewed staged hash `f1e6f82bdd5a4035a8230306b3d519b03ac1a61a` as GREEN. Codex only flagged regenerated untracked `.omo/run-continuation` session metadata, which was not staged or committed.
   
   ## Full-suite notes
   
   - `./gradlew clean` completed successfully.
   - `./gradlew aggregateViolations --continue` was attempted and failed on unrelated existing compile errors:
     - `grails-validation/src/main/groovy/grails/validation/Validateable.groovy`: unable to resolve `groovy.transform.Virtual`
     - `grails-data-graphql/core/.../Arguable.groovy` and `ComplexTyped.groovy`: static type checking cannot find `withDelegate(Closure, Object)`
   - `./gradlew :grails-test-report:check --continue` was attempted and exceeded the 20 minute tool timeout after running broad test output.
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4937119185

   ## βœ… All tests passed βœ…
   
   🏷️ Commit: 0f1cf52699879b69ba64affe893e979766c09314
   ▢️ Tests:  12228 executed
   βšͺ️ Checks: 59/59 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-5171532954

   ## βœ… All tests passed βœ…
   
   🏷️ Commit: afabbc5c2a4b797c1c47751962c0fc834c199b89
   ▢️ Tests:  56229 executed
   βšͺ️ Checks: 60/60 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4883385749

   ## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/15824?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `74.41860%` with `11 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 49.6016%. Comparing base ([`c88070e`](https://app.codecov.io/gh/apache/grails-core/commit/c88070e6c95441559d3a085b0a0405d0cdc72ae6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`9943118`](https://app.codecov.io/gh/apache/grails-core/commit/994311865b7d4c4d2c50c6d96c49c750b4b600be?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/15824?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...in/groovy/grails/spring/BeanBuilderXmlSupport.java](https://app.codecov.io/gh/apache/grails-core/pull/15824?src=pr&el=tree&filepath=grails-spring%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fspring%2FBeanBuilderXmlSupport.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLXNwcmluZy9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL3NwcmluZy9CZWFuQnVpbGRlclhtbFN1cHBvcnQuamF2YQ==) | 80.6452% | [4 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/15824?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ing/src/main/groovy/grails/spring/BeanBuilder.java](https://app.codecov.io/gh/apache/grails-core/pull/15824?src=pr&el=tree&filepath=grails-spring%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fspring%2FBeanBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLXNwcmluZy9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL3NwcmluZy9CZWFuQnVpbGRlci5qYXZh) | 58.3333% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/15824?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   [![Impacted file tree graph](https://app.codecov.io/gh/apache/grails-core/pull/15824/graphs/tree.svg?width=650&height=150&src=pr&token=qJrryhpk7u&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/grails-core/pull/15824?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##                8.0.x     #15824        +/-   ##
   ==================================================
   + Coverage     49.5810%   49.6016%   +0.0206%     
   - Complexity      16695      16710        +15     
   ==================================================
     Files            1947       1948         +1     
     Lines           92598      92626        +28     
     Branches        16179      16179                
   ==================================================
   + Hits            45911      45944        +33     
   + Misses          39589      39577        -12     
   - Partials         7098       7105         +7     
   ```
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/15824?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ” | |
   |---|---|---|
   | [...ing/src/main/groovy/grails/spring/BeanBuilder.java](https://app.codecov.io/gh/apache/grails-core/pull/15824?src=pr&el=tree&filepath=grails-spring%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fspring%2FBeanBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLXNwcmluZy9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL3NwcmluZy9CZWFuQnVpbGRlci5qYXZh) | `53.6000% <58.3333%> (+0.6900%)` | :arrow_up: |
   | [...in/groovy/grails/spring/BeanBuilderXmlSupport.java](https://app.codecov.io/gh/apache/grails-core/pull/15824?src=pr&el=tree&filepath=grails-spring%2Fsrc%2Fmain%2Fgroovy%2Fgrails%2Fspring%2FBeanBuilderXmlSupport.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLXNwcmluZy9zcmMvbWFpbi9ncm9vdnkvZ3JhaWxzL3NwcmluZy9CZWFuQnVpbGRlclhtbFN1cHBvcnQuamF2YQ==) | `80.6452% <80.6452%> (ΓΈ)` | |
   
   ... and [5 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/15824/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4929163710

   ## βœ… All tests passed βœ…
   
   🏷️ Commit: 5b3d00464804129eead9e883a47d430c3bdecd52
   ▢️ Tests:  4713 executed
   βšͺ️ Checks: 59/59 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#discussion_r3591420330


##########
grails-spring/src/main/groovy/grails/spring/BeanBuilder.java:
##########
@@ -234,11 +218,11 @@ public void xmlns(Map<String, String> definition) {
 
             Assert.notNull(uri, "Namespace definition cannot supply a null URI");
 
-            final NamespaceHandler namespaceHandler = namespaceHandlerResolver.resolve(uri);
+            final NamespaceHandler namespaceHandler = getXmlSupport().resolveNamespaceHandler(uri);
             if (namespaceHandler == null) {
                 throw new BeanDefinitionParsingException(
                       new Problem("No namespace handler found for URI: " + uri,
-                            new Location(readerContext.getResource())));
+                            new Location(getXmlSupport().getReaderContext(beanBuildResource).getResource())));

Review Comment:
   Confirmed already addressed on this branch. The unknown-namespace error path builds its `Location` directly from the already-available `beanBuildResource` (`new Problem("No namespace handler found for URI: " + uri, new Location(beanBuildResource))`), so it does **not** call `getXmlSupport().getReaderContext(...)` and therefore never forces `XmlBeanDefinitionReader` / unrefreshed-context initialization on the error path. The regression test `testUnknownNamespaceDoesNotInitializeXmlReaderForErrorLocation` locks this in by making context initialization throw and asserting the namespace-handler error still surfaces. Merged the latest `8.0.x` as well.
   



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4986987375

   ## βœ… All tests passed βœ…
   
   🏷️ Commit: a3b910951c21f8e2404e6b5bedc2d1bf5c2ccaa2
   ▢️ Tests:  46898 executed
   βšͺ️ Checks: 59/59 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4986244181

   ## Review feedback addressed
   
   Merged the latest `8.0.x` and pushed.
   
   **Copilot review comment (unknown-namespace error path initializing XML reader/context):** already satisfied on this branch. The `"No namespace handler found for URI: ..."` error builds its `Location` directly from the already-available `beanBuildResource` (`new Location(beanBuildResource)`), so it does **not** call `getXmlSupport().getReaderContext(...)` and never forces `XmlBeanDefinitionReader` / unrefreshed-context initialization in the exception path. The regression test `testUnknownNamespaceDoesNotInitializeXmlReaderForErrorLocation` locks this in by making context initialization throw and asserting the namespace-handler error still surfaces.
   
   **Follow-up (documented, not changed here):** the lazy XML refactor means `setClassLoader` no longer dispatches to the protected `initializeBeanBuilderForClassLoader` hook. No in-repo `BeanBuilder` subclass overrides that hook, so this only affects hypothetical external subclasses; noting it here in case we want to preserve the hook dispatch in a follow-up.
   
   Local verification: `:grails-spring:test --tests BeanBuilderTests` passes.
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#discussion_r3523623388


##########
grails-spring/src/main/groovy/grails/spring/BeanBuilder.java:
##########
@@ -234,11 +218,11 @@ public void xmlns(Map<String, String> definition) {
 
             Assert.notNull(uri, "Namespace definition cannot supply a null URI");
 
-            final NamespaceHandler namespaceHandler = namespaceHandlerResolver.resolve(uri);
+            final NamespaceHandler namespaceHandler = getXmlSupport().resolveNamespaceHandler(uri);
             if (namespaceHandler == null) {
                 throw new BeanDefinitionParsingException(
                       new Problem("No namespace handler found for URI: " + uri,
-                            new Location(readerContext.getResource())));
+                            new Location(getXmlSupport().getReaderContext(beanBuildResource).getResource())));

Review Comment:
   In the error path for an unknown XML namespace, this code calls getXmlSupport().getReaderContext(...) just to obtain a Resource for Location. Creating the XmlReaderContext will also lazily create the XmlBeanDefinitionReader, which in turn forces springConfig.getUnrefreshedApplicationContext() initialization. That adds unnecessary work in an exception path and can mask the intended "No namespace handler found" error if context initialization fails.
   
   You can use the already-available beanBuildResource directly when constructing Location to avoid initializing XML reader/context infrastructure for this error message.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4883541646

   ## βœ… All tests passed βœ…
   
   🏷️ Commit: 994311865b7d4c4d2c50c6d96c49c750b4b600be
   ▢️ Tests:  47645 executed
   βšͺ️ Checks: 45/45 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-5208595820

   I'm not sure why this was merged. We need to revert it until this can be reviewed as I did not agree with this PR.  
   
   The premise doesn't hold up
   
   Issue #14915 (type: blocker) says BeanBuilder "is ultimately backed by xml" and needs reworking. That was never true for the common path. In the pre-merge BeanBuilder.java, the XML fields are referenced in exactly four places:
   
   - 161, 171–174 β€” construction
   - 226, 237, 241 β€” xmlns
   - 955 β€” createDynamicElementReader (the namespace DSL)
   
   Ordinary bean(Foo) { ... } in resources.groovy went through springConfig.addBeanDefinition / programmatic BeanDefinition all along. The XML reader was constructed and then never used unless you wrote xmlns or imported a .xml.
   
   It doesn't reduce the XML coupling
   
   importBeans(*.xml) still uses XmlBeanDefinitionReader. xmlns still uses DefaultNamespaceHandlerResolver + XmlReaderContext + BeanDefinitionParserDelegate. They moved into BeanBuilderXmlSupport and are built on first use. If Spring actually drops those classes, Grails is in exactly the same spot.
   
   And there's no pressure today β€” I checked the jars on the classpath:
   
   β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
   β”‚              Class              β”‚ spring-beans 7.0.8 β”‚
   β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
   β”‚ XmlBeanDefinitionReader         β”‚ not deprecated     β”‚
   β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
   β”‚ DefaultNamespaceHandlerResolver β”‚ not deprecated     β”‚
   β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
   β”‚ BeanDefinitionParserDelegate    β”‚ not deprecated     β”‚
   β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€
   β”‚ XmlReaderContext                β”‚ not deprecated     β”‚
   β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜
   
   Same for MvcNamespaceHandler in spring-webmvc 7.0.8. Nothing was breaking or even warning.
   
   My BeanRegistrar question was never answered β€” the author's reply pivoted to a comparison table against #16019 and argued "land this for Spring 7 BeanBuilder compatibility," but there was no compatibility break to fix. BeanRegistrar is the rework #14915 asked for and it isn't here.
   
   What it actually buys
   
   Real but small: BeanBuilder construction no longer forces getUnrefreshedApplicationContext() β†’ initialiseApplicationContext(), so a BeanBuilder that never builds a context no longer creates a GrailsApplicationContext. It also cleans up a latent bug where setClassLoader(null) passed the raw null param to the reader while the resolver got the defaulted field.
   
   What I'd have flagged
   
   Broken protected extension hook. initializeSpringConfig() and setClassLoader() no longer dispatch to protected initializeBeanBuilderForClassLoader(ClassLoader). It survives as dead code that now forces XML init if a subclass calls it β€” inverting the PR's own intent. grails.spring.BeanBuilder is public API. WebBeanBuilder is the only in-repo subclass and doesn't override it, but external plugin subclasses silently stop being called. The author documented this in the PR body as a "follow-up" instead of fixing it.
   
   Unremarked behavior change in importBeans. The new helper adds beanReader.setBeanClassLoader(classLoader), which the old code did not do. That switches XML-imported definitions from deferred beanClassName resolution to eager class resolution at import time, using BeanBuilder's classloader rather than the target factory's. The new test asserts createBeanDefinition('xmlBean').beanClass == String, which only passes because of that change β€” so the test enshrines it. Not mentioned anywhere in the PR description.
   
   Duplicate test FQN. The new grails-spring/src/test/groovy/grails/spring/BeanBuilderTests.groovy collides by fully-qualified name with the existing 1002-line grails-test-suite-uber/src/test/groovy/grails/spring/BeanBuilderTests.groovy β€” which already covers the refactored paths (testImportSpringXml, testUseSpringNamespaceAsMethod, testUseTwoSpringNamespaces, testSpringNamespaceBean). Per CLAUDE.md #11 that suite should have been extended, not shadowed. Two grails.spring.BeanBuilderTests in aggregate reports is a maintenance hazard.
   
   Docs oversell it. whatsNew and upgrading80x now tell users Grails 8 "keeps the resources.groovy BeanBuilder DSL backward compatible on Spring Framework 7," implying a Spring 7 problem was solved. Nothing was broken, and XML is still how xmlns works.
   
   Merged without human approval. reviewDecision is empty β€” the only reviews are Copilot (COMMENTED) and the author's own comment. Per CONTRIBUTING, "all other changes" require 1 reviewer, review-then-commit.
   
   Suggested follow-up
   
   Reopen #14915 (or file the BeanRegistrar work fresh) since the blocker is closed but unaddressed, and a small follow-up to restore the initializeBeanBuilderForClassLoader dispatch, de-dupe the test class name, and either revert or document the setBeanClassLoader change. Want me to draft that issue text or a post-merge review comment?


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4929264506

   FYI: spring started enforcing certain configuration like AMPQ / rabbit to be associated to an application context.  Why are we maintaining the link to the bean builder dsl that Spring has stated will no longer be supported?  Can't we adjust the beanDsl to use a BeanRegistrar instead which is the programmatic way Spring suggests using now? 


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-5123469858

   ## How this relates to #16019
   
   These two PRs are complementary, not alternatives.
   
   **This PR (#15824)** keeps the **existing** BeanBuilder path working cleanly on Spring 7. It moves Spring XML reader / namespace / parser setup behind a lazy helper so ordinary `resources.groovy` and programmatic BeanBuilder DSL registration no longer eagerly initialize XML infrastructure. XML `importBeans`, `xmlns`, and related paths still work, but only initialize XML support when those features are actually used. No application rewrite is required; public BeanBuilder behavior stays compatible.
   
   **#16019** is the **forward** registration model. It adds `@GrailsBeans`, which compiles a `beans = { bean(...) ... }` DSL into real `@AutoConfiguration` / `@Bean` methods (with Boot ordering and `@Conditional*` support), and moves several framework plugins onto that path plus `beanRegistrar()` where plugin runtime state is needed. It does **not** refactor `BeanBuilder` or fix the eager-XML issue this PR addresses, and it is not a drop-in replacement for evaluating existing `resources.groovy` / `doWithSpring` BeanBuilder DSLs.
   
   | | #15824 (this PR) | #16019 |
   | --- | --- | --- |
   | Goal | Compatibility fix for current BeanBuilder | New Spring-native bean authoring path |
   | Touches `BeanBuilder` | Yes (lazy XML) | No |
   | App `resources.groovy` | Keeps working unchanged | Not a substitute |
   | Plugin bean registration | Unchanged public DSL | `@GrailsBeans` / `beanRegistrar()` / generated autoconfig |
   | Scope | Small, focused | Large feature + framework conversions |
   
   **Recommendation:** land this PR for Spring 7 BeanBuilder compatibility; continue #16019 as the longer-term way to declare beans. Merging #16019 does not make this PR unnecessary while BeanBuilder / `resources.groovy` / `doWithSpring` still exist


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-5124275918

   ## βœ… All tests passed βœ…
   
   🏷️ Commit: c47fb1ebdb88083a86b21ce306504f00702dca10
   ▢️ Tests:  33193 executed
   βšͺ️ Checks: 59/59 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-4883294747

   <!-- Bito Reply -->
   The suggested change is correct and aligns with the goal of avoiding unnecessary initialization of Spring XML infrastructure in the error path. By using `beanBuildResource` directly when constructing the `Location` object, you bypass the need to call `getXmlSupport().getReaderContext(beanBuildResource)`, which would otherwise trigger the lazy initialization of the `XmlBeanDefinitionReader` and the associated `ApplicationContext`.
   
   This change effectively prevents the potential masking of the "No namespace handler found" error and improves performance in the exception path.
   
   **grails-spring/src/main/groovy/grails/spring/BeanBuilder.java**
   ```
   final NamespaceHandler namespaceHandler = getXmlSupport().resolveNamespaceHandler(uri);
               if (namespaceHandler == null) {
                   throw new BeanDefinitionParsingException(
                         new Problem("No namespace handler found for URI: " + uri,
                               new Location(beanBuildResource)));
               }
   ```


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-5208597957

   @borinquenkid will you please revert this merge and reopen this. 


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "borinquenkid (via GitHub)" <gi...@apache.org>.
borinquenkid merged PR #15824:
URL: https://github.com/apache/grails-core/pull/15824


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Keep BeanBuilder DSL independent of XML setup [grails-core]

Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #15824:
URL: https://github.com/apache/grails-core/pull/15824#issuecomment-5005615009

   ## βœ… All tests passed βœ…
   
   🏷️ Commit: a1b7309d20a9526a4931cb57ec68d09ae6e4d619
   ▢️ Tests:  12697 executed
   βšͺ️ Checks: 59/59 completed
   
   ---
   _Learn more about TestLens at [testlens.app](https://testlens.app)._
   


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org