Ease creating Location headers in REST controllers [SPR-8020]

[spring-projects-issues]

Oliver Drotbohm opened SPR-8020 and commented

With it's annotation model Spring MVC provides a convenient way to write REST based server side components. Unfortunately creating fully-qualified URLs for the Location header (for POST requests especially) requires us to have HttpServletRequest and HttpServletResponse in the controller methods signature just to hand it to a helper method to copy the static part of the request URL (everything up to the servlet context) into a String and piping it back to the response.

So as HttpHeaders already has a setLocation(URI uri) method, I wonder whether Spring MVC could simply "expand" this URI to prepend the path up to the servlet context in case the URI does not start with a protocol string. Beyond that, having a @ResponseHeaders annotation which allows you to bind the response headers to a controller method parameter of type HttpHeaders would round of the support. This way a controller could look something like this:

@Controller
public class CustomerController {

  private static final String CUSTOMERS = "/customers";
  private static final String CUSTOMER = CUSTOMERS + "/{id}";

  @ResponseStatus(HttpStatus.CREATED)
  @RequestMapping(value = CUSTOMERS, method = RequestMethod.POST)
  public void createCustomer(@RequestBody Customer customer, @ResponseHeaders HttpHeaders headers) {
    Customer result = repository.save(customer);
    headers.setLocation(new UriTemplate(CUSTOMER).expand(result.getId()));
  }

  @RequestMapping(value = CUSTOMER, method = RequestMethod.GET)
  public @ResponseBody Customer customer(@PathVariable("id") Long id) {
    return repository.findOne(id);
  }
}

---

Affects: 3.0.5, 3.1 M1

Referenced from: commits 60ee0bb

3 votes, 5 watchers

[spring-projects-issues]

Eugen Paraschiv commented

This would indeed be a useful feature and a good step forward for first class REST support in Spring. As it stands now, the only way to do it is indeed to work with the low level request and response, which is not ideal.

[spring-projects-issues]

Rossen Stoyanchev commented

Oliver, a ServletUriComponentsBuilder was added recently. It is a sub-class of the UriComponentsBuilder with methods to build a URI from information in the HttpServletRequest.

For example the following prepares a URL based on the current request including the context path, and the literal part of the servlet mapping (assuming something like "/main/*"):

String url = 
  ServletUriComponentsBuilder.fromServletMapping(request).path("/customers/{id}").build()
    .expand(repository.findOne(id)).encode().toUriString();

This is available with 3.1.0.BUILD-SNAPSHOT. Please, give it a try! Section 16.7 "Building URIs" covers the subject and also there are test cases in UriComponentsTests, UriComponentBuilderTests, and ServletUriComponentBuilderTests.

[spring-projects-issues]

Rossen Stoyanchev commented

With regards to @ResponseHeaders although it looks pretty good in this scenario, the semantics become less clear in combination with a ResponseEntity return value. Not something that you would normally do but nevertheless possible. A secondary issue is that HttpHeaders can be used client and server side and modifying setLocation() as suggested would require access to the HttpServletRequest.

Use of ReponseEntity is simple and unambiguous and suitable in this scenario. In combination with my previous comment it could look something like this:

@RequestMapping(value = CUSTOMERS, method = RequestMethod.POST)
public ResponseEntity createCustomer(@RequestBody Customer customer, HttpServletRequest request) {
  Customer result = repository.save(customer);

  URI location = 
    ServletUriComponentsBuilder.fromServletMapping(request).path("/customers/{id}").build()
      .expand(result.getId()).toUri();
			
  HttpHeaders headers = new HttpHeaders();
  headers.setLocation(location);
  return new ResponseEntity(headers, HttpStatus.CREATED);
}

You can also leave out the HttpServletRequest entirely, which relies on ThreadLocal access:

@RequestMapping(value = CUSTOMERS, method = RequestMethod.POST)
public ResponseEntity createCustomer(@RequestBody Customer customer) {
  Customer result = repository.save(customer);

  URI location = 
    ServletUriComponentsBuilder.fromCurrentServletMapping().path("/customers/{id}").build()
      .expand(result.getId()).toUri();
			
  HttpHeaders headers = new HttpHeaders();
  headers.setLocation(location);
  return new ResponseEntity(headers, HttpStatus.CREATED);
}

[spring-projects-issues]

Mike Whittemore commented

Perhaps this would be hiding too many details, but it may be useful to provide a builder that can streamline this sort of thing. For example something like the following:

ResponseEntity response = new ResponseBuilder()
  .withStatus(HttpStatus.CREATED)
  .withLocationHeader("/customers/{id}", result.getId()) // variable argument list
  .build();

For usage patterns that do not return ResponseEntity objects, perhaps an alternative build() signature would update the HttpServletResponse that is in hiding in the framework. Perhaps:

new ResponseBuilder()
  ...
  .updateResponse(); // updates response object instead of returning an entity

[spring-projects-issues]

Oliver Drotbohm commented

I essentially like the stage Rossen has brought this to already. I esp. like not needing to get the HttpServletRequest injected into the query method just for the sake of piping it into the builder. However I also think that

….path("/customers/{id}").build().expand(result.getId()).toUri();

still deserves a shortcut of some kind. I wouldn't even mind just boiling the four method calls together to

….buildAndEncodePath("/customers/{id}", result.getId());

as I think esp. the intermediate build() step is a bit cumbersome and probably hard to discover. Beyond that I am fine with the current solution. The solution Mike proposed has some charm as well and is a nice one-stop-shop for creating ResponseEntity instances. Although it does not yet seem to take the explicit "from servlet mapping" into account.

[spring-projects-issues]

Rossen Stoyanchev commented

We can add support for a UriComponentsBuilder argument, which would be created via ServletUriComponentsBuilder.fromServletMapping(request) as well as add a buildAndEncode shortcut method. It would look like this:

@RequestMapping(value = CUSTOMERS, method = RequestMethod.POST)
public ResponseEntity createCustomer(@RequestBody Customer customer, UriComponentsBuilder builder) {
  Customer result = repository.save(customer);

  HttpHeaders headers = new HttpHeaders();
  headers.setLocation(builder.path("/customers/{id}").buildAndExpand(result.getId()).toUri());
  return new ResponseEntity(headers, HttpStatus.CREATED);
}

Good enough?

[spring-projects-issues]

Oliver Drotbohm commented

Looks great! (I think there's a closing parenthesis missing after ….getId()).

[spring-projects-issues]

Eugen Paraschiv commented

The support for this seems to be coming along nicely, but still looks like it can be further improved.
I'm referring to the fact that the added support relies on the @Controller method returning a ResponseEntity; an improvement would be to have a way to not just build the Location, but also update the headers of the request without needing to go to the full construction of the Response Entity (which is significant boilerplate - for instance, in the example above, it's half the code). This would be just the other side of the coin - updating the response without directly referencing it (there already is a shortcut to get the UriComponentsBuilder without directly referencing the request).
Also, related to the usage of HttpHeaders in both a client and server context - why would that be problematic? The object would hold the Location in it's headers map; on the client, setting the Location would simply change that value (which presumably is what you want); on the server, it would mean changing the value and updating the response (which, again, is presumably what you want).
Thanks for the great work.
Eugen.

[spring-projects-issues]

Eugen Paraschiv commented

Sorry, it should be "update the headers of the RESPONSE without ... " above (unable to edit).