Nazewnictwo adresów REST API

Posiadam w mojej aplikacji REST kontroler z kilkoma metodami pod adreseami

@PostMapping(value = "/sendMessage")
@GetMapping(value = "/getSentMessages")
@GetMapping(value = "/getReceivedMessages")
@DeleteMapping(value = "/removeSentMessage")
@DeleteMapping(value = "/removeReceivedMessage")
@GetMapping(value = "/searchSentMessages")

i chciałym przerobić te adresy na typowo RESTowe, więc np. adres

@PostMapping(value = "/sendMessage")

mogę zmienić na

@PostMapping(value = "/message")

A co w przypadku gdybym chciał z adresu

@GetMapping(value = "/getReceivedMessages")

również utworzyć typowo RESTowy adres, to jak powinien on wyglądać, może

@GetMapping(value = "/messages/received")

Czy takie nazewnictwo jest odpowiednie?

jak chcesz wysłać wiadomość to
POST /message
chcesz pobrać wiadomość
GET /message/{id}

Mam dwa DTO:

public class SentMessageDTO {
    ...
    private String recipient;

i

public class ReceivedMessageDTO {
     ...
    private String sender;

W przypadku pobierania danych wiadomości wysłane i odebranej trochę się różnią, dlatego pobieranie danych za pomocą jednego kontrolera trochę nie pasuje, bo według tego sposobu, aby pobierać wiadomość tylko zapytaniem

@GetMapping("/message/{id}")

konieczne by było stworzenie DTO z nadawcą i odbiorcą, a potrzebuje tylko jednej z tych zmiennych w zależności jaka to jest wiadomość. W sytuacji, gdy potrzebuję wiadomości wysłanej, to w DTO nie potrzebuję kto jest nadawcą i ta zmienna nie będzie użyta.

Słabe rozwiązanie. Jak chcesz zrobić jako zasób REST to zrób to solidnie bez zbędnego skomplikowania i tyle. Jak nie wiesz jak coś ma działać to najlepiej zasugerować się czymś co już istnieje. W przypadku wiadomości dobrym przykładem jest poczta. Każda wiadomość składa się z odbiorcy, nadawcy, treści i załączników. Normalnie możesz wyświetlić wszystkie wiadomości (GET na kolekcji wiadomości), wyświetlić wiadomości wysłane i odebrane (pewnego typu filtr na kolekcji). Jakiś czas temu pisałem podobny kod, co prawda nie jestem z niego dumny no ale skoro wisi na gh to niech tam wisi. W moim przypadku wiadomość sie tworzy tylko podając adresata, nadawca jest ustawiany jako zauthentykowany użytkownik, z kolei przy pobraniu wiadomości dostaje się wszystko: odbiorce, nadawce, czas wysłania itp. Dodatkowo możesz filtrować całość za pomocą enuma który pozwala ci pobrać wiadomości wysłane, otrzymane oraz wszystkie (tym razem bym wywalił enuma ALL bo jest trochę bezużyteczny).

Controller

PS: Polecam lombok'a

@Interpod: dzięki za rady. Przeanalizuję Twój projekt, aby zobaczyć jak powinno to wyglądać.
Mam jeszcze takie pytanko. Otóż posiadam jeden kontroler z metodami, które służą do walidacji pól w jQuery

@ApiOperation(value = "Return false if the user by username exists")
    @GetMapping("/checkUsernameAtRegistering")
@ApiOperation(value = "Return false if the user by e-mail exists")
    @GetMapping("/checkEmailAtRegistering")
@ApiOperation(value = "Return true if the user by username exists")
    @GetMapping("/checkUsername")
@ApiOperation(value = "Return true if the given password is the same as the database")
    @GetMapping("/checkPassword")

jak powinno wyglądać REST API dla tego typu metod. A potrzebuję tych metod do walidowania formularzy w js.

Ja osobiście zrobilem validacje na backendzie, w taki sposób ze skleiłem customowego validatora:

RegisterValidator

Następnie ten validator validator jest używany w controllerze przy rejestracji:

AuthController

Gdzie jak coś sie posypie to jest rzucany exception który następnie jest tłumaczony na odpowiedni response.
Jeżeli chciałbyś faktycznie to na frontendzie zrobić co wg mnie troche mija sie z celem to jedynym sensownym rozwiązaniem mogło by być:

GET users/{username}

Tylko tutaj fajnie by było dodać case'a który robił by jakaś walidacje czy jeżeli masz prawo do obejrzenia profilu użytkownika to zwraca ci DTO całe (również zawierające poufne dane typu hasło itp), a jeżeli nie to zwróciłoby jakieś okrojone i wtedy we froncie mógłbyś sprawdzić czy sie matchują jakieś pola i zwrócić informacje o błędzie.
Ewentualnie coś takiego jeszcze:

GET publicprofiles/{username}

I wtedy ten endpoint zwracałby pola które musisz sprawdzić przy rejestracji. (PS: Obydwa sposoby są co najmniej słabe).

No tyle że jednak lepszym rozwiązaniem jest po prostu wysłanie POST'a do rejestracji i jak wróci ci status odpowiedzi i ResponseBody po prostu sprawdzic czy status jest 404 i wtedy odczytać z ResposeBody opis błedów, ewentualnie 201 i wtedy wyciągnąć z headera location link i wypluć użytkownikowi.

**Teraz zauważ że w przypadku pierwszych 2 sposobów aby użytkownik się zarejestrował i dał mu odpowiedź musi zawsze polecieć 2 zapytania do backendu. Co jest tragicznym rozwiązaniem. **Dlatego że może wystąpić sytuacja że jak już pobierzesz te profile będziesz sprawdzał czy istnieje juz cos takiego na backendzie i w tym czasie inny użytkownik zrobi to samo, albo nawet zaczął przed tobą. I wystąpi coś na wzór wyścigu. Między twoim pierwszym i drugim requestem poleci POST utworzenia użytkownika o danych które do tej pory nie istniały. Ty będziesz posiadał informacje że użytkownik o tych danych może być utworzony, a tutaj się w miedzy czasie ktos zarejestrował tymi danymi. Wyślesz POST'a i dostaniesz albo InternalServerError albo inna odpowiedź jeżeli ustawiłeś sobie mapowanie Internali (chyba że nie ustawiłeś sobie constrains'ów na bazie na unique). Dlatego lepszym rozwiązaniem jest jednak korzystanie z jedno krokowej rejestracji.

Dużo solidnej lekcji. Dzisiaj przez cały dzień będę próbował to zrobić i zrefaktoryzować swój kod według tego co mi napisałeś. W takim razie na froncie zostawię tylko walidację długości i składni, a takie elementy jak sprawdzanie czy username istnieje itd., to zastosuję ten walidator tak jak Ty to zrobiłeś.

@Interpod:

A co sądzicie o takim rozwiązania?

@GetMapping(value = "/api/message/{id}")
    public
    HttpEntity<? extends MessageDTO> getMessage(
            @ApiParam(value = "Message type", required = true) @RequestParam MessageType messageType,
            @ApiParam(value = "The message ID", required = true) @PathVariable Long id
    ) {
        return messageService.getMessage(id, authorizationService.getUserId(), messageType)
                .map(message -> {
                    if(messageType == MessageType.RECEIVED) {
                        return ResponseEntity.ok().body(converterMessageToReceivedMessageDTO.convert(message));
                    } else /* if(messageType == MessageType.SENT) */ {
                        return ResponseEntity.ok().body(converterMessageToSentMessageDTO.convert(message));
                    }
                }).orElseGet(() -> ResponseEntity.notFound().build());
    }

Musi być podział na dwa typy wiadomości, wysłaną i odebraną, bo np. przy odebranej odbiorca może zobaczyć swoją datę przeczytania, ale z punktu nadawcy nie właściwe jest, aby on widział również datę przeczytania. To taka tajemnica niedostępna dla nadawcy.
Z obiektami zrobiłem to tak:

@Data
public class MessageDTO {
    private Long id;
    private String sender;
    private String recipient;
    private String subject;
    private String text;
    private Date dateOfRead;
}

@JsonInclude(JsonInclude.Include.NON_NULL)
public class ReceivedMessageDTO extends MessageDTO {} // Przy odbiorcy usuwam pole 'recipient' dzięki @JsonIclude

@JsonInclude(JsonInclude.Include.NON_NULL)
public class SentMessageDTO extends MessageDTO {} // Przy nadawcy usuwam pole 'dateOfRead' i 'sender' dzięki @JsonIclud

PS: Trochę nie pasują mi te obiekty. Macie pomysł jak zrobić to lepiej? Albo np. przyjmowany MessageType zadeklarować jako Optional i w razie jego braku zaprogramować ustalenie typu i dopiero przejść do bloku return.

URL - jest OK, o to właśnie chodzi :).

