Architektura aplikacji - hexagonal/podział na pakiety - kilka pytań

0

Cześć,
tworzę projekt w ramach nauki. Do tej pory stosowałem package by feature - na przykład wyglądało to tak: miałem pakiet registration bez sub-pakietów, a w nim wszystkie klasy związane z rejestracją użytkownika, wszystkie klasy były publiczne.

Postanowiłem to poprawić i zrobić jakiś refaktor. Tak oto trafiłem na pojęcie hexagonal architecture. Trochę poczytałem m.in. ten wątek:
Architektura heksagonalna to oszustwo

ważne posty odnośnie architektury heksagonalnej z tego wątku:

some_ONE napisał(a):

Ja heksagonalną/czystą architekturę rozumiem tak, że mam wewnętrzną warstwę biznesową i ona powinna wiedzieć jak najmniej o zewnętrznym świecie. Dlatego jak muszę odpytać jakiś zewnętrzny serwis (REST, SOAP, a może bezpośrednio bazę systemu legacy) to wtedy tworzę port i warstwy biznesowej nie obchodzi co jest pod spodem.
Ale to, że jakiś serwis biznesowy ma coś wstrzyknięte, to jeszcze nie znaczy, że wszystkie zależności mają być portami schowanymi za interfejsem. Jak w handlerze mam walidator to nie tworzę portu walidatora, bo to nie ma sensu. Jak mam pomocniczy serwis określający czy dany użytkownik może wykonać akcję to nie tworzę do niego portu, bo to znowu nie ma sensu.

Przynajmniej tak ja to widzę ;)

Grzyboo napisał(a):

Też rozumiem to jak some_ONE wspomniał - porty tworzysz do zewnętrznych(pozaaplikacyjnych) usług (REST service, baza danych, operacje na filesystemie itp.), a nie do jakiś drobnych klas, które wydzieliłeś, żeby zachować SRP. A do tych zewnętrznych serwisów prawie zawsze i tak chcesz mieć wiele implementacji: do testów, do środowisk nie-produkcyjnych itp.
Tworzenie tony single-implementation interfejsów tylko dla zasady jest głupie i szkodliwe.

Shalom napisał(a):

Dla mnie generalnie rozbija się to o bardzo prosty koncept: domain bez zależności.
Domena nie wie nic o tym ze komunikujemy się z jakimiś zewnętrznymi systemami i nigdzie w domenie nie ma referencji do żadnej "zewnętrznej" klasy (oczywiście nie mówię tu o jakimś vavr czy innych libkach), bo w moim przypadku moduł domain nie ma dependency na nic takiego.
To automatycznie wymusza, żeby wszystkie "zewnętrze" zależności były owrapowane jakimś "naszym" modułem (adapterem), który spina naszą domenę z tym zewnętrznym serwisem a sam jest wystawiony w domenie jako jakiś nasz interfejs (port).

Na podstawie tych postów, zrobiłem refaktor pakietu registration na coś takiego, tworząc port i adapter dla zewnętrznej usługi - bazy danych:

registration
├── adapter
│   └── registrationdb
│       ├── RoleRepositoryAdapter
│       └── UserRepositoryAdapter
├── application
│   ├── controller
│   │   └── RegistrationController
│   └── dto
│       └── RegisterRequest
├── domain
│   ├── exception
│   │   ├── CredentialValidationException
│   │   └── RoleNotFoundException
│   ├── model
│   │   ├── Role
│   │   ├── User
│   │   └── UserRole
│   ├── ports
│   │   ├── RoleRepository
│   │   └── UserRepository
│   └── service
│       ├── EmailExistenceValidationStrategy
│       ├── EmailFormatValidationStrategy
│       ├── PasswordEncoderService
│       ├── PasswordFormatValidationStrategy
│       ├── RegistrationValidation
│       ├── RegistrationValidationStrategy
│       ├── UserFactory
│       ├── UsernameExistenceValidationStrategy
│       ├── UsernameValidationStrategy
│       ├── UserRegistration
│       ├── UserRegistrationService
│
  1. W tym rozwiązaniu część klas będzie musiała być publiczna, z uwagi na to, że w Javie prywatne klasy są dostępne tylko w obrębie swojego pakietu. Czyli exception, dto itp. - klasy publiczne.

Podział odpowiedzialności port-adapter wygląda tak:

port

public interface UserRepositoryPort {

    int save(User user);
    void assignRoleToUser(int userId, int roleId);
}

adapter

class UserRepositoryAdapter implements UserRepositoryPort {

    private final UserDAO userDAO;

    public UserRepositoryPortAdapter(UserDAO userDAO) {
        this.userDAO = userDAO;
    }

    @Override
    public int save(User user) {
        return userDAO.save(user);
    }

    @Override
    public void assignRoleToUser(int userId, int roleId) {
        userDAO.assignRoleToUser(userId, roleId);
    }
}

dao

@Repository
public interface UserDAO {

    @SqlUpdate("INSERT INTO users (username, email, password) VALUES (:username, :email, :password)")
    @GetGeneratedKeys
    int save(@BindBean User user);

    @SqlUpdate("INSERT INTO user_roles (user_id, role_id) VALUES (:userId, :roleId)")
    void assignRoleToUser(@Bind("userId") int userId, @Bind("roleId") int roleId);
}

Czy może tak być?

  1. Encji User używam w funkcjonalności rejestracji - pakiet registration. Co jeśli tej samej encji będę musiał użyć w nowej funkcjonalności, przykładowo login feature. Powinienem stworzyć jakiś wspólny pakiet typu common i tam ją przechowywać?

  2. Czy mógłbym zastosować jakieś lepsze podejście do podziału na pakiety?

Dzięki za pomoc

1

Jest dobrze.

Chociaz ja (php) posluguje sie troche inna nomenklatura w nazewnictwie warstw:

|
├── infrastructure
|    └── repository
|         ├── repo1
|         └── ...
└── domain
     ├── entity
     |    ├── entity1
     |    └── ...
     └── interface
          ├── repo1Interface
          └── ...
1

Kilka pytań:

  • czy repozytoria zwracają tylko modele biznesowe? Czy może jednak jakieś klasy bazodanowe uciskają?
  • czy na pewno w kontrolerze jest tylko obsługa http, i na pewno nie ma logiki?
  • ten pakiet service wygląda trochę jak kitchensink. Pokażesz kod tych service'ów? Bo jakby nie patrzeć takie service' też mogą być częścią domeny, zależnie od tego co w nich jest.

Jaki jest sens tego? UserRepositoryAdapter wygląda jakby nic nie robił i był po nic. Czemu DAO nie może implementować UserRepositoryPort?

Ornstein napisał(a):
  1. W tym rozwiązaniu część klas będzie musiała być publiczna, z uwagi na to, że w Javie prywatne klasy są dostępne tylko w obrębie swojego pakietu. Czyli exception, dto itp. - klasy publiczne.

No tak jak w każdej aplikacji. Wiadomo że klasa ma być publiczna, jeśli coś innego ma jej użyć.

Podział odpowiedzialności port-adapter wygląda tak:

Czy może tak być?

Czyli u Ciebie port i adapter to w zasadzie interfejs i implementacja? Bo jeśli tak, to może tak być.

  1. Encji User używam w funkcjonalności rejestracji - pakiet registration. Co jeśli tej samej encji będę musiał użyć w nowej funkcjonalności, przykładowo login feature. Powinienem stworzyć jakiś wspólny pakiet typu common i tam ją przechowywać?

Musisz rozważyć wady i zalety obu podejść. W gruncie rzeczy musisz wybrać pomiędzy DRY a Loose-Couplingiem, zależnie od projektu jedno albo drugie brzozę odpowiednie.

  1. Czy mógłbym zastosować jakieś lepsze podejście do podziału na pakiety?

Nie ma dobrego i złego podejścia. Różne architektury są dobre do różnych

0
Riddle napisał(a):
  • czy repozytoria zwracają tylko modele biznesowe? Czy może jednak jakieś klasy bazodanowe uciskają?

Zwracają tylko modele biznesowe. Wyniki zapytań mapuję na obiekty biznesowe.

  • czy na pewno w kontrolerze jest tylko obsługa http, i na pewno nie ma logiki?

Jest tylko obsługa http.

@RestController
@RequestMapping("/api/v1/users")
public class RegistrationController {

    private final UserRegistrationService registration;

    public RegistrationController(UserRegistrationService registration) {
        this.registration = registration;
    }

    @PostMapping("/register")
    public ResponseEntity<ApiResponse> register(@Valid @RequestBody RegisterRequest request) {
        ApiResponse registrationResponse = registration.register(request);
        return new ResponseEntity<>(registrationResponse, HttpStatus.CREATED);
    }
}

Jaki jest sens tego? UserRepositoryAdapter wygląda jakby nic nie robił i był po nic. Czemu DAO nie może implementować UserRepositoryPort?

DAO i port to interfejsy. Nie mogę implementować interfejsu w interfejsie. Musiałbym coś tutaj zmienić.

  • ten pakiet service wygląda trochę jak kitchensink. Pokażesz kod tych service'ów? Bo jakby nie patrzeć takie service' też mogą być częścią domeny, zależnie od tego co w nich jest.

Tutaj wkradł mi się mały błąd, ponieważ użyłem ChatGPT do wygenerowania drzewka z podziałem na pakiety na podstawie screenshota z IDE. Chat popełnił błąd, a ja w ogóle nie zwróciłem na to uwagi. Pakiet service to sub-pakiet pakietu domain. Poprawiłem wcześniejszy post.

Kod tych klas wygląda tak:

wejście

@Slf4j
@Service
public class UserRegistrationService {

    private final RegistrationValidation validation;
    private final ApplicationMessageLoader message;
    private final UserRegistration registration;

    public UserRegistrationService(RegistrationValidation validation,
                                   ApplicationMessageLoader message,
                                   UserRegistration registration) {

        this.validation = validation;
        this.message = message;
        this.registration = registration;
    }

    public ApiResponse register(RegisterRequest request) {
        validation.validateRequest(request);
        registration.registerUser(request);
        return new ApiResponse(message.getMessage("user.registration.success"));
    }
}

Walidacja

class RegistrationValidation {
    
    private final List<RegistrationValidationStrategy> strategies;

    public RegistrationValidation(List<RegistrationValidationStrategy> strategies) {
        this.strategies = strategies;
    }

    public void validateRequest(RegisterRequest request) {
        for (RegistrationValidationStrategy strategy : strategies) {
            strategy.validate(request);
        }
    }
}

interface RegistrationValidationStrategy {
    void validate(RegisterRequest registerRequest) throws CredentialValidationException;
}

class ValidationRegex {

    public static final String USERNAME_PATTERN = "^[a-zA-Z0-9]{3,20}$";
    public static final String EMAIL_PATTERN = "^[a-zA-Z0-9._%+-]{1,15}@[a-zA-Z0-9.-]{2,15}\\.[a-zA-Z]{2,6}$";
    public static final String PASSWORD_PATTERN = "^(?=.*[0-9])(?=.*[a-z])(?=.*[A-Z])(?=.*[@#$%^&+=])(?=\\S+$).{8,}$";
}

@Service
@Order(1)
public class UsernameValidationStrategy implements RegistrationValidationStrategy {

    private final ApplicationMessageLoader message;

    public UsernameValidationStrategy(ApplicationMessageLoader message) {
        this.message = message;
    }

    @Override
    public void validate(RegisterRequest request) throws CredentialValidationException {
        if (!request.username().matches(ValidationRegex.USERNAME_PATTERN)) {
            throw new CredentialValidationException(message.getMessage("validation.username.error"));
        }
    }
}

@Service
@Order(2)
public class EmailFormatValidationStrategy implements RegistrationValidationStrategy {

    private final ApplicationMessageLoader message;

    public EmailFormatValidationStrategy(ApplicationMessageLoader message) {
        this.message = message;
    }

    @Override
    public void validate(RegisterRequest request) throws CredentialValidationException {
        if (!request.email().matches(ValidationRegex.EMAIL_PATTERN)) {
            throw new CredentialValidationException(message.getMessage("validation.email.error"));
        }
    }
}

@Service
@Order(3)
public class PasswordFormatValidationStrategy implements RegistrationValidationStrategy {

    private final ApplicationMessageLoader message;

    public PasswordFormatValidationStrategy(ApplicationMessageLoader message) {
        this.message = message;
    }

    @Override
    public void validate(RegisterRequest request) throws CredentialValidationException {
        if (!request.password().matches(ValidationRegex.PASSWORD_PATTERN)) {
            throw new CredentialValidationException(message.getMessage("validation.password.error"));
        }
    }
}

@Service
@Order(4)
public class UsernameExistenceValidationStrategy implements RegistrationValidationStrategy {

    private final ApplicationMessageLoader message;
    private final UserDAO userDAO;

    public UsernameExistenceValidationStrategy(ApplicationMessageLoader message, UserDAO userDAO) {
        this.message = message;
        this.userDAO = userDAO;
    }

    @Override
    public void validate(RegisterRequest request) {
        if (userDAO.existsByUsername(request.username()) > 0) {
            throw new CredentialValidationException(message.getMessage("username.exist"));
        }
    }
}

@Service
@Order(5)
public class EmailExistenceValidationStrategy implements RegistrationValidationStrategy {

    private final ApplicationMessageLoader message;
    private final UserDAO userDAO;

    public EmailExistenceValidationStrategy(ApplicationMessageLoader message, UserDAO userDAO) {
        this.message = message;
        this.userDAO = userDAO;
    }

    @Override
    public void validate(RegisterRequest request) {
        if (userDAO.existsByEmail(request.email()) > 0) {
            throw new CredentialValidationException(message.getMessage("email.exist"));
        }
    }
}

Tworzenie użytkownika

class UserRegistration {

    private final UserFactory userFactory;
    private final RoleDAO roleDAO;
    private final UserDAO userDAO;

    public UserRegistration(UserFactory userFactory, RoleDAO roleDAO, UserDAO userDAO) {
        this.userFactory = userFactory;
        this.roleDAO = roleDAO;
        this.userDAO = userDAO;
    }

    @Transactional
    public void registerUser(RegisterRequest request) {
        Role role = roleDAO.findByName(UserRole.ROLE_USER).orElseThrow(
                () -> new RoleNotFoundException("Role not found"));

        User user = userFactory.createUser(request, role);

        int userId = userDAO.save(user);

        for (Role roles : user.getRoles()) {
            userDAO.assignRoleToUser(userId, roles.getId());
        }
    }
}

class UserFactory {

    private final PasswordEncoderService passwordEncoder;

    public UserFactory(PasswordEncoderService passwordEncoder) {
        this.passwordEncoder = passwordEncoder;
    }

    public User createUser(RegisterRequest request, Role role) {
        User user = new User();
        user.setUsername(request.username());
        user.setEmail(request.email());
        user.setPassword(passwordEncoder.encodePassword(request.password()));
        user.setRoles(Collections.singleton(role));
        return user;
    }
}

class PasswordEncoderService {

    private final PasswordEncoder passwordEncoder;

    public PasswordEncoderService(PasswordEncoder passwordEncoder) {
        this.passwordEncoder = passwordEncoder;
    }

    public String encodePassword(String password) {
        return passwordEncoder.encode(password);
    }
}
2
Ornstein napisał(a):
Riddle napisał(a):

Jaki jest sens tego? UserRepositoryAdapter wygląda jakby nic nie robił i był po nic. Czemu DAO nie może implementować UserRepositoryPort?

DAO i port to interfejsy. Nie mogę implementować interfejsu w interfejsie. Musiałbym coś tutaj zmienić.

Oczywiście że możesz

interface DAO extends UserRepositoryPort {
}
Ornstein napisał(a):

Pakiet service to sub-pakiet pakietu domain. Poprawiłem wcześniejszy post.

Nie ma czegoś takiego jak sub-pakiet w javie. Pakiet foo oraz foo.bar nie mają ze sobą nic wspólnego. Owszem - przestrzenie nazw są mapowane na hierarchię systemu plików, tak. Ale foo.bar nie ma dostępu do prywatnych czy nawet package-private elementów foo.

Ornstein napisał(a):

Kod tych klas wygląda tak:

  • Rozdmuchane te klasy jak nie wiem co, i nic Ci to nie daje. Klasy RegistrationValidation, ValidationRegex, UsernameValidationStrategy, EmailFormatValidationStrategy, PasswordFormatValidationStrategy, EmailExistenceValidationStrategy najlepiej jakby wszystkie były w UserRegistrationService.
  • UserRegistrationService to nie powinien być service Springowy, bo przecież to ma być część Twojej logiki bieznesowej. A jeśli chcesz żeby UserRegistrationService pozostał @Service, to to co robi ten service czyli "validate request", "register user" oraz "return new api response" należałoby wynieść do innej klasy która nie jest servicem springowym.
  • UserRegistration też chyba włączyłbym w UserRegistrationService
  • Te consty w ValidationRegex też należałoby przenieść najbliżej miejsca gdzie są używane.
  • PasswordEncoderService nie ma sensu. Inline.
  • Jeśli UserFactory jest użyte raz, to moim zdaniem należałoby inline'ować je do miejsca gdzie są użyte.
  • Nie powinno być @Transactional w domenie biznesowej.
  • Nie wiem jak wygląda ApplicationMessageLoader, ale jeśli zwraca tylko normalne stringi, to też jest do wyrzucenia.

Podsumowanko

Co na plus:

  • Fajnie że controllery mają tylko obsługę HTTP
  • Fajnie że DAO zwracają tylko encje domenowe
  • Zakładam że masz testy pod to? Wiec też fajnie

Co na minus:

  • Praktycznie wszystkie klasy które wkleiłeś w ostatnim poście są rozdmuchane i dodane po nic. Cała ta logika powinna być w jednym miejscu - walidacja, tworzenie instancji usera, przekazanie go do DAO, zwrócenie message'a. To jest jedna odpowiedzialność, bo to jest stworzenie usera. Jak rozumiem, nie powinno się dać móc stworzyć usera bez zrobienia tych kroków, więc nie ma sensu rozdzielać tych klas. Przekombinowałeś to.

Tak by to mogło wyglądać:

@Service
public class UserRegistrationService {
    private final UserRegistration registration;

    public UserRegistrationService(UserRegistration registration) {
        this.registration = registration;
    }

    public ApiResponse register(RegisterRequest request) {
        return registration.registerUser(request);
    }
}

class UserRegistration {
    private final ApplicationMessageLoader message;
    private final RoleDAO roleDAO;
    private final UserDAO userDAO;
    private final PasswordEncoder passwordEncoder;

    public UserRegistration(RoleDAO roleDAO, UserDAO userDAO, PasswordEncoder passwordEncoder, ApplicationMessageLoader message) {
        this.userFactory = userFactory;
        this.roleDAO = roleDAO;
        this.userDAO = userDAO;
        this.passwordEncoder = passwordEncoder;
    }

    public void registerUser(RegisterRequest request) {
        validate(request);
        Role role = roleDAO.findByName(UserRole.ROLE_USER).orElseThrow(() -> new RoleNotFoundException("Role not found"));
        User user = userFactory.createUser(request, role);
        int userId = userDAO.save(user);
        for (Role roles : user.getRoles()) {
            userDAO.assignRoleToUser(userId, roles.getId());
        }
        return new ApiResponse(message.getMessage("user.registration.success"));
    }

    private void validate(RegisterRequest request) {
        if (!request.username().matches("^[a-zA-Z0-9]{3,20}$")) {
            throw new CredentialValidationException(message.getMessage("validation.username.error"));
        }
        if (!request.email().matches("^[a-zA-Z0-9._%+-]{1,15}@[a-zA-Z0-9.-]{2,15}\\.[a-zA-Z]{2,6}$")) {
            throw new CredentialValidationException(message.getMessage("validation.email.error"));
        }
        if (!request.password().matches("^(?=.*[0-9])(?=.*[a-z])(?=.*[A-Z])(?=.*[@#$%^&+=])(?=\\S+$).{8,}$")) {
            throw new CredentialValidationException(message.getMessage("validation.password.error"));
        }
        if (userDAO.existsByUsername(request.username()) > 0) { // ten interfejs powinieś poprawić, tak żeby `existsByEmail` zwracał boolean
            throw new CredentialValidationException(message.getMessage("username.exist"));
        }
        if (userDAO.existsByEmail(request.email()) > 0) { // ten interfejs powinieś poprawić, tak żeby `existsByEmail` zwracał boolean
            throw new CredentialValidationException(message.getMessage("email.exist"));
        }
    }

    private User createUser(RegisterRequest request, Role role) {
        User user = new User();
        user.setUsername(request.username());
        user.setEmail(request.email());
        user.setPassword(passwordEncoder.encode(request.password()));
        user.setRoles(Collections.singleton(role));
        return user;
    }
}

A jeśli faktycznie potrzebujesz po coś dodać @Transactional gdzieś, to napisz failujący test pod to, i dopisz to gdzieś w poza logiką biznesową, albo nałóżna to odpowiedni interfejs i wtedy dodaj do logiki biznesowej. Jeśli nie jesteś w stanie napisać failującego testu, to może to jest sygnał że jednak nie potrzebujesz tego.

0
  • Nie wiem jak wygląda ApplicationMessageLoader, ale jeśli zwraca tylko normalne stringi, to też jest do wyrzucenia.

ApplicationMessageLoader ładuje wiadomości z pliku properties.

@Service
public class ApplicationMessageLoader {

    private final Properties messages = new Properties();

    public ApplicationMessageLoader() {
        this(ApplicationMessageLoader.class.getClassLoader().getResourceAsStream("messages.properties"));
    }

    ApplicationMessageLoader(InputStream propertiesStream) {
        try {
            if (propertiesStream != null) {
                messages.load(propertiesStream);
            } else {
                throw new IOException("Properties file is null");
            }
        } catch (IOException e) {
            throw new RuntimeException("Failed to load messages.properties");
        }
    }

    public String getMessage(String key) {
        return messages.getProperty(key, "Default message for " + key);
    }

    public String getMessage(String key, Object... params) {
        String message = messages.getProperty(key, "Default message for " + key);
        return MessageFormat.format(message, params);
    }
}
1
Ornstein napisał(a):
  • Nie wiem jak wygląda ApplicationMessageLoader, ale jeśli zwraca tylko normalne stringi, to też jest do wyrzucenia.

ApplicationMessageLoader ładuje wiadomości z pliku peoperties.

A potrzebujesz tego? Obstawiam że na 99% i tak zawsze te message'a są takie same, więć to jest niepotrzebne skomplikowanie.

Obstawiam że na 99.99% taki kod byłby okej:

@Override
public void validate(RegisterRequest request) {
  if (!request.username().matches("^[a-zA-Z0-9]{3,20}$")) {
      throw new CredentialValidationException("Malformed username");
  }
  if (!request.email().matches("^[a-zA-Z0-9._%+-]{1,15}@[a-zA-Z0-9.-]{2,15}\\.[a-zA-Z]{2,6}$")) {
      throw new CredentialValidationException("Invalid email format");
  }
  if (!request.password().matches("^(?=.*[0-9])(?=.*[a-z])(?=.*[A-Z])(?=.*[@#$%^&+=])(?=\\S+$).{8,}$")) {
      throw new CredentialValidationException("jakiś stały message");
  }
  // ...

Bez tego ApplicationMessageLoader i bez ładowania prioperties'ów.

0
Riddle napisał(a):
Ornstein napisał(a):
  • Nie wiem jak wygląda ApplicationMessageLoader, ale jeśli zwraca tylko normalne stringi, to też jest do wyrzucenia.

ApplicationMessageLoader ładuje wiadomości z pliku peoperties.

A potrzebujesz tego? Faktycznie chcesz żeby Twoje wiadomości się zmieniały w runtime'ie? Bo po to są pliki prioperties. Obstawiam że na 99% i tak zawsze te message'a są takie same, więć to jest niepotrzebne skomplikowanie.

Zrobiłem to dlatego, że chciałem mieć wszystkie wiadomości w jednym miejscu. Założyłem, że gdy będę chciał którąś zmodyfikować, łatwiej będzie mi to zrobić w ogólnym pliku niż szukać po klasach.

1
Ornstein napisał(a):

Zrobiłem to dlatego, że chciałem mieć wszystkie wiadomości w jednym miejscu. Założyłem, że gdy będę chciał którąś zmodyfikować, łatwiej będzie mi to zrobić w ogólnym pliku niż szukać po klasach.

Okej, kumam. Ale na pewno plik properties nie jest dobrym miejscem na to.

Aczkolwiek szczerze mówiąc takie podejście też jest średnie. Powinieneś trzymać te stringi w miejscu ich najbliższego użycia. One nie mają logiki żadnej za sobą, nic takiego, więc z punktu widzenia programu są magiczną wartością, nie należy ich wydzielać. Jak będziesz chciał zmienić jakąś konkretną wiadomość to po prostu sobie wyszukasz w IDE tą wiadomość.

A jeśli już faktycznie chcesz je mieć wszystkie w jednym miejscu, to najlepiej byłoby napisać test pod to, MessageTest, który ma tyle testów ile message'ów. To by nawet miało sens. Tylko że nikt czegoś takiego nie robi, bo to jest niepotrzebne, bo to są "gołe dane", bez żadnej logiki.

Jedyny case gdzie widziałbym sens dla czegoś takiego, to gdyby za tymi message'ami była jakaś logika - np. chciałbyś wyświetlać różne stringi dla różnych userów (jak w translacjach np.). Ale jeśli nie, to to jest zupełnie niepotrzebne.

Nie rozwiązuj problemów których nie masz. YAGNI.

0
Riddle napisał(a):

Co na minus:

  • Praktycznie wszystkie klasy które wkleiłeś w ostatnim poście są rozdmuchane i dodane po nic. Cała ta logika powinna być w jednym miejscu - walidacja, tworzenie instancji usera, przekazanie go do DAO, zwrócenie message'a. To jest jedna odpowiedzialność, bo to jest stworzenie usera. Jak rozumiem, nie powinno się dać móc stworzyć usera bez zrobienia tych kroków, więc nie ma sensu rozdzielać tych klas. Przekombinowałeś to.

Stanowczo nie.

Proces rejestracj to po prostu zapisanie rekordu w bazie. Nie znam java'y ale z pewnoscia java zna pojecie middleware i to wlasnie w middleware powinno zachodzic pre-procesowanie, persystencji:

  • middleware: request->dto
  • middleware: dto->validation
  • middleware: i co tam jeszcze komu przyjdzie do glwowy, np autoryzacja, autentykacja itp, itd ...

Bez zbedneto kodu w kontrolerze, dto przekazane jest przez kontroler do serwisu, w ktorym:

  • service: dto->conversion->dao
  • service: service->action/transation(dao)->repository (wywolanie odpowiednej akcji/transakcji)

Potem zwrotka z sewisu trafia do responder'a.
A na koncu odebrany z respondera response wypluwany jest przez controller na swiat.

Nawiazujac jeszcze do Single Responsibility wlasciwie funkcjonuje w dwoch odslonach tzn albo jako pojedyncze zadanie do wykonania, albo pojedynczy powod do zmiany.
Z powodu tego pierwszego, mi osobiscie stanowczo nie lezy mnogosc dostepnych publicznych metod w UserRegistration, co raczej nie idzie w parze z pojedyncza odpowiedzialnoscia w sensie funkcjonalnosci. Nie wiem jak w Java'ie ale pojedyncze zadanie, w php, wyjatkowo latwo weryfikuje sie wylacznym stosowaniem __invoke() natomiast jesli chodzi o pojedyczy powod do zmiany to poza np potencjalnym kolejnym atrybutem w encji, widze jeszcze conajmniej dwa.

Reasumujac, stanowczo nie zgadzam sie ze stwierdzeniem iz: Cała ta logika powinna być w jednym miejscu

0
proximus-prime napisał(a):

Proces rejestracj to po prostu zapisanie rekordu w bazie.

W niektórych aplikacjach tak, w niektórych nie.

proximus-prime napisał(a):

Nie znam java'y ale z pewnoscia java zna pojecie middleware i to wlasnie w middleware powinno zachodzic pre-procesowanie, persystencji:

  • middleware: request->dto
  • middleware: dto->validation
  • middleware: i co tam jeszcze komu przyjdzie do glwowy, np autoryzacja, autentykacja itp, itd ...

Bez zbedneto kodu w kontrolerze, dto przekazane jest przez kontroler do serwisu, w ktorym:

  • service: dto->conversion->dao
  • service: service->action/transation(dao)->repository (wywolanie odpowiednej akcji/transakcji)

Potem zwrotka z sewisu trafia do responder'a.
A na koncu odebrany z respondera response wypluwany jest przez controller na swiat.

No to jest jeden ze sposobów, i nawet by działał.

Ale ciężko w czymś takim zrobić vertical slicing; a po drugie ten "middleware" najpiewniej będzie z jakiegoś framework'a, więc teraz mamy naszą logikę przywiązaną do frameworka.

Możesz powiedzieć co jest nie tak z przykładem który napisałem tutaj? Architektura aplikacji - hexagonal/podział na pakiety - kilka pytań

proximus-prime napisał(a):

Nawiazujac jeszcze do Single Responsibility wlasciwie funkcjonuje w dwoch odslonach tzn albo jako pojedyncze zadanie do wykonania, albo pojedynczy powod do zmiany.

Ja zazwyczaj o tym myślę praktycznie zawsze w tym drugim znaczeniu (i myślę że też to miał autor na myśli). "Zadanie do wykonania" za bardzo dotyka szczegółów implementacyjnych IMO.

proximus-prime napisał(a):

Z powodu tego pierwszego, mi osobiscie stanowczo nie lezy mnogosc dostepnych publicznych metod w UserRegistration, co raczej nie idzie w parze z pojedyncza odpowiedzialnoscia w sensie funkcjonalnosci. Nie wiem jak w Java'ie ale pojedyncze zadanie, w php, wyjatkowo latwo weryfikuje sie wylacznym stosowaniem __invoke()

Sorry. Masz rację - w moim przykładzie był public bo po prostu kopiowałem z kodu wklejonego przez @Ornstein, ale one jak najbardziej powinny być private. Poprawiłem je, tak że teraz jest tylko jedna public.

Dzięki za zwrócenie uwagi.

proximus-prime napisał(a):

natomiast jesli chodzi o pojedyczy powod do zmiany to poza np potencjalnym kolejnym atrybutem w encji, widze jeszcze conajmniej dwa.

Jakie? Ta klasa będzie wymagała zmiany, tylko wtedy kiedy zmieni się sposób rejestracji usera. IMO "rejestarcja usera" to jest ta odpowiedzialność. Wiadomo że można to podzielić na mniejsze kroki (typu wczytanie usera, zapisanie usera, zwalidowanie usera), ale one wszystkie są częścią "rejestracji usera".

0
Riddle napisał(a):

W niektórych aplikacjach tak, w niektórych nie.

Nie twierdze, ze wiecej widzialem od Ciebie, ale wg mnie zawsze jest to kwestia, spiecia z odpowiednim serwisem, albo pojedynczej akcji, albo pojedynczej transakcji

Riddle napisał(a):

No to jest jeden ze sposobów, i nawet by działał.

Ale ciężko w czymś takim zrobić vertical slicing; a po drugie ten "middleware" najpiewniej będzie z jakiegoś framework'a, więc teraz mamy naszą logikę przywiązaną do frameworka.

Dziala zawsze o ile kod jest framework agnostic i nie godzisz sie ze stwierdzeniem, ze Symfony to jest cos najlepszego co zdarzylo sie w php 🙂

Jesli chodzi o verical slicing to wrecz przeciwnie, ostatnio spinalem trywialnego middlware handlera, z serwerem websocket'owym, a teraz wlasciwe ten sam kod uzywam w dla pluginu grpc w workerze roadrunner'a.

W przypadku http jest to o tyle latwe ze dzieki psr7 request staje sie idealnym pasem transmisyjnym od pierwszego middleware do serwisu.

Riddle napisał(a):

Możesz powiedzieć co jest nie tak z przykładem który napisałem tutaj? Architektura aplikacji - hexagonal/podział na pakiety - kilka pytań

Z grubsza rzecz biorac, to wlasnie to vertical slicing jest dla mnie nieoczywisce, z tego tez powodu sam rozpisalem to troche dokladniej.

Riddle napisał(a):

Ja zazwyczaj o tym myślę praktycznie zawsze w tym drugim znaczeniu (i myślę że też to miał autor na myśli). "Zadanie do wykonania" za bardzo dotyka szczegółów implementacyjnych IMO.

Ja natomiast zawsze mialem z tym problem, dopoki ktos gdzies stwierdzil, ze tak naparawde to na poczatku chodzilo o funkcjonalnosc, a potem uncle Ben to tego przysiadl i zdefiniowal to po swojemu tzn powodem do zmiany.

Riddle napisał(a):

? Jakie? Ta klasa będzie wymagała zmiany, tylko wtedy kiedy zmieni się sposób rejestracji usera. IMO "rejestarcja usera" to jest ta odpowiedzialność. Wiadomo że można to podzielić na mniejsze kroki (typu wczytanie usera, zapisanie usera, zwalidowanie usera), ale one wszystkie są częścią "rejestracji usera".

Jesli mam osobny serwis rejestracji, to dto nawet nie powinno do niego docierac jesli nie zostalo zwalidowane.

Sens umieszczania tych pojedynczych krokow w osobnych klasach polega na tym ze jesli zostana spiete w serwisie rejestracji jako osobne, majace swoja wyjatkowa odpowiedzialnosc klasy, to dla samego serwisu rejestracji nie bedzie mialo znaczenia, np zmiana wlasnego kodu walidacji na gotowa zewnetrzna biiblioteke (to jest powod do zmian walidatora).

Serwis rejestracji, majacy spiety ze soba akcje lub transakcje, nie powinien byc zalezny od tego na jakim data source odbywa sie persystencja stad interfejsy. Ale to nie wszystko, bo sam strzal akcja/transakcja pokazuje kolejny powod do zmiany, dzisiaj moze to byc generyczna zwrotki z repo, ale jutro wymagania moga sie zmienic. To jest dla akcji/transakcji powod do zmiany, bo operujac na interfejsie repozytorium nie musi przejmowac sie zmianami na/w datasource.

To tak pokrotce, bez ironii i z wyrazami szacunku 😃

0
proximus-prime napisał(a):
Riddle napisał(a):

W niektórych aplikacjach tak, w niektórych nie.

Nie twierdze, ze wiecej widzialem od Ciebie, ale wg mnie zawsze jest to kwestia, spiecia z odpowiednim serwisem, albo pojedynczej akcji, albo pojedynczej transakcji

No wiadomo że dowolną ilość kroków można schować do jednej funkcji lub jednego pod-serwisu, ale jest dużo aplikacji które robią kilka rzeczy jak user się rejestruje: chociażby wysyłka maila do autora. Wtedy logika biznesowa powinna mieć:

userDao.storeUserInDatabase(user);
notification.notifyUserCreated(user); // to jest interfejs, który dostaje implementacje, która wysyła maila
Riddle napisał(a):

Jesli mam osobny serwis rejestracji, to dto nawet nie powinno do niego docierac jesli nie zostalo zwalidowane.

Sens umieszczania tych pojedynczych krokow w osobnych klasach polega na tym ze jesli zostana spiete w serwisie rejestracji jako osobne, majace swoja wyjatkowa odpowiedzialnosc klasy, to dla samego serwisu rejestracji nie bedzie mialo znaczenia, np zmiana wlasnego kodu walidacji na gotowa zewnetrzna biiblioteke (to jest powod do zmian walidatora).

tylko widzisz, te klasy - walidacja, zapisanie w bazie, operacje, one są koncepcyjnie blisko siebie (cohesion), często jak dodajesz pole to dodajesz też walidacje; więc moim zdaniem to trochę nie ma sensu żeby to rozbijać na dwie klasy.

A jednocześnie nie widzę sensu czemu by to robić.

Riddle napisał(a):

Serwis rejestracji, majacy spiety ze soba akcje lub transakcje, nie powinien byc zalezny od tego na jakim data source odbywa sie persystencja stad interfejsy. Ale to nie wszystko, bo sam strzal akcja/transakcja pokazuje kolejny powod do zmiany, dzisiaj moze to byc generyczna zwrotki z repo, ale jutro wymagania moga sie zmienic. To jest dla akcji/transakcji powod do zmiany, bo operujac na interfejsie repozytorium nie musi przejmowac sie zmianami na/w datasource.

Jak masz odpowiednio zrobione Dependency Inversion, to żadne z problemów o których piszesz nie wystąpią; nawet jak wszystko jest w jednej klasie tak jak pokazałem.

0
Riddle napisał(a):

No wiadomo że dowolną ilość kroków można schować do jednej funkcji lub jednego pod-serwisu, ale jest dużo aplikacji które robią kilka rzeczy jak user się rejestruje: chociażby wysyłka maila do autora.

Powtorze raz jeszcze, nie twierdze, ze wiecej widzialem od Ciebie, ale wg mnie zawsze jest to kwestia, spiecia z odpowiednim serwisem, albo pojedynczej akcji, albo pojedynczej transakcji, albo kilku akcji/transakcji.

Nie ma takiej funkcjonalnosci, ktorej procesowanie nie jest mozliwe w oparciu o chain middlewares, z "wyplutym" ostatecznie do serwisu dto'sem, ktorego trzeba jedynie atakowac wylacznie akcjami/transakcjami. Wlacznie z wysylka maila.

Riddle napisał(a):

tylko widzisz, te klasy - walidacja, zapisanie w bazie, operacje, one są koncepcyjnie blisko siebie (cohesion), często jak dodajesz pole to dodajesz też walidacje; więc moim zdaniem to trochę nie ma sensu żeby to rozbijać na dwie klasy.

A jednocześnie nie widzę sensu czemu by to robić.

Tak i ta kohezja, ktora w php utrwalila sie niechlubnym wdarciem Doctrine na salony kodu, pozamiatala doszczetnie separation of concern mieszajac w jednym miejscu za pomoca adnotacji/atrybutow logike aplikacji z konfiguracja.

Stad tez ja jestem osobiscie ogromnym zwolennikiem strukturalnej modularyzacji kodu wokol poszczegolnych elementow domeny.

Riddle napisał(a):

Jak masz odpowiednio zrobione Dependency Inversion, to żadne z problemów o których piszesz nie wystąpią; nawet jak wszystko jest w jednej klasie tak jak pokazałem.

No coz, nie zgadzam sie z tym 🙂 , ale dalsza pociagniecie tego watku to dyskusja nad wyzszoscia swiat Bozego Narodzenia nad swietami Wielkiejnocy.

Moge tylko uszanowac Twoje odmienne zdanie i podziekowac za fajna wymiane pogladow.

Nie miej mi prosze zle, ale juz wlasciwie nic wiecej nie mam do powiedzenia.

3

Hexagonala można wdrażać na różne sposoby, to znaczy nie ma jednego sposobu na podział na pakiety i na samo nazewnictwo pakietów. Mało tego, jakby aplikacja się wystarczająco skomplikowała, to możesz mieć nadal package-by-feature w obrębie heksagonalowych pakietów.

Ja heksagonala wdrażałem tak:

  • jedna spójna konwencja w wszystkich aplikacjach (mikroserwisach) w całym produkcie
  • pakiet adapterów dzielę na inbound i outbound (driving i driven)
  • jeżeli aplikacja jest złożona to inbound dzielę np. na api, messaging; natomiast outbound na persistence, messaging, external_service_name
  • pakiet domeny dzielę na porty, serwisy, model (wyjątki, domain eventy, value objekty, itd.)
  • cała domena jest niezależna od technologii typu Spring (u Ciebie tego nie ma)
  • serwisy korzystają z portów, porty (technology-agnostic) są implementowane przez adaptery (technology-specific)
  • domena nie korzysta bezpośrednio z adapterów, ever
  • pilnować mocnej enkapsulacji, public wywalać tam gdzie psułoby to enkapsulację
2
Hodor napisał(a):

Hexagonala można wdrażać na różne sposoby, to znaczy nie ma jednego sposobu na podział na pakiety i na samo nazewnictwo pakietów. Mało tego, jakby aplikacja się wystarczająco skomplikowała, to możesz mieć nadal package-by-feature w obrębie heksagonalowych pakietów.

Ja heksagonala wdrażałem tak:

  • jedna spójna konwencja w wszystkich aplikacjach (mikroserwisach) w całym produkcie
  • pakiet adapterów dzielę na inbound i outbound (driving i driven)
  • jeżeli aplikacja jest złożona to inbound dzielę np. na api, messaging; natomiast outbound na persistence, messaging, external_service_name
  • pakiet domeny dzielę na porty, serwisy, model (wyjątki, domain eventy, value objekty, itd.)
  • cała domena jest niezależna od technologii typu Spring (u Ciebie tego nie ma)
  • serwisy korzystają z portów, porty (technology-agnostic) są implementowane przez adaptery (technology-specific)
  • domena nie korzysta bezpośrednio z adapterów, ever
  • pilnować mocnej enkapsulacji, public wywalać tam gdzie psułoby to enkapsulację

O kurcze 😯 Jak rzadko widzę tutaj post, z który się tak bardzo zgadzam. Well played, 10/10.

Jedyne co mi trochę nie pasuje to "jedna spójna konwencja we wszystkich mikroserwisach" - pytanie, masz na myśli po prostu serwisy (architektura zorientowana serwisowo)? Czy masz na myśli na prawdę pełnoprawne mikroserwisy (czyli są niezależnie deployowalne)? Bo jeśli to są pełnoprawne mikroserwisy, to raczej trudno jest w nich utrzymać spójną konwencję. Jakby - mikroserwisy to jest trade między niezaleznością i spójnością. Ciężko mi sobie wyobrazić architekturę która jednocześnie jest niezależna od innych, oraz jednocześnie ma spójną konwencję. No albo albo. Chyba że to nie są mikroserwisy, tylko architektura zorientowana serwisowo gdzie niezależny deployment nie jest wymagany - wtedy spoko.

0

Skoro cała domena ma być niezależna od technologii typu Spring, to co w sytuacji, kiedy w niektórych klasach domenowych używam zależności Springa? Przykładowo feature login:

@Service
public class LoginFacade {
    private final LoginService login;
//...
}

class LoginService {
    private final UserAuthentication authentication;
//...
}

Klasa UserAuthentication odpowiada za uwierzytelnienie użytkownika.

@Component
class UserAuthentication {
    private final AuthenticationManager authManager;

    UserAuthentication(AuthenticationManager authManager) {
        this.authManager = authManager;
    }

    Authentication authenticateUser(LoginRequest request) {
        try {
            Authentication auth = authManager.authenticate(
                    new UsernamePasswordAuthenticationToken(
                            request.usernameOrEmail(), request.password()));
            SecurityContextHolder.getContext().setAuthentication(auth);
            return auth;
        } catch (BadCredentialsException e) {
            throw new BadCredentialsException("Login failed due to invalid credentials");
        }
    }
}

AuthenticationManager to zależność Springa. Teraz tak, żeby ta klasa domenowa nie była zależna od Springa, wychodzi na to, że muszę stworzyć port i adapter dla AuthenticationManager, tak? No bo jak inaczej?

Wyglądałoby to tak:

Port

public interface UserAuthenticationPort {
    Authentication authenticateUser(LoginRequest request);
}

Adapter

@Component
class UserAuthenticationAdapter implements UserAuthenticationPort {

    private final AuthenticationManager authManager;

    UserAuthenticationAdapter(AuthenticationManager authManager) {
        this.authManager = authManager;
    }

    @Override
    public Authentication authenticateUser(LoginRequest request) {
        try {
            Authentication auth = authManager.authenticate(
                    new UsernamePasswordAuthenticationToken(
                            request.usernameOrEmail(), request.password()));
            SecurityContextHolder.getContext().setAuthentication(auth);
            return auth;
        } catch (BadCredentialsException e) {
            throw new BadCredentialsException("Login failed due to invalid credentials");
        }
    }
}

Klasa domenowa

class UserAuthentication {
    private final UserAuthenticationPort authentication;

    UserAuthentication(UserAuthenticationPort authentication) {
        this.authentication = authentication;
    }

    Authentication authenticateUser(LoginRequest request) {
        return authentication.authenticateUser(request);
    }
}

Co myślicie?

2
Ornstein napisał(a):

Skoro cała domena ma być niezależna od technologii typu Spring, to co w sytuacji, kiedy w niektórych klasach domenowych używam zależności Springa? Przykładowo feature login:

No musisz na to nałożyć odpowiednią abstrakcje, oczywiście. W jakiś sposób zaenkapsulować springa. Tak na prawdę musisz wykorzystać polimorfizm, żeby osiągnąć Dependency Inversion. Ports and adapter to na pewno nie jest jedyny sposób żeby to zrobić; ale na 100% to musi być polimorficzne.

Tylko to trzeba zrobić mądrze.

  • ❗ Nie wychodź od swojego serwisu springowego który chcesz "wcisnąć" w domenę biznesową. Wtedy powstanie tzn. leaky abstraction.
  • ✔️ Zacznij od domeny biznesowej, i zastanów się jak możesz otworzyć swoją domenę na przyjęcie implementacji która dostarczy logowanie, ale tak żeby nie być zależnym od frameworka. Jakie są absolutnie niezbędne elementy których domena biznesowa wymaga? Login i hasło tylko? Jeśli tak, to abstrakcja powinna wiedzieć tylko o loginie i haśle. Jakie są absolutnie niezbędne rzeczy jakie domena ma dostać od zalogowanego użykownika? Tylko nazwę? To tylko to abstrakcja powinna dać. Jak to zrobisz, to po prostu napisz logowanie ze springiem pasującą do tej abstrakcji. Nie koniecznie powstanie w ten sposób serwis logowania który masz teraz, i to bardzo dobrze.

Tylko widzisz, @Ornstein Twoje przykłady zawierają bardzo mało logiki. Przykład który podałeś wygląda trochę tak jakby tam w ogóle nie było domeny biznesowej związanej z logowaniem.

1
Riddle napisał(a):

Tylko widzisz, @Ornstein Twoje przykłady zawierają bardzo mało logiki. Przykład który podałeś wygląda trochę tak jakby tam w ogóle nie było domeny biznesowej związanej z logowaniem.

Ten przykład który podałem, mógłby być lepszy.

Po zmianach feature login wygląda tak:

Kontroler

@RestController
@RequestMapping("/api/v1/users")
public class LoginController {

    private final LoginFacade login;

    public LoginController(LoginFacade login) {
        this.login = login;
    }

    @PostMapping("/login")
    public ResponseEntity<ApiResponse> login(@Valid @RequestBody LoginRequest request,
                                             HttpServletResponse response) {
        String sessionToken = login.userLogin(request);

        Cookie sessionCookie = new Cookie("session_id", sessionToken);
        sessionCookie.setHttpOnly(true);
        sessionCookie.setPath("/");
        response.addCookie(sessionCookie);

        return ResponseEntity.ok(new ApiResponse("Logged successfully."));
    }
}

Fasada

@Service
public class LoginFacade {

    private final LoginHandler login;

    public LoginFacade(LoginHandler login) {
        this.login = login;
    }

    public String userLogin(LoginRequest request) {
        return login.login(request);
    }
}

Domena

class LoginHandler {
    private final UserSessionCreator userSessionCreator;
    private final UserRepositoryPort userRepository;
    private final UserAuthenticationPort userAuth;
    private final TokenHasher tokenHasher;
    private final TokenGenerator tokenGenerator;

   LoginHandler(TokenGenerator tokenGenerator,
                UserSessionCreator userSessionCreator,
                UserRepositoryPort userRepository,
                UserAuthenticationPort userAuth,
                TokenHasher tokenHasher) {
        this.tokenGenerator = tokenGenerator;
        this.userSessionCreator = userSessionCreator;
        this.userRepository = userRepository;
        this.userAuth = userAuth;
        this.tokenHasher = tokenHasher;
   }

    public String login(LoginRequest request) {
       String username = userAuth.authenticateUser(request.usernameOrEmail(), request.password());
       User user = userRepository.findByUsername(username).orElseThrow(
                () -> new UserNotFoundException("User not found with username :" + username));

        String sessionToken = tokenGenerator.generateToken();
        String hashedToken = tokenHasher.hashToken(sessionToken);
        userSessionCreator.createUserSession(hashedToken, user.getId());
        return sessionToken;
    }
}

class UserSessionCreator {
    private final SessionRepositoryPort sessionRepository;

    UserSessionCreator(SessionRepositoryPort sessionRepository) {
        this.sessionRepository = sessionRepository;
    }

    public void createUserSession(String hashedToken, int userId) {
        Session session = createSession(hashedToken, userId);
        sessionRepository.save(session);
    }

    private Session createSession(String hashedToken, Integer userId) {
        Session session = new Session();
        session.setSessionId(hashedToken);
        session.setUserId(userId);
        session.setCreateAt(new Date());
        session.setExpiresAt(new Date(System.currentTimeMillis() + (24 * 60 * 60 * 1000)));

        return session;
    }
}

Port i adapter dla uwierzytelnienia usera

public interface UserAuthenticationPort {
    String authenticateUser(String usernameOrEmail, String password);
}

@Service
class UserAuthenticationAdapter implements UserAuthenticationPort {

    private final AuthenticationManager authManager;

    UserAuthenticationAdapter(AuthenticationManager authManager) {
        this.authManager = authManager;
    }

    @Override
    public String authenticateUser(String usernameOrEmail, String password) {
        try {
            Authentication auth = authManager.authenticate(new UsernamePasswordAuthenticationToken(usernameOrEmail, password));
            SecurityContextHolder.getContext().setAuthentication(auth);
            return auth.getName();
        } catch (BadCredentialsException e) {
            throw new BadCredentialsException("Login failed due to invalid credentials.");
        }
    }
}
1
Ornstein napisał(a):
Riddle napisał(a):

Tylko widzisz, @Ornstein Twoje przykłady zawierają bardzo mało logiki. Przykład który podałeś wygląda trochę tak jakby tam w ogóle nie było domeny biznesowej związanej z logowaniem.

Ten przykład który podałem, mógłby być lepszy.

Po zmianach feature login wygląda tak:

  • LoginController bardzo dobry
  • UserSessionCreator bardzo dobry
  • UserAuthenticationAdapter i UserAuthenticationPort bardzo dobry. A nie myślałeś żeby uprościć nazwy? np interfejs mógłby się nazywać Authentication a implementacja SpringAuthentication? Nie ma potrzeby pisać "user authentication", bo wiadomo że to user się autentykuje.
  • Fajnie że masz osobne klasy na TokenHasher i TokenGenerator - bo jak się domyślam robią inne rzeczy; ale LoginHandler nie musi o nim wiedzieć. Ja bym na Twoim miejscu dodał nową klase, np TokenMaker czy jakoś tak, który w sobie ma TokenHasher oraz TokenGenerator i wystawia tylko pojedynczą metodę getToken().

Rozumiem że LoginFacade jest po to żeby wstrzyknąć LoginHandler jako @Service springowy. Fajny pomysł. Ciekawe czy da się to zrobić bez dodatkowej klasy (i bez dodawania @Service do LoginHandler)? 🤔 Pewnie nie. Chyba zmieniłbym nazwę, bo fasada się kojarzy za bardzo z wzorcem projektowym fasada. Może LoginHandlerService?

0
Riddle napisał(a):
Ornstein napisał(a):
Riddle napisał(a):

Tylko widzisz, @Ornstein Twoje przykłady zawierają bardzo mało logiki. Przykład który podałeś wygląda trochę tak jakby tam w ogóle nie było domeny biznesowej związanej z logowaniem.

Ten przykład który podałem, mógłby być lepszy.

Po zmianach feature login wygląda tak:

  • LoginController bardzo dobry
  • UserSessionCreator bardzo dobry
  • UserAuthenticationAdapter i UserAuthenticationPort bardzo dobry. A nie myślałeś żeby uprościć nazwy? np interfejs mógłby się nazywać Authentication a implementacja SpringAuthentication? Nie ma potrzeby pisać "user authentication", bo wiadomo że to user się autentykuje.
  • Fajnie że masz osobne klasy na TokenHasher i TokenGenerator - bo jak się domyślam robią inne rzeczy; ale LoginHandler nie musi o nim wiedzieć. Ja bym na Twoim miejscu dodał nową klase, np TokenMaker czy jakoś tak, który w sobie ma TokenHasher oraz TokenGenerator i wystawia tylko pojedynczą metodę getToken().

Rozumiem że LoginFacade jest po to żeby wstrzyknąć LoginHandler jako @Service springowy. Fajny pomysł. Ciekawe czy da się to zrobić bez dodatkowej klasy (i bez dodawania @Service do LoginHandler)? 🤔 Pewnie nie. Chyba zmieniłbym nazwę, bo fasada się kojarzy za bardzo z wzorcem projektowym fasada. Może LoginHandlerService?

Uprościłem te nazwy klas, teraz powinno wyglądać to lepiej.

Miałbym jeszcze takie pytanie. Mam klasę DeleteUser i metodę deleteUserById, która wykonuje operacje takie jak usunięcie użytkownika, usunięcie ról użytkownika, usunięcie aktywnych sesji użytkownika. Chcę wykonać te operacje w ramach jednej transakcji. Do tej pory używałem adnotacji @Transactional, ale chcę z niej zrezygnować, żeby odseparować domenę od Springa. Pomyślałem, żeby zrobić port i adapter i w adapterze skorzystać z TransactionTemplate.

Port

public interface TransactionalExecutorPort {
    <T> T execute(Supplier<T> action);
}

Adapter

@Service
class TransactionalExecutorAdapter implements TransactionalExecutorPort {

    private final TransactionTemplate transactionTemplate;

    TransactionalExecutorAdapter(TransactionTemplate transactionTemplate) {
        this.transactionTemplate = transactionTemplate;
    }

    @Override
    public <T> T execute(Supplier<T> action) {
        return transactionTemplate.execute(status -> action.get());
    }
}

A następnie wykorzystać port w domenie:

class DeleteUser {

    private final UserRepositoryPort userRepository;
    private final SessionRepositoryPort sessionRepository;
    private final TransactionalExecutorPort transactionalExecutor;

    DeleteUser(UserRepositoryPort userRepository,
               SessionRepositoryPort sessionRepository,
               TransactionalExecutorPort transactionalExecutor) {
        this.userRepository = userRepository;
        this.sessionRepository = sessionRepository;
        this.transactionalExecutor = transactionalExecutor;
    }

    ApiResponse deleteUserById(int userId) {
        return transactionalExecutor.execute(() -> {
            User user = userRepository.findById(userId).orElseThrow(() -> new UserNotFoundException(
                    "User not found with id: " + userId));

            userRepository.deleteUserRoles(userId);
            sessionRepository.deleteUserSessions(userId);

            userRepository.delete(user);

            return new ApiResponse(String.format("User with ID %d deleted successfully.", userId));
        });
    }
}

Czy takie podejście będzie dobre?
Druga kwestia, załóżmy, że mam pakiet A, pakiet B, pakiet C i w każdym pakiecie potrzebuję wykonać jakieś operacje w ramach transakcji. Zastanawiam się, jak to dobrze zorganizować. Pomyślałem, aby adapter przenieś do jakiegoś pakietu common, a następnie w pakietach A,B,C stworzyć porty, które będą używane w obrębie danego pakietu. Mogłoby tak być?

1

@Ornstein Okej. To że chcesz odseparować Springa od domeny to jest bardzo dobra rzecz! Pochwalam.

Tylko nie obraz się, ale sposób w jaki to zrobiłeś w Twoim przykładzie raczej tego nie osiąga 😕

Jak do tego podejść:

Rozumiem że chcesz usunąć usera razem z sesją i wszystkim innym. Pytanie: czy to jest jedyny use case? Czy potrzebujesz kiedyś usunąć usera, ale bez usuwania sesji?

Bo jeśli to jest jedyny use case to wystarczy Ci interfejs:

interface DeleteUser {
  void deleteEverthingById(int id);
}

A implementacja sparingowa już może usuwać wszystko co trzeba. Domena nie musi wiedzieć co dokładnie jest usuwane.

0
Riddle napisał(a):

Rozumiem że chcesz usunąć usera razem z sesją i wszystkim innym. Pytanie: czy to jest jedyny use case? Czy potrzebujesz kiedyś usunąć usera, ale bez usuwania sesji?

Chcę usunąć usera i wszystko, co jest do niego przypisane.

Riddle napisał(a):

Bo jeśli to jest jedyny use case to wystarczy Ci interfejs:

interface DeleteUser {
  void deleteEverthingById(int id);
}

A implementacja sparingowa już może usuwać wszystko co trzeba. Domena nie musi wiedzieć co dokładnie jest usuwane.

Mówisz, że powinienem usunąć te porty, które są abstrakcją dla DAO, i wyodrębnić operacje usuwania do osobnej klasy?

Coś takiego?

Port

public interface DeleteUserPort {
    void deleteEverythingById(int id);
}

Adapter

@Service
class DeleteUserAdapter implements DeleteUserPort {

    private final UserDAO userRepository;
    private final SessionDAO sessionRepository;
    private final TransactionTemplate transactionTemplate;

    public DeleteUserAdapter(UserDAO userDAO,
                             SessionDAO sessionDAO,
                             TransactionTemplate transactionTemplate) {
        this.userRepository = userDAO;
        this.sessionRepository = sessionDAO;
        this.transactionTemplate = transactionTemplate;
    }

    @Override
    public void deleteEverythingById(int id) {
        transactionTemplate.executeWithoutResult(status -> {
            User user = userRepository.findById(id).orElseThrow(() -> new UserNotFoundException(
                    "User not found with id: " + id));

            userRepository.deleteUserRoles(id);
            sessionRepository.deleteUserSessions(id);

            userRepository.delete(user);
        });
    }
}

Domena

class DeleteUser {

    private final DeleteUserPort deleteUserPort;

    public DeleteUser(DeleteUserPort deleteUserPort) {
        this.deleteUserPort = deleteUserPort;
    }

    public ApiResponse delete(int userId) {
        deleteUserPort.deleteEverythingById(userId);
        return new ApiResponse(String.format("User with ID %d deleted successfully.", userId));
    }
}
1
Ornstein napisał(a):
Riddle napisał(a):

Rozumiem że chcesz usunąć usera razem z sesją i wszystkim innym. Pytanie: czy to jest jedyny use case? Czy potrzebujesz kiedyś usunąć usera, ale bez usuwania sesji?

Chcę usunąć usera i wszystko, co jest do niego przypisane.

Riddle napisał(a):

Bo jeśli to jest jedyny use case to wystarczy Ci interfejs:

interface DeleteUser {
  void deleteEverthingById(int id);
}

A implementacja sparingowa już może usuwać wszystko co trzeba. Domena nie musi wiedzieć co dokładnie jest usuwane.

Mówisz, że powinienem usunąć te porty, które są abstrakcją dla DAO, i wyodrębnić operacje usuwania do osobnej klasy?

Coś takiego?

No chyba tak. Wygląda okej. W przyszłości, jeśli do Twojego DeleteUser nie dojdzie żadna nowa odpowiedzialność, to można by się zastanowić czy nie zrobić inline tej klasy, bo ona wnosi bardzo mało.

Dodatkowo, co Ty masz z tym żeby nazywać klasy takimi dziwnymi nazwami. Czemu te klasy się nazywają DeleteUserAdapter i DeleteUserPort? Ja bym interfejs nazwał po prostu DeleteUser (po co pisać że to port), a klasę bym nazwał tak, żeby było wiadomo co ta implementacja robi, np TransactionDeleteUser implements DeleteUser.

To nie jest tak, że jak używasz wzorca ports and adapters, to od razu musisz nazywać wszystko port i adapter.

0

Tutaj jednej rzeczy nie rozumiem. Z jednej strony czytam, że domena powinna wiedzieć jak najmniej o szczegółach technicznych. Z drugiej strony napisałeś:

Riddle napisał(a):

W przyszłości, jeśli do Twojego DeleteUser nie dojdzie żadna nowa odpowiedzialność, to można by się zastanowić czy nie zrobić inline tej klasy, bo ona wnosi bardzo mało.

Mając te dwie klasy:

@Service
class TransactionDeleteUser implements DeleteUser {

    private final UserDAO userRepository;
    private final SessionDAO sessionRepository;
    private final TransactionTemplate transactionTemplate;

    public DeleteUserAdapter(UserDAO userDAO,
                             SessionDAO sessionDAO,
                             TransactionTemplate transactionTemplate) {
        this.userRepository = userDAO;
        this.sessionRepository = sessionDAO;
        this.transactionTemplate = transactionTemplate;
    }

    @Override
    public void deleteEverythingById(int id) {
        transactionTemplate.executeWithoutResult(status -> {
            User user = userRepository.findById(id).orElseThrow(() -> new UserNotFoundException(
                    "User not found with id: " + id));

            userRepository.deleteUserRoles(id);
            sessionRepository.deleteUserSessions(id);

            userRepository.delete(user);
        });
    }
}

class DeleteUser {

    private final DeleteUserPort deleteUserPort;

    public DeleteUser(DeleteUserPort deleteUserPort) {
        this.deleteUserPort = deleteUserPort;
    }

    public ApiResponse delete(int userId) {
        deleteUserPort.deleteEverythingById(userId);
        return new ApiResponse(String.format("User with ID %d deleted successfully.", userId));
    }
}

Jeśli chciałbym je połączyć, czy nie pomieszam przypadkiem domeny ze szczegółami technicznymi?

Riddle napisał(a):

Dodatkowo, co Ty masz z tym żeby nazywać klasy takimi dziwnymi nazwami. Czemu te klasy się nazywają DeleteUserAdapter i DeleteUserPort? Ja bym interfejs nazwał po prostu DeleteUser (po co pisać że to port), a klasę bym nazwał tak, żeby było wiadomo co ta implementacja robi, np TransactionDeleteUser implements DeleteUser.

To nie jest tak, że jak używasz wzorca ports and adapters, to od razu musisz nazywać wszystko port i adapter.

Ok, rozumiem.

1
Ornstein napisał(a):

Tutaj jednej rzeczy nie rozumiem. Z jednej strony czytam, że domena powinna wiedzieć jak najmniej o szczegółach technicznych. Z drugiej strony napisałeś:

Riddle napisał(a):

W przyszłości, jeśli do Twojego DeleteUser nie dojdzie żadna nowa odpowiedzialność, to można by się zastanowić czy nie zrobić inline tej klasy, bo ona wnosi bardzo mało.

Mając te dwie klasy:
[...]
Jeśli chciałbym je połączyć, czy nie pomieszam przypadkiem domeny ze szczegółami technicznymi?

Ale ja proponowałem tutaj żeby TransactionDeleteUser zostało tak jak jest. Sugerowałem żeby zrobić inline klasy DeleteUser. Bo ona ma tylko metodę delete() która nie robi prawie nic oprócz wołania deleteEverythingById() i zwrócenia ApiResponse.

0
Riddle napisał(a):
Ornstein napisał(a):

Tutaj jednej rzeczy nie rozumiem. Z jednej strony czytam, że domena powinna wiedzieć jak najmniej o szczegółach technicznych. Z drugiej strony napisałeś:

Riddle napisał(a):

W przyszłości, jeśli do Twojego DeleteUser nie dojdzie żadna nowa odpowiedzialność, to można by się zastanowić czy nie zrobić inline tej klasy, bo ona wnosi bardzo mało.

Mając te dwie klasy:
[...]
Jeśli chciałbym je połączyć, czy nie pomieszam przypadkiem domeny ze szczegółami technicznymi?

Ale ja proponowałem tutaj żeby TransactionDeleteUser zostało tak jak jest. Sugerowałem żeby zrobić inline klasy DeleteUser. Bo ona ma tylko metodę delete() która nie robi prawie nic oprócz wołania deleteEverythingById() i zwrócenia ApiResponse.

A teraz rozumiem, o co chodzi.

Wydaje mi się, że mniej więcej rozumiem cały koncept separacji. Zróbmy mały test - najlepiej na przykładzie. Feature weather - pobieranie i przypisywanie pogody użytkownikowi dla lokalizacji, którą podał.

Port:

public interface WeatherDataFetcher {
    WeatherDto fetchWeatherData(String locationName) throws IOException;
}

Adapter:

@Service
class WeatherDataClient implements WeatherDataFetcher {

    @Override
    public WeatherDto fetchWeatherData(String locationName) throws IOException {
        // implementacja np. jakieś WeatherAPI
    }
}

Domena:

class UserWeatherDataHandler {
   private final WeatherDataFetcher weatherDataFetcher;
   private final UserWeatherDataSaver userWeatherDataSaver;

    UserWeatherDataHandler(WeatherDataFetcher weatherDataFetcher,
                           UserWeatherDataSaver userWeatherDataSaver) {
        this.weatherDataFetcher = weatherDataFetcher;
        this.userWeatherDataSaver = userWeatherDataSaver;
    }

    public void fetchAndSaveWeather(int userId, String locationName) {
        WeatherDto weather = weatherDataFetcher.fetchWeather("Warszawa");
        userWeatherDataSaver.saveWeatherToUser(weather, userId);
    }
}

Klasa która zawiera szczegóły techniczne - i tutaj odbywa się całą magia, czyli przypisanie "pogody" użytkownikowi:

class UserWeatherDataSaver {
}

Jest dobrze?

1

No w sumie tak.

Jedyne do czego bym się doczepił, to to że ta Twoja klasa UserWeatherDataHandler nie ma logiki żadnej. Sens istnienia tej klasy UserWeatherDataHandler jest trochę... wątpliwy. No ale technicznie może być.

0
Riddle napisał(a):

No w sumie tak.

Jedyne do czego bym się doczepił, to to że ta Twoja klasa UserWeatherDataHandler nie ma logiki żadnej. Sens istnienia tej klasy UserWeatherDataHandler jest trochę... wątpliwy. No ale technicznie może być.

W sumie napisałem ją po to, aby połączyć logikę z obu klas. Gdybym dodał jakiś try -> catch, może miałaby większy sens.

1
Ornstein napisał(a):

W sumie napisałem ją po to, aby połączyć logikę z obu klas. Gdybym dodał jakiś try -> catch, może miałaby większy sens.

No ale to nie jest zadanie domeny przecież - takie "łaczenie logiki obu klas".

W domenie ma być to co robi aplikacja. Jeśli robiłbyś np. aplikacje do liczenia podatków, to w domenie powinno być wszystko co związane z podatkami, ale wszystko co jest np związane z parsowaniem exceli powinno być poza domeną.

Zarejestruj się i dołącz do największej społeczności programistów w Polsce.

Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.

Utwórz konto