Walidacja kluczy obcych w warstwie domeny

W encji Rate mam taki konstruktor:

        public Rate(int productId, Guid customerId, int stars)
        {
            Guard.InRange(stars, 1, 6, "Liczba gwiazek nie jest poprawna."); // rzuca wyjątek z kodem NotValid

            if (productId <= 0)
                throw new MyStoreException(Code.NotSpecified, "ID produktu nie jest poprawne.");

            if (customerId == Guid.Empty)
                throw new MyStoreException(Code.NotSpecified, "ID klienta nie jest poprawne.");

            ProductId = productId;
            CustomerId = customerId.ToString();
            Stars = stars;
        }

Chciałbym tłumaczyć wyjątki z warstwy domeny na jakieś czytelne odpowiedzi dla użytkownika. O ile ma to sens w przypadku, gdy użytkownik poda (w jakiś sposób) niepoprawną ilość gwiazdek i walidacja tego nie wyłapie, o tyle w sytuacji, gdy ID produktu albo klienta nie jest poprawne, raczej sensu to nie ma, bo przyczyną tych błędów będą prawdopodobnie luki w samej aplikacji (np. binding nie zadziała), a nie użytkownik. Co użytkownik dowiedziałby się z informacji, że ID klienta nie jest poprawne? Zrobiłem to tak, że w domenie rzucam wyjątki z kodem NotValid, które są tłumaczone na Bad Request i wyjątki z kodem NotSpecified, które są tłumaczone na Internal Server Error (w odpowiedzi jakaś ogólna informacja, że coś nie działa, a sama wiadomość z wyjątku idzie do logów). Pytania:

1. Czy ma to sens? Nie byłoby lepiej zawsze zwracać 400 albo 500?
2. Czy nie lepiej byłoby przekazywać do konstruktora encje, zamiast tylko ich ID? np. Rate(Product product, Customer customer, int stars)?

Jak tak patrzę po repozytoriach devmentors, to oni tych kluczy raczej w ogóle nie walidują, przykład: https://github.com/devmentors/Pacco.Services.Parcels/blob/master/src/Pacco.Services.Parcels.Core/Entities/Parcel.cs

Wydaje mi się, że generalnie taki błąd wyskoczy tylko gdy ktoś próbuje nam popsuć system, a zatem nie ma potrzeby zwracać dobrych komunikatów błędu

Pytanie tylko czy poleci wyjątek przy Save - wydaje mi się, że tak, bo na poziomie bazy wywali, że nie ma takiego keya? (jeżeli relacyjna)

Niemniej jednak bym pozostał przy sprawdzaniu.

Wyjątek przy Save na pewno poleci i użytkownik dostanie 500 automatycznie. Skoro i tak ten błąd wyskoczy tylko przy próbie zepsucia aplikacji, to takie sprawdzanie IMO niewiele wnosi, ale dalej nie jestem przekonany - w końcu chcę mieć zawsze nieskazitelnie czyste encje.

Do tego sam Piotr czasami sprawdza poprawność kluczy, a czasami nie: https://github.com/devmentors/DNC-DShop.Services.Discounts/blob/master/src/DShop.Services.Discounts/Domain/Discount.cs

One Source of Truth.
Dwóch sprawdzających? Wyjdzie bokiem.

Repozytoriami z devmentors bym się bardzo nie sugerował, bo ile nagrania są fajne i wartościowe, to kod taki sobie żeby nie powiedzieć słaby, jak już to spoglądaj na repetytorium Kamila Grzybka zupełnie inny poziom.

Ja bym się skłaniał ku opcji z przesyłaniem encji, idki używamy tylko do pobierania encji z repozytoriów, a dalej już staramy się operować na encjach, w końcu programujemy obiektowo tak :)?

Czyli w serwisie aplikacyjnym pobierasz wszystkie potrzebne encje, jak nie istnieje jakaś rzucasz błędem, a potem orkiestrujesz działanie na nich.

nobody01 napisał(a):

1. Czy ma to sens? Nie byłoby lepiej zawsze zwracać 400 albo 500?

400 możesz zwracać tylko wtedy, jeśli użytkownik jest sam w stanie naprawić swój request. Czy to taka sytuacja? Chyba nie.

2. Czy nie lepiej byłoby przekazywać do konstruktora encje, zamiast tylko ich ID? np. Rate(Product product, Customer customer, int stars)?

No raczej.

Meh, sam już nie wiem. Okazuje się, że Kamil czasami jednak wstrzykuje ID (value object) zamiast encji: https://github.com/kgrzybek/sample-dotnet-core-cqrs-api/blob/master/src/SampleProject.Domain/Payments/Payment.cs Najrozsądniejsze wydaje się być chyba wstrzykiwanie encji tylko wtedy, gdy potrzebnych jest o niej więcej danych niż tylko ID. Wstrzykiwanie za każdym razem encji może powodować spowolnienie np. w sytuacji, gdy mamy w JWT ID klienta i nie możemy go po prostu przekazać, tylko musimy słać zapytanie do bazy, by pobrać całą encję.

Jak tak patrzę, to ta opcja ze wstrzykiwaniem idków jest dużo popularniejsza, mimo że niepoprawna w sensie OOP. Weźmy np. to: https://github.com/dotnet-architecture/eShopOnContainers/blob/dev/src/Services/Ordering/Ordering.Domain/AggregatesModel/OrderAggregate/Order.cs

No tak, Kamil wstrzykuje VO a nie zwykłego inta czy guida. VO będący idkiem tylko jednego typu jest pełnoprawnym zmienikiem instancji tego typu, czego o int czy guid powiedzieć nie możemy.