Spring @RestController on class is ignored when advised and spring.aop.proxy-target-class is false

Spring Version: 6.2.11 Spring Boot Version: 3.4.10 Minimal Reproduction: reproduction.zip

Use case

We generate OpenAPI interfaces and model classes from an OpenAPI spec. Previously our projects used swagger-codegen, but we are moving to openapi-generator, which happens to add an @Validated annotation on the generated interfaces, when adding any validation annotations to the generated code. These generated interfaces are implemented by our hand-made code - classes implementing @RestController or @Controller, depending on who implemented it. Our projects all use spring.aop.proxy-target-class=false.

The problem

The effect we see with the addition of this @Validated is that the new controllers stopped being registered as endpoints or having requests routed to them by Spring, even though they are annotated with these controller annotations.

Things tried

If we comment out the @Validated in the generated interfaces, the controllers work normally again.

We set a flag to the generator to add @Controller annotations to the interfaces, which brought the traffic to our controller class again, but it is being handled using @Controller mappings even when our implementation class is annotated with @RestController (for instance, if I have return type of String and the implementation returns "Hello world", we now get an error that no static resource exists for "Hello World").

If I switch spring.aop.proxy-target-class=true, the controllers work normally again.

If I annotate individual controller classes with @Scope(proxyMode = org.springframework.context.annotation.ScopedProxyMode.TARGET_CLASS) (which I think just switches that one bean's advice to use CGLIB proxies), it works normally.

By "normally", I mean that, where the fixes don't cause author issues, the implementation methods seem to be getting the same inputs from requests and producing the same responses as they did before.

None of these options are particularly attractive to us, because it seems like @RestController on the implementation class should be honored, regardless of whether it is being advised or not.

Why is this happening?

I traced through what was happening with a debugger, and here is what I think is happening:

1. Component scan picks up the implementation class and adds it to the context - as part of this, the @Validation is found and it's determined the bean needs to be wrapped in a proxy. Since I've indicated in properties to use JDK proxies where possible, it creates a JDK proxy using my interface, then adds the proxy to the Spring context.
2. The Web mapping code looks through beans for ones it should map to endpoints. On each one, it finds the type of bean it is. It then calls a method to determine if the type is a handler - if it is, it will proceed to process the types methods. In this case, the type is a proxy type, which it seems happy enough to accept.

4. The check for handler seems to be just looking for controller annotations using a "Hierarchy" search, which recursively looks for at direct annotations, interface annotations, and parent annotations. None of these directions account for proxy target annotations. The search stops as soon as matching annotations are found. In this case, that means as soon as it finds @Controller on the interface (if it has one), or not at all (if it doesn't have one).

Some ideas

Having spent some time on this, I have a few ideas of possible ways this could be addressed - of course I don't know if any of these are feasible:

1. In step 2, AbstractBeanFactory returns the type of the bean in context as being a proxy class. The code for this seems pretty simple:

I'm not sure how something would find it useful to see a random Proxy class as the type, besides just knowing it is a proxy, so I think maybe it would make sense for the proxy target type to be returned here in the case of a proxy. If not, at least processCandidateBean could find the target type if it is a proxy, and use that.

1. If the bean type in processCandidateBean is left as proxy, maybe the code which searches for controller annotations could use a different search method which is able to find annotations on target types and give them priority over interfaces they extend.

Objective/Summary

It seems like documentation I have found mostly suggests that @RestController on an implementation class should always override the one in the interface, and that the implementation class in this scenario would be the ideal location to have this annotation.

I think it is very confusing that this isn't the case with spring.aop.proxy-target-class=false.

Right now it seems like it is something that could be fixed without too much trouble, but of course do not know for sure and wanted to at least raise the issue.

Thanks in advance for any insights here! If it is a valid concern/issue and someone has some ideas on direction for a fix but no priority for it, I would also be interested in trying my hand at a pull request.

Comment From: Kronen

I think it is very confusing that this isn't the case with spring.aop.proxy-target-class=true.

Maybe you meant false?

Comment From: snydergd

@Kronen yes, you are right. I'll edit it. Thanks

Comment From: mdeinum

Although I can see the issue you have with this, but ultimatly I think the problem is with the OpenAPI generator. The fact that it adds @Validated also changes the way validation is done and with recent versions of Spring it isn't even needed anymore.

Comment From: snydergd

I think this extends beyond just @Validated. I might make my implementation class advised in another way, perhaps with an aspect, which would also cause this confusing behavior when target-proxy-class is false.

Comment From: rstoyanchev

Indeed as of 6.1, method validation is supported in spring-web where it is better integrated with controller methods. See the section on validation for controllers, and the second note under that:

If a controller has a class level @Validated, then method validation is applied through an AOP proxy. In order to take advantage of the Spring MVC built-in support for method validation added in Spring Framework 6.1, you need to remove the class level @Validated annotation from the controller.

Comment From: rstoyanchev

In addition, since proxying is more general than just validation, the described behavior is expected. For interface proxying, we expect annotations to be on the proxy interfaces.

This is mentioned in the reference docs where it says that you'll need to use class-based proxying or otherwise the controller annotation should be on the interface.

Comment From: snydergd

I am a little surprised that there is no interest in changing it to be more intuitive. It kind of seems like a documented shortcoming/quirk, rather than an intentional/desired behavior, but I appreciate the review and conclusion all the same - glad to see there is documentation on it. Thank you!

Comment From: rstoyanchev

When you proxy by interface, the interface defines what you choose to expose, and consequently we go by the interface too. In other words this is very much intentional.

If anything the documentation in the referenced section could use an update.

Comment From: snydergd

You may have a better view of developer experience than me, so there is a grain of salt here, but in our environment people aren't keenly aware of the type of proxy being created or even necessarily with what the proxies are and how they work, which is one reason we use Spring - so that even the newest person on the team can see the patterns and understand. For the most part they leave the implementation details of applying the annotations to Spring according to the patterns they see/use.

This is a case where many examples online may put @RestController on the implementation class, because they use the default CGLIB proxies, but in our environment it doesn't work and this can be very confusing. You could say we should not use interface-based proxies here to avoid this, but I don't see why we have to make this choice when the annotations could work the same way no matter the type of proxy being used.

I don't see how it is beneficial for the implementation class annotation to be ignored here, to where I might think I'm using the @RestController, when truly I am using @Controller.

Comment From: rstoyanchev

Thanks for the perspective.

First, did you consider my earlier https://github.com/spring-projects/spring-framework/issues/35819#issuecomment-3542335967, why do you want method validation applied via AOP when it is now supported in spring-web?

Second, does this really come down to the generator being able to put @Controller, but not @RestController on the interface? That seems like a real limitation that should be feasible and meaningful to address.

If RestController is the only annotation that needs to be noticed on the class, looking behind the behind the interface seems like working around a quirk. We would have look at the class and all base classes for no real reason, at best a performance slowdown, but possibly leading to surprises if any annotations are actually present on concrete classes which would prevent seeing those on the interface. It also opens up a lot of complexity as this could actually be relied on, putting some annotations on the class and some on an interface.

Comment From: snydergd

It means a lot that you are taking the time to hear this perspective - I really appreciate it!

Our main concerns have been resolved

We have now found an option for the generator we are moving to (openapi-generator-maven-plugin), of useSpringBuiltInValidation, which removes @Validated on the interfaces - I believe doing as you said and using spring-web's own validation instead. We do plan to use this and in our case I think it resolves our main concerns quite nicely, since we are setting patterns for use of this generator.

I was also wondering if we could generate with @RestController or @Controller conditionally to resolve this, but the problem in our case is that the generator runs once per project, while the annotation on implementations could potentially vary between controller classes on our existing projects.

There are still concerns

Like you say, my concerns only apply when people add or keep @RestController annotations on classes in cases where they become wrapped in an interface-based proxy. People adding @RestController should hopefully see the document you mentioned (or in our case, we should also add it to our patterns/documentation).

I'm concerned the type of proxy used on a controller could switch without the developer realizing it. For example, by changing whether a controller has @Transactional, @Validated, etc. directly. In this case, they haven't necessarily seen this documentation, or potentially even realize they have changed the use of proxy.

• If bean becomes wrapped in interface-based proxy:
• Some methods will start using @Controller semantics rather than @RestController ones.
• If bean stops being wrapped in an interface-based proxy:
• @RestController annotations on the class, if present will surprisingly start being used (similar to your concern about changing this behavior)

Summary

There's nothing people can't get around, and the cases where it could be confusing are maybe small, but I still think it can be confusing and in an ideal world I think it would be better for cglib and interface proxies to act the same. I still don't understand the benefit of having it behave differently - unless I'm in the nitty-gritty, I would expect class annotations to override the interface. I know even if it would be one way in an ideal world that doesn't mean it's worth it to go there today.

Comment From: rstoyanchev

I'm glad that it's resolved for you.

For your concerns, simply adding @Transactional on the controller class won't cause an issue. You would need to create an interface first, and at that point you need to decide where you put all annotations (not just RestController, but all mapping annotations, method and method parameter, transaction, security, etc). Do they go on the class, on the interface, a mix of both? It seems confusing to rely on a mix of both.

Comment From: snydergd

I don't know how common our scenario is, but would imagine somewhat common. We don't touch the interfaces except through modifying the OpenAPI Spec and regenerating. There are definitely annotations we want to come from the spec and thus interface (for example, valid values for a field), but annotations such as @Transactional I think would make more sense as part of our implementation code and cannot come through the specification. My concerns are under the assumption that the class is already implementing an interface.

I see what you are saying that a mix of both class and interface annotations adds confusion. I think if I were to get an error or warning that @RestController is going to be ignored on the implementation class vs. being silently ignored like it is now, that would also take care of any concern I have. This way there would be no unpleasant surprises of it starting to take effect or losing effect when placed on the controller implementation class.

Comment From: snydergd

@rstoyanchev Do you think it would be worth considering having a warning/error generate when an interface-based proxy is created around a class having @Controller or @RestController? I don't know how difficult it would be, but I can look into it more if you think it could be worth a look.

Comment From: rstoyanchev

@snydergd there are a number of reasons I'm not in favour.

The question of overriding annotations, e.g. having Controller (on the interface) overridden with RestController (on the class), would be an odd one to allow unless you understand the specific issue with openapi-generator. It means for any Spring application, not just this scenario, you have to look at both the interface and the class.

We could say, look on the class only if it isn't on the interface, but then it could be argued that class it's inconsistent with how it would work if the same interface and class were used with class-based proxying or without proxying.

As a question of consistency, should the same be done for request mappings? It seems to me that overriding annotations from the interface (created by openapi-generator) with class annotations would mean deviating from the schema. Also there is the factor of making it difficult to work with if you have to look through the full hierarchy to assemble your understanding of the mappings.

There is a performance angle, for Controller/RestController, we would need to consider every bean that is an interface proxy as a potential controller.

Overall I see this as a situation that's very specific to OpenAPI codegen, and for that specifically for the Controller/RestController annotation, and not request mappings.

Perhaps this is something that could be raised with that openapi-generator project to allow more flexibility for controller vs restcontroller within a project?

Or perhaps there is some way to do it. After all RestController is just a convenience meta-annotation for Controller + ResponseBody on the class level, which then helps to have ResponseBody inherited, but you could also have ResponseBody on every controller methods instead. Then you wouldn't need RestController.

If you decide to pursue this further, or have more findings, please do share.

Comment From: snydergd

@rstoyanchev Thanks again for discussing this with me - again I really appreciate it!

We could say, look on the class only if it isn't on the interface, but then it could be argued that class it's inconsistent with how it would work if the same interface and class were used with class-based proxying or without proxying.

What I am seeing is the opposite of this -- today the behavior is different between using interface-based proxy and using class-based or no proxying. Interface-based proxying means class annotations have no effect. The other two mean they do.

Saying annotations are not allowed on a class, when it extends an interface and is proxied with interface-based proxies, does make sense for the reasons you have said. However, my concern is that, with it being different than cglib or non-proxy variants, the behavior could be difficult to understand and explain in some scenarios, where the annotations on the class seem to have no effect. That is why my suggestion (that I'm now asking about) is to have a nice error or warning in this scenario so that developers quickly understand and can fix the problem. This is what I saw for some validation annotation processing - an error that the annotation cannot be on both class and interface - and this was very easy to understand and resolve.

There is a performance angle, for Controller/RestController, we would need to consider every bean that is an interface proxy as a potential controller.

For producing an error/warning, you would only need to consider the proxies tied to controllers you find, and with the work you are already doing on these beans, I can't see how a small check on the target object would be substantial - especially with this only being done once on context load.

As a question of consistency, should the same be done for request mappings? It seems to me that overriding annotations from the interface (created by openapi-generator) with class annotations would mean deviating from the schema. Also there is the factor of making it difficult to work with if you have to look through the full hierarchy to assemble your understanding of the mappings.

The question of overriding annotations, e.g. having Controller (on the interface) overridden with RestController (on the class), would be an odd one to allow unless you understand the specific issue with openapi-generator. It means for any Spring application, not just this scenario, you have to look at both the interface and the class.

Again, I think it could make a lot of sense to only support the annotations on the interface in these cases as well, as you are describing, but a warning in this case would be ideal, since it is not the case for cglib and unproxied controllers.

Perhaps this is something that could be raised with that openapi-generator project to allow more flexibility for controller vs restcontroller within a project?

I think that now the generator is not the issue here - where an issue might come up still is if someone adds @Transactional to the class implementing the interface (as an example). If the generator did support generating the interfaces with this annotation, that could be an approach to solve the problem, but since it's perfectly reasonable for them to think they can add it to the class, I think that the developer may not realize the need to make the change in the generator, and there could be some confusion, which would be great to avoid.

Hopefully this outlines what my concern is, as I do not think it was clear before :)