Enable REST controller method parameter annotations on an interface [SPR-11055]

[spring-projects-issues]

Paweł Mendelski opened SPR-11055 and commented

Please, enable inheritance of controller related annotations from implemented interfaces.

I would like to share rest service interface between client and server. This way I could provide a really convenient mechanism - client-proxy, like the one RestEasy provides.

Shared interface:

@RequestMapping("/random")
public interface RandomDataController {

	@RequestMapping(value = "/{type}", method = RequestMethod.GET)
	@ResponseBody
	RandomData getRandomData(
			@PathVariable(value = "type") RandomDataType type, @RequestParam(value = "size", required = false, defaultValue = "10") int size);
}

Server implementation:

@Controller
public class RandomDataImpl implements RandomDataController {

	@Autowired
	private RandomGenerator randomGenerator;

	@Override
	public RandomData getPathParamRandomData(RandomDataType type, int size) {
		return randomGenerator.generateRandomData(type, size);
	}
}

Client code:

// ...
RandomDataController randomDataController = SpringMvcProxyFactory.create(RandomDataController.class, "http://localhost:8080");
RandomData rd = randomDataController.getRandomData(RandomDataType.ALPHA, 100);
// ...

At the moment I cannot write SpringMvcProxyFactory because parameter annotations from interface RandomDataController are not inherited by RandomDataControllerImpl.

---

Affects: 4.3.3

Issue Links:

• @RequestBody annotation in interface is ignored [SPR-15714] #20270 @RequestBody annotation in interface is ignored ("is duplicated by")
• Inconsistent interpretation of parameter annotations in interface-based handler methods [SPR-14021] #18593 Inconsistent interpretation of parameter annotations in interface-based handler methods ("is duplicated by")
• mvc Parameter annotations are not inherited [SPR-14526] #19095 mvc Parameter annotations are not inherited ("is duplicated by")
• Spring Mvc Annotations in argument of a method (like @Path) annotated with @RequestMapping are not supported when you create a @RestController via an Interface [SPR-14805] #19371 Spring Mvc Annotations in argument of a method (like @Path) annotated with @RequestMapping are not supported when you create a @RestController via an Interface ("is duplicated by")
• RequestHeader annotation does not work if it's put on an interface [SPR-15023] #19590 RequestHeader annotation does not work if it's put on an interface ("is duplicated by")
• Consider reusing @RequestMapping annotations as an interface for HTTP clients [SPR-12213] #16827 Consider reusing @RequestMapping annotations as an interface for HTTP clients ("is duplicated by")
• Allow controller parameter annotations (@RequestBody, @PathVariable...) to be defined on interfaces or parent classes [SPR-15046] #19612 Allow controller parameter annotations (@RequestBody, @PathVariable...) to be defined on interfaces or parent classes ("is duplicated by")
• Supports annotating parameter annotations like @PathVariable on interface method [SPR-16110] #20658 Supports annotating parameter annotations like @PathVariable on interface method ("is duplicated by")
• Aliases in parameter annotations on a controller interface are not evaluated [SPR-17460] #21992 Aliases in parameter annotations on a controller interface are not evaluated
• MethodParameter.isOptional() fails with ArrayIndexOutOfBoundsException [SPR-17629] #22160 MethodParameter.isOptional() fails with ArrayIndexOutOfBoundsException
• Introduce proxy-based REST client similar to HttpInvokerProxyFactoryBean [SPR-12131] #16747 Introduce proxy-based REST client similar to HttpInvokerProxyFactoryBean
• getMappingForMethod failed when implement a interface method(with @RequestMapping) in super class [SPR-17223] #21756 getMappingForMethod failed when implement a interface method(with @RequestMapping) in super class
• Annotations on generic interface methods not found by AnnotationUtils [SPR-16060] #20609 Annotations on generic interface methods not found by AnnotationUtils
• Subclass-resolved generic return type declarations for handler methods [SPR-16877] #21416 Subclass-resolved generic return type declarations for handler methods
• @DateTimeFormat annotations are necessary both on field and constructor parameter in the immutable form class [SPR-16840] #21380 @DateTimeFormat annotations are necessary both on field and constructor parameter in the immutable form class

Referenced from: commits 790d515, 1f5d0fa

30 votes, 37 watchers

[spring-projects-issues]

George Georgovassilis commented

There is an implementation here: https://github.com/ggeorgovassilis/spring-rest-invoker
I would be happy to contribute it as a module or to spring-web if there is interest

[spring-projects-issues]

George Georgovassilis commented

Upon re-reading the ticket's description, the submitter asks for a common interface that would work both for controllers that expose REST APIs and clients that consume them. The implementation above generally requires interfaces that might not make sense when implemented by a controller.

[spring-projects-issues]

Rossen Stoyanchev commented

Controller annotations can be inherited from a base class or interface. Method parameter annotations however cannot be. They must be directly on the controller method in all cases. This ticket is a request to change that.

Indeed your proposal has similar purpose but does not depend on this ticket since in your case the interfaces are only used for generating a client proxy.

That said once the ability to create a client proxy from an interface with the same annotations is available, it becomes a natural question: should it be possible for a controller and a client proxy to share an interface?

[spring-projects-issues]

George Georgovassilis commented

In that case I'd like to decouple my implementation from the concrete decision whether a single or multiple interfaces are to be used. New ticket #16747.

[spring-projects-issues]

Matt Benson commented

This issue also applies to parent classes.

[spring-projects-issues]

Matt Benson commented

PR submitted: #976

[spring-projects-issues]

Rossen Stoyanchev commented

Wow this is a very extensive effort and a perfect example of the sort of thing that you could discuss (beforehand) on our contributor's mailing list.

Style and programming model issues aside, there is a whole separate discussion if we should even re-use the same annotations for client-proxy generation or create more dedicated annotations for the purpose (see #16827). Alternatively with what we have today wouldn't it be easy to generate interfaces on the fly from a controller's methods and annotations? That way method parameter annotations can remain close to the class as promoted by the programming model today.

[spring-projects-issues]

Matt Benson commented

TBH I decided to take my chances rather than subject my inbox to the burden of another mailing list. With regard to client proxies, spring-cloud is already doing this with their support for Netflix Feign, and until recently their documentation (at least somewhat erroneously) reported that it was possible to use a Spring-annotated REST interface for client and server alike. The non-inherited nature of parameter annotations is in fact the missing ingredient here. If people are going to use interfaces for client proxies, I can't see how forcing them to define their REST API in multiple places does anything other than expose them to the risk that these definitions will fall out of sync.

My interpretation of "on the fly" is "at runtime." I don't see how that helps clients who want to consume the hypothetical generated interface. Certainly it would be possible to generate such an interface, but it seems like this would be more a tooling step or, at best, an annotation processor, and getting these generated types into a consumable client artifact would be quite a build-system-dependent task. It is not at all clear to me that this solution would be better than a basic, Java-based one.

[spring-projects-issues]

Matt Benson commented

With the recent closure of the development forum, do any other Spring developers have any thoughts on the proposed fix?

[spring-projects-issues]

Rossen Stoyanchev commented

If we did implement this feature to allow method parameter annotations on supertypes it would most likely be exposed through AnnotationUtils which we currently use to find method and type level annotations on supertypes (see for example RequestMappingHandlerMapping). The various argument resolver implementations would delegate to AnnotationUtils unlike the PR which introduces the same as base class functionality. Also the current algorithm in AnnotationUtils for method and type level annotations is also a lot more involved and so would be the one for method parameters. This is not really meant as criticism but just saying that the problem is more complex. We are however taking a look at this still. There are several related tickets so I'm going to assign this one also to Juergen Hoeller for consideration.

From a Spring MVC perspective I think basic support for the Spring Cloud with Netflix Feign use case (i.e. allow the same interface to be used on client and server side)is fine as a use case but allowing a method parameter to be anywhere is another matter. It means for a controller method with several arguments the annotations for each argument could literally be in a different superclass. I'm not sure it should be so wide open. Some meaningful constraints focusing on the target use cases we want to support should perferably be in order.

[spring-projects-issues]

Alexandre Navarro commented

+1

[spring-projects-issues]

Elias O commented

One more use case for method param annotation inheritance:
I'm using Swagger Codegen to generate API interfaces from OpenAPI specs during build process so that I can implement them directly in my code. Those generated interfaces contain all Spring MVC related annotations so I don't need to care about annotating my methods. Except method params. I had to copy them once again because Spring doesn't see them.

[spring-projects-issues]

Yura Nosenko commented

@Elias O, have a similar story with RAML. It's a huge gap, since the method-level annotations work as expected. This feature is a MUST!

[spring-projects-issues]

Yura Nosenko commented

When it comes to validation, there is a workaround - @Validated. It will actually make jsr303 param annotations inheritable.

[spring-projects-issues]

Rossen Stoyanchev commented

Do you mean that Swagger can't generate a class for a server stub, and same for RAML?

[spring-projects-issues]

Elias O commented

Rossen Stoyanchev that mean that if you use Swagger codegen, you can choose an "API [interfaces] only" option that generates API interfaces like this:

public interface Resource {
  @RequestMapping
  void do(@RequestParam String name, @RequestBody DTO value);
}

so that you may expect your controller to look like this:

@RestController
public class ResourceImpl implements Resource {
  void do(String name, DTO value) {
    // do stuff
  }
}

ResourceImpl#do will inherit @RequestMapping, but @RequestParam and @RequestBody won't, therefore name and value will be null (AFAIR).

The only way to make it work is to duplicate arg annotations:

@RestController
public class ResourceFixedImpl implements Resource {
  void do(@RequestParam String name, @RequestBody DTO value) {
    // do stuff
  }
}

In real cases, such interfaces can have tons of annotations (moreover, arg annotations tend to become very long) and it makes sense to leave them aside from the implementation.

To overcome this bug, currently swagger can generate api + implementation (with annotated params) + delegation interface, so that users can implement a delegation interface only.

[spring-projects-issues]

Alexandre Navarro commented

Hi,

I will explain our use case why we want to implement @FeignClient interface in the @RestController.

In my firm, we have developped java microservice (around 20) with spring, spring-boot and spring-cloud with @FeignClient since 3-4 years. Our different microservices are in different git repo, can be released independently and at first each microservice implements their own @FeignClient if they need to request another microservice, so our microservices are really independent each other. One day, someone corrected a typo from "portofolio" to "portfolio" in an argument of an annotation of a @RestController, he changed in the controller and some feign clients but missed one feign client. We had a bug in production we have to release a fix. Since this moment, we decided to change our strategy with @FeignClient, we decided to share it, not duplicate it. Now, our microservice are splitted in 2 modules, one for the microservice itself, one for the api which include the POJO used as argument or result of the @RestController an @FeignClient Interface. If another microservice2 needs to call microservice1, he will take microservice2-api as lib and it uses it in order not to copy all Feign interfaces everywhere. Clearly, we are happier with shared @FeignClient with shared-lib-api (we only used the api lib internally to the team not with the others which consume our microservice). Nevertheless, today, as the @RestController can't implement the @FeignClient interface because the annotation in the argument are not taken, we need to duplicate the different annotations between @RestController and @FeignClient and we added some IntegrationTest to test if our @FeignClient interface are coherent with the @RestController, which is clearly painful and can be fixed if a @RestController can implement @FeignClient interface.

So this is the reason why I really want to have this feature.

I understand you have to set some rules if for instance some annotations in the implementation are different in the interface (potentially they can be contradictory).

In my point of view, you should take first the annotations of the interface and potentially add / override it if the implementation add / override some annotations. It can be useful because

• some annotations are not implemented in Feign (like regex validation in parameter are yet not supported in feign) and globally annotation in server side (@RestController) are always implemented first compared to annotation in client side (@FeignClient)
• but also to let implementer to decide to override or not something from interface.

This is just my point of view on the rule to adopt between interfact / implemenation annotations, maybe you have some other arguments to decide apply different rules between annotations declared in @ClientFeign vs @RestController

Thanks in advance to implement this feature, it will be really really useful.

@Juergen Hoeller : Just to remember, we discussed about it at the Spring BOF at Devoxx France 2 weeks ago.

 

[spring-projects-issues]

Damian Sima commented

Hi guys, can we have an update from the eng team about this?

I mean the issue was open un 2013 and it is still active but unresolved, are we gonna have a feature for this? 

Cheers!

[spring-projects-issues]

Andrey Gliznetsov commented

I'd like to add that some annotations actually work on interface, f.e. @RequestParam, but not  @RequestBody nor @PathVariable

[spring-projects-issues]

Juergen Hoeller commented

I'm happy to report that this is finally resolved in master now, in time for the 5.1 RC1 release!

Feel free to give it an early try against the upcoming 5.1.0.BUILD-SNAPSHOT...

[spring-projects-issues]

George Georgovassilis commented

Thank you Juergen Hoeller

[spring-projects-issues]

Damian Sima commented

you rock Juergen Hoeller

[polluxxing]

@jhoeller hi,how can I achieve the same effect in spring5.0.x?