From 7e38b5a2e0416831cfe1657e379b03fde3709abc Mon Sep 17 00:00:00 2001 From: Piotr Dec Date: Wed, 22 May 2024 23:28:22 +0200 Subject: [PATCH] fix: Request validation in UserService --- .../eu/ztsh/wymiana/service/UserService.java | 6 +- .../eu/ztsh/wymiana/validation/Adult.java | 5 +- .../wymiana/validation/InstanceValidator.java | 20 +++++++ .../validation/InstanceValidatorFactory.java | 18 ++++++ .../validation/ValidationFailedException.java | 20 +++++++ .../wymiana/web/model/UserCreateRequest.java | 2 + .../java/eu/ztsh/wymiana/EntityCreator.java | 7 ++- .../ztsh/wymiana/service/UserServiceTest.java | 56 +++++++++++++++---- .../eu/ztsh/wymiana/util/UserMapperTest.java | 18 +++--- 9 files changed, 126 insertions(+), 26 deletions(-) create mode 100644 src/main/java/eu/ztsh/wymiana/validation/InstanceValidator.java create mode 100644 src/main/java/eu/ztsh/wymiana/validation/InstanceValidatorFactory.java create mode 100644 src/main/java/eu/ztsh/wymiana/validation/ValidationFailedException.java diff --git a/src/main/java/eu/ztsh/wymiana/service/UserService.java b/src/main/java/eu/ztsh/wymiana/service/UserService.java index 01cba7c..d23dc2c 100644 --- a/src/main/java/eu/ztsh/wymiana/service/UserService.java +++ b/src/main/java/eu/ztsh/wymiana/service/UserService.java @@ -4,8 +4,8 @@ import eu.ztsh.wymiana.data.repository.UserRepository; import eu.ztsh.wymiana.exception.UserAlreadyExistsException; import eu.ztsh.wymiana.model.User; import eu.ztsh.wymiana.util.UserMapper; +import eu.ztsh.wymiana.validation.InstanceValidator; import eu.ztsh.wymiana.web.model.UserCreateRequest; -import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; @@ -16,8 +16,10 @@ import java.util.Optional; public class UserService { private final UserRepository userRepository; + private final InstanceValidator validator; - public User create(@Valid UserCreateRequest request) { + public User create(UserCreateRequest request) { + validator.validate(request); if (userRepository.findById(request.pesel()).isPresent()) { throw new UserAlreadyExistsException(request); } diff --git a/src/main/java/eu/ztsh/wymiana/validation/Adult.java b/src/main/java/eu/ztsh/wymiana/validation/Adult.java index affba90..0d7448f 100644 --- a/src/main/java/eu/ztsh/wymiana/validation/Adult.java +++ b/src/main/java/eu/ztsh/wymiana/validation/Adult.java @@ -16,9 +16,12 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME; @Constraint(validatedBy = AdultValidator.class) @Documented public @interface Adult { - String message() default "{jakarta.validation.constraints.Adult.message}"; + String message() default MESSAGE; Class[] groups() default { }; Class[] payload() default { }; + + String MESSAGE = "The person has not reached the age of 18"; + } diff --git a/src/main/java/eu/ztsh/wymiana/validation/InstanceValidator.java b/src/main/java/eu/ztsh/wymiana/validation/InstanceValidator.java new file mode 100644 index 0000000..664f5c4 --- /dev/null +++ b/src/main/java/eu/ztsh/wymiana/validation/InstanceValidator.java @@ -0,0 +1,20 @@ +package eu.ztsh.wymiana.validation; + +import jakarta.validation.Validator; + +public class InstanceValidator { + + private final Validator validator; + + public InstanceValidator(Validator validator) { + this.validator = validator; + } + + public void validate(T t, Class... classes) { + var violations = validator.validate(t, classes); + if (!violations.isEmpty()) { + throw new ValidationFailedException(violations); + } + } + +} diff --git a/src/main/java/eu/ztsh/wymiana/validation/InstanceValidatorFactory.java b/src/main/java/eu/ztsh/wymiana/validation/InstanceValidatorFactory.java new file mode 100644 index 0000000..69289ab --- /dev/null +++ b/src/main/java/eu/ztsh/wymiana/validation/InstanceValidatorFactory.java @@ -0,0 +1,18 @@ +package eu.ztsh.wymiana.validation; + +import jakarta.validation.Validation; +import jakarta.validation.ValidatorFactory; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class InstanceValidatorFactory { + + @Bean + public InstanceValidator instanceValidator() { + try (ValidatorFactory factory = Validation.buildDefaultValidatorFactory()) { + return new InstanceValidator(factory.getValidator()); + } + } + +} diff --git a/src/main/java/eu/ztsh/wymiana/validation/ValidationFailedException.java b/src/main/java/eu/ztsh/wymiana/validation/ValidationFailedException.java new file mode 100644 index 0000000..7ed17fa --- /dev/null +++ b/src/main/java/eu/ztsh/wymiana/validation/ValidationFailedException.java @@ -0,0 +1,20 @@ +package eu.ztsh.wymiana.validation; + +import jakarta.validation.ConstraintViolation; +import lombok.Getter; + +import java.util.Set; +import java.util.stream.Collectors; + +@Getter +public class ValidationFailedException extends RuntimeException { + + private final transient Set violations; + + public ValidationFailedException(Set> violations) { + super("Validation failed: %s".formatted(violations.stream() + .map(ConstraintViolation::getMessage).collect(Collectors.joining(System.lineSeparator())))); + this.violations = violations; + } + +} diff --git a/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java b/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java index 35c7a94..6c15a62 100644 --- a/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java +++ b/src/main/java/eu/ztsh/wymiana/web/model/UserCreateRequest.java @@ -3,7 +3,9 @@ package eu.ztsh.wymiana.web.model; import eu.ztsh.wymiana.validation.Adult; import jakarta.validation.constraints.Min; import jakarta.validation.constraints.NotNull; +import lombok.Builder; +@Builder public record UserCreateRequest( @NotNull String name, @NotNull String surname, diff --git a/src/test/java/eu/ztsh/wymiana/EntityCreator.java b/src/test/java/eu/ztsh/wymiana/EntityCreator.java index 77ae953..4f7fcd7 100644 --- a/src/test/java/eu/ztsh/wymiana/EntityCreator.java +++ b/src/test/java/eu/ztsh/wymiana/EntityCreator.java @@ -23,8 +23,11 @@ public class EntityCreator { return new UserEntityBuilder(); } - public static UserCreateRequest userRequest() { - return new UserCreateRequest(Constants.NAME, Constants.SURNAME, Constants.PESEL, Constants.PLN); + public static UserCreateRequest.UserCreateRequestBuilder userRequest() { + return UserCreateRequest.builder().name(Constants.NAME) + .surname(Constants.SURNAME) + .pesel(Constants.PESEL) + .pln(Constants.PLN); } public static class UserEntityBuilder { diff --git a/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java b/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java index 4a9b94b..8a0f9fc 100644 --- a/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java +++ b/src/test/java/eu/ztsh/wymiana/service/UserServiceTest.java @@ -5,7 +5,11 @@ import eu.ztsh.wymiana.RepositoryBasedTest; import eu.ztsh.wymiana.data.repository.UserRepository; import eu.ztsh.wymiana.exception.UserAlreadyExistsException; import eu.ztsh.wymiana.util.UserMapper; +import eu.ztsh.wymiana.validation.Adult; +import eu.ztsh.wymiana.validation.InstanceValidator; +import eu.ztsh.wymiana.validation.ValidationFailedException; import jakarta.transaction.Transactional; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -17,44 +21,72 @@ class UserServiceTest extends RepositoryBasedTest { private final UserService userService; @Autowired - public UserServiceTest(UserRepository userRepository) { + public UserServiceTest(UserRepository userRepository, InstanceValidator instanceValidator) { super(userRepository); - userService = new UserService(userRepository); + userService = new UserService(userRepository, instanceValidator); } @Test @Transactional + @DisplayName("Create new user") void createNewUserTest() { - userService.create(EntityCreator.userRequest()); + userService.create(EntityCreator.userRequest().build()); var entity = EntityCreator.user().build(); expect(entity); } @Test - void createDuplicatedUser() { - var first = EntityCreator.userRequest(); - var second = EntityCreator.userRequest(); + @DisplayName("Try to create user that already exists") + void createDuplicatedUserTest() { + var first = EntityCreator.userRequest().build(); + var second = EntityCreator.userRequest().build(); userService.create(first); assertThatThrownBy(() -> userService.create(second)) - .isInstanceOf(UserAlreadyExistsException.class) - .hasMessage("User with PESEL %s already exists".formatted(EntityCreator.Constants.PESEL)); + .isInstanceOf(UserAlreadyExistsException.class) + .hasMessage("User with PESEL %s already exists".formatted(EntityCreator.Constants.PESEL)); + } + + @Test + @DisplayName("Try to create another user with same PESEL") + void createDuplicatedPeselTest() { + var first = EntityCreator.userRequest().build(); + var second = EntityCreator.userRequest() + .name("Jan") + .surname("Kowalski") + .pln(30.30) + .build(); + userService.create(first); + assertThatThrownBy(() -> userService.create(second)) + .isInstanceOf(UserAlreadyExistsException.class) + .hasMessage("User with PESEL %s already exists".formatted(EntityCreator.Constants.PESEL)); + } + + @Test + @DisplayName("Try to create too young user") + void youngUserTest() { + var request = EntityCreator.userRequest().pesel("08280959342").build(); + assertThatThrownBy(() -> userService.create(request)) + .isInstanceOf(ValidationFailedException.class) + .hasMessageContaining(Adult.MESSAGE); } @Test @Transactional + @DisplayName("Get existing user") void getExistingUserTest() { var entity = EntityCreator.user().build(); userRepository.save(entity); var userOptional = userService.get(EntityCreator.Constants.PESEL); var expected = UserMapper.entityToPojo(entity); assertThat(userOptional) - .isNotEmpty() - .get() - .usingRecursiveComparison() - .isEqualTo(expected); + .isNotEmpty() + .get() + .usingRecursiveComparison() + .isEqualTo(expected); } @Test + @DisplayName("Try get non-existing user") void getNonExistingUserTest() { var userOptional = userService.get(EntityCreator.Constants.PESEL); assertThat(userOptional).isEmpty(); diff --git a/src/test/java/eu/ztsh/wymiana/util/UserMapperTest.java b/src/test/java/eu/ztsh/wymiana/util/UserMapperTest.java index f3fc91d..0479332 100644 --- a/src/test/java/eu/ztsh/wymiana/util/UserMapperTest.java +++ b/src/test/java/eu/ztsh/wymiana/util/UserMapperTest.java @@ -15,23 +15,23 @@ class UserMapperTest { void entityToPojoTest() { var entity = EntityCreator.user().build(); var expected = new User( - EntityCreator.Constants.NAME, - EntityCreator.Constants.SURNAME, - EntityCreator.Constants.PESEL, - Map.of("PLN", new Currency("PLN", EntityCreator.Constants.PLN)) + EntityCreator.Constants.NAME, + EntityCreator.Constants.SURNAME, + EntityCreator.Constants.PESEL, + Map.of("PLN", new Currency("PLN", EntityCreator.Constants.PLN)) ); assertThat(UserMapper.entityToPojo(entity)) - .usingRecursiveComparison() - .isEqualTo(expected); + .usingRecursiveComparison() + .isEqualTo(expected); } @Test void requestToEntityTest() { - var request = EntityCreator.userRequest(); + var request = EntityCreator.userRequest().build(); var expected = EntityCreator.user().build(); assertThat(UserMapper.requestToEntity(request)) - .usingRecursiveComparison() - .isEqualTo(expected); + .usingRecursiveComparison() + .isEqualTo(expected); } }