spring-guides / tut-rest

Building REST services with Spring :: Learn how to easily build RESTful services with Spring
https://spring.io/guides/tutorials/rest/
515 stars 412 forks source link

Complete Building REST services with Spring tutorial with OrderNotFoundAdvice Class #101

Open oulanbator opened 2 years ago

oulanbator commented 2 years ago

It may not be really relevant for "oppening an issue", but since the last part of the tutorial intends to put all together the Order associated classes, in the same way that we built the Employee classes (in order to build a "real" RESTful service), i felt like something was missing here : the OrderNotFoundAdvice Class. Indeed, as we left our code at the end of the tutorial, a bad request on orders (i.e. curl -v localhost:8080/orders/5) send back a 500 error (internal server error) instead of the 404 we should expect. It is probably just a small piece of code, and almost no explanations to add in the tutorial as the benefits are already explained earlier, when building the Employee classes. It's my first contribution to a project on Github so I don't really know how to help more from here, but I could easily write the missing paragraph if needed.

robertmcnees commented 1 month ago

Hi @oulanbator. Thank you for your issue. I see that there are inconsistencies in the links module. If you would like to submit a PR, you could contribute a new OrderNotFoundAdvice similar to the existing EmployeeNotFoundAdvice class. Make sure that the README.adoc is also updated, as some people may be building the code from scratch when going through the tutorial so all relevant instructions need to be declared.

This part seems like a separate issue to me:

Indeed, as we left our code at the end of the tutorial, a bad request on orders (i.e. curl -v localhost:8080/orders/5) send back a 500 error (internal server error) instead of the 404 we should expect.

I didn't have this issue when I went through the tutorial. While the approach between Order and Employee is inconsistent, as you called out, I did not find that it caused an unexpected result. Can you elaborate?

I'll keep this issue open for a future PR.