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

Adding ConditionalOnProperty to spring-core [SPR-13244] #17835

Closed
spring-projects-issues opened this issue Jul 16, 2015 · 9 comments
Closed

Adding ConditionalOnProperty to spring-core [SPR-13244] #17835

spring-projects-issues opened this issue Jul 16, 2015 · 9 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 16, 2015

Meang Akira Tanaka opened SPR-13244 and commented

This issue seems to be a variant of #16372, however I have created a new feature request as the description is not exactly the same thing.

I would like to suggest that beans can be enabled based upon whether some property or a value is set or not set in the Environment of the application context.

The concrete usage which I have is somewhat related to SWS-911, where a URL connection factory has to be created based upon proxy settings in the environment, however I can think that there must be other valid usage scenarios, where beans should be registered based on some settings in the environment.

Currently the spring core already has a limit support for Conditional Bean registration, however this is based on the profile.

It has been argued in #16372 that this feature exists in Spring-boot, however it is not necessary the case that one uses spring-boot.

As an alternative to provide an implementation I would like to suggest that ConditionalOnXX are moved to Spring-core from Spring-boot.


Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 26, 2015

Meang Akira Tanaka commented

Hi Stephane

I would like to work on this issue, but before I start to work on this issue I would like to know if the basic principle of moving @Conditional to Spring-core seems like a bad idea or not? i.e. can you already tell whether it will rejected.
since it seems like you are the only one who have an opinion about it in #16372

BR

Meang

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Hey Meang,

I don't think I am the only one having an opinion about it. I believe that @ConditionalOnExpression is fine per se but we need to be careful as when we do the move as we first need a proper deprecated period before actually removing the code that we moved.

@ConditionalOnProperty is really different since it has this notion of prefix that implies relaxed binding. I am not keen on moving relaxed binding in Spring core. In the end, I am not convinced at all that moving those bits is worth it since nothing should prevent you (in theory) to use Spring Boot for that. This is a new feature after all.

Thoughts?

@spring-projects-issues
Copy link
Collaborator Author

Meang Akira Tanaka commented

Hey Stephane

I haven't work with spring and spring-boot for more than a year, so bare with my lack of understanding of spring.

As far I understand Spring-core is more or less a set of components that can be assembled together in arbitrary fashion and form any form of application architecture that one sees fit, however spring-boot as I understand it is an opionated application architecture and therefore more locked on how certain things should be putted together.

In that context and IMHO it seems for me more natural that @ConditionalXX to be part of spring-core rather than spring-boot, as @ConditionalXX may be useful in order to configure certain behaviour in the application.

You are right that one can pull spring-boot into the application, but it seems for me like a big dependency for using one small feature.

I am not keen on moving relaxed binding in Spring core
Is that because relaxed binding is opionated. Maybe I am unsure why you are not keen on that?

This is a new feature after all.
Does that means that there is a feature freeze on spring-core?

My issue stems from an issue I had while working on a project, where it was needed to pass through a proxy when using spring-ws under certain circumstances, which was when working in certain environments.

In order to do that I could have chosen to use xml configuration and then alter it as it went through different environment, however it seemed more elegant for me to let the configuration of a proxy decide whether spring-ws needed to be injected with a connection that should go through a proxy or not, and thereby avoiding that the proxy setting being configured one place and the injection of the connection being configured another place.

Personally I am more fan of JavaConfig as they provide a tighter binding to the classes being wired together and makes refactoring easier, however JavaConfig has one big weakness, which is that it needs to be recompiled if one needs to alter the application context, which I would like to avoid when passing through the environments while developing.

I know that Spring is a framework that needs to cater for all kinds of need and one should not see it from one single use case, however I just though that it would be beneficial to be able to use JavaConfig while still have the flexibility of altering the behavior through the environment

What do you think?

Have a nice day

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

You are right that one can pull spring-boot into the application, but it seems for me like a big dependency for using one small feature.

How big is that exactly? Have you tried? We do have a problem of having all our auto-configuration code in spring-boot-autoconfigure, granted but I wouldn't call that a "big dependency" either.

I am not keen to move the relaxed binding because it is very opinionated and we are still working on it right now. Even if we feel that moving that code is the right move, I don't see how it could happen until we have finalized those improvements. Back to your original request, the contract of @ConditionalOnProperty is heavily based on relaxed binding. The prefix attribute is typically what I am talking about. It makes no sense to move @ConditionalOnProperty in its current form without the relaxed binding.

Please don't twist my words. Of course there isn't a code freeze on spring-core, where does that come from? Spring Framework continues to do what it ever did and Spring 5 will be packed with tons of new features. My point is that Spring Framework is not opinionated (that's what Spring Boot does). So if you want a new feature that is opinionated, I don't see the requirement of using Spring Boot as a problem. If you are in a situation where you can't or won't upgrade an existing app, then you won't be able to use that new feature. That's all I am saying.

I also think that if you absolutely don't want to move to Spring Boot (even in a minimal form), nothing prevents you from writing your own condition, even as an exercice or as a prototype. Since you know your requirements, it might be surprisingly straightforward.

@spring-projects-issues
Copy link
Collaborator Author

Meang Akira Tanaka commented

How big is that exactly? Have you tried? We do have a problem of having all our auto-configuration code in spring-boot-autoconfigure, granted but I wouldn't call that a "big dependency" either.

I haven't really tried and I should have tried before coming with such a bold statement. sorry for that, what I meant was it is an additional dependency.

I am not keen to move the relaxed binding because it is very opinionated and we are still working on it right now. Even if we feel that moving that code is the right move, I don't see how it could happen until we have finalized those improvements. Back to your original request, the contract of @ConditionalOnProperty is heavily based on relaxed binding. The prefix attribute is typically what I am talking about. It makes no sense to move @ConditionalOnProperty in its current form without the relaxed binding.

Ok I get your argument. I'll wait until it will be finalized and check the result out :)

Please don't twist my words. Of course there isn't a code freeze on spring-core, where does that come from?

I apologize if you think I was trying to twist your words, I think I misread the sentence you wrote in the previous reply: "this is a new feature after all" and I thought it implied that therefore it should be in spring-boot.

I also think that if you absolutely don't want to move to Spring Boot (even in a minimal form), nothing prevents you from writing your own condition, even as an exercice or as a prototype. Since you know your requirements, it might be surprisingly straightforward.

You are right. I already implemented one :) and it was quite straightforward once you know what needs to be implemented :)

Anyway I guess it concludes this JIRA ticket... thanks for taking you time to look into it and again I apologize if it seemed like I twisted your words.

Have a nice day

Meang

@spring-projects-issues
Copy link
Collaborator Author

Stéphane Nicoll commented

Thanks for the discussion Meang.

@spring-projects-issues
Copy link
Collaborator Author

Meang Akira Tanaka commented

Hi Stephane

I just saw that Juergen changed the fix version from General Backlog to 5.0 Backlog.

Should I understand that this feature will appear in the next major version of Spring?

If that is the case if you need some extra hands to do something about it I am willing to provide those extra pair of hands.

Just let me know what needs to be done...

br

Meang

@sdeleuze
Copy link
Contributor

Related to #15920, not sure yet if that's a duplicate or not.

@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Jul 26, 2021
@jhoeller jhoeller modified the milestones: 6.x Backlog, General Backlog Dec 23, 2023
@snicoll
Copy link
Member

snicoll commented Jan 2, 2024

It is yes. I think we should bring these somewhat consistently if we ever decided to do so.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Jan 2, 2024
@snicoll snicoll removed this from the General Backlog milestone Jan 2, 2024
@snicoll snicoll added the status: duplicate A duplicate of another issue label Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants