From d9c5448d3e091e319ca1a868deab9e681ea9c7b5 Mon Sep 17 00:00:00 2001 From: Ivan Holotsvan Date: Wed, 22 Jan 2025 16:29:29 +0200 Subject: [PATCH 1/3] more meaningful error messages in EventDtoRequestValidator --- .../handler/CustomExceptionHandler.java | 15 ---- .../validator/EventDtoRequestValidator.java | 34 +++++--- .../EventDtoRequestValidatorTest.java | 77 +++++++++++++------ 3 files changed, 77 insertions(+), 49 deletions(-) diff --git a/core/src/main/java/greencity/exception/handler/CustomExceptionHandler.java b/core/src/main/java/greencity/exception/handler/CustomExceptionHandler.java index 7a6390f84a..c71db04a99 100644 --- a/core/src/main/java/greencity/exception/handler/CustomExceptionHandler.java +++ b/core/src/main/java/greencity/exception/handler/CustomExceptionHandler.java @@ -479,21 +479,6 @@ public final ResponseEntity handleUnsupportedSortException( return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(exceptionResponse); } - /** - * Customize the response for EventDtoValidationException. - * - * @param ex the exception - * @param request the current request - * @return a {@code ResponseEntity} message - */ - @ExceptionHandler(EventDtoValidationException.class) - public final ResponseEntity handleEventDtoValidationException( - EventDtoValidationException ex, WebRequest request) { - ExceptionResponse exceptionResponse = new ExceptionResponse(getErrorAttributes(request)); - log.warn(ex.getMessage(), ex); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(exceptionResponse); - } - /** * Customize the response for WrongIdException. * diff --git a/core/src/main/java/greencity/validator/EventDtoRequestValidator.java b/core/src/main/java/greencity/validator/EventDtoRequestValidator.java index 8500507481..627a50e19d 100644 --- a/core/src/main/java/greencity/validator/EventDtoRequestValidator.java +++ b/core/src/main/java/greencity/validator/EventDtoRequestValidator.java @@ -39,20 +39,30 @@ public class EventDtoRequestValidator */ @Override public boolean isValid(Object value, ConstraintValidatorContext context) { - if (value instanceof AddEventDtoRequest addEventDtoRequest) { - validateDateLocations(addEventDtoRequest.getDatesLocations()); - convertToUTC(addEventDtoRequest.getDatesLocations()); - validateEventDateLocations(addEventDtoRequest.getDatesLocations()); - validateTags(addEventDtoRequest.getTags()); - } else if (value instanceof UpdateEventRequestDto updateEventDto) { - validateDateLocations(updateEventDto.getDatesLocations()); - convertToUTC(updateEventDto.getDatesLocations()); - validateEventDateLocations(updateEventDto.getDatesLocations()); - validateTags(updateEventDto.getTags()); - } else { + try { + switch (value) { + case AddEventDtoRequest addEventDtoRequest -> { + validateDateLocations(addEventDtoRequest.getDatesLocations()); + convertToUTC(addEventDtoRequest.getDatesLocations()); + validateEventDateLocations(addEventDtoRequest.getDatesLocations()); + validateTags(addEventDtoRequest.getTags()); + } + case UpdateEventRequestDto updateEventDto -> { + validateDateLocations(updateEventDto.getDatesLocations()); + convertToUTC(updateEventDto.getDatesLocations()); + validateEventDateLocations(updateEventDto.getDatesLocations()); + validateTags(updateEventDto.getTags()); + } + default -> { + return false; + } + } + return true; + } catch (EventDtoValidationException ex) { + context.disableDefaultConstraintViolation(); + context.buildConstraintViolationWithTemplate(ex.getMessage()).addConstraintViolation(); return false; } - return true; } private void validateDateLocations(List dates) { diff --git a/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java b/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java index c32ec63c68..fdd34ecce9 100644 --- a/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java +++ b/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java @@ -4,76 +4,103 @@ import greencity.dto.event.AddEventDtoRequest; import greencity.dto.event.UpdateEventDateLocationDto; import greencity.dto.event.UpdateEventRequestDto; -import greencity.exception.exceptions.EventDtoValidationException; import greencity.exception.exceptions.InvalidURLException; import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.util.List; +import jakarta.validation.ConstraintValidatorContext; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.springframework.test.context.junit.jupiter.SpringExtension; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; @ExtendWith(SpringExtension.class) class EventDtoRequestValidatorTest { @InjectMocks private EventDtoRequestValidator validator; + @Mock + private ConstraintValidatorContext constraintValidatorContext; + + @Mock + private ConstraintValidatorContext.ConstraintViolationBuilder violationBuilder; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + when(constraintValidatorContext.buildConstraintViolationWithTemplate(anyString())) + .thenReturn(violationBuilder); + when(violationBuilder.addConstraintViolation()).thenReturn(constraintValidatorContext); + } + @Test void withoutDatesException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventDtoWithoutDates(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDtoRequest, null)); + boolean isValid = validator.isValid(addEventDtoRequest, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for empty date locations"); } @Test void withZeroDatesException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventDtoWithZeroDates(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDtoRequest, null)); + boolean isValid = validator.isValid(addEventDtoRequest, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for zero date locations"); } @Test void withTooManyDatesException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventDtoWithTooManyDates(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDtoRequest, null)); + boolean isValid = validator.isValid(addEventDtoRequest, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for too many date locations"); } @Test void withStartDateInPastException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventWithPastStartDate(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDtoRequest, null)); + boolean isValid = validator.isValid(addEventDtoRequest, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for past start date"); } @Test void withStartDateAfterFinishDateException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventWithStartDateAfterFinishDate(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDtoRequest, null)); + boolean isValid = validator.isValid(addEventDtoRequest, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for start date after finish date"); } @Test void withoutAddressAndLinkException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventWithoutAddressAndLink(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDtoRequest, null)); + boolean isValid = validator.isValid(addEventDtoRequest, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for event without address and link"); } @Test void withInvalidLinkException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventWithInvalidLink(); - assertThrows(InvalidURLException.class, () -> validator.isValid(addEventDtoRequest, null)); + assertThrows(InvalidURLException.class, + () -> validator.isValid(addEventDtoRequest, constraintValidatorContext)); } @Test void withTooManyTagsException() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getEventWithTooManyTags(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDtoRequest, null)); + boolean isValid = validator.isValid(addEventDtoRequest, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for too many tags"); } @Test void validEvent() { AddEventDtoRequest addEventDtoRequest = ModelUtils.getAddEventDtoRequest(); - assertTrue(validator.isValid(addEventDtoRequest, null)); + assertTrue(validator.isValid(addEventDtoRequest, constraintValidatorContext)); } @Test @@ -82,32 +109,36 @@ void saveEventWithSameDates() { ZonedDateTime zonedDateTime = ZonedDateTime.now(ZoneOffset.UTC).plusHours(2L); addEventDto.getDatesLocations().forEach(e -> e.setStartDate(zonedDateTime)); addEventDto.getDatesLocations().forEach(e -> e.setFinishDate(zonedDateTime)); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(addEventDto, null)); + boolean isValid = validator.isValid(addEventDto, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false same event with same dates"); } @Test void updateWithTooManyTagsException() { UpdateEventRequestDto updateEventDto = ModelUtils.getUpdateEventDtoWithTooManyDates(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(updateEventDto, null)); + boolean isValid = validator.isValid(updateEventDto, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for too many tags"); } @Test void updateWithEmptyDateLocations() { UpdateEventRequestDto updateEventDto = ModelUtils.getUpdateEventDtoWithEmptyDateLocations(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(updateEventDto, null)); + boolean isValid = validator.isValid(updateEventDto, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for empty date locations"); } @Test void updateEventDtoWithoutDates() { UpdateEventRequestDto updateEventDto = ModelUtils.getUpdateEventDtoWithoutDates(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(updateEventDto, null)); - + boolean isValid = validator.isValid(updateEventDto, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false event without dates"); } @Test void updateWithInvalidLinkException() { UpdateEventRequestDto updateEventDto = ModelUtils.getUpdateEventWithoutAddressAndLink(); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(updateEventDto, null)); + boolean isValid = validator.isValid(updateEventDto, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for invalid link"); } @Test @@ -115,13 +146,14 @@ void updateWithoutLinkAndCoordinates() { UpdateEventRequestDto updateEventDto = ModelUtils.getUpdateEventDto(); updateEventDto.getDatesLocations().forEach(e -> e.setOnlineLink(null)); updateEventDto.getDatesLocations().forEach(e -> e.setCoordinates(null)); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(updateEventDto, null)); + boolean isValid = validator.isValid(updateEventDto, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for dto without link and coordinates"); } @Test void validEventUpdate() { UpdateEventRequestDto updateEventDto = ModelUtils.getUpdateEventDto(); - assertTrue(validator.isValid(updateEventDto, null)); + assertTrue(validator.isValid(updateEventDto, constraintValidatorContext)); } @Test @@ -130,13 +162,14 @@ void updateEventWithSameDates() { ZonedDateTime zonedDateTime = ZonedDateTime.now(ZoneOffset.UTC).plusHours(2L); updateEventDto.getDatesLocations().forEach(e -> e.setStartDate(zonedDateTime)); updateEventDto.getDatesLocations().forEach(e -> e.setFinishDate(zonedDateTime)); - assertThrows(EventDtoValidationException.class, () -> validator.isValid(updateEventDto, null)); + boolean isValid = validator.isValid(updateEventDto, constraintValidatorContext); + assertFalse(isValid, "Validation should fail and return false for event with same dates"); } @Test void invalidObjectType() { Object value = new Object(); - assertFalse(validator.isValid(value, null)); + assertFalse(validator.isValid(value, constraintValidatorContext)); } @Test @@ -148,7 +181,7 @@ void invalidDates() { .onlineLink("http://localhost:8060/swagger-ui.html#/") .build())) .tags(List.of("first", "second", "third")).build(); - assertThrows(EventDtoValidationException.class, - () -> validator.isValid(updateEventRequestDto, null)); + boolean isValid = validator.isValid(updateEventRequestDto, constraintValidatorContext); + assertFalse(isValid); } } \ No newline at end of file From 34cf905523e45b54262f2517c5ff420e7acdd03a Mon Sep 17 00:00:00 2001 From: Ivan Holotsvan Date: Wed, 22 Jan 2025 16:44:04 +0200 Subject: [PATCH 2/3] small refactoring --- .../validator/EventDtoRequestValidator.java | 37 +++++++++++++------ .../EventDtoRequestValidatorTest.java | 2 +- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/greencity/validator/EventDtoRequestValidator.java b/core/src/main/java/greencity/validator/EventDtoRequestValidator.java index 627a50e19d..af1e5cd36f 100644 --- a/core/src/main/java/greencity/validator/EventDtoRequestValidator.java +++ b/core/src/main/java/greencity/validator/EventDtoRequestValidator.java @@ -41,18 +41,10 @@ public class EventDtoRequestValidator public boolean isValid(Object value, ConstraintValidatorContext context) { try { switch (value) { - case AddEventDtoRequest addEventDtoRequest -> { - validateDateLocations(addEventDtoRequest.getDatesLocations()); - convertToUTC(addEventDtoRequest.getDatesLocations()); - validateEventDateLocations(addEventDtoRequest.getDatesLocations()); - validateTags(addEventDtoRequest.getTags()); - } - case UpdateEventRequestDto updateEventDto -> { - validateDateLocations(updateEventDto.getDatesLocations()); - convertToUTC(updateEventDto.getDatesLocations()); - validateEventDateLocations(updateEventDto.getDatesLocations()); - validateTags(updateEventDto.getTags()); - } + case AddEventDtoRequest addEventDtoRequest -> + validateEventDto(addEventDtoRequest.getDatesLocations(), addEventDtoRequest.getTags()); + case UpdateEventRequestDto updateEventDto -> + validateEventDto(updateEventDto.getDatesLocations(), updateEventDto.getTags()); default -> { return false; } @@ -86,6 +78,12 @@ private void convertToUTC(List dates } private void validateEventDateLocations(List eventDateLocationDtos) { + validateUniqueDates(eventDateLocationDtos); + validateDateRanges(eventDateLocationDtos); + validateLocationDetails(eventDateLocationDtos); + } + + private void validateUniqueDates(List eventDateLocationDtos) { Set startDateSet = new HashSet<>(); Set finishDateSet = new HashSet<>(); @@ -96,14 +94,22 @@ private void validateEventDateLocations if (!startDateSet.add(startDate) || !finishDateSet.add(finishDate)) { throw new EventDtoValidationException(ErrorMessage.SAME_EVENT_DATES); } + } + } + private void validateDateRanges(List eventDateLocationDtos) { + for (T eventDateLocationDto : eventDateLocationDtos) { if (eventDateLocationDto.getStartDate().isBefore(ZonedDateTime.now(ZoneOffset.UTC)) || eventDateLocationDto.getStartDate().isAfter(eventDateLocationDto.getFinishDate()) || eventDateLocationDto.getStartDate().isAfter(ZonedDateTime.now(ZoneOffset.UTC) .plusYears(MAX_YEARS_OF_PLANNING))) { throw new EventDtoValidationException(ErrorMessage.EVENT_START_DATE_AFTER_FINISH_DATE_OR_IN_PAST); } + } + } + private void validateLocationDetails(List eventDateLocationDtos) { + for (T eventDateLocationDto : eventDateLocationDtos) { if (eventDateLocationDto.getOnlineLink() == null && eventDateLocationDto.getCoordinates() == null) { throw new EventDtoValidationException(ErrorMessage.NO_EVENT_LINK_OR_ADDRESS); } @@ -120,4 +126,11 @@ private void validateTags(List tags) { throw new EventDtoValidationException(ErrorMessage.WRONG_COUNT_OF_TAGS_EXCEPTION); } } + + private void validateEventDto(List datesLocations, List tags) { + validateDateLocations(datesLocations); + convertToUTC(datesLocations); + validateEventDateLocations(datesLocations); + validateTags(tags); + } } \ No newline at end of file diff --git a/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java b/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java index fdd34ecce9..c3035b88d2 100644 --- a/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java +++ b/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java @@ -182,6 +182,6 @@ void invalidDates() { .build())) .tags(List.of("first", "second", "third")).build(); boolean isValid = validator.isValid(updateEventRequestDto, constraintValidatorContext); - assertFalse(isValid); + assertFalse(isValid, "Validation should fail for null dates in UpdateEventRequestDto"); } } \ No newline at end of file From e6444439780ec19bef1d4adca39cee83aeba802b Mon Sep 17 00:00:00 2001 From: Ivan Holotsvan Date: Thu, 23 Jan 2025 10:54:23 +0200 Subject: [PATCH 3/3] remove opening of mock --- .../java/greencity/validator/EventDtoRequestValidatorTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java b/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java index c3035b88d2..47fcfed12b 100644 --- a/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java +++ b/core/src/test/java/greencity/validator/EventDtoRequestValidatorTest.java @@ -14,7 +14,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; -import org.mockito.MockitoAnnotations; import org.springframework.test.context.junit.jupiter.SpringExtension; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -35,7 +34,6 @@ class EventDtoRequestValidatorTest { @BeforeEach void setUp() { - MockitoAnnotations.openMocks(this); when(constraintValidatorContext.buildConstraintViolationWithTemplate(anyString())) .thenReturn(violationBuilder); when(violationBuilder.addConstraintViolation()).thenReturn(constraintValidatorContext);