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

Document how to override Spring Data REST exposed resources using manual controller implementations [DATAREST-535] #910

Closed
spring-projects-issues opened this issue May 7, 2015 · 10 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented May 7, 2015

Oliver Drotbohm opened DATAREST-535 and commented

To override the resources automatically exported using manual controllers the controllers written have to consider certain aspects to make sure, they're not picked up by the standard Spring MVC handler mapping:

  • use @BasePathAwareController instead of @Controller or @RestController
  • not use @RequestMapping on the type level

Issue Links:

  • DATAREST-566 Investigate overriding Spring Data REST entity endpoints while using Spring Security

Referenced from: pull request #177, and commits 394319f, 2afa6a6, ba6d0f8, c97034f, 48d4c3a

Backported to: 2.3.1 (Fowler SR1), 2.1.6 (Dijkstra SR6)

1 votes, 4 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 13, 2015

Jason Young commented

Can you mention whether we should use @RepositoryRestController on custom controllers? I use this and others use this but the API docs imply it's for SDR internal use, so that part is unclear

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 25, 2015

Jason Young commented

Also, are you sure these are the things to do to integrate custom controllers? Using SDR 2.3, I have a @Controller that uses @RequestMapping on the type and method level to override a SDR search URL (repository method that just returns Strings) and it works fine. If I deviate from that, using @BasePathAwareController, and just a @RequestMapping on the method (whether including the base path or not), I only ever get "PersistentEntity must not be null!" which is what I get when I leave it to the SDR implementation

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 26, 2015

Greg Turnquist commented

@BasePathAwareController is definitely needed to support moving the base URI of the REST endpoints. For example, if you configure the service to be served from /api, @BasePathAwareController takes the burden off of you to know this.

So far, my usage of the annotation was wrapped around a custom controller that fetched entities using the related Spring Data repository. I haven't tinkered with it returning something else

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 26, 2015

Greg Turnquist commented

Can you assemble a small project on github that depicts the problem you are seeing. We have test cases like https://github.com/spring-projects/spring-data-rest/blob/master/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java#L603-L612, which test https://github.com/spring-projects/spring-data-rest/blob/master/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/AuthorsController.java, a custom controller, and they work fine. A better reproduction of your exact issue may help figure out what is wrong

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 26, 2015

Jason Young commented

I'll see what I can do, and will keep you posted. Thanks!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 27, 2015

Jason Young commented

I've made a small project that demonstrates what I'm seeing: https://github.com/thinkbigthings/DATAREST-535

The signature of the repository whose search url I'm overriding returns a List<String>. I'm not sure that's the issue, it seems like it would happen regardless of the repository.

It's also possible that I just don't what @BasePathAwareController is supposed to do. Should I expect it to automatically prefix the urls I provide in my @RequestMappings?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 29, 2015

Greg Turnquist commented

I made some tweaks and submitted it for review at https://github.com/thinkbigthings/DATAREST-535/pull/1. You don't have to commit it, but can instead use the PR to review my comments and feedback. In my PR, there is never a "PersistentyEntity must not be null" when invoking the injected repository's listProducers() call

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 30, 2015

Jason Young commented

Greg, thanks for experimenting with this! Since your changes fixed the problem I experimented with it some more, and found another clue.

When I make the recommended changes to use @BasePathAwareController and remove /api from the method @RequestMapping (leaving all else the same, including security) it uses the SDR implementation based on the repository method, ignoring the controller. From there, if all I do is remove the @PreAuthorize("isAuthenticated()") from the controller method, then it uses the custom controller implementation which is what we actually want. So I think @PreAuthorize and Spring Security is somehow interfering with @BasePathAwareController

I removed @PreAuthorize from the controller method in my original project, and applied @BasePathAwareController with modified path in the @RequestMapping, and my original project works as you would expect now. So that solves my problem, I guess I didn't need that @PreAuthorize there anyway given the overall security configuration.

Maybe in the context of this jira issue, mention not to use security annotations at the controller method level?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 5, 2015

Jason Young commented

I just checked out the latest reference guide (http://docs.spring.io/spring-data/rest/docs/2.4.0.M1/reference/html/#customizing-sdr.overriding-sdr-response-handlers) and saw no mention that @PreAuthorize would break @BasePathAwareController. Was this intentional? And will there be a follow up jira issue to investigate why it breaks?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 6, 2015

Greg Turnquist commented

The original intent of this issue was to update the docs. Discovering this Spring Security integration issue is best handled with a separate ticket

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