HT-14 implementation of Pure Rest API #1
Conversation
mykolaFilimonov
commented
Oct 22, 2023
mykolaFilimonov
commented
- Uniform interface used springframework.hateoas and links added to all endpoints at least every implement self link, and usually on list of all elements of the same type;
- Client-server, we implement only server side, we do not care about client side, so we can use browser, Postman, and any what we want;
- Layered we have simple app without additional layers between client and server
- Stateless all services and controllers not save any data, and can extract data only by id directly from repo
- Cachable used ShallowEtagHeaderFilter as auto implemented Etag handler, and this bullet require more learning from my side
- Uniform interface used springframework.hateoas and links added to all endpoints at least every implement self link, and usually on list of all elements of the same type; - Client-server, we implement only server side, we do not care about client side, so we can use browser, Postman, and any what we want; - Layered we have simple app without additional layers between client and server - Stateless all services and controllers not save any data, and can extract data only by id directly from repo - Cachable used ShallowEtagHeaderFilter as auto implemented Etag handler, and this bullet require more learning from my side
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
|
||
| @Controller | ||
| @ResponseBody |
There was a problem hiding this comment.
You can use '@RestController' instead that is itself annotated with @controller and @responsebody.
There was a problem hiding this comment.
Good point, I have just done it as a test, and one more time show @RestController it is easier and more understandable than just annotation
| .map(noteAssembler::toModel) | ||
| .toList(); | ||
|
|
||
| return CollectionModel.of( |
There was a problem hiding this comment.
Why don't you just move this logic to NoteAssembler class?
btw, RepresentationModelAssembler has a default implementation of toCollectionModel()
There was a problem hiding this comment.
thanks, not checked additional methods, it very helpful
| @Repository | ||
| public class UserRepository { | ||
|
|
||
| private final static Map<String, User> users = new HashMap<>(); |
There was a problem hiding this comment.
It is better to use ConcurrentHashMap, since HashMap is not thread-safe
| user.setNotes(new ArrayList<>()); | ||
| } | ||
| note.setUserUuid(userUuid); | ||
| user.getNotes().add(note); |
There was a problem hiding this comment.
You can encapsulate this logic in the User class via the helper method and use it here instead(e.g. user.addNote(note))
There was a problem hiding this comment.
thanks for suggestion, valid point fixed)
| if (!users.containsKey(user.getId())) { | ||
| users.put(user.getId(), user); |
There was a problem hiding this comment.
Better to use here putIfAbscent method which is atomical in ConcurrentHashMap and thus there will be no race condition in between of containsKey operation and put operation.
| if (user.getId() == null) { | ||
| user.setId(UUID.randomUUID().toString()); | ||
| } | ||
| if (!users.containsKey(user.getId())) { |
There was a problem hiding this comment.
It's an interesting part of the code in concept "what if there is already UUID in the map?". Do you expect duplicates?
If so, then better to make it in loop until unique UUID is found.
| return users.values().stream().toList(); | ||
| } | ||
| public void addUser(User user) { | ||
| if (user.getId() == null) { |
There was a problem hiding this comment.
How can you have here non-null user ID? It's create operation, right?
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.ExceptionHandler; | ||
|
|
||
| @org.springframework.web.bind.annotation.ControllerAdvice |
There was a problem hiding this comment.
Any reason to put here full qualified name with package?
| } | ||
|
|
||
| public List<Note> getNotes() { | ||
| return Collections.unmodifiableList(notes); |
There was a problem hiding this comment.
If notes field is null there will be NullPointerException thrown.
|
|
||
| public List<Note> getAllNotes(String userUuid) { | ||
| var user = this.getUser(userUuid); | ||
| return user.getNotes() == null ? Collections.emptyList() : user.getNotes(); |
There was a problem hiding this comment.
Better to handle this case in one place - getNotes method itself.
