spring-projects / spring-hateoas-examples

Collection of examples on how (and why) to build hypermedia-driven apps with Spring HATEOAS
Apache License 2.0
384 stars 186 forks source link

Possible error in examples for "HATEOAS with Affordances" #32

Closed MormonJesus69420 closed 4 years ago

MormonJesus69420 commented 4 years ago

Hello, I am quite new to the whole Spring ecosystem, so excuse me if I am wrong. I just started new job that uses Spring and I am trying my best to learn up to date info in regards to Spring.

When reading through guide about HATEOAS and Affordances I came across following snippet:

return new EntityModel<>(savedEmployee,
            linkTo(methodOn(EmployeeController.class).findOne(savedEmployee.getId())).withSelfRel()
                .andAffordance(afford(methodOn(EmployeeController.class).updateEmployee(null, savedEmployee.getId())))
                .andAffordance(afford(methodOn(EmployeeController.class).deleteEmployee(savedEmployee.getId()))),
            linkTo(methodOn(EmployeeController.class).findAll()).withRel("employees")
        ).getId() >>> ISSUE
            .map(Link::getHref)
            .map(href -> {
                try {
                    return new URI(href);
                } catch (URISyntaxException e) {
                    throw new RuntimeException(e);
                }
            })
            .map(uri -> ResponseEntity.noContent().location(uri).build())
            .orElse(ResponseEntity.badRequest().body("Unable to create " + employee));

The method call getId() that I marked with >>> ISSUE seems to cause problems for me. When I used it my IDE (IntelliJ IDEA) complained that this method does not exists in the EnityModel<Employee> class. I couldn't find any reference to getId() method in documentation for both the current Spring HATEOAS API documentation, or the 1.0.0 API documentation.

When reading through source code I found that getId() wasn't actually used, it uses .getLink(IanaLinkRelations.SELF), which when used in my code worked immediately. It might be good idea to update the README.adoc file to reflect that.

Also, when reading through the current API documenation I found that the constructor EntityModel(T content, Link... links) is deprecated and that EntityModel.of(T content, Link... links) should be used instead. I think it might be good to update the example code and guide to use the newer and not deprecated solution.

Lastly, it is nice that RepresentationModelAssembler<> is mentioned and kept out of the example for simplicity, but it might be a good idea to point to an example where it is used, or have a short snippet showing how it can be implemented and used. I followed the guide found on Spring.io, so you might consider linking to it as an example.

odrotbohm commented 4 years ago

Where did you find the example code exactly?

MormonJesus69420 commented 4 years ago

Oh right, I forgot to link to the examples. You can find there here https://github.com/spring-projects/spring-hateoas-examples/blob/master/affordances/README.adoc#defining-your-domain the getId() method is used both in newEmployee(@RequestBody Employee employee) and updateEmployee(@RequestBody Employee employee, @PathVariable long id) methods.

odrotbohm commented 4 years ago

Got it. The getId() refers to a previous version of the API that returned the self link for that method. I.e. you can replace that call with ….getRequiredLink(IanaLinkRelations.SELF) and should be good to go. We'll update the example. Thanks for reporting this.

MormonJesus69420 commented 4 years ago

No problem. I just was confused when reading the tutorial and it didn't work, but looking through sourcecode I found out how to fix it. Also just wanted to say that your talk "REST Beyond the Obvious" from 2018 was very interesting and informative.