Twoje uzasadnienie dot. logiki biznesowej jest jak najbardziej słuszne. Osobiście natomiast zaimplementowałbym to trochę inaczej. Wg mnie jeżeli pytam się o zasób /ap/message, to oczekuję, że zawsze w dokumencie wynikowym będę mieć przewidywalny zestaw atrybutów. Innymi słowy, nie powinno być tak, że jak pytam się o wiadomość nr 1, to atrybut dateOfRead istnieje, a jak o wiadomość nr 2, to go nie ma. Zamiast tego, możesz po prostu przysyłać w tych atrybutach puste wartości.

Kiedy lista atrybutów może być zmienna? Wtedy, kiedy mam na to wpływ:

• wersjonowanie API - tu mam wpływ na to, z jaką wersją API gadam i jaki zestaw atrybutów mnie obowiązuje,
• kiedy API pozwala np. poprzez jakiś query param poprosić o konkretne atrybuty.

@zyxist: dzięki za odpowiedź. Dostałem jeszcze taką odpowiedź na https://stackoverflow.com/questions/46203232/rest-method-for-getmessages?noredirect=1#comment79369937_46203232, aby utworzyć oddzielne adresy dla pobierania wiadomości wysłane i odebranej. Co o tym myślisz? Ważny jest fragment 'Furthermore, if a new messageType is added, the existing method would have to be changed which violates the open/close principle'.

Na StackOverflow masz jeszcze jedną fajną propozycję, mianowicie by zbudować sobie adresy w stylu /api/message/sent/ oraz /api/message/received/ - wtedy widać, że chodzi o różne typy tej samej wiadomości, ale będące różnymi zasobami. I tu wg mnie już mogą być różne zestawy pól.

To, które z rozwiązań jest lepsze, należy już bardziej do zagadnień logiki biznesowej. Dla każdego z nich można znaleźć dobre uzasadnienie. Jeśli klient wymaga, by móc odpytać się o każdą wiadomość, podając tylko jej ID, to rozwiązanie ze StackOverflow odpada. Jeśli klient będzie oddzielnie pracował z wiadomościami odebranymi i wysłanymi i zawsze zna typ wiadomości, o który chce się zapytać, podejście ze StackOverflow jest OK i pozwoli na zróżnicowanie zestawów atrybutów.

PS. Jeśli chodzi o zasadę otwarte/zamknięte, to da się to zaimplementować tak, by można było dodawać nowe typy wiadomości bez jej łamania :).

@zyxist:
Teraz mój kontroler wygląda w taki sposób https://pastebin.com/XuHgNLVW. Jednak według mnie trochę panuje tutaj za duży burdel. Używanych jest bardzo dużo obiektów i potrzebnych do nich konwerterów, co trochę psuje wygląda całościowy. Mimo to wiadomo co gdzie jest.

Klawiatur napisał(a):

@zyxist:
Teraz mój kontroler wygląda w taki sposób https://pastebin.com/XuHgNLVW. Jednak według mnie trochę panuje tutaj za duży burdel. Używanych jest bardzo dużo obiektów i potrzebnych do nich konwerterów, co trochę psuje wygląda całościowy. Mimo to wiadomo co gdzie jest.

O kurde,

1. Lombok
2. Wywal autowired
3. Autowired Constructor > Getter > ............ nigdy? > field
4. Robisz coś takiego

addMessage(@ApiParam(value = "Message", required = true) @RequestBody @Valid SendMessageDTO sendMessageDTO,  UriComponentsBuilder uriComponentsBuilder)

Pierwsze pytanie co daje ci annotacja valid w tym przypadku? Czy ona po prostu jest żeby byla? Żeby coś to robilo to musi być coś w stylu:

addMessage(@RequestBody @Valid SendMessageDTO messageDTO, Errors errors, ...,) {
   ifErrorsThrowRuntime(errors);
}

Zwracanie HttpEntity wgl trochę tylko rozwala jakość tego. Jak znajdę chwile to ogarnę resztę tego kodu.

1. Co jest nie tak z Lombok?
   2,3) W kontrolerze nie ma chyba znaczenia czy autowired czy konstruktor. Jakbym testował kontroler, to używałbym MockMvc i nie ma potrzeby tworzenia konstruktora.
2. To jest pod kontrolą https://github.com/JonkiPro/REST-Web-Services/blob/master/src/main/java/com/service/app/rest/controller/advice/ErrorFieldsExceptionHandler.java Może wpiszę coś w komentarzu, bo ktoś też może pomyśleć, że ta walidacja nie działa.

Teraz moje adresy wyglądają w ten sposób https://zapodaj.net/b1a1febdc87fc.png.html https://zapodaj.net/3e5b6cb8c2488.png.html