Отрефакторить код регистрации пользователя

22. Отрефакторить код регистрации пользователя

Условие задачи:
Дан код регистрации пользователя. В процессе регистрации пользователь сохраняется через DAO, формируется отчёт, отправляется уведомление и пишется лог.

Нужно провести code review, найти ошибки компиляции, проблемы с архитектурой и безопасностью, а затем выполнить рефакторинг.

Код:

public interface UserDao {
    void save(UserDto userDto);
}

public class UserDaoImplementation implements UserDao {

    @Override
    public void save(UserDto userDto) {
        System.out.println("Save user.");
    }
}

public class UserDaoFastImplementation implements UserDao {

    @Override
    public void save(UserDto userDto) {
        System.out.println("Save user.");
    }

    public void saveFast(String username, String password) {
        System.out.println("Save user optimized.");
    }
}

public record UserDto(String username, String password) {
}

public class Main {

    public static void main(String[] args) {
        var logger = LoggerFactory.getLogger("org.example");
        var userDao = new UserDaoImplementation();
        var jsonMapper = new JsonMapper();
        var notificationService = new EmailNotificationService();

        var registerService = new RegisterUserService(
                logger,
                jsonMapper,
                userDao,
                notificationService
        );

        var newUserDto = new UserDto(
                "test-username",
                "test-password"
        );

        registerService.register(newUserDto);
    }
}

public class RegisterUserService {

    private final Logger logger;
    private final JsonMapper jsonMapper;
    private final UserDao userDao;
    private final EmailNotificationService emailNotificationService;

    public RegisterUserService(
            Logger logger,
            JsonMapper jsonMapper,
            UserDao userDao,
            EmailNotificationService emailNotificationService
    ) {
        this.logger = logger;
        this.jsonMapper = jsonMapper;
        this.userDao = userDao;
        this.emailNotificationService = emailNotificationService;
    }

    public void register(UserDto userDto) {
        logger.info(
                "Start registration for user: {}",
                userDto.username()
        );

        if (userDao instanceof UserDaoFastImplementation) {
            ((UserDaoFastImplementation) userDao).saveFast(
                    userDto.username(),
                    userDto.password()
            );
        } else {
            userDao.save(userDto);
        }

        try {
            saveReportToFile(userDto);
        } catch (IOException e) {
            logger.error(
                    "Save report to file for user {} failed!",
                    userDto.username()
            );
        }

        emailNotificationService.sendEmailNotification();

        logger.info(
                "User: {} with password: {}, registered",
                userDto.username(),
                userDto.password()
        );
    }

    private void saveReportToFile(UserDto userDto)
            throws IOException {

        FileOutputStream outputStream =
                new FileOutputStream(
                        "/tmp/registered-users-report.csv"
                );

        byte[] strToBytes =
                jsonMapper.writeValueAsBytes(userDto);

        outputStream.write(strToBytes);
        outputStream.close();
    }
}

public final class EmailNotificationService {

    public void sendEmailNotification() {
        System.out.println("Send email notification");
    }
}

Спойлеры к решению

Подсказки

💡 Проверь, откуда импортируется UserDto в RegisterUserService.
💡 RegisterUserService не должен знать конкретные реализации UserDao.
💡 Пароль нельзя писать в лог и отчёт.
💡 Работу с файлом лучше вынести из сервиса регистрации.
💡 Проверь соответствие расширения файла его содержимому и режим открытия файла.

Решение

Проблемы:

в RegisterUserService указан неправильный импорт UserDto: DTO находится в org.example.dto;
instanceof UserDaoFastImplementation нарушает полиморфизм и OCP;
saveFast() отсутствует в контракте UserDao;
сервис регистрации одновременно занимается persistence, отчётом, уведомлением и логированием;
отсутствует валидация UserDto;
пароль попадает в лог;
пароль попадает в файл вместе с UserDto;
для реальной регистрации пароль нельзя сохранять в открытом виде;
FileOutputStream закрывается вручную;
файл имеет расширение .csv, но записывается JSON;
файл перезаписывается при каждой регистрации;
ошибка сохранения отчёта фактически только логируется, нужно явно определить допустима ли регистрация без отчёта;
System.out.println() в DAO и notification лучше заменить нормальной реализацией/логированием.

Как лучше исправить

Сервис должен работать только с интерфейсом:

public interface UserDao {
    void save(UserDto userDto);
}

Оптимизированная реализация должна переопределять тот же метод:

public class UserDaoFastImplementation implements UserDao {

    @Override
    public void save(UserDto userDto) {
        System.out.println("Save user optimized.");
    }
}

Тогда instanceof больше не нужен.

public class RegisterUserService {

    private final Logger logger;
    private final UserDao userDao;
    private final RegistrationReportService reportService;
    private final EmailNotificationService notificationService;

    public RegisterUserService(
            Logger logger,
            UserDao userDao,
            RegistrationReportService reportService,
            EmailNotificationService notificationService
    ) {
        this.logger = logger;
        this.userDao = userDao;
        this.reportService = reportService;
        this.notificationService = notificationService;
    }

    public void register(UserDto userDto) {
        validate(userDto);

        logger.info(
                "Start registration for user: {}",
                userDto.username()
        );

        userDao.save(userDto);
        reportService.save(userDto.username());
        notificationService.sendEmailNotification();

        logger.info(
                "User {} registered",
                userDto.username()
        );
    }

    private void validate(UserDto userDto) {
        if (userDto == null
                || userDto.username() == null
                || userDto.username().isBlank()
                || userDto.password() == null
                || userDto.password().isBlank()) {
            throw new IllegalArgumentException(
                    "Invalid user data"
            );
        }
    }
}

Отчёт лучше вынести в отдельный компонент и не сохранять в нём пароль:

public class RegistrationReportService {

    private static final Path REPORT =
            Path.of("/tmp/registered-users-report.csv");

    public void save(String username) {
        try {
            Files.writeString(
                    REPORT,
                    username + System.lineSeparator(),
                    StandardOpenOption.CREATE,
                    StandardOpenOption.APPEND
            );
        } catch (IOException e) {
            throw new IllegalStateException(
                    "Unable to save registration report",
                    e
            );
        }
    }
}

Для настоящей системы пароль перед сохранением должен быть захеширован подходящим password encoder. В логах и отчётах пароль хранить нельзя.

Главная идея рефакторинга: убрать зависимость от конкретной реализации DAO, разделить обязанности и исключить утечку пароля.