spring-projects / spring-hateoas

Spring HATEOAS - Library to support implementing representations for hyper-text driven REST web services.
https://spring.io/projects/spring-hateoas
Apache License 2.0
1.04k stars 477 forks source link

Allow moving hypermedia concerns into serialization layer #523

Closed kewne closed 7 years ago

kewne commented 7 years ago

Spring HATEOAS currently requires the use of Resource as the return type for handler methods, which makes migrating legacy Spring MVC applications to HATEOAS backwards incompatible because the JSON format changes.

If the hypermedia concerns were moved into the serialization layer, i.e., into the appropriate HttpMessageConverter, every legacy Spring MVC application would instantly become hypermedia-enabled and links could be added incrementally.

In my mind, controller methods would look like regular Spring MVC methods:

// OrderController
@GetMapping("/order/{id}")
Order getOrder(String id) {
    return repository.getOrder(id);
}

and the same URL could be used for RPC and HATEOAS style calls.

gregturn commented 7 years ago

Why not use ResponseEntity<?> as your return type?

kewne commented 7 years ago

That would work too, what I meant was that everything supported by regular Spring MVC would be supported and the serialization result would depend on content negotiation and the specifics of the chosen format. If that format is hypermedia-enabled, the link (and form) building logic would be called, instead of being explicitly called in or from controller code.

odrotbohm commented 7 years ago

The controller is the natural home of transforming protocol details into service layer API calls, hence, I don't see a reason why we'd want to move that responsibility to another place. The sole reason we try to keep the Resource API free from media type specific details. That said, nothing in Spring HATEOAS actually forces you to implement the Resource creation in a controller in the first place. You could write a ResponseBodyAdvice to take care of that in a separate component if you wish.

Summarizing, I don't even agree on the premise as it's a claim without evidence. It's not even a design goal to make existing controllers "work" in a hypermedia driven way. Neither it is to align with anything RPC.

kewne commented 7 years ago

My suggestion is about allowing controllers to produce both hypermedia-enabled and regular media type representations for "normal" Spring MVC methods.

As it is (correct me if I'm wrong), handler methods that return Resource are assumed to be specific to hypermedia (even the JSON representation changes), which constrains the generality of the methods because I can't serve regular media types and hypermedia ones from the same method. The use case that convinced me to create the issue is migrating older Spring MVC to HATEOAS, since removing the Resource stuff from controllers makes them the same as plain MVC and could be a huge driver for Spring HATEOAS adoption as I no longer have to duplicate everything to return Resource.

ResponseBodyAdvice could be an option for implementing what I have in mind but it doesn't seem to allow changing the return type.

gregturn commented 7 years ago

As it is (correct me if I'm wrong), handler methods that return Resource are assumed to be specific to hypermedia

Yes, because Spring HATEOAS's Resource type = REST resource

constrains the generality of the methods because I can't serve regular media types and hypermedia ones from the same method

You can serve ANYTHING from one Spring MVC method, because you can use ResponseEntity<?> as the return type of your method. And you're not confined to hypermedia types. See my example below:

@RestController
class SampleController {

    @GetMapping("/employees/{id}")
    ResponseEntity<?> employeeDetails(@PathVariable long id, WebRequest request) {

        String acceptHeader = request.getHeader(HttpHeaders.ACCEPT);
        if (acceptHeader != null) {
            if (acceptHeader.equals(MediaTypes.HAL_JSON_VALUE)) {
                return ResponseEntity.ok(new Resource<Employee>(employeeService.halRecord()));
            } else if (acceptHeader.equals(MediaType.APPLICATION_PDF_VALUE)) {
                return ResponseEntity.ok(employeeService.hoursWorkedAsPdf());
            } else {
                return ResponseEntity
                    .badRequest()
                    .body("We can't handle Accept type " + acceptHeader);
            }
        }

        return ResponseEntity
            .badRequest()
            .body("Must send an Accept header!");

    }
}

This method looks up the current method's ACCEPT header, then switches to one of four possible results, error handling included.

kewne commented 7 years ago

The problem I see with the example you provided is that it "re-implements" the content negotiation that is already built into Spring MVC (the Accept header can be very complex). At first glance, I think the example can be rewritten as two methods with different mappings using @GetMapping(produces = ...), which would delegate content negotiation to the framework and improve type safety (by returning explicit types instead of ResponseEntity<?>).

Having to do this kind of branching is precisely the reason I opened the issue: links are only relevant for hypermedia-capable media types. What I was suggesting was allowing something like:

@RestController
class SampleController {

    @GetMapping("/employees/{id}")
    ResponseEntity<Employee> employeeDetails(@PathVariable long id) {
        return ResponseEntity.ok(employeeService.getEmployeeDetails(id)));
    }
}

The message converter components would then convert my POJO into the adequate representation, whether it's plain JSON, HAL or PDF.

After interacting with the framework a bit more and having tried to implement this myself, it's becoming clear to me that Spring HATEOAS was built on a different philosophy than what I was requesting. If this still isn't something that you want to see in the framework let me know and I'll close it.

gregturn commented 7 years ago

I crafted that example because I sensed you were hinged on fitting things into one method. In reality I too would split stuff up into different methods based on types.

The only difference is the hypermedia ones need to wrap the return object in our Resource type.

I recently created https://github.com/spring-projects/spring-hateoas-examples/tree/master/basics. Perhaps it would be of interest to you. It shows how simple it can be to build hypermedia. Other examples are coming.

I highly doubt we are going to move the library such that a non-resource type will generate hypermedia, if that's what you're asking.

kewne commented 7 years ago

I highly doubt we are going to move the library such that a non-resource type will generate hypermedia, if that's what you're asking.

That answers my "question", I'm closing this. Thank you for the time and effort in answering my questions 👍

gregturn commented 7 years ago

Best of luck in your coding efforts. 😉