Re: [PR] 8.0.x - make configurable: allow-bean-definition-overriding / allow-circular-references [grails-core]

["codeconsole (via GitHub)" <gi...@apache.org>]

codeconsole commented on code in PR #15765:
URL: https://github.com/apache/grails-core/pull/15765#discussion_r3502716876


##########
grails-testing-support-core/src/main/groovy/org/grails/testing/GrailsApplicationBuilder.groovy:
##########
@@ -166,11 +167,15 @@ class GrailsApplicationBuilder {
             ((AnnotationConfigRegistry) context).register(ClassUtils.forName(it, classLoader))
         }
 
-        def beanFactory = (context.beanFactory as DefaultListableBeanFactory).tap {
-            allowBeanDefinitionOverriding = true
-            allowCircularReferences = true
-        }
+        def beanFactory = context.beanFactory as DefaultListableBeanFactory
         prepareContext(context, beanFactory)
+        // Bean definition overriding and circular references default to true (the historical Grails
+        // behavior) but can be turned off via the standard spring.main.* properties. These are read
+        // after prepareContext so that application.yml (loaded by ConfigDataApplicationContextInitializer)
+        // and other property sources are available, and before refresh so the values take effect.
+        def environment = context.environment

Review Comment:
   Good catch — you're right that the placement was off. The flags were set after `prepareContext`, so the auto-config class registrations and the Grails post-processor bean definition had already been registered; `allow-bean-definition-overriding` only governed the refresh-time registrations, not the earlier ones.
   
   I've reordered `createMainContext` so the environment is initialized (via `ConfigDataApplicationContextInitializer`) and the `spring.main.*` flags are set on the bean factory immediately after the context is created — before any bean definitions are registered and before `refresh()`. So the settings now govern the whole context lifecycle.
   
   (For the circular-references half specifically: no application beans were being instantiated before `refresh()` — `prepareContext` only registered definitions plus one pre-built singleton via `registerSingleton` — but registering the definitions first was still the wrong order, which is what your comment flagged.)
   
   Verified green across the harness consumers with the reorder: `grails-testing-support-core`, `grails-databinding`, `grails-fields`, `grails-core`, and `grails-test-suite-web`.



-- 
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