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

Spring Data REST registered ConversionService not used when overriding individual resources [DATAREST-423] #803

Closed
spring-projects-issues opened this issue Dec 7, 2014 · 14 comments
Assignees

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Dec 7, 2014

Thibaud Lepretre opened DATAREST-423 and commented

I have a business object User and in my application I have to set property active to false when deleting User.

I don't want to expose my business logic on REST model so I wanted to override DELETE /user/\{id\} behavior to patch user to set active to false.

In order to achieve that I write my own controller with following piece of code:

@RepositoryRestController
@ResponseBody
@RequestMapping("/users")
public class UserController {

    @Inject
    private UserService userService;

    @ResponseStatus(value = HttpStatus.NO_CONTENT)
    @RequestMapping(method = RequestMethod.DELETE, value = "/{user}")
    public void delete(@PathVariable("user") User user) {
        user.setActive(false);
        userService.save(user);
    }
}

Method is correctly called but I get an org.springframework.beans.ConversionNotSupportedException...

So I tried to add a custom converter (String -> User) like describe here: http://docs.spring.io/spring-data/rest/docs/1.1.0.M1/reference/htmlsingle/#d4e110

But the converter is never called.

When debugging I saw that https://github.com/spring-projects/spring-framework/blob/master/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java#L169 is NULL that why my converter is not called.

I will try to find more and create PR if possible but atm I don't have more information


Affects: 2.3 M1 (Fowler)

Backported to: 2.2.2 (Evans SR2)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 7, 2014

Oliver Drotbohm commented

The conversion should work in the first place. Make sure your MVC configuration either uses the defaultConversionService bean that Spring Data REST registers as ConversionService or explicitly use @EnableSpringDataWebSupport in your MVC configuration. The latter will activate the to-entity-conversion for MVC controllers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 7, 2014

Thibaud Lepretre commented

I publish a sample project https://github.com/kakawait/DATAREST-423 to reproduce my issue:

Scenario: DELETE /api/users/1

As you can see I explicitly use @EnableSpringDataWebSupport but without success

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 7, 2014

Oliver Drotbohm commented

Awesome, would you mind adding a test case that fails?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 7, 2014

Thibaud Lepretre commented

I push a commit with a basic Integration test to highlight my problem, please consider using mvn clean test to reproduce

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 8, 2014

Thibaud Lepretre commented

Can we consider as a bug? If yes I will try to fix it with PR (if I find time because vacancies are coming)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Oliver Drotbohm commented

You filed a ticket, let me look into it :)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Oliver Drotbohm commented

You seem to mix up quite a few things here. I can get this to work if I remove @RepositoryRestController annotation (which is an internal one by the way and shouldn't be used from user code). and replace it with @RestController and augment the mapping to /api/users/1. The basic difference is that with @RepositoryRestController the actual method invocation is piped through a slightly different pipeline (to be able to apply ResourceProcessor beans). However, this pipeline doesn't seem to be aware of the ConversionService instance we create in RepositoryRestMvcConfiguration.

I have a fix handy but want to make sure I equip it with enough tests to prevent regressions so that I thought it might be worth mentioning the workaround

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Thibaud Lepretre commented

Yes you'r correct if I'm changing to @RestController It's works but It will break SDR. Indeed by setting @RestController the /api/users mapping will be managed by RequestMappingHandlerMapping and all SDR will not more accessible and exception HttpRequestMethodNotSupportedException will be throw.

RequestMappingHandler is called before RepositoryRestHandlerMapping, SDR mapping will not be resolved anymore.

I "hacked" my code by adding @RepositoryRestController in order to do not managed my controller by RequestMappingHandlerMapping and it works. SDR still working and my controller override the SDR as needed. But conversion issue occurs in this configuration

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Oliver Drotbohm commented

What exactly do you mean by "all SDR will not more accessible"? I just changed your controller to use @RestController and the augmented mapping and SDR continues to work as expected but also uses the manual implementation in UserController for DELETE requests for individual users

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Thibaud Lepretre commented

Hum I will retry and write some teat cases if needed

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Oliver Drotbohm commented

This should be fixed in master and the 2.2.x bugfix branch. Feel free to give the snapshots a try

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Thibaud Lepretre commented

Before any further investigations, I want to be sure that my use case is clear:

I'm using SDR for my UserRepository, I fixes mapping on /api/users.
Then I wanted to override SDR delete behavior, so DELETE /api/users/\{id} (keep in mind that I want to keep the same path) so I write a custom controller with @RequestMapping(method = RequestMethod.DELETE, value = "/api/users/\{id\}").

If I annotated my custom controller with @RestController: override (and any possible conversion) works well but after when I try to request any SDR API (for example HEAD /api/users/1) I get org.springframework.web.client.HttpClientErrorException: 405 Method Not Allowed. Test reproduction here: https://github.com/kakawait/DATAREST-423/commit/e83d917ddd8d59e34ea88b5529da0f5508d1dc90

If I annotated my custom controller with @RepositoryRestController: overrides works well and SDR API still accessible (not more 405). But conversion is no more supported

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Thibaud Lepretre commented

Ho I posted to late :) Thanks I will try fix asap. Thanks so much

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 9, 2014

Thibaud Lepretre commented

Tested with 2.3.0.BUILD-SNAPSHOT It's perfect for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants