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

SpringMvcContract does not support path parameters defined in interface's @RequestMapping annotation #1023

Closed
jebeaudet opened this issue May 12, 2016 · 3 comments
Assignees
Milestone

Comments

@jebeaudet
Copy link
Contributor

jebeaudet commented May 12, 2016

In SpringMvcContract, the path defined in the interface's class @RequestMapping annotation is prepended at the end of the parseAndValidateMetadata(Class<?> targetType, Method method) method. While this works fine for most cases, it does not support path parameters defined there that apply to all methods. This is because the PathVariableParameterProcessor expect it to be in the MethodMetadata at the time or it adds it as a form param as a fallback.

Example of a non working interface at the moment:

    @RequestMapping("/prepend/{anotherId}")
    public interface Example {
        @RequestMapping(value = "/test/{id}", method = RequestMethod.GET)
        ResponseEntity<TestObject> getTest(@PathVariable String anotherId, 
                                           @PathVariable String id);

        @RequestMapping(method = RequestMethod.POST)
        TestObject postTest(@PathVariable String anotherId, 
                            @RequestBody TestObject object);
    }

I'll submit a PR shortly! Thanks again for the great lib.

@spencergibb
Copy link
Member

Using @RequestMapping on an interface level is not supported because of the way spring MVC works. It assumes they are real controllers. There is a path attribute on @FeignClient.

@jebeaudet
Copy link
Contributor Author

Hmm, we're not using @FeignClient here because we're generating clients with the Feign.Builder() to use in some functional tests.

We're using the pattern defined in the documentation with one interface defining the resource and a controller implementing it and a client extending the interface in separate projects. AFAIK, using annotation on the interface worked properly when implementing it in our controller but I'll have to verify that with my colleague tomorrow. You're saying that this shouldn't work in the first place is that it?

@jebeaudet
Copy link
Contributor Author

We're implementing the pattern defined here http://cloud.spring.io/spring-cloud-static/spring-cloud.html#spring-cloud-feign-inheritance

It's working well having all the @RequestMapping annotations on the shared interface with the client extending the interface and the resource implementing the interface. The only thing not working is the support for query param on the class lever @RequestMapping annotation which this PR provides a fix.

dsyer pushed a commit that referenced this issue May 17, 2016
…pping

This commit adds the support for query string defined in the class'
@RequestMapping Annotation that is applied to all method of a
controller.

Fxies gh-1023, fixes gh-1024
@dsyer dsyer closed this as completed May 17, 2016
@dsyer dsyer reopened this May 17, 2016
@jebeaudet jebeaudet changed the title SpringMvcContract does not support query string defined in interface's @RequestMapping annotation SpringMvcContract does not support path parameters defined in interface's @RequestMapping annotation Jun 8, 2016
@spencergibb spencergibb added this to the 1.1.2 milestone Jun 9, 2016
@spencergibb spencergibb self-assigned this Jun 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants