Wątek przeniesiony 2024-03-04 13:24 z Java przez Riddle.

Pytanie odnośnie struktury kodu

0

Cześć,
napisałem kod odpowiedzialny za usuwanie użytkownika. Mam problem z oceną, czy zrobiłem to tak, jak należy, a dokładniej chodzi mi o jedną rzecz. Funkcjonalność klas takich jak RoleDeleterImpl, SessionDeleterImpl, ogranicza się do wykorzystania metod zdefiniowanych w DAO. Zrobiłem to po to, żeby można było to łatwiej przetestować. Otrzymałem komentarz, że stworzyłem dużo bezwartościowego kodu, ponieważ te moje klasy robią praktycznie to samo, co DAO i tak nie powinno być. Jaka jest Wasza opinia na ten temat? Czy mogę i powinienem tak robić? Ewentualnie co powinienem zmienić.

Dzięki za pomoc

UserDAO:

public interface UserDAO {

    @SqlQuery("SELECT * FROM users WHERE id = :id")
    @RegisterBeanMapper(User.class)
    Optional<User> findById(@Bind("id") int id);

    @SqlUpdate("DELETE FROM users WHERE id = :id")
    void delete(@BindBean User user);

    @SqlUpdate("DELETE FROM user_roles WHERE user_id = :userId")
    void deleteUserRoles(@Bind("userId") int userId);
}

SessionDAO:

public interface SessionDAO {

    @SqlUpdate("DELETE FROM sessions WHERE user_id = :userId")
    void deleteSessionsByUserId(@Bind("userId") int userId);
}

Klasy:

@Service
public class RoleDeleterImpl implements RoleDeleter {

    private final UserDAO userDAO;

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

    @Override
    public void deleteUserRoles(int userId) {
        userDAO.deleteUserRoles(userId);
    }
}
@Service
public class SessionDeleterImpl implements SessionDeleter {

    private final SessionDAO sessionDAO;

    public SessionDeleterImpl(SessionDAO sessionDAO) {
        this.sessionDAO = sessionDAO;
    }

    @Override
    public void deleteUserSessions(int userId) {
        sessionDAO.deleteSessionsByUserId(userId);
    }
}
@Service
public class UserRetrievalImpl implements UserRetrieval {

    private final UserDAO userDAO;

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

    @Override
    public User findByUserId(int userId) {
        return userDAO.findById(userId).orElseThrow(() -> new UserNotFoundException(
                "User not found with id: " + userId));
    }
}
@Service
public class UserDeleterImpl implements UserDeleter {

    private final UserDAO userDAO;
    private final UserRetrieval userRetrieval;

    public UserDeleterImpl(UserDAO userDAO, UserRetrieval userRetrieval) {
        this.userDAO = userDAO;
        this.userRetrieval = userRetrieval;
    }

    @Override
    public void deleteUser(int userId) {
        User user = userRetrieval.findByUserId(userId);
        userDAO.delete(user);
    }
}
@Service
public class UserDeletionManagerImpl implements UserDeletionManager {
    private final RoleDeleter roleDeleter;
    private final UserDeleter userDeleter;
    private final SessionDeleter sessionDeleter;


    public UserDeletionManager(RoleDeleterImpl roleDeleter,
                               UserDeleterImpl userDeleter,
                               SessionDeleterImpl sessionDeleter) {
        this.roleDeleter = roleDeleter;
        this.userDeleter = userDeleter;
        this.sessionDeleter = sessionDeleter;
    }

    @Transactional
    public void executeDeletion(int userId) {
        roleDeleter.deleteUserRoles(userId);
        sessionDeleter.deleteUserSessions(userId);
        userDeleter.deleteUser(userId);
    }
}
@Service
@Slf4j
class DeleteUserServiceImpl implements DeleteUserService {

    private final UserDeletionManager deletionManager;
    private final ApplicationMessageService messageService;

    public DeleteUserServiceImpl(UserDeletionManager deletionManager, ApplicationMessageService messageService) {
        this.deletionManager = deletionManager;
        this.messageService = messageService;
    }

    @Override
    public ApiResponse deleteUser(int userId) throws DatabaseException {
        deletionManager.executeDeletion(userId);
        log.info("User with ID {} deleted successfully", userId);
        return new ApiResponse(messageService.getMessage("user.delete.success", userId));
    }
}
0

Ok, zacznę pokazywać jak piszę, w wolnej chwili nagram na YT.

ps. to jedna z ocen jakie ostatnio dostałem "@johnny_Be_good: weź się w końcu zamknij i przestań rozprzestrzeniać te swoje rozwiązania z d**y — abrakadaber 2024-03-02 15:21"

3

No faktycznie trochę to dziwnie wygląda.

Jedna rzecz - niepotrzebnie masz interfejsy na wszystko, one są tutaj zupełnie po nic. Takie interfejsy mają sens jak próbujesz odseparować moduły od siebie, ale w Twoim przypadku nie ma.
Druga rzecz - po co Ci klasy SessionDeleterImpl, UserDeleterImpl, UserRetrievalImpl? Czemu UserDeletionManagerImpl miałby po prostu nie mieć w sobie DAO? Noi czemu te dwa DAO są osobne, zamiast być jednym?
Po trzecie - czemu ta klasa nazywa się "manager"? Czemu nie po prostu DeleteUser. Dopisanie "manager" nie dodaje żadnych nowych informacji do tej klasy.

Więc tak, zgadzam się. Te klasy są niepotrzebne. I jeśli mówisz że napisałeś je po to żeby "łatwiej je testować", to nie wiem czy sobie nie zrobiłeś sam pod górę, bo tego Twojego UserDeletionManager (który nie powinien się tak nazywać), wygląda jakby tak samo łatwo dało się go przetestować.

0
Riddle napisał(a):

No faktycznie trochę to dziwnie wygląda.

Jedna rzecz - niepotrzebnie masz interfejsy na wszystko, one są tutaj zupełnie po nic.
Druga rzecz - po co Ci klasy SessionDeleterImpl, UserDeleterImpl, UserRetrievalImpl? Czemu UserDeletionManagerImpl miałby po prostu nie mieć w sobie DAO? Noi czemu te dwa DAO są osobne, zamiast być jednym?
Po trzecie - czemu ta klasa nazywa się "manager"? Czemu nie po prostu DeleteUser. Dopisanie "manager" nie dodaje żadnych nowych informacji do tej klasy.

Więc tak, zgadzam się. Te klasy są niepotrzebne. I jeśli mówisz że napisałeś je po to żeby "łatwiej je testować", to nie wiem czy sobie nie zrobiłeś sam pod górę, bo tego Twojego UserDeletionManager (który nie powinien się tak nazywać), wygląda jakby tak samo łatwo dało się go przetestować.

To tak będzie lepiej?

public interface UserDAO {

    @SqlQuery("SELECT * FROM users WHERE id = :id")
    @RegisterBeanMapper(User.class)
    Optional<User> findById(@Bind("id") int id);

    @SqlUpdate("DELETE FROM users WHERE id = :id")
    void delete(@BindBean User user);

    @SqlUpdate("DELETE FROM user_roles WHERE user_id = :userId")
    void deleteUserRoles(@Bind("userId") int userId);

    @SqlUpdate("DELETE FROM sessions WHERE user_id = :userId")
    void deleteUserSessions(@Bind("userId") int userId);
}
@Service
public class DeleteUser {

    private final UserDAO userDAO;

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

    @Transactional
    public void delete(int userId) {
        userDAO.deleteUserRoles(userId);
        userDAO.deleteUserSessions(userId);

        User user = userDAO.findById(userId).orElseThrow(() -> new UserNotFoundException(
                "User not found with id: " + userId));

        userDAO.delete(user);
    }
}
@Service
@Slf4j
public class DeleteUserService {

    private final DeleteUser delete;
    private final ApplicationMessageService message;

    public DeleteUserService(DeleteUser deletionManager,
                             ApplicationMessageService message) {
        this.delete = deletionManager;
        this.message = message;
    }

    public ApiResponse deleteUser(int userId) throws DatabaseException {
        delete.delete(userId);
        log.info("User with ID {} deleted successfully", userId);
        return new ApiResponse(message.getMessage("user.delete.success", userId));
    }
}
0

Jest lepiej.

1

Lepiej jakiegoś orma, zrób lub użyj, a nie jakieś takie dziwne coś, że specjalna klasa tylko do usuwania usera.
Ten kod jest podatny na sql injection, więc nie polecam.
Po co Ci interfejs, który możesz użyć tylko dla użytkownika, interfejsy powinny być abstrakcyjne, że sobie je możesz zaimplementować do różnych modeli, a nie tylko dla usera, taki interfejs nie ma żadnego sensu.

1
omenomn2 napisał(a):

Lepiej jakiegoś orma, zrób lub użyj, a nie jakieś takie dziwne coś, że specjalna klasa tylko do usuwania usera.

Yyy... a o dependency inversion i SRP pan słyszał?

omenomn2 napisał(a):

Ten kod jest podatny na sql injection, więc nie polecam.

Nie jest, przecież używa JPA 😐

omenomn2 napisał(a):

Po co Ci interfejs, który możesz użyć tylko dla użytkownika, interfejsy powinny być abstrakcyjne, że sobie je możesz zaimplementować do różnych modeli, a nie tylko dla usera, taki interfejs nie ma żadnego sensu.

To szczegół implementacyjny JPA.

@Ornstein nie polecam słuchać tych rewelacji. Jest dobrzeń tak jak jest.

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

Lepiej jakiegoś orma, zrób lub użyj, a nie jakieś takie dziwne coś, że specjalna klasa tylko do usuwania usera.

Yyy... a o dependency inversion i SRP pan słyszał?

omenomn2 napisał(a):

Ten kod jest podatny na sql injection, więc nie polecam.

Nie jest, przecież używa JPA 😐

omenomn2 napisał(a):

Po co Ci interfejs, który możesz użyć tylko dla użytkownika, interfejsy powinny być abstrakcyjne, że sobie je możesz zaimplementować do różnych modeli, a nie tylko dla usera, taki interfejs nie ma żadnego sensu.

To szczegół implementacyjny JPA.

@Ornstein nie polecam słuchać tych rewelacji. Jest dobrzeń tak jak jest.

A czy ta klasa poniżej, którą zaproponowałem, przestrzega SRP? Bo tak patrząc, to odpowiada za wyszukanie użytkownika i jego usunięcie. Dlatego we wcześniejszej wersji wydzieliłem wyszukanie użytkownika do oddzielnej klasy.

@Service
public class DeleteUser {

    private final UserDAO userDAO;

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

    @Transactional
    public void delete(int userId) {
        User user = userDAO.findById(userId).orElseThrow(() -> new UserNotFoundException(
            "User not found with id: " + userId));

        userDAO.deleteUserRoles(userId);
        userDAO.deleteUserSessions(userId);
        userDAO.delete(user);
    }
}
1
Ornstein napisał(a):

A czy ta klasa poniżej, którą zaproponowałem, przestrzega SRP? Bo tak patrząc, to odpowiada za wyszukanie użytkownika i jego usunięcie. Dlatego we wcześniejszej wersji wydzieliłem wyszukanie użytkownika do oddzielnej klasy.

Tak, przestrzega.

Tak na prawdę nie odpowiada za wyszukanie. Wyszukanie to jest tylko szczegół implementacyjny prawdziwego zachowania jakim jest usunięcie.

Widać jasno że odpowiedzialnością tej klasy jest: usuń użytkownika po id. Nie ma innej, jest tylko jedna. Więc przestrzega SRP.

Łamałaby SRP gdyby wynik tego wyszukiwania w jakiś sposób wyciekłby poza tą klasę. Ale tak długo jak nie wycieka, to jedyną odpowiedzialnością pozostaje usuwanie, tak jak powinno być.

2

SRP łatwiej zrozumieć jak się wie po co się to stosuje i jakie są z tego korzyści, a stosuje się głównie po to żeby dało się użyć logiki w innym miejscu jeśli jest potrzebna. Na przykład klasa która wyciąga dane z bazy i je jakoś obrabia łamie SRP bo jeśli później będziesz potrzebował obrobić dane z innego źródła (na przykład lokalnie w pamięci) to masz pecha bo musisz tę logikę wtedy wyłuskiwać albo robić kopiuj wklejkę. Klasa która zapisze i odczyta usera nie łamie SRP, co najwyżej może łamać ISP.
Przy okazji zyskujemy czytelność i ładniejszą organizację kodu ale to są jakby efekty uboczne tego.

0

Nie jest, przecież używa JPA 😐

Racja, źle spojrzałem.

Yyy... a o dependency inversion i SRP pan słyszał?

Za bardzo bierzesz do siebie srp, tu nie chodzi o to, żeby robić klasy z jedną metodą.
Tylko bardziej o to, żeby klasa zajmowała się pojedynczym kontekstem.
A odnośnie dependency inversion to właśnie w tym kodzie abstrakcja jest zła, bo jest implementacją, więc to nie jest szczegół implementacyjny.

omenomn2 napisał(a):
Po co Ci interfejs, który możesz użyć tylko dla użytkownika, interfejsy powinny być abstrakcyjne, że sobie je możesz zaimplementować do różnych modeli, a nie tylko dla usera, taki interfejs nie ma żadnego sensu.

To szczegół implementacyjny JPA.

No właśnie to błąd pod kątem dependency inversion

0
omenomn2 napisał(a):

Za bardzo bierzesz do siebie srp, tu nie chodzi o to, żeby robić klasy z jedną metodą.
Tylko bardziej o to, żeby klasa zajmowała się pojedynczym kontekstem

Napisałeś a nie jakieś takie dziwne coś, że specjalna klasa tylko do usuwania usera., tak jakby klasa specjalnie pod usuwanie usera była czymś złym - nie jest. Osobna klasa która usuwa usera moim zdaniem jest całkiem spoko.

omenomn2 napisał(a):

A odnośnie dependency inversion to właśnie w tym kodzie abstrakcja jest zła, bo jest implementacją, więc to nie jest szczegół implementacyjny.

A to z kolei napisałem, bo Ty napisałeś Lepiej jakiegoś orma, zrób. A ORM przecież to jest szczegół implementacyjny, i nie należy go umieszczać w warstwie biznesowej - a pytanie autora dotyczy również warstwy biznesowej, dlatego dodanie ORM do wartswy biznesowej złamałoby DI.

Poza tym cytując cały Twój pierwszy post:

omenomn2 napisał(a):

Po co Ci interfejs, który możesz użyć tylko dla użytkownika, interfejsy powinny być abstrakcyjne, że sobie je możesz zaimplementować do różnych modeli, a nie tylko dla usera, taki interfejs nie ma żadnego sensu.

Tutaj rozumiem odnosisz się do tego, że jesli ktoś chce zrobić usuwanie "czegoś", to powinien tego użyć do usuwania np userów, postów, wiadomości, etc? Bo jeśli tak, to moim zdaniem nie masz racji.

omenomn2 napisał(a):

To szczegół implementacyjny JPA.

No właśnie to błąd pod kątem dependency inversion

Jeśli mówimy o interfejsie tym pod JPA, to to jest szczegół implementacyjny JPA tak jak napisałem, i chcąc być zgodnym z DI, należałoby to schować w abstrakcji.

Jeśli jednak mówimy o interfejsie domenowym, to nie łamie to DI w żaden sposób.

0

A to z kolei napisałem, bo Ty napisałeś Lepiej jakiegoś orma, zrób. A ORM przecież to jest szczegół implementacyjny, i nie należy go umieszczać w warstwie biznesowej - a pytanie autora dotyczy również warstwy biznesowej, dlatego dodanie ORM do wartswy biznesowej złamałoby DI.

Człowiek wrzucił kod do oceny, więc oceniłem, że nie ma sensu, stosować czystych zapytań w kodzie, bo są do tego biblioteki, które robią to lepiej.
Czy to szczegół implementacyjny, czy nie to nie ma znaczenia pod kątem mojej wypowiedzi, znaczenie ma to, co można zrobić lepiej w tym kodzie.

0
omenomn2 napisał(a):

[...] że nie ma sensu, stosować czystych zapytań w kodzie, bo są do tego biblioteki, które robią to lepiej.

Nic nie rozumiem z tego zdania 🧐 Co to są te "czyste zapytania"?

omenomn2 napisał(a):

Czy to szczegół implementacyjny, czy nie to nie ma znaczenia pod kątem mojej wypowiedzi, znaczenie ma to, co można zrobić lepiej w tym kodzie.

No czyli co konkretnie?

0

DELETE FROM users WHERE id = :id

to jest zapytanie np.

Lepiej użyć biblioteki, która robi coś takiego $user->delete(), $post->delete(), itd. czyli po prostu orm.

No czyli co konkretnie?

No czyli konkretnie to lepiej użyć biblioteki, która robi coś takiego $user->delete(), $post->delete(), itd. czyli po prostu orm, a nie używać bezpośrednio zapytań,
Są dwie drogi, albo pisać własny orm, albo użyć gotowego, ale droga w przykładzie tego kodu jest najgorsza, czyli pisanie zapytań pod każdy osobny przypadek/potrzebę.

Query raw - nie spotkałeś się z takim pojęciem ?

0
omenomn2 napisał(a):

Lepiej użyć biblioteki, która robi coś takiego $user->delete(), $post->delete(), itd. czyli po prostu orm.

Spora grupa developerów nie uważa, że tak jest lepiej. Dużo ludzi odbija się od ORMów, bo niby są fajne, ale często robią magię pod spodem i nie pozwalają na pełne wykorzystanie potencjału bazy.

Do tego ORM to nie do końca to co piszesz. ORMem nazwa się wszystko to co nazwa wskazuje: coś co mapuje relacje bazodanowe na obiekty. Przykładowo w podejściu micro ORM możesz mieć coś takiego:

class Person {
  @Field("name")
  string name;

  @Field("surname")
  string surname
}

List<Person> AllPeople(Database db) {
  return db.Exec("SELECT name, surname from people;").To<List<Person>>();
}

gdzie całkowity brak ORMa wygląda tak:

List<Person> AllPeople(Database db) {
  return db.Exec("SELECT name, surname from people;").map(row -> new Person(row["name"].asString(), row["surname"].asString()))
}

Zauważ, że w pierwszym przykładzie ORM sam tworzy mapowanie. W podejściu numer 2 operujemy na czystym formacie tego co zwróciła baza danych

0
omenomn2 napisał(a):

DELETE FROM users WHERE id = :id

to jest zapytanie np.

Lepiej użyć biblioteki, która robi coś takiego $user->delete(), $post->delete(), itd. czyli po prostu orm.

Wszystko jedno. Oba to szczegół implementacyjny.

omenomn2 napisał(a):

No czyli co konkretnie?

No czyli konkretnie to lepiej użyć biblioteki, która robi coś takiego $user->delete(), $post->delete(), itd. czyli po prostu orm, a nie używać bezpośrednio zapytań,

Abstrakcja na sql (jak np orm) ma swoje wady i zalety.

omenomn2 napisał(a):

Są dwie drogi, albo pisać własny orm, albo użyć gotowego, ale droga w przykładzie tego kodu jest najgorsza, czyli pisanie zapytań pod każdy osobny przypadek/potrzebę.

Query raw - nie spotkałeś się z takim pojęciem ?

To jest bardziej złożona sprawa, niż takie proste palnięcie "orm jest lepszy niż gole sql".

0

Spora grupa developerów nie uważa, że tak jest lepiej. Dużo ludzi odbija się od ORMów, bo niby są fajne, ale często robią magię pod spodem i nie pozwalają na pełne wykorzystanie potencjału bazy.

Jak się nie potrafi z nich korzystać to nie pozwalają.

Przykładowo w podejściu micro ORM możesz mieć coś takiego

Nie chodziło mi o mikro orm, tylko porządną bibliotekę, która nie tylko mapuje obiekty, ale wykonuję operacje na bazie danych.

To jest bardziej złożona sprawa, niż takie proste palnięcie "orm jest lepszy niż gole sql".

Mam inne zdanie.

0
omenomn2 napisał(a):

Nie chodziło mi o mikro orm, tylko porządną bibliotekę, która nie tylko mapuje obiekty, ale wykonuję operacje na bazie danych.

To co napisałem właśnie to robi. Tobie chodzi o dodatkowy ficzer tj. zapytania są generowanie na podstawie modelu

0

A jak oceniacie strukturę kodu odpowiedzialnego za logowanie?

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

    private final SessionInitialization session;
    private final ApplicationMessageLoader message;

    public LoginController(SessionInitialization session,
                           ApplicationMessageLoader message) {
        this.session = session;
        this.message = message;
    }

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

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

        return ResponseEntity.ok(new ApiResponse(message.getMessage("login.success")));
    }
}
@Service
public class SessionInitialization {

    private final UserSessionCreator sessionCreator;

    public SessionInitialization(UserSessionCreator sessionCreator) {
        this.sessionCreator = sessionCreator;
    }

    public String initializeSession(LoginRequest request) {
        Session session = sessionCreator.createUserSession(request);
        return session.getSessionId();
    }
}
@Service
public class UserSessionCreator {

    private final SessionCreation sessionCreation;
    private final UserAuthentication userAuth;
    private final TokenGenerator tokenGenerator;
    private final SessionDAO sessionDAO;
    private final UserDAO userDAO;

    public UserSessionCreator(SessionCreation sessionCreation,
                              UserAuthentication userAuth,
                              TokenGenerator tokenGenerator,
                              SessionDAO sessionDAO,
                              UserDAO userDAO) {
        this.sessionCreation = sessionCreation;
        this.userAuth = userAuth;
        this.tokenGenerator = tokenGenerator;
        this.sessionDAO = sessionDAO;
        this.userDAO = userDAO;
    }

    public Session createUserSession(LoginRequest request) {
        String token = tokenGenerator.generateToken();
        Authentication auth = userAuth.authenticateUser(request);
        User user = userDAO.findByUsername(auth.getName()).orElseThrow(
                () -> new UserNotFoundException("User not found with username :" + auth.getName()));

        Session session = sessionCreation.createSession(token, user.getId());
        sessionDAO.save(session);

        return session;
    }
}
@Service
@Slf4j
public class UserAuthentication {

    private final AuthenticationManager authManager;
    private final ApplicationMessageLoader message;

    public UserAuthentication(AuthenticationManager authManager,
                              ApplicationMessageLoader message) {
        this.authManager = authManager;
        this.message = message;
    }

    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) {
            log.error("Error during login for user: {}", request.usernameOrEmail(), e);
            throw new BadCredentialsException(message.getMessage("login.failure"));
        }
    }
}
@Service
public class SessionCreation {

    private final TokenHasher tokenHasher;

    public SessionCreation(TokenHasher tokenHasher) {
        this.tokenHasher = tokenHasher;
    }

    public Session createSession(String token, Integer userId) {
        String hashedToken = tokenHasher.generateHashedToken(token);

        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;
    }
}
@Service
public class TokenHasher {

    private final Base64.Encoder encoder = Base64.getUrlEncoder();

    public String generateHashedToken(String token) {
        try {
            MessageDigest digest = MessageDigest.getInstance("SHA-256");
            byte[] hashedBytes = digest.digest(token.getBytes(StandardCharsets.UTF_8));
            return encoder.encodeToString(hashedBytes);
        } catch (NoSuchAlgorithmException e) {
            throw new IllegalStateException("SHA-256 algorithm not found", e);
        }
    }
}
@Service
public class TokenGenerator {

    private final SecureRandom secureRandom = new SecureRandom();

    private final Base64.Encoder encoder = Base64.getUrlEncoder();

    public String generateToken() {
        byte[] randomBytes = new byte[24];
        secureRandom.nextBytes(randomBytes);
        return encoder.encodeToString(randomBytes);
    }
}
1
Ornstein napisał(a):

A jak oceniacie strukturę kodu odpowiedzialnego za logowanie?

  1. Zasadność istnienia SessionInitialization trochę wątpliwa. Ja bym zrobił inline tej klasy albo przeniósł zwracanie tego id do UserSessionCreator
  2. Czy faktycznie masz usecase na to żeby TokenHasher rzucił ten wyjątek? Imo ten wyjątek nigdy nie poleci. Ja bym rzucił po prostu new RuntimeException(e) i się nie przejmował. Chyba że faktycznie myślisz, że taka sytuacja może się zdarzyć, to należałoby napisać pod to test i ją obsłużyć poprawnie.

Reszta chyba git.

1 użytkowników online, w tym zalogowanych: 0, gości: 1