MODCAL-124: Fix issue with legacy exceptions not associating with cal…

…endars (#159)

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);
 }

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) {

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);
 }

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<PeriodDTO> periodsToMigrate;
+  protected List<Calendar> 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<PeriodDTO> 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);
       }
     }

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(

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<OpeningDayRelativeDTO> 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<ExceptionRange> convertOpeningDayRelativeDTOToExceptionRanges(
-    LocalDate startDate,
-    LocalDate endDate,
-    List<OpeningDayRelativeDTO> 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<OpeningDayRelativeDTO> openings = period.getOpeningDays();
 
     if (openings.size() != 1) {
       throw new IllegalArgumentException(
@@ -91,7 +88,7 @@ public static List<ExceptionRange> 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<ExceptionRange> 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<ExceptionRange> convertOpeningDayRelativeDTOToExceptionRanges
       }
     }
 
-    return Arrays.asList(builder.build());
+    return builder.build();
   }
 
   /**
@@ -223,7 +223,7 @@ private static List<NormalOpening> consolidateNormalOpenings(List<NormalOpening>
     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<Calendar> toCalendars(List<PeriodDTO> periods) {
+    log.debug("Converting periods: {}", periods);
+
+    List<Calendar> calendars = periods
+      .stream()
+      .filter(period -> !isExceptional(period))
+      .map(PeriodUtils::convertOpeningPeriodToCalendar)
+      .collect(Collectors.toCollection(ArrayList::new));
+    List<PeriodDTO> 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<Calendar> 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<ExceptionRange> 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;
+  }
 }