Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt a spring boot Condition [SPR-16065] #20614

Closed
spring-issuemaster opened this issue Oct 12, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Oct 12, 2017

pas filip opened SPR-16065 and commented

I would like to propose that some or all of the implementations of org.springframework.context.annotation.Condition
from spring boot be adopted by spring core.

Specifically:

  • OnBeanCondition
  • OnClassCondition
  • OnExpressionCondition
  • OnJavaCondition
  • OnJndiCondition
  • OnPropertyCondition
  • OnResourceCondition

Optionally:

  • OnWebApplicationCondition

These are conditions that would add nice value to the spring core framework.
I am aware that spring boot is opinionated but I would argue that these features make sense to exist in spring core so it's users can benefit from it.
I'm not sure if it was a conscious design decision to not include these in spring core originally or if things just evolved naturally within the spring boot project.

Currently I have only three options, none of which I like, which are:

  1. Roll my own implementation for every project I'm confronted with that uses spring framework without spring boot.
  2. Import the spring boot autoconfigure module into my projects.
  3. Migrate my projects to spring boot.

The reasons why I don't like these options are:

  1. Rolling my own just doesn't seem very dry. Feels a lot like reinventing the wheel. :)
  2. The spring boot autoconfigure seems to be a rather large module. Feels rather heavy. :)
  3. A viable option; but feels like a large hurdle to jump to just use those condition features. :)

It looks like the package org.springframework.boot.autoconfigure.condition would be a good candidate for a separate module.

I've also created a JIRA issue in the spring boot project for this. (See issue Create a separate module for generic conditions)
If extracted into a small module the barrier to importing it would already be greatly reduced.
Which would make the 2nd option already much more viable although it feels like an omission in spring core.

Ideally though I think these implementations would sit better in spring core or perhaps a new module along the lines of spring core autoconfigure or spring core condition.
Unfortunately convention would bid the package name be changed and here I'd propose the following:

  • Introduce code into spring core so users of spring core and boot can start using these "new" spring core features.
  • Deprecate the existing spring boot code.
  • On the next major release version of spring boot remove the existing implementations. This would be a big breaking change although easy to refactor/migrate.

Affects: 4.3.12, 5.0 GA

Issue Links:

  • #15592 Port @ConditionalOn... from Spring Boot into Spring Core ("duplicates")
  • #17835 Adding ConditionalOnProperty to spring-core ("duplicates")
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 12, 2017

Brian Clozel commented

This has been discussed already in several issues, including #17835 and #15592.

While some conditions could be considered for Spring Framework, quite a few of those depend on opinions and constraints held in Spring Boot. We can't enforce those in all Spring Framework applications, so such Conditions could come with some important limitations.

While I agree option 1) isn't wise, 2) wouldn't work anyway because of those limitations. Now if you make the appropriate changes to your application to make it work anyway, turning your app in a Spring Boot app would only mean deleting configuration and refactoring a bit your build file. You'd achieve 3).

Phil's comment just gives one example of that. Conditions like @OnWebApplicationCondition really depend on Spring Boot classes anyway.

Please comment/subscribe on the other issues.
Thanks!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 12, 2017

pas filip commented

I understand the concerns for not migrating all of the conditions although in the proposal I made I singled out the condtions that don't have any of these issues.
I think there is room for consideration which conditions would be appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.