They could live in org.springframework.boot.autoconfigure.web
Comment From: philwebb
https://github.com/philwebb/spring-boot/tree/gh-14939
Comment From: restolho
I'll take this
Comment From: restolho
@philwebb was checking this but found an issue. Both classes have dependencies on OnWebApplicationCondition.java and that one has dependencies on a few more ... it's a snow ball.
Should i just move the classes and make the OnWebApplicationCondition.java public? Do you have any sugestion?
Comment From: wilkinsona
Thanks for the offer, but @philwebb already has this one. Please take a look at the changes in the branch that he linked to in the comment about yours if you are interested.
Comment From: restolho
Oh sorry. Since it was "open" assumed it was still on the todo list. Thanks @wilkinsona
Comment From: wilkinsona
It’s just waiting to be finished off and then merged once we’ve started work on a new minor release. If you are looking for something to contribute, issues labelled ideal-for-contribution are hopefully a good place to start.
Comment From: restolho
@wilkinsona Thanks for the hint. Will check issues with that label.
Comment From: snicoll
@philwebb what is the status of that branch? Is it ready to be merged?
Comment From: philwebb
I can't remember to be honest. I'll have to review it again.
Comment From: snicoll
In the meantime we've had the war deployment conditions that need to move as well. I've rebased Phil's branch and I am looking into it.
Comment From: snicoll
I've been rebasing the original commit from 2018 (ugh) and I am now of the opinion that we should create an org.springframework.boot.autoconfigure.web.condition package with the following conditions:
ConditionalOnEnabledResourceChain(from the root package)ConditionalOnNotWebApplication(from o.s.autoconfigure.condition)ConditionalOnWebApplication(from o.s.autoconfigure.condition)ConditionalOnMissingFilterBean(from the servlet sub-package)ConditionalOnNotWarDeployment(from o.s.autoconfigure.condition)ConditionalOnWarDeployment(from o.s.autoconfigure.condition)
Here are some of the things I've noticed reviewing the current state of affairs:
- Moving
ConditionalOnWebApplicationrequires to play with import controls asorg.springframework.boot.autoconfigure.web.is not allowed to import classes fromorg.springframework.boot.web. That wasn't the case for the original condition package so it might be a little bit artificial - I've tried to move
ConditionalOnNotWarDeploymentto the servlet package as it makes sense to move it there. Case in point we haveConditionalOnMissingFilterBeanthere already. However this condition is used inEmbeddedWebServerFactoryCustomizerAutoConfigurationwhich is in theembeddedpackage and the logical import direction is that the servlet package can import the embedded package, not the other way around.
While less precise I think moving those conditions in a mirror condition sub-package would prevent further problems like this going forward.