Zwracanie zawsze tego samego obiektu w kontrolerze

Question

Witam,

napisałem serwis który zwraca odpowiedz dla klienta zgodnie z moim schematem czyli wiadomość, obiekt, status wcześniej zwracałem tylko obiekt i robiłem w ten sposób:

    public ResponseEntity<Set<UserDTO>> getAllUsers() {
        return new ResponseEntity<Set<UserDTO>>(mapUserListToUserDTOList(userService.getAllUsers()), HttpStatus.OK);
    }

po stworzeniu serwisu zwracam w ten sposób:

    public ResponseEntity<SuccessResponse> getAllUsers() {
        return responseService.successResponse("Users list", mapUserListToUserDTOList(userService.getAllUsers()), HttpStatus.OK);
    }

i jak wcześniej zwracałem ResponseEntity<Set<UserDTO>> tak teraz zwracam ResponseEntity<SuccessResponse> praktycznie wszędzie, czasami zdarzy się że zwracam ResponseEntity<ErrorResponse>. I tutaj jest moje pytanie czy takie rozwiązanie jest dobre? Mam trochę wątpliwości, ponieważ robiąc tak jak wcześniej wiedziałem co dana funkcja zwraca a teraz tylko wiem że zwraca status pozytywny a nie wiem jaki obiekt dokładnie, próbowałem jakoś wykorzystać typy generyczne żeby wyszło coś w tym stylu ResponseEntity<SuccessResponse<Set<UserDTO>> ale nie wyszło mi to do końca :/

Answer 1

Mam trochę wątpliwości, ponieważ robiąc tak jak wcześniej wiedziałem co dana funkcja zwraca a teraz tylko wiem że zwraca status pozytywny

 

Rozpracujmy to tak, żebym Ci wskazał  jak myśleć o danym endpoincie ( twój endpoint to getAllUsers) :
 

 getAllUsers()

Generalnie endpoint  dla klienta działa tak, ze zwrócić powinna listę użytkownik - to jest reakcja oczekiwana od strony klienta, status wystarcza, dlaczego? Bo rezultatem będzie, zwrócna lista- jako widok, oraz status że wszystko poszło okej. 
Nie musisz zwracać obiektu, natomiast powinieneś zwrócić uwagę na jedno "żeby obsługiwać błędy" tzn. zachowanie po stronie klienta powinno być zarówno dla niepowodzenia np-> status ( dla serwera, co poszło nie tak ) + informcja o braku elementów, jak i dla powodzenia -> status

Sprowadza się też problem do tego, co sawiera twoja encja "SuccessResponse" bo nie jest ona ni jak powiązana z endpointem i to błąd w architekturze rest, bo wrapujesz obiekt, który z punktu widzenia KLIENTA jest bezużyteczny a twoja metoda jest nie jasna, ma zwracać listę użytkowników, a zajmuje się obiektem "success/lub jego braku"
Więc co ostatecznie? 

Podsumowanie: generalnie nie jest to dobre rozwiązanie i powinieneś zwrócić "obiekt transferowany" tak jak wskazuje twoja metoda czyli 

 ResponseEntity<UserDTO> getAllUsers()

natomiast powinieneś skorzystać z "global exception handlera np" i obsłużyć sobie :) obiekt SuccessRespons -> jako rezultat np błędu/powodzenia i tego co ma się dziać w tej sytuacji, zachowanie jakieś "np logoi do bazy przy succes" z pomocą @controleradvice
 https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/bind/annotation/ControllerAdvice.html

W twoim przypadku dużo zależy od tego jakie API tworzysz, ale na daną chwile to co robisz to "OVER-ENGINERING"  SZTUKA DL A SZTUKI niepotrzebne łamanie zasad i komplikowanie sobie życia, zrozumienia innym programistą i przyszłą refaktoryzacją, czy debugowaniem 

Comments

dziękuje za tak obszerne wytłumaczenie, teraz rozumiem że moje wątpliwości co do tego podejścia były słuszne. Na początku chciałem żeby wszystkie odpowiedzi dla których akcja się udała (np pobranie listy wszystkich użytkowników) miały ten sam schemat odpowiedzi 

{
"message" : "Users list",
"data": userDTO,
"status": 200
}

przy pisaniu testów integracyjnych odczułem "ból" tego rozwiązania bo za każdym razem musiałem najpierw odpowiedz mapować do SuccessResponse a dopiero potem successResponse.data() do np. UserDTO. Pytanie czy przy pomocy @ControllerAdvice dałoby radę coś takiego zrobić a w kontrolerze mieć normalnie?

ResponseEntity<UserDTO> getAllUsers()

Bo jeżeli dobrze rozumiem to @ControllerAdvice łapię wyjątki.

---

@ControllerAdvic 
Wyłapuje wyjątki, on jest tylko aktywny ( czuły na wystąpenie wyjątku, stąd nie jest dla niego istotna sygnatura tej metody co ona zwraca), po prostu jeśli w CIELE - metody getAllUsers(), wyrzucisz wyjaek np... "orElseTHrow(" new MyEntityErrorResponse()"

- to wystarczy że stworzysz klase implementującą i zarządzającą obiektami dla ControlerAdvica i tam zajmiesz się obsługą 
MyEntityErrorResponse() jako klasy o nowym wyjątku 

---

Teraz już rozumiem, bardzo dziękuje za poświęcony czas :)