From 6e9cc7fbbe2f77e71b2197a11ec4f50e952e7088 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Fri, 24 Feb 2023 08:07:30 -0600 Subject: [PATCH] MODCAL-124: Fix issue with legacy exceptions not associating with calendars (#159) --- .../domain/mapper/ExceptionHourMapper.java | 5 + .../domain/mapper/ExceptionRangeMapper.java | 3 + .../domain/mapper/NormalOpeningMapper.java | 5 + .../calendar/service/CustomTenantService.java | 32 ++-- .../org/folio/calendar/utils/DateUtils.java | 4 +- .../org/folio/calendar/utils/PeriodUtils.java | 176 +++++++++++++----- .../migration/NormalMigrationTest.java | 161 ++++++++++++++-- .../calendar/testconstants/Calendars.java | 18 +- .../testconstants/ExceptionRanges.java | 8 + .../folio/calendar/testconstants/Periods.java | 7 + .../PeriodUtilsCalendarConversionTest.java | 15 +- ...iodUtilsConflictedOrphanExceptionTest.java | 32 ++++ ...tilsExceptionalCalendarConversionTest.java | 21 ++- .../utils/PeriodUtilsExceptionalTest.java | 168 +++-------------- .../utils/PeriodUtilsNullConversionTest.java | 18 -- src/test/resources/database-migrate-data.sql | 16 +- 16 files changed, 428 insertions(+), 261 deletions(-) create mode 100644 src/test/java/org/folio/calendar/unit/utils/PeriodUtilsConflictedOrphanExceptionTest.java diff --git a/src/main/java/org/folio/calendar/domain/mapper/ExceptionHourMapper.java b/src/main/java/org/folio/calendar/domain/mapper/ExceptionHourMapper.java index f283c995..2282c1cb 100644 --- a/src/main/java/org/folio/calendar/domain/mapper/ExceptionHourMapper.java +++ b/src/main/java/org/folio/calendar/domain/mapper/ExceptionHourMapper.java @@ -3,12 +3,17 @@ import org.folio.calendar.domain.dto.ExceptionalOpeningDTO; import org.folio.calendar.domain.entity.ExceptionHour; import org.mapstruct.Mapper; +import org.mapstruct.Mapping; import org.mapstruct.factory.Mappers; @Mapper(componentModel = "spring") public interface ExceptionHourMapper { ExceptionHourMapper INSTANCE = Mappers.getMapper(ExceptionHourMapper.class); + @Mapping(target = "exceptionId", ignore = true) ExceptionalOpeningDTO toDto(ExceptionHour source); + + @Mapping(target = "id", ignore = true) + @Mapping(target = "exception", ignore = true) ExceptionHour fromDto(ExceptionalOpeningDTO source); } diff --git a/src/main/java/org/folio/calendar/domain/mapper/ExceptionRangeMapper.java b/src/main/java/org/folio/calendar/domain/mapper/ExceptionRangeMapper.java index d8cde39d..0ecdec4a 100644 --- a/src/main/java/org/folio/calendar/domain/mapper/ExceptionRangeMapper.java +++ b/src/main/java/org/folio/calendar/domain/mapper/ExceptionRangeMapper.java @@ -8,6 +8,7 @@ import org.folio.calendar.domain.entity.ExceptionRange; import org.folio.calendar.domain.entity.ExceptionRange.ExceptionRangeBuilder; import org.mapstruct.Mapper; +import org.mapstruct.Mapping; import org.mapstruct.factory.Mappers; // This class directly relates to ExceptionRange and, due to MapStruct's automatic implementation @@ -17,6 +18,8 @@ public interface ExceptionRangeMapper { ExceptionRangeMapper INSTANCE = Mappers.getMapper(ExceptionRangeMapper.class); + @Mapping(target = "calendarId", ignore = true) + @Mapping(target = "opening", ignore = true) ExceptionRangeDTO toDto(ExceptionRange source); default ExceptionRange fromDto(ExceptionRangeDTO source) { diff --git a/src/main/java/org/folio/calendar/domain/mapper/NormalOpeningMapper.java b/src/main/java/org/folio/calendar/domain/mapper/NormalOpeningMapper.java index e0160b77..b6eb4e19 100644 --- a/src/main/java/org/folio/calendar/domain/mapper/NormalOpeningMapper.java +++ b/src/main/java/org/folio/calendar/domain/mapper/NormalOpeningMapper.java @@ -3,12 +3,17 @@ import org.folio.calendar.domain.dto.NormalHoursDTO; import org.folio.calendar.domain.entity.NormalOpening; import org.mapstruct.Mapper; +import org.mapstruct.Mapping; import org.mapstruct.factory.Mappers; @Mapper(componentModel = "spring") public interface NormalOpeningMapper { NormalOpeningMapper INSTANCE = Mappers.getMapper(NormalOpeningMapper.class); + @Mapping(target = "calendarId", ignore = true) NormalHoursDTO toDto(NormalOpening source); + + @Mapping(target = "id", ignore = true) + @Mapping(target = "calendar", ignore = true) NormalOpening fromDto(NormalHoursDTO source); } diff --git a/src/main/java/org/folio/calendar/service/CustomTenantService.java b/src/main/java/org/folio/calendar/service/CustomTenantService.java index 9fcaf37b..51fa62e4 100644 --- a/src/main/java/org/folio/calendar/service/CustomTenantService.java +++ b/src/main/java/org/folio/calendar/service/CustomTenantService.java @@ -39,7 +39,7 @@ public class CustomTenantService extends TenantService { protected final CalendarValidationService calendarValidationService; protected final TranslationService translationService; - protected List periodsToMigrate; + protected List calendarsToMigrate; /** * Constructor for a new CustomTenantService. Directly wraps the original {@link TenantService TenantService} constructor. @@ -61,7 +61,7 @@ public CustomTenantService( this.calendarService = calendarService; this.calendarValidationService = calendarValidationService; this.translationService = translationService; - this.periodsToMigrate = new ArrayList<>(); + this.calendarsToMigrate = new ArrayList<>(); } /** @@ -89,12 +89,21 @@ protected void beforeLiquibaseUpdate(TenantAttributes attributes) { .configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false) .setSerializationInclusion(Include.NON_NULL); - this.periodsToMigrate = - jdbcTemplate.query(GET_RMB_OPENINGS, new RMBOpeningMapper(jdbcTemplate, mapper)); - this.periodsToMigrate.removeAll(Collections.singletonList(null)); - - log.info(String.format("Found %d periods to migrate", this.periodsToMigrate.size())); - log.debug(this.periodsToMigrate); + List periodsToMigrate = jdbcTemplate.query( + GET_RMB_OPENINGS, + new RMBOpeningMapper(jdbcTemplate, mapper) + ); + periodsToMigrate.removeAll(Collections.singletonList(null)); + + log.info("Found {} periods to migrate", periodsToMigrate.size()); + log.debug(periodsToMigrate); + + this.calendarsToMigrate = PeriodUtils.toCalendars(periodsToMigrate); + log.info( + "Converted {} periods into {} calendars", + periodsToMigrate.size(), + calendarsToMigrate.size() + ); } } @@ -103,14 +112,13 @@ protected void beforeLiquibaseUpdate(TenantAttributes attributes) { */ @Override protected void afterLiquibaseUpdate(TenantAttributes attributes) { - for (PeriodDTO period : this.periodsToMigrate) { + for (Calendar calendar : calendarsToMigrate) { try { - log.info(String.format("Attempting to save calendar with ID %s", period.getId())); - Calendar calendar = PeriodUtils.toCalendar(period); + log.info("Attempting to save converted calendar {}", calendar); this.calendarValidationService.validate(calendar); this.calendarService.saveCalendar(calendar); } catch (RuntimeException e) { - log.error(String.format("Could not save calendar with ID %s", period.getId())); + log.error("Could not save calendar {}", calendar); log.error(e); } } diff --git a/src/main/java/org/folio/calendar/utils/DateUtils.java b/src/main/java/org/folio/calendar/utils/DateUtils.java index 13cd6556..a18fd370 100644 --- a/src/main/java/org/folio/calendar/utils/DateUtils.java +++ b/src/main/java/org/folio/calendar/utils/DateUtils.java @@ -71,8 +71,8 @@ public static boolean contains(ChronoLocalDate date, ChronoLocalDate start, Chro * Check if a date range is contained within another (inclusive) range * @param testStart the first date of the range to test, inclusive * @param testEnd the last date of the range to test, inclusive - * @param start the first date of the range, inclusive - * @param end the last date of the range, inclusive + * @param start the first date of the range that should include test, inclusive + * @param end the last date of the range that should include test, inclusive * @return if the test range is contained within the other range */ public static boolean containsRange( diff --git a/src/main/java/org/folio/calendar/utils/PeriodUtils.java b/src/main/java/org/folio/calendar/utils/PeriodUtils.java index 25901e2c..2fe508d8 100644 --- a/src/main/java/org/folio/calendar/utils/PeriodUtils.java +++ b/src/main/java/org/folio/calendar/utils/PeriodUtils.java @@ -2,10 +2,14 @@ import java.time.LocalDate; import java.util.ArrayList; -import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Optional; +import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import lombok.experimental.UtilityClass; +import lombok.extern.log4j.Log4j2; import org.folio.calendar.domain.entity.Calendar; import org.folio.calendar.domain.entity.Calendar.CalendarBuilder; import org.folio.calendar.domain.entity.ExceptionHour; @@ -25,45 +29,38 @@ * Primarily kept for legacy reasons to facilitate the conversion of old RMB Period * objects to the new Calendar format */ +@Log4j2 @UtilityClass public class PeriodUtils { /** - * Determine if a a list of OpeningDayRelativeDTO is intended for an exception or a calendar (distinct in legacy, although both use Periods) + * Determine if a Period is intended for an exception or a calendar (distinct in legacy, but both use Periods) * - * @param openings a list of {@link org.folio.calendar.domain.dto.OpeningDayRelativeDTO} objects - * @return if the list refers to an exception or normal opening + * @param period the period to test + * @return if the period refers to an exception or normal opening */ - public static boolean areOpeningsExceptional(Iterable openings) { - for (OpeningDayRelativeDTO opening : openings) { - if (opening.getWeekdays() == null) { - return true; - } - } - return false; + public static boolean isExceptional(PeriodDTO period) { + return period.getOpeningDays().stream().anyMatch(opening -> opening.getWeekdays() == null); } /** - * Convert period openings to exceptions. Due to the simple nature of legacy exceptions, + * Convert exception period openings to exceptions. Due to the simple nature of legacy exceptions, * this will result in exactly one {@link org.folio.calendar.domain.entity.ExceptionRange}. * - * @param startDate the first day of the exception - * @param endDate the last day of the exception - * @param openings a list of {@link org.folio.calendar.domain.dto.OpeningDayRelativeDTO} objects - * @return a {@link java.util.List} of the corresponding {@code ExceptionRange} + * @param period the period to convert + * @return the corresponding {@code ExceptionRange} */ - public static List convertOpeningDayRelativeDTOToExceptionRanges( - LocalDate startDate, - LocalDate endDate, - List openings - ) { + public static ExceptionRange convertExceptionalPeriodToExceptionRanges(PeriodDTO period) { UUID exceptionId = UUID.randomUUID(); ExceptionRangeBuilder builder = ExceptionRange .builder() .id(exceptionId) - .startDate(startDate) - .endDate(endDate); + .name(period.getName()) + .startDate(period.getStartDate().getValue()) + .endDate(period.getEndDate().getValue()); + + List openings = period.getOpeningDays(); if (openings.size() != 1) { throw new IllegalArgumentException( @@ -91,7 +88,7 @@ public static List convertOpeningDayRelativeDTOToExceptionRanges if (!Boolean.TRUE.equals(opening.isOpen())) { // no time information implies closure // therefore, we want no openings - return Arrays.asList(builder.build()); + return builder.build(); } // open all day @@ -100,14 +97,17 @@ public static List convertOpeningDayRelativeDTOToExceptionRanges builder.opening( ExceptionHour .builder() - .startDate(startDate) + .startDate(period.getStartDate().getValue()) .startTime(TimeConstants.TIME_MIN) - .endDate(endDate) + .endDate(period.getEndDate().getValue()) .endTime(TimeConstants.TIME_MAX) .build() ); } else { - for (LocalDate date : DateUtils.getDateRange(startDate, endDate)) { + for (LocalDate date : DateUtils.getDateRange( + period.getStartDate().getValue(), + period.getEndDate().getValue() + )) { builder = builder.opening( ExceptionHour @@ -121,7 +121,7 @@ public static List convertOpeningDayRelativeDTOToExceptionRanges } } - return Arrays.asList(builder.build()); + return builder.build(); } /** @@ -223,7 +223,7 @@ private static List consolidateNormalOpenings(List return normalOpenings; } - public static Calendar toCalendar(PeriodDTO period) { + protected static Calendar convertOpeningPeriodToCalendar(PeriodDTO period) { // basic info CalendarBuilder calendarBuilder = Calendar .builder() @@ -239,20 +239,112 @@ public static Calendar toCalendar(PeriodDTO period) { calendarBuilder = calendarBuilder.servicePoint(servicePointAssignment); // create hours - if (areOpeningsExceptional(period.getOpeningDays())) { - calendarBuilder.exceptions( - convertOpeningDayRelativeDTOToExceptionRanges( - period.getStartDate().getValue(), - period.getEndDate().getValue(), - period.getOpeningDays() - ) - ); - } else { - calendarBuilder.normalHours( - convertOpeningDayRelativeDTOToNormalOpening(period.getOpeningDays()) - ); - } + calendarBuilder.normalHours( + convertOpeningDayRelativeDTOToNormalOpening(period.getOpeningDays()) + ); return calendarBuilder.build(); } + + public static List toCalendars(List periods) { + log.debug("Converting periods: {}", periods); + + List calendars = periods + .stream() + .filter(period -> !isExceptional(period)) + .map(PeriodUtils::convertOpeningPeriodToCalendar) + .collect(Collectors.toCollection(ArrayList::new)); + List exceptions = periods.stream().filter(PeriodUtils::isExceptional).toList(); + + log.info("Found {} calendars and {} exceptions", calendars.size(), exceptions.size()); + log.debug("Calendars: {}", calendars); + log.debug("Exceptions: {}", exceptions); + + for (PeriodDTO exceptionPeriod : exceptions) { + ExceptionRange exception = convertExceptionalPeriodToExceptionRanges(exceptionPeriod); + log.info( + "Searching for a calendar to host exception {} on service point {}", + exception, + exceptionPeriod.getServicePointId() + ); + + Optional parentCalendar = calendars + .stream() + // ensure we're only looking at calendars for this SP + .filter((Calendar cal) -> + cal + .getServicePoints() + .stream() + .anyMatch(spa -> spa.getServicePointId().equals(exceptionPeriod.getServicePointId())) + ) + // calendar must be within the date range of this exception + .filter((Calendar cal) -> + DateUtils.containsRange( + exception.getStartDate(), + exception.getEndDate(), + cal.getStartDate(), + cal.getEndDate() + ) + ) + // calendar must not already have an exception with this date range + .filter((Calendar cal) -> { + if ( + cal + .getExceptions() + .stream() + .anyMatch(otherException -> + DateUtils.overlaps( + exception.getStartDate(), + exception.getEndDate(), + otherException.getStartDate(), + otherException.getEndDate() + ) + ) + ) { + // theoretically, this indicates an unrecoverable issue, since + // there should only be able to be one exception per date per SP + log.info( + "Calendar {} seems like where this exception belongs, but another exception conflicted...", + cal + ); + return false; + } + return true; + }) + .findAny(); + + if (parentCalendar.isPresent()) { + Set newExceptionSet = new HashSet<>(parentCalendar.get().getExceptions()); + newExceptionSet.add(exception); + + parentCalendar.get().setExceptions(newExceptionSet); + + log.info("Exception stored in calendar {}", parentCalendar.get().getName()); + } else { + log.error( + "Could not find a parent for exception {}; creating orphaned exception", + exceptionPeriod + ); + // this goes at the back of the list so that, in the event + // an orphaned exception cannot be added, it will be excluded + // rather than the original calendar + calendars.add( + Calendar + .builder() + .name(String.format("Orphaned exception (%s)", exception.getName())) + .servicePoint( + ServicePointCalendarAssignment + .builder() + .servicePointId(exceptionPeriod.getServicePointId()) + .build() + ) + .startDate(exception.getStartDate()) + .endDate(exception.getEndDate()) + .exception(exception) + .build() + ); + } + } + return calendars; + } } diff --git a/src/test/java/org/folio/calendar/integration/migration/NormalMigrationTest.java b/src/test/java/org/folio/calendar/integration/migration/NormalMigrationTest.java index 3195fa38..11a650c1 100644 --- a/src/test/java/org/folio/calendar/integration/migration/NormalMigrationTest.java +++ b/src/test/java/org/folio/calendar/integration/migration/NormalMigrationTest.java @@ -2,14 +2,18 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import java.util.List; import org.folio.calendar.domain.dto.CalendarCollectionDTO; -import org.folio.calendar.domain.dto.CalendarDTO; +import org.folio.calendar.domain.entity.Calendar; import org.folio.calendar.domain.mapper.CalendarMapper; import org.folio.calendar.integration.api.calendar.BaseCalendarApiTest; -import org.folio.calendar.testconstants.Calendars; +import org.folio.calendar.testconstants.Dates; +import org.folio.calendar.testconstants.ExceptionRanges; +import org.folio.calendar.testconstants.Names; +import org.folio.calendar.testconstants.NormalOpenings; import org.folio.calendar.testconstants.UUIDs; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -34,24 +38,145 @@ void testGetCalendars() { .as(CalendarCollectionDTO.class); // fix comparison nuances for valid calendar - collection.getCalendars().forEach(c -> c.setId(UUIDs.UUID_A)); - assertThat( - "The regular period was properly converted", - collection.getCalendars().stream().map(CalendarDTO::getNormalHours).toList(), - hasItem( - containsInAnyOrder( - calendarMapper.toDto(Calendars.CALENDAR_FULL_EXAMPLE_F).getNormalHours().toArray() - ) + List calendars = collection + .getCalendars() + .stream() + .map(calendarMapper::fromDto) + .toList(); + calendars.forEach(Calendar::clearIds); + + assertThat("Five periods were consolidated into three calendars", calendars, hasSize(3)); + + Calendar withExceptions = calendars + .stream() + .filter(cal -> cal.getName().equals(Names.NAME_1)) + .findFirst() + .get(); + Calendar withoutExceptions = calendars + .stream() + .filter(cal -> cal.getName().equals(Names.NAME_4)) + .findFirst() + .get(); + Calendar orphaned = calendars + .stream() + .filter(cal -> cal.getName().equals("Orphaned exception (replay quake aloft routine)")) + .findFirst() + .get(); + + checkCalendarWithExceptions(withExceptions); + checkCalendarWithoutExceptions(withoutExceptions); + checkCalendarOrphaned(orphaned); + } + + void checkCalendarWithExceptions(Calendar calendar) { + assertThat( + "Calendar with exceptions has expected service points", + calendar.getServicePoints(), + hasSize(1) + ); + assertThat( + "Calendar with exceptions has expected service points", + calendar.getServicePoints().stream().toList().get(0).getServicePointId(), + is(UUIDs.UUID_0) + ); + assertThat( + "Calendar with exceptions has expected start date", + calendar.getStartDate(), + is(Dates.DATE_2021_01_01) + ); + assertThat( + "Calendar with exceptions has expected end date", + calendar.getEndDate(), + is(Dates.DATE_2021_12_31) + ); + assertThat( + "Calendar with exceptions has expected normal openings", + calendar.getNormalHours(), + containsInAnyOrder( + NormalOpenings.MONDAY_04_00_TO_14_59, + NormalOpenings.TUESDAY_ALL_DAY, + NormalOpenings.WEDNESDAY_23_00_TO_SUNDAY_23_59 + ) + ); + assertThat( + "Calendar with exceptions has expected exceptions", + calendar.getExceptions(), + containsInAnyOrder( + ExceptionRanges.OPEN_ALL_DAY_MAR_16_TO_APR_30.withName(Names.NAME_2), + ExceptionRanges.CLOSED_2021_07_04_TO_2021_09_22.withName(Names.NAME_3) ) ); + } + + void checkCalendarWithoutExceptions(Calendar calendar) { + assertThat( + "Calendar without exceptions has expected service points", + calendar.getServicePoints(), + hasSize(1) + ); + assertThat( + "Calendar without exceptions has expected service points", + calendar.getServicePoints().stream().toList().get(0).getServicePointId(), + is(UUIDs.UUID_1) + ); assertThat( - "The exceptional period was properly converted", - collection - .getCalendars() - .stream() - .filter(c -> !c.getExceptions().isEmpty()) - .anyMatch(c -> c.getExceptions().get(0).getOpenings().size() == 4), - is(true) + "Calendar without exceptions has expected start date", + calendar.getStartDate(), + is(Dates.DATE_2021_05_01) + ); + assertThat( + "Calendar without exceptions has expected end date", + calendar.getEndDate(), + is(Dates.DATE_2021_09_22) + ); + assertThat( + "Calendar without exceptions has expected normal openings", + calendar.getNormalHours(), + containsInAnyOrder( + NormalOpenings.MONDAY_23_00_TO_23_59, + NormalOpenings.MONDAY_00_00_TO_12_30, + NormalOpenings.THURSDAY_ALL_DAY + ) + ); + assertThat( + "Calendar without exceptions has no exceptions", + calendar.getExceptions(), + hasSize(0) + ); + } + + void checkCalendarOrphaned(Calendar calendar) { + assertThat( + "Calendar with orphaned exception has expected service points", + calendar.getServicePoints(), + hasSize(1) + ); + assertThat( + "Calendar with orphaned exception has expected service points", + calendar.getServicePoints().stream().toList().get(0).getServicePointId(), + is(UUIDs.UUID_5) + ); + assertThat( + "Calendar with orphaned exception has expected start date", + calendar.getStartDate(), + is(Dates.DATE_2021_01_01) + ); + assertThat( + "Calendar with orphaned exception has expected end date", + calendar.getEndDate(), + is(Dates.DATE_2021_01_04) + ); + assertThat( + "Calendar with orphaned exception has no normal openings", + calendar.getNormalHours(), + hasSize(0) + ); + assertThat( + "Calendar with orphaned exception has expected exceptions", + calendar.getExceptions(), + containsInAnyOrder( + ExceptionRanges.OPEN_04_00_TO_14_59_JAN_1_THRU_JAN_4.withName(Names.NAME_3) + ) ); } } diff --git a/src/test/java/org/folio/calendar/testconstants/Calendars.java b/src/test/java/org/folio/calendar/testconstants/Calendars.java index b1155ded..70f15054 100644 --- a/src/test/java/org/folio/calendar/testconstants/Calendars.java +++ b/src/test/java/org/folio/calendar/testconstants/Calendars.java @@ -186,30 +186,32 @@ public class Calendars { public static final Calendar CALENDAR_FULL_EXCEPTIONAL_A = CALENDAR_2021_01_01_TO_2021_12_31 .withId(UUIDs.UUID_A) .withServicePoints(Set.of(ServicePointCalendarAssignments.ASSIGNMENT_SP_0)) - .withName(Names.NAME_1) + .withName("Orphaned exception (" + Names.NAME_1 + ")") .withNormalHours(Set.of()) - .withExceptions(Set.of(ExceptionRanges.CLOSED_2021_01_01_TO_2021_12_31)); + .withExceptions(Set.of(ExceptionRanges.CLOSED_2021_01_01_TO_2021_12_31.withName(Names.NAME_1))); public static final Calendar CALENDAR_FULL_EXCEPTIONAL_B = CALENDAR_2021_01_01_TO_2021_01_04 .withId(UUIDs.UUID_B) .withServicePoints(Set.of(ServicePointCalendarAssignments.ASSIGNMENT_SP_0)) - .withName(Names.NAME_2) + .withName("Orphaned exception (" + Names.NAME_2 + ")") .withNormalHours(Set.of()) - .withExceptions(Set.of(ExceptionRanges.OPEN_ALL_DAY_JAN_1_THRU_JAN_4)); + .withExceptions(Set.of(ExceptionRanges.OPEN_ALL_DAY_JAN_1_THRU_JAN_4.withName(Names.NAME_2))); public static final Calendar CALENDAR_FULL_EXCEPTIONAL_C = CALENDAR_2021_01_01_TO_2021_01_04 .withId(UUIDs.UUID_C) .withServicePoints(Set.of(ServicePointCalendarAssignments.ASSIGNMENT_SP_5)) - .withName(Names.NAME_3) + .withName("Orphaned exception (" + Names.NAME_3 + ")") .withNormalHours(Set.of()) - .withExceptions(Set.of(ExceptionRanges.OPEN_04_00_TO_14_59_JAN_1_THRU_JAN_4)); + .withExceptions( + Set.of(ExceptionRanges.OPEN_04_00_TO_14_59_JAN_1_THRU_JAN_4.withName(Names.NAME_3)) + ); public static final Calendar CALENDAR_FULL_EXCEPTIONAL_D = CALENDAR_2021_01_01_TO_2021_01_01 .withId(UUIDs.UUID_D) .withServicePoints(Set.of(ServicePointCalendarAssignments.ASSIGNMENT_SP_9)) - .withName(Names.NAME_4) + .withName("Orphaned exception (" + Names.NAME_4 + ")") .withNormalHours(Set.of()) - .withExceptions(Set.of(ExceptionRanges.OPEN_00_00_TO_14_59_JAN_1)); + .withExceptions(Set.of(ExceptionRanges.OPEN_00_00_TO_14_59_JAN_1.withName(Names.NAME_4))); public static final Calendar CALENDAR_COMBINED_EXAMPLE_A = CALENDAR_2021_01_01_TO_2021_04_30 .withServicePoints( diff --git a/src/test/java/org/folio/calendar/testconstants/ExceptionRanges.java b/src/test/java/org/folio/calendar/testconstants/ExceptionRanges.java index 639a1ba0..1776e12e 100644 --- a/src/test/java/org/folio/calendar/testconstants/ExceptionRanges.java +++ b/src/test/java/org/folio/calendar/testconstants/ExceptionRanges.java @@ -59,6 +59,14 @@ public class ExceptionRanges { .openings(Arrays.asList()) .build(); + public static final ExceptionRange CLOSED_2021_07_04_TO_2021_09_22 = ExceptionRange + .builder() + .name(Names.NAME_1) + .startDate(Dates.DATE_2021_07_04) + .endDate(Dates.DATE_2021_09_22) + .openings(Arrays.asList()) + .build(); + public static final ExceptionRange OPEN_ALL_DAY_JAN_1_THRU_JAN_4 = ExceptionRange .builder() .startDate(Dates.DATE_2021_01_01) diff --git a/src/test/java/org/folio/calendar/testconstants/Periods.java b/src/test/java/org/folio/calendar/testconstants/Periods.java index 9bba4b02..a4f84220 100644 --- a/src/test/java/org/folio/calendar/testconstants/Periods.java +++ b/src/test/java/org/folio/calendar/testconstants/Periods.java @@ -210,4 +210,11 @@ public class Periods { .withServicePointId(UUIDs.UUID_0) .withName(Names.NAME_3) .withOpeningDays(Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_CLOSED)); + + public static final PeriodDTO PERIOD_EXCEPTIONAL_INVALID_NULL_OPENING = Periods.PERIOD_FULL_EXCEPTIONAL_G.withOpeningDays( + Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_INVALID_NULL_OPENING) + ); + public static final PeriodDTO PERIOD_EXCEPTIONAL_INVALID_MULTIPLE_OPENINGS = Periods.PERIOD_FULL_EXCEPTIONAL_G.withOpeningDays( + Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_INVALID_MULTIPLE_OPENINGS) + ); } diff --git a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsCalendarConversionTest.java b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsCalendarConversionTest.java index a54617f6..c87fe2d0 100644 --- a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsCalendarConversionTest.java +++ b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsCalendarConversionTest.java @@ -3,6 +3,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import java.util.Arrays; import org.folio.calendar.domain.entity.Calendar; import org.folio.calendar.testconstants.Calendars; import org.folio.calendar.testconstants.Periods; @@ -14,7 +15,10 @@ class PeriodUtilsCalendarConversionTest { @Test void testFullCalendarConversionA() { - Calendar calendar = PeriodUtils.toCalendar(Periods.PERIOD_FULL_EXAMPLE_A).withId(UUIDs.UUID_A); + Calendar calendar = PeriodUtils + .toCalendars(Arrays.asList(Periods.PERIOD_FULL_EXAMPLE_A)) + .get(0) + .withId(UUIDs.UUID_A); calendar.clearIds(); assertThat( "A converted period with openings across many days is represented equivalently as a calendar", @@ -25,7 +29,10 @@ void testFullCalendarConversionA() { @Test void testFullCalendarConversionB() { - Calendar calendar = PeriodUtils.toCalendar(Periods.PERIOD_FULL_EXAMPLE_B).withId(UUIDs.UUID_B); + Calendar calendar = PeriodUtils + .toCalendars(Arrays.asList(Periods.PERIOD_FULL_EXAMPLE_B)) + .get(0) + .withId(UUIDs.UUID_B); calendar.clearIds(); assertThat( "A converted period with openings on few days is represented equivalently as a calendar", @@ -36,7 +43,9 @@ void testFullCalendarConversionB() { @Test void testEmptyCalendarConversion() { - Calendar calendar = PeriodUtils.toCalendar(Periods.PERIOD_WITH_NO_OPENINGS_NOR_EXCEPTIONS); + Calendar calendar = PeriodUtils + .toCalendars(Arrays.asList(Periods.PERIOD_WITH_NO_OPENINGS_NOR_EXCEPTIONS)) + .get(0); calendar.clearIds(); assertThat( "A converted period with no openings nor exceptions represented equivalently as a calendar", diff --git a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsConflictedOrphanExceptionTest.java b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsConflictedOrphanExceptionTest.java new file mode 100644 index 00000000..c11b5547 --- /dev/null +++ b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsConflictedOrphanExceptionTest.java @@ -0,0 +1,32 @@ +package org.folio.calendar.unit.utils; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; + +import java.util.Arrays; +import java.util.List; +import org.folio.calendar.domain.entity.Calendar; +import org.folio.calendar.testconstants.Calendars; +import org.folio.calendar.testconstants.Periods; +import org.folio.calendar.utils.PeriodUtils; +import org.junit.jupiter.api.Test; + +class PeriodUtilsConflictedOrphanExceptionTest { + + @Test + void testDoubleOrphan() { + List calendars = PeriodUtils.toCalendars( + Arrays.asList(Periods.PERIOD_FULL_EXCEPTIONAL_A, Periods.PERIOD_FULL_EXCEPTIONAL_A) + ); + calendars.forEach(Calendar::clearIds); + + assertThat( + "Two calendars are returned, one for each found", + calendars, + contains( + Calendars.CALENDAR_FULL_EXCEPTIONAL_A.withId(null), + Calendars.CALENDAR_FULL_EXCEPTIONAL_A.withId(null) + ) + ); + } +} diff --git a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalCalendarConversionTest.java b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalCalendarConversionTest.java index bfc6ed22..1c4f2d2e 100644 --- a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalCalendarConversionTest.java +++ b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalCalendarConversionTest.java @@ -3,9 +3,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import java.util.Arrays; import org.folio.calendar.domain.entity.Calendar; import org.folio.calendar.testconstants.Calendars; -import org.folio.calendar.testconstants.Names; import org.folio.calendar.testconstants.Periods; import org.folio.calendar.testconstants.UUIDs; import org.folio.calendar.utils.PeriodUtils; @@ -15,10 +15,11 @@ class PeriodUtilsExceptionalCalendarConversionTest { @Test void testFullExceptionalCalendarConversionA() { - Calendar calendar = PeriodUtils.toCalendar(Periods.PERIOD_FULL_EXCEPTIONAL_A); + Calendar calendar = PeriodUtils + .toCalendars(Arrays.asList(Periods.PERIOD_FULL_EXCEPTIONAL_A)) + .get(0); calendar.clearIds(); - // legacy exceptions have no names - calendar.getExceptions().forEach(e -> e.setName(Names.NAME_1)); + assertThat( "A converted closure exceptional period is represented equivalently as a calendar", calendar.withId(UUIDs.UUID_A), @@ -28,7 +29,9 @@ void testFullExceptionalCalendarConversionA() { @Test void testFullExceptionalCalendarConversionB() { - Calendar calendar = PeriodUtils.toCalendar(Periods.PERIOD_FULL_EXCEPTIONAL_B); + Calendar calendar = PeriodUtils + .toCalendars(Arrays.asList(Periods.PERIOD_FULL_EXCEPTIONAL_B)) + .get(0); calendar.clearIds(); assertThat( "A converted all-day opening exceptional period is represented equivalently as a calendar", @@ -39,7 +42,9 @@ void testFullExceptionalCalendarConversionB() { @Test void testFullExceptionalCalendarConversionC() { - Calendar calendar = PeriodUtils.toCalendar(Periods.PERIOD_FULL_EXCEPTIONAL_C); + Calendar calendar = PeriodUtils + .toCalendars(Arrays.asList(Periods.PERIOD_FULL_EXCEPTIONAL_C)) + .get(0); calendar.clearIds(); assertThat( "A converted partial-day opening exceptional period is represented equivalently as a calendar", @@ -50,7 +55,9 @@ void testFullExceptionalCalendarConversionC() { @Test void testFullExceptionalCalendarConversionD() { - Calendar calendar = PeriodUtils.toCalendar(Periods.PERIOD_FULL_EXCEPTIONAL_D); + Calendar calendar = PeriodUtils + .toCalendars(Arrays.asList(Periods.PERIOD_FULL_EXCEPTIONAL_D)) + .get(0); calendar.clearIds(); assertThat( "A converted partial-day opening exceptional period is represented equivalently as a calendar", diff --git a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalTest.java b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalTest.java index c23db9d3..2b8ed2c1 100644 --- a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalTest.java +++ b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsExceptionalTest.java @@ -1,19 +1,12 @@ package org.folio.calendar.unit.utils; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThrows; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import org.folio.calendar.domain.entity.ExceptionRange; -import org.folio.calendar.domain.legacy.dto.OpeningDayRelativeDTO; -import org.folio.calendar.testconstants.Dates; -import org.folio.calendar.testconstants.ExceptionRanges; +import org.folio.calendar.domain.legacy.dto.PeriodDTO; import org.folio.calendar.testconstants.OpeningDayRelativeConstants; +import org.folio.calendar.testconstants.Periods; import org.folio.calendar.utils.PeriodUtils; import org.junit.jupiter.api.Test; @@ -23,7 +16,9 @@ class PeriodUtilsExceptionalTest { void testRegularClosureIsNotExceptional() { assertThat( "A non-exceptional opening (for closure) is not exceptional", - PeriodUtils.areOpeningsExceptional(Arrays.asList(OpeningDayRelativeConstants.MONDAY_CLOSED)), + PeriodUtils.isExceptional( + PeriodDTO.builder().openingDay(OpeningDayRelativeConstants.MONDAY_CLOSED).build() + ), is(false) ); } @@ -32,8 +27,11 @@ void testRegularClosureIsNotExceptional() { void testRegularOpeningIsNotExceptional() { assertThat( "A non-exceptional opening (for opening hours) is not exceptional", - PeriodUtils.areOpeningsExceptional( - Arrays.asList(OpeningDayRelativeConstants.MONDAY_OPEN_04_00_TO_14_59) + PeriodUtils.isExceptional( + PeriodDTO + .builder() + .openingDay(OpeningDayRelativeConstants.MONDAY_OPEN_04_00_TO_14_59) + .build() ), is(false) ); @@ -43,8 +41,8 @@ void testRegularOpeningIsNotExceptional() { void testExceptionalClosureIsExceptional() { assertThat( "An exceptional opening (for closure) is exceptional", - PeriodUtils.areOpeningsExceptional( - Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_CLOSED) + PeriodUtils.isExceptional( + PeriodDTO.builder().openingDay(OpeningDayRelativeConstants.EXCEPTIONAL_CLOSED).build() ), is(true) ); @@ -54,156 +52,46 @@ void testExceptionalClosureIsExceptional() { void testExceptionalOpeningIsExceptional() { assertThat( "An exceptional opening (for different opening hours) is exceptional", - PeriodUtils.areOpeningsExceptional( - Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_OPEN_04_00_TO_14_59) + PeriodUtils.isExceptional( + PeriodDTO + .builder() + .openingDay(OpeningDayRelativeConstants.EXCEPTIONAL_OPEN_04_00_TO_14_59) + .build() ), is(true) ); } @Test - void testConversionOfNoExceptionsToExceptionRanges() { - List list = new ArrayList<>(); + void testConversionOfTooManyOpenings() { assertThrows( - "A list of no openings cannot be converted to exception range(s)", + "A period must only have one openingDays", IllegalArgumentException.class, - () -> - PeriodUtils.convertOpeningDayRelativeDTOToExceptionRanges( - Dates.DATE_2021_01_01, - Dates.DATE_2021_12_31, - list - ) + () -> PeriodUtils.convertExceptionalPeriodToExceptionRanges(Periods.PERIOD_FULL_EXAMPLE_B) ); } @Test - void testConversionOfMultipleExceptionsToExceptionRanges() { - List list = Arrays.asList( - OpeningDayRelativeConstants.EXCEPTIONAL_CLOSED, - OpeningDayRelativeConstants.EXCEPTIONAL_OPEN_04_00_TO_14_59 - ); + void testConversionOfNullOpeningList() { assertThrows( - "A list of multiple openings cannot be converted to exception range(s)", + "A period must only have one openingDays", IllegalArgumentException.class, () -> - PeriodUtils.convertOpeningDayRelativeDTOToExceptionRanges( - Dates.DATE_2021_01_01, - Dates.DATE_2021_12_31, - list + PeriodUtils.convertExceptionalPeriodToExceptionRanges( + Periods.PERIOD_EXCEPTIONAL_INVALID_NULL_OPENING ) ); } @Test - void testConversionOfMultipleHourPairsToExceptionRanges() { - List list = Arrays.asList( - OpeningDayRelativeConstants.EXCEPTIONAL_INVALID_MULTIPLE_OPENINGS - ); + void testConversionOfMultiOpeningInOneList() { assertThrows( - "An opening with multiple hour ranges cannot be converted to exception range(s)", + "A period must only have one openingDays", IllegalArgumentException.class, () -> - PeriodUtils.convertOpeningDayRelativeDTOToExceptionRanges( - Dates.DATE_2021_01_01, - Dates.DATE_2021_12_31, - list - ) - ); - } - - @Test - void testConversionOfClosureToExceptionRanges() { - List result = PeriodUtils.convertOpeningDayRelativeDTOToExceptionRanges( - Dates.DATE_2021_01_01, - Dates.DATE_2021_12_31, - Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_CLOSED) - ); - assertThat( - "An exceptional closure produces exactly one equivalent ExceptionRange", - result, - hasSize(1) - ); - - // override default ID - result.get(0).setId(null); - assertThat( - "An exceptional closure produces exactly one equivalent ExceptionRange with no openings", - result.get(0).getOpenings(), - hasSize(0) - ); - - assertThat( - "An exceptional closure can be equivalently represented as an ExceptionRange", - result, - hasItem( - ExceptionRanges - .withExceptionId(ExceptionRanges.CLOSED_2021_01_01_TO_2021_12_31, result.get(0).getId()) - .withName("Untitled exception") - ) - ); - } - - @Test - void testConversionOfAllDayOpeningToExceptionRanges() { - List result = PeriodUtils.convertOpeningDayRelativeDTOToExceptionRanges( - Dates.DATE_2021_01_01, - Dates.DATE_2021_01_04, - Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_OPEN_ALL_DAY) - ); - assertThat( - "An exceptional all-day opening produces exactly one equivalent ExceptionRange", - result, - hasSize(1) - ); - - // override default ID - result.get(0).setId(null); - assertThat( - "An exceptional all-day opening produces exactly one equivalent ExceptionRange", - result.get(0).getOpenings(), - hasSize(1) - ); - - assertThat( - "An exceptional all-day opening can be equivalently represented as an ExceptionRange", - result, - hasItem( - ExceptionRanges.withExceptionId( - ExceptionRanges.OPEN_ALL_DAY_JAN_1_THRU_JAN_4, - result.get(0).getId() - ) - ) - ); - } - - @Test - void testConversionOfPartialDayOpeningToExceptionRanges() { - List result = PeriodUtils.convertOpeningDayRelativeDTOToExceptionRanges( - Dates.DATE_2021_01_01, - Dates.DATE_2021_01_04, - Arrays.asList(OpeningDayRelativeConstants.EXCEPTIONAL_OPEN_04_00_TO_14_59) - ); - - assertThat( - "An exceptional partial-day opening produces exactly one equivalent ExceptionRange", - result, - hasSize(1) - ); - assertThat( - "An exceptional partial-day opening across four days produces exactly four equivalent ExceptionRanges", - result.get(0).getOpenings(), - hasSize(4) - ); - - assertThat( - "An exceptional partial-day opening can be equivalently represented as an ExceptionRange", - result, - hasItem( - ExceptionRanges.withExceptionId( - ExceptionRanges.OPEN_04_00_TO_14_59_JAN_1_THRU_JAN_4, - result.get(0).getId() + PeriodUtils.convertExceptionalPeriodToExceptionRanges( + Periods.PERIOD_EXCEPTIONAL_INVALID_MULTIPLE_OPENINGS ) - ) ); } } diff --git a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsNullConversionTest.java b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsNullConversionTest.java index 964bf0e0..1c12fb8a 100644 --- a/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsNullConversionTest.java +++ b/src/test/java/org/folio/calendar/unit/utils/PeriodUtilsNullConversionTest.java @@ -5,30 +5,12 @@ import java.util.Arrays; import java.util.List; import org.folio.calendar.domain.legacy.dto.OpeningDayRelativeDTO; -import org.folio.calendar.testconstants.Dates; import org.folio.calendar.testconstants.OpeningDayRelativeConstants; import org.folio.calendar.utils.PeriodUtils; import org.junit.jupiter.api.Test; class PeriodUtilsNullConversionTest { - @Test - void testNullOpeningDayRelativeExceptional() { - List invalidOpeningList = Arrays.asList( - OpeningDayRelativeConstants.EXCEPTIONAL_INVALID_NULL_OPENING - ); - assertThrows( - "A null OpeningHourRange in a OpeningDayRelative cannot be converted to an exception range", - IllegalArgumentException.class, - () -> - PeriodUtils.convertOpeningDayRelativeDTOToExceptionRanges( - Dates.DATE_2021_01_01, - Dates.DATE_2021_01_02, - invalidOpeningList - ) - ); - } - @Test void testNullOpeningDayRelativeNormal() { List invalidOpeningList = Arrays.asList( diff --git a/src/test/resources/database-migrate-data.sql b/src/test/resources/database-migrate-data.sql index ca0f03c5..9da7d424 100644 --- a/src/test/resources/database-migrate-data.sql +++ b/src/test/resources/database-migrate-data.sql @@ -1,28 +1,22 @@ --- Harvested from a mod-calendar 1.12.0 using the contents of AbstractExistingCalendarTest +-- Harvested from a mod-calendar 1.15.0 INSERT INTO "test_mod_calendar"."openings" ("id", "jsonb", "creation_date", "created_by") VALUES ( 'cccccccc-cccc-4ccc-8ccc-cccccccccccc', - '{"id": "cccccccc-cccc-4ccc-8ccc-cccccccccccc", "name": "replay quake aloft routine", "endDate": "2021-01-04T00:00:00.000+00:00", "startDate": "2021-01-01T00:00:00.000+00:00", "exceptional": true, "servicePointId": "55555555-5555-5555-5555-555555555555"}', + '{"id": "cccccccc-cccc-4ccc-8ccc-cccccccccccc", "name": "replay quake aloft routine", "endDate": "2021-01-04T00:00:00.000+00:00", "startDate": "2021-01-01T00:00:00.000+00:00", "exceptional": true, "servicePointId": "55555555-5555-4555-8555-555555555555"}', NULL, NULL ), ( 'dddddddd-dddd-4ddd-8ddd-dddddddddddd', - '{"id": "dddddddd-dddd-4ddd-8ddd-dddddddddddd", "name": "supplier grouped bride lazily", "endDate": "2021-09-22T00:00:00.000+00:00", "startDate": "2021-05-01T00:00:00.000+00:00", "exceptional": false, "servicePointId": "11111111-1111-1111-1111-111111111111"}', - NULL, - NULL - ), - ( - 'bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb', - '{"id": "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb", "name": "comic sublime upscale utilize", "endDate": "2021-09-22T00:00:00.000+00:00", "startDate": "2021-05-01T00:00:00.000+00:00", "exceptional": false, "servicePointId": "00000000-0000-4000-8000-000000000000"}', + '{"id": "dddddddd-dddd-4ddd-8ddd-dddddddddddd", "name": "supplier grouped bride lazily", "endDate": "2021-09-22T00:00:00.000+00:00", "startDate": "2021-05-01T00:00:00.000+00:00", "exceptional": false, "servicePointId": "11111111-1111-4111-8111-111111111111"}', NULL, NULL ), ( 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa', - '{"id": "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", "name": "sectional proving blanching deputy", "endDate": "2021-04-30T00:00:00.000+00:00", "startDate": "2021-01-01T00:00:00.000+00:00", "exceptional": false, "servicePointId": "00000000-0000-4000-8000-000000000000"}', + '{"id": "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", "name": "sectional proving blanching deputy", "endDate": "2021-12-31T00:00:00.000+00:00", "startDate": "2021-01-01T00:00:00.000+00:00", "exceptional": false, "servicePointId": "00000000-0000-4000-8000-000000000000"}', NULL, NULL ), @@ -77,4 +71,4 @@ VALUES '{"id": "71f194d5-98ee-417c-95f4-39ccb99c33f9", "openingId": "00000000-0000-4000-8000-000000000000", "openingDays": [{"openingDay": {"open": false, "allDay": true, "exceptional": true, "openingHour": [{"endTime": "23:59", "startTime": "00:00"}]}}]}', NULL, NULL - ); \ No newline at end of file + );