Conversation
| * Created by Facundo on 1/29/2018. | ||
| */ | ||
| @Configuration | ||
| public class ServiceConfig { |
There was a problem hiding this comment.
es necesaria esta clase?
There was a problem hiding this comment.
Es una buena practica no usar el autowired directamente en el service, despues cuando haces los UT no te levanta todo el contexto de spring. Pero sigo la forma en lo hiciste vos y lo saco leo
| * Created by Facundo on 1/29/2018. | ||
| */ | ||
| @RestController | ||
| @RequestMapping("/customers") |
There was a problem hiding this comment.
le pongamos "/api/customers" para seguir la misma convención de Users
| ResponseEntity<List<Customer>> responseEntity; | ||
| List<Customer> customers = customerService.getCustomers(); | ||
| if (customers == null || customers.isEmpty()) { | ||
| responseEntity = new ResponseEntity<>(HttpStatus.NOT_FOUND); |
There was a problem hiding this comment.
Es mejor devolver una lista vacía. Un NOT FOUND puede confundir con que el web service no existe
There was a problem hiding this comment.
Agree. Se podría mandar un OK o un NO_CONTENT.
| ResponseEntity<Customer> responseEntity; | ||
| Customer customerStored = customerService.addCustomer(customer); | ||
| if (customerStored == null) { | ||
| responseEntity = new ResponseEntity<>(HttpStatus.NOT_FOUND); |
There was a problem hiding this comment.
si esto es Null en realidad debería explotar. Es un caso no esperado. InternalServerError. Sacá el If directamente
| public ResponseEntity<Customer> updateCustomer(@RequestBody Customer customer) { | ||
| ResponseEntity<Customer> responseEntity; | ||
| Customer customerStored = customerService.updateCustomer(customer); | ||
| if (customerStored == null) { |
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; |
There was a problem hiding this comment.
Si se puede evitar el comodin (*) en los import mejor. Evitar su uso es una buena practica, no se gana en performance pero se evita la ambiguedad de impotar clases que no se utilizan.
| this.customerService = customerService; | ||
| } | ||
|
|
||
| @RequestMapping(method = RequestMethod.GET, produces = "application/JSON") |
There was a problem hiding this comment.
"application/JSON" no es el content-type estandar del iana (http://www.iana.org/assignments/media-types/media-types.xhtml). Conviene directamente usar los tipos provistos por la clase org.springframework.http.MediaType para evitar estos errores:
MediaType.APPLICATION_JSON_VALUE
| } | ||
|
|
||
| @RequestMapping(method = RequestMethod.PUT,produces = "application/JSON") | ||
| public ResponseEntity<Customer> updateCustomer(@RequestBody Customer customer) { |
There was a problem hiding this comment.
Si se envia un jsonBody con un ID que no existe, falla con un 500 en vez de un 404.
Me parece que queda mejor haciendo que la URL incluya el ID que se quiere modificar, ej: /api/customer/
Y en el request body los datos que se quieren modificar. Entre el controller/service tienen que hacerse responsables de:
- buscar el customer por el ID solicitado
- devolver 404 si no existe
- actualizar el customer si existe
| @@ -1,2 +1,7 @@ | |||
| spring.datasource.url=jdbc:postgresql://localhost:5432/template | |||
There was a problem hiding this comment.
Para que esto ande de una con el script "start.bat" es necesario que la URL de la BD apunte a el host "db", ej:
jdbc:postgresql://db:5432/template
4 participants
No description provided.