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

Cannot declare a Spring MVC controller with @Bean (without @Controller) since version 6 #29650

Closed
reda-alaoui opened this issue Dec 6, 2022 · 8 comments
Labels
status: invalid An issue that we don't feel is valid

Comments

@reda-alaoui
Copy link

reda-alaoui commented Dec 6, 2022

Affects: 6.0.2

Since #22154 , it seems not possible anymore to declare a Controller class via an @Bean method.

Before we could do that:

@RequestMapping
class MyController {

}

@ConditionalOnSomething
@AutoConfiguration
class MyConfiguration {

  @Bean
  public MyController myController() {
     return new MyController();
  }
}

Now, because of

, the controller is not considered as a request mapping handler because it is missing @Controller .

But annotating it make it discoverable by component scan (enabled by the test classes for example), which prevents us from declaring the component conditionally.

Is there something I am missing here?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 6, 2022
@reda-alaoui
Copy link
Author

I think it relates to #28978

@bclozel
Copy link
Member

bclozel commented Dec 7, 2022

This change has been introduced in #22154.
I'm closing this issue as this is the expected behavior now.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Dec 7, 2022
@bclozel bclozel added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 7, 2022
@reda-alaoui
Copy link
Author

reda-alaoui commented Dec 7, 2022

@bclozel I know it has been introduced in #22154. I read #22154 before creating this issue.
But #22154 discussion was about many pre existing use cases that would break but did not cover controller conditional declaration. #22154 forgot this use case.

Can you please re-open this issue? It would at least allow someone to tell me that conditional controller declaration is supported via another technique or not supported anymore at all.

@bclozel
Copy link
Member

bclozel commented Dec 7, 2022

Conditional controller declaration is still supported. Adding @Controller to MyController should work. Note that component scanning auto-configuration packages should not be done for this very reason: components and configuration classes would be picked up when they're only meant to be discovered through the auto-configuration process.

@reda-alaoui
Copy link
Author

reda-alaoui commented Dec 7, 2022

To test my boot starter library, I need to declare a class like this in my test directory:

@SpringBootApplication
public class App {

  public static void main(String[] args) {
    new SpringApplication(App.class).run(args);
  }
}

@SpringBootApplication automatically enables component scanning on the library package. This leads to a duplicate MyController declaration (when MyController is annotated with @Controller) failing the tests bootstrap. How should I test my starter library?

@reda-alaoui
Copy link
Author

reda-alaoui commented Dec 7, 2022

Moreover, from a usability POV, #22154 creates a difference between @RequestMapping injectables and the other injectables. I can conditionally declare any injectable without annotating it with @Component, but now we have this special case where request mappers need to be indirectly annotated with @Component. Again it looks like #22154 didn't consider this use case.

@bclozel
Copy link
Member

bclozel commented Dec 7, 2022

@SpringBootApplication automatically enables component scanning on the library package. This leads to a duplicate MyController declaration (when MyController is annotated with @controller) failing the tests bootstrap. How should I test my starter library?

You need to organize your auto-configuration library and its testing support so that the auto-configuration packages are never scanned. This is called out in the Spring Boot reference documentation:

Auto-configurations must be loaded only by being named in the imports file. Make sure that they are defined in a specific package space and that they are never the target of component scanning. Furthermore, auto-configuration classes should not enable component scanning to find additional components. Specific @imports should be used instead.

As for

Moreover, from a usability POV, #22154 creates a difference between @RequestMapping injectables and the other injectables. I can conditionally declare any injectable without annotating it with @component, but now we have this special case where request mappers need to be indirectly annotated with @component. Again it looks like #22154 didn't consider this use case.

@RequestMapping has no specific meaning when it comes to application beans. Interfaces can be annotated with those and they're never considered in the application context. Only @Controller is meta-annotated to signal that controllers are meant to be components.

This is an important behavior change and I understand that this is quite inconvenient for you, but considering all the related issues and reasons listed in #22154 I don't think we're going to change this behavior back. Especially since the case you're reporting is invalid in the first place, since you were using this behavior without knowing it to work around the fact that the package+scanning arrangement of your library is not ideal. You could very much run into another issue that's related to scanning there without having controllers involved.

@reda-alaoui
Copy link
Author

reda-alaoui commented Dec 7, 2022

I don't think we're going to change this behavior back.

I am not asking for a revert. I think the notion of @Controller should be decoupled from @Component. That way, the behaviour of all injectables would remain the same without defeating the intent of #22154 . Similar to #28978 .

That way controllers would be declared like this by starter libraries:

@RequestMappingController
@RequestMapping
class MyController {

}

And like this for component scanning:

@Controller
@RequestMappingController
@RequestMapping
class MyController {

}

A meta anotation coupling @Controller with @RequestMappingController could be imagined to avoid the need of passing 3 annotations for the second example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants