Skip to content

Register the data binding plugin#15518

Open
matrei wants to merge 2 commits into7.1.xfrom
matrei/register-databinding-plugin
Open

Register the data binding plugin#15518
matrei wants to merge 2 commits into7.1.xfrom
matrei/register-databinding-plugin

Conversation

@matrei
Copy link
Contributor

@matrei matrei commented Mar 19, 2026

The data binding plugin was no longer registered
as a plugin since it became a Spring AutoConfiguration.

The data binding plugin was no longer registered
as a plugin since it became a Spring AutoConfiguration.
@testlens-app

This comment has been minimized.

@testlens-app
Copy link

testlens-app bot commented Mar 19, 2026

✅ All tests passed ✅

🏷️ Commit: 0ee0a7e
▶️ Tests: 42572 executed
⚪️ Checks: 35/35 completed


Learn more about TestLens at testlens.app.

*/
class DataBindingGrailsPlugin extends Plugin {

def version = GrailsUtil.getGrailsVersion()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you have a dependency on this plugin? I had intentionally removed it in 2b46cdb because it didn't configure anything anymore. I'm trying to understand why we would care that this is a plugin?

@bito-code-review
Copy link

The PR refactors the databinding plugin by removing the abstract class with configuration logic and replacing it with a minimal Plugin subclass. This keeps it as a Grails plugin for framework loading and modularity, while moving constants to DataBindingConfigurationProperties. No direct dependency remains, as tests now inline the values.

Copy link
Contributor

@jamesfredley jamesfredley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only question is why the empty plugin shell needs to exist.

@matrei
Copy link
Contributor Author

matrei commented Mar 19, 2026

I think we should be consistent in how we deal with modules that have plugin "traits" vs other modules. In this case, the module is registering beans in the application context (as it always has). Also, being a plugin will make it available in the plugin manager.

@jdaugherty
Copy link
Contributor

The issue with the plugin is you can't load "before" that plugin since its beans, etc will be loaded prior, so we're only adding a plugin to show in the modules.

@matrei
Copy link
Contributor Author

matrei commented Mar 20, 2026

The issue with the plugin is you can't load "before" that plugin since its beans, etc will be loaded prior, so we're only adding a plugin to show in the modules.

Yes, but I still think its valuable to have all "plugin" modules listed in the plugin manager.
Otherwise we have a third kind of module dependency to reason about.

@jdaugherty
Copy link
Contributor

Yes, but I still think its valuable to have all "plugin" modules listed in the plugin manager. Otherwise we have a third kind of module dependency to reason about.

My only worry is now you have a plugin that people think they can load before, evict, etc and yet they cannot. We have other libraries that do not have plugins in them but add significant functionality (look at grails-web-common). It would be nice as part of any re-architecture of the plugins if we could address this.

@matrei
Copy link
Contributor Author

matrei commented Mar 21, 2026

My only worry is now you have a plugin that people think they can load before, evict, etc and yet they cannot. We have other libraries that do not have plugins in them but add significant functionality (look at grails-web-common). It would be nice as part of any re-architecture of the plugins if we could address this.

  • Does bean registration have to correlate with plugin load ordering, or can we let the features available for the Spring context handle that for us? @Conditional*, @Lazy, ObjectProvider<T> etc.
  • What would be the use case for loading before dataBinding?
  • Is there a use case nowadays for having one plugin evict another? I see this as problematic in conjunction with @Configuration as beans will already be registered when eviction starts. Doesn't it make more sense to just remove the plugin dependency in the application. Maybe we can stop startup with a message if "evicted" (incompatible) plugins are on the classpath?
  • I don't see any plugin "traits" in grails-web-common. (I may be missing something)

Plugin "traits" as I see them (one or more of):

  • Grails artefacts (controllers, url-mappings, services, taglibs, cli
    commands etc.)
  • Beans registered in the application context
  • Application resources
  • Lifecycle event handlers
  • Trait injectors*
  • Groovy Extension Modules*
  • Metadata
    • Title
    • Description
    • Author
    • Grails version compatibility range
    • License
    • Documentation url
    • Scm details

* This is not a plugin trait in and of itself, but can be part of a plugin

@jdaugherty
Copy link
Contributor

* Does bean registration have to correlate with plugin load ordering, or can we let the features available for the Spring context handle that for us? `@Conditional*`, `@Lazy`, `ObjectProvider<T>` etc.

@matrei The existing architecture supports loading the beans in the order defined by the plugin load order. So yes.

* What would be the use case for loading before `dataBinding`?

Does that matter? The issue is defining a plugin implies that one can load before/after it. This isn't supported currently.

* Is there a use case nowadays for having one plugin evict another? I see this as problematic in conjunction with `@Configuration` as beans will already be registered when eviction starts. Doesn't it make more sense to just remove the plugin dependency in the application. Maybe we can stop startup with a message if "evicted" (incompatible) plugins are on the classpath?

Isn't this debating the architecture of the plugins (I agree they need reworked). I'm just going for a consistent behavior.

* I don't see any plugin "traits" in `grails-web-common`. (I may be missing something)

That library defines a grails.factories file which is often shipped a long side plugins since it's used to populate GrailsArtifacts.

Plugin "traits" as I see them (one or more of):

* Grails artefacts (controllers, url-mappings, services, taglibs, cli
  commands etc.)

* Beans registered in the application context

* Application resources

* Lifecycle event handlers

* Trait injectors*

* Groovy Extension Modules*

* Metadata
  
  * Title
  * Description
  * Author
  * Grails version compatibility range
  * License
  * Documentation url
  * Scm details
  • This is not a plugin trait in and of itself, but can be part of a plugin

Unfortunately, we moved bean wiring to @Configuration and as a result I think we really need to have a discussion about formalizing the plugin architecture in 8. I agree with you on the plugin traits. I think we need to define a hard contract / interface for Grails Plugins and document these behaviors.

@matrei
Copy link
Contributor Author

matrei commented Mar 22, 2026

Does bean registration have to correlate with plugin load ordering, or can we let the features available for the Spring context handle that for us? @conditional*, @lazy, ObjectProvider etc.

The existing architecture supports loading the beans in the order defined by the plugin load order. So yes.

This is kind of muddled waters with modern Spring conventions.
I think we moved some bean registration to @Configuration for a reason with Spring Boot 3, and this move decoupled bean registration from plugin load order. There may be workarounds we can put in place for 8 or 9 where we bring this back, but for 7 we are where we are.

The issue with the plugin is you can't load "before" that plugin since its beans, etc will be loaded prior, so we're only adding a plugin to show in the modules.

What would be the use case for loading before dataBinding?

Does that matter? The issue is defining a plugin implies that one can load before/after it. This isn't supported currently.

We have multiple other plugins in grails-core using @AutoConfiguration that suffer the same issue. If there is no use case to load before dataBinding, why break the convention?

Is there a use case nowadays for having one plugin evict another? I see this as problematic in conjunction with @configuration as beans will already be registered when eviction starts. Doesn't it make more sense to just remove the plugin dependency in the application. Maybe we can stop startup with a message if "evicted" (incompatible) plugins are on the classpath?

Isn't this debating the architecture of the plugins (I agree they need reworked). I'm just going for a consistent behavior.

It was an open question. I don't have the answer, maybe someone else does.

I don't see any plugin "traits" in grails-web-common. (I may be missing something)

That library defines a grails.factories file which is often shipped a long side plugins since it's used to populate GrailsArtifacts.

I wouldn't consider this a plugin "trait" in an of itself.
As you say, there are multiple modules having a grails.factories file.

I'm trying to understand why we would care that this is a plugin?

In my mind:

  1. It has historically been a plugin.
  2. Keeping it consistent what a plugin is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants