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

GET on RestRepository not possible, if a RestController for the same path is available [DATAREST-522] #896

Closed
spring-projects-issues opened this issue Apr 16, 2015 · 16 comments
Assignees

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Apr 16, 2015

Felix Huch opened DATAREST-522 and commented

We have a Spring Data REST project, with several @RepositoryRestResource annotated Spring Data Repositories.

Example:

@RepositoryRestResource(path = "person")
public interface PersonRepository extends PagingAndSortingRepository<Person, Integer> {}

We want to override the POST/PATCH method of one of these repositories. So we annotate a RestController with @RestController and the annotation @RequestMapping has the same path as the repository.

Example:

@RestController
@RequestMapping("person")
public class PersonController{

	@RequestMapping(method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON_VALUE)
	public ResponseEntity<Boolean> savePerson(@RequestBody Person person) {
...
}

The POST/PATCH works fine. But now a GET to /person (to receive all Persons) throws an HttpRequestMethodNotSupportedException and says: Request method 'GET' not supported.

A GET to /person/1 instead is working just fine.

We had a look at https://jira.spring.io/browse/DATAREST-490 and the @BasePathAwareController, but this doesnt seem to change anything


Affects: 2.3 GA (Fowler)

Attachments:

Issue Links:

  • DATAREST-490 Repository controllers not invoked if resource is handled manually in dedicated media type

Backported to: 2.3.1 (Fowler SR1)

0 votes, 5 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 16, 2015

Oliver Drotbohm commented

Try two things: re-add @BasePathAwareController and remove the type-level @RequestMapping and you should see this working. The latter causes the standard Spring MVC infrastructure to pick up the controller and the corresponding handler mapping see the partially matching @RequestMapping (not completely matching as you try GET, not POST. This triggers the exception and then the effect described in DATAREST-490 kicks in. By moving to @BasePathAwareController you end up in a different handler mapping that cycles through the registered handler mappings for @BasePathAwareController, the repository controllers etc

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 16, 2015

Felix Huch commented

First, thanks for this fast and great support!

We have tried these two ways and the result is the same.

@RestController
@BasePathAwareController
public class PersonController {

	@RequestMapping(value = "person", method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON_VALUE)
	public ResponseEntity<Boolean> savePerson(@RequestBody Person person) {
		return new ResponseEntity<Boolean>(true, HttpStatus.OK);
	}
}
@BasePathAwareController
public class PersonController {

	@RequestMapping(value = "person", method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON_VALUE)
	public ResponseEntity<Boolean> savePerson(@RequestBody Person person) {
		return new ResponseEntity<Boolean>(true, HttpStatus.OK);
	}
}

A GET on /person results in a HttpRequestMethodNotSupportedException
And a GET on /person/1 works fine.
The POST/PATCH works fine too

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 16, 2015

Oliver Drotbohm commented

Hm, weird. Do you have a tiny sample I can look at?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 17, 2015

Felix Huch commented

I added a minimal sample project to this ticket. The resources are available under /api/*

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 17, 2015

Oliver Drotbohm commented

Adding the following test case works for me:

@RunWith(SpringJUnit4ClassRunner.class)
@WebAppConfiguration
@ContextConfiguration(classes = AppConfig.class)
public class AppTest {

	@Autowired WebApplicationContext context;

	MockMvc mvc;

	@Before
	public void setUp() {
		this.mvc = MockMvcBuilders.webAppContextSetup(context).build();
	}

	@Test
	public void testname() throws Exception {

		mvc.perform(MockMvcRequestBuilders.get("/person/1").accept(MediaType.APPLICATION_JSON)).andExpect(
				MockMvcResultMatchers.status().isOk());
	}
}

When providing samples, please make sure there's something to execute, that clearly shows what you're expecting and what's wrong. I'm not gonna deploy anything anywhere. Ideally a mvn clean test (or equivalent in Gradle) should reproduce what's considered broken

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 17, 2015

Oliver Drotbohm commented

Nevermind, I see, you'd like to override /person and that breaks for me, too. That said, this just emphasizes my last note :)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 17, 2015

Felix Huch commented

Thanks! I will keep that in mind ;-)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 17, 2015

Thibaud Lepretre commented

Before Fowler train, I achieve your needs like following: http://stackoverflow.com/questions/27712038/spring-data-rest-and-collections-with-unique-constraints/27802520#27802520.

Since Fowler train HandlerMapping repositoryExporterHandlerMapping does not exist it now RequestMappingHandlerMapping but principle was the same.

I had the same needs as you that why I recreate the handlerMapping chain with delegation to avoid RequestMappingHandlerMapping to throw exception before testing Spring Data Rest controller.

My SO answer can be "patch" solution for you or help investigation. I really hope that Spring Data Rest provides such feature in futur!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 17, 2015

Oliver Drotbohm commented

The issue was us not handling exceptions other HttpMediaTypeNotAcceptableException in DelegatingHandlerMapping that might indicate a partial match found in the primary HandlerMapping we deploy. This should be fixed in both master as well as the 2.3.x bugfix branch. Feel free to give the snapshots a try

@spring-projects-issues
Copy link
Author

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

Felix Huch commented

Unfortunately its not working.

We added

<repositories>
 <repository>
      <id>spring-libs-snapshot</id>
      <url>https://repo.spring.io/libs-snapshot</url>
 </repository>
</repositories>

and

<spring.data.rest.version>2.3.1.BUILD-SNAPSHOT</spring.data.rest.version>

to our pom.xml.

The tests look like

   
@Test
public void checkGET() throws Exception {		   
     mvc.perform(MockMvcRequestBuilders.get("/person")
          .accept(MediaType.APPLICATION_JSON))
          .andExpect(MockMvcResultMatchers.status().isOk());
}

@Test
public void checkPOST() throws Exception {		
     mvc.perform(MockMvcRequestBuilders.post("/person")
          .accept(MediaType.APPLICATION_JSON))
          .andExpect(MockMvcResultMatchers.status().isOk());
}

and we get

[main] WARN  o.s.web.servlet.PageNotFound - Request method 'GET' not 
supported
...
java.lang.AssertionError: Status 
Expected :200
Actual   :405

The @DelegatingHandlerMapping is the new one

@spring-projects-issues
Copy link
Author

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

Oliver Drotbohm commented

When I run your sample application's test case checkPOST I see the latest build snapshot being used (April 18th). I then get a 400 status code which looks correct as you don't send any data. I see the same expected 400 on 2.4.0 snapshots.

It might make sense to rather create a repo that you update than referring to an attachment here that I have to modify by some means, as the latter is just begging for inconsistencies in what you use and what I am trying to reproduce here

@spring-projects-issues
Copy link
Author

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

Felix Huch commented

I'm sorry for the circumstances. I created a github-repo: https://github.com/felixhuch/spring-data-rest-playground
This shows what i mean.
Thanks ;-)

@spring-projects-issues
Copy link
Author

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

Oliver Drotbohm commented

As indicated in my very first comment you need to switch to @BasePathAwareController and need to make sure not to use @RequestMapping on the type level as this will cause the root Spring MVC handler mapping to pick up the controller and cause the effects I described here. Following the suggestions makes your tests pass for me

@spring-projects-issues
Copy link
Author

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

Felix Huch commented

Jap :-) Thank you very much!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 6, 2017

Robert Rackl commented

Just in case some else also stumbles over this: "No @RequestMapping on the type level" means: Do not configure the path on the method. You can only configure one resource path on the whole class: This is what worked for me in the end:

@RepositoryRestController
@RequestMapping("person")   // only set the path mapping here!
public class PersonController{
 
	@RequestMapping(method = RequestMethod.POST)   // no value  for path here!
	public @ResponseBody PersistentEntityResource savePerson(@RequestBody Person person, PersistentEntityResourceAssembler resourceAssembler)) {
...
}

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 6, 2017

Robert Rackl commented

Actually ... this still doesn't work. I'll open a new 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