jdaugherty commented on code in PR #15939:
URL: https://github.com/apache/grails-core/pull/15939#discussion_r3547923444


##########
grails-core/src/main/groovy/org/grails/plugins/DefaultGrailsPlugin.java:
##########
@@ -391,13 +391,17 @@ public void 
doWithRuntimeConfiguration(RuntimeSpringConfiguration springConfig)
         b.setVariable("resolver", getResolver());
 
         if (plugin instanceof Plugin) {
-            Closure c = ((Plugin) plugin).doWithSpring();
+            Plugin pluginObject = (Plugin) plugin;
+            BeanBuilder bb = new BeanBuilder(getParentCtx(), springConfig, 
grailsApplication.getClassLoader());
+            bb.setBinding(b);
+            // Legacy closure-returning hook: doWithSpring() returns a 
bean-defining closure
+            Closure c = pluginObject.doWithSpring();
             if (c != null) {
-                BeanBuilder bb = new BeanBuilder(getParentCtx(), springConfig, 
grailsApplication.getClassLoader());
-                bb.setBinding(b);
                 c.setDelegate(bb);
                 bb.invokeMethod("beans", new Object[]{c});
             }
+            // Method-based hook: doWithSpring(BeanBuilder) registers beans 
directly against the builder
+            pluginObject.doWithSpring(bb);

Review Comment:
   Don't we want to error if they define both?  shouldn't we document that the 
closure form is legacy and will be removed in a future version?  You can output 
a warning even.



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

Reply via email to