From 4783c94b8e33f767d5941cd566f55c80ed1bdfd3 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 27 Nov 2024 16:59:44 -0500 Subject: [PATCH 1/5] fix(CheckMonitoredTrip): Correctly unsnooze snoozed trips at midnight next calendar day. --- .../tripmonitor/jobs/CheckMonitoredTrip.java | 37 ++++++++++--- .../jobs/CheckMonitoredTripTest.java | 53 +++++++++++++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index d8c44769f..c14bc3d14 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -28,6 +28,7 @@ import java.time.LocalDate; import java.time.ZoneId; import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -670,6 +671,18 @@ public boolean shouldSkipMonitoredTripCheck(boolean persist) throws Exception { return true; } + // For trips that are snoozed, see if they should be unsnoozed first. + if (trip.snoozed) { + if (shouldUnsnoozeTrip()) { + // Clear previous journey state and matching itinerary as we want to start afresh. + // This will let + previousMatchingItinerary = null; + } else { + LOG.info("Skipping: Trip is snoozed."); + return true; + } + } + if (isPrevMatchingItineraryNotConcluded()) { // Skip checking the trip the rest of the time that it is active if the trip was deemed not possible for the // next possible time during a previous query to find candidate itinerary matches. @@ -678,12 +691,6 @@ public boolean shouldSkipMonitoredTripCheck(boolean persist) throws Exception { return true; } - // skip checking the trip if it has been snoozed - if (trip.snoozed) { - LOG.info("Skipping: Trip is snoozed."); - return true; - } - matchingItinerary = previousMatchingItinerary; targetZonedDateTime = DateTimeUtils.makeOtpZonedDateTime(previousJourneyState.targetDate, trip.tripTime); } else { @@ -929,4 +936,22 @@ private Locale getOtpUserLocale() { private String getTripUrl() { return String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, trip.id); } + + /** + * Whether a trip should be unsnoozed and monitoring should resume. + * @return true if the current time is after the calendar day (midnight) after the matching trip start day, false otherwise. + */ + public boolean shouldUnsnoozeTrip() { + ZoneId otpZoneId = DateTimeUtils.getOtpZoneId(); + var midnightAfterPreviousTrip = ZonedDateTime + .ofInstant( + previousJourneyState.matchingItinerary.startTime.toInstant().plus(1, ChronoUnit.DAYS), + otpZoneId + ) + .withHour(0) + .withMinute(0) + .withSecond(0); + + return DateTimeUtils.nowAsZonedDateTime(otpZoneId).isAfter(midnightAfterPreviousTrip); + } } diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 9bc9a3eb6..0d4051090 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -746,4 +746,57 @@ private void assertCheckMonitoredTrip( assertEquals(expectedAttempts, monitoredTrip.attemptsToGetMatchingItinerary); assertEquals(expectedTripStatus, monitoredTrip.journeyState.tripStatus); } + + @ParameterizedTest + @MethodSource("createCanUnsnoozeTripCases") + void canUnsnoozeTrip(ZonedDateTime time, boolean shouldUnsnooze) throws Exception { + MonitoredTrip monitoredTrip = PersistenceTestUtils.createMonitoredTrip( + user.id, + OtpTestUtils.OTP2_DISPATCHER_PLAN_RESPONSE.clone(), + false, + OtpTestUtils.createDefaultJourneyState() + ); + monitoredTrip.id = UUID.randomUUID().toString(); + // Mark trip as snoozed + monitoredTrip.snoozed = true; + Persistence.monitoredTrips.create(monitoredTrip); + + // Mock the current time + DateTimeUtils.useFixedClockAt(time); + + // After snoozed trip is over, trip checks on that trip should not be skipped + CheckMonitoredTrip check = new CheckMonitoredTrip(monitoredTrip, this::mockOtpPlanResponse); + + // Add artifacts of prior monitoring (e.g. monitoring was active until a few minutes before trip start) + JourneyState journeyState = monitoredTrip.journeyState; + journeyState.targetDate = "2020-06-09"; + journeyState.tripStatus = TripStatus.TRIP_UPCOMING; + journeyState.lastCheckedEpochMillis = Instant + .ofEpochMilli(monitoredTrip.itinerary.startTime.getTime()) + .minus(10, ChronoUnit.MINUTES) + .toEpochMilli(); + journeyState.matchingItinerary = monitoredTrip.itinerary; + check.previousJourneyState = journeyState; + check.previousMatchingItinerary = monitoredTrip.itinerary; + + assertEquals(shouldUnsnooze, check.shouldUnsnoozeTrip()); + check.shouldSkipMonitoredTripCheck(); + + MonitoredTrip modifiedTrip = Persistence.monitoredTrips.getById(monitoredTrip.id); + assertEquals(!shouldUnsnooze, modifiedTrip.snoozed); + + // Clear the created trip. + PersistenceTestUtils.deleteMonitoredTrip(modifiedTrip); + } + + private static Stream createCanUnsnoozeTripCases() { + // (Trips from above response above starts on Tuesday, June 9, 2020 at 8:40am and ends at 8:58am.) + return Stream.of( + // At 9:00am on Tuesday, June 9, 2020 (right after trip ends), snoozed trip should remain snoozed. + Arguments.of(noonMonday8June2020.withDayOfMonth(9).withHour(9).withMinute(0), false), + // At 00:00am on Wednesday, June 10, 2020, snoozed trip should be unsnoozed + // but it is too early for the trip to be analyzed again. + Arguments.of(noonMonday8June2020.withDayOfMonth(10).withHour(0).withMinute(0), true) + ); + } } From dbfcd418a66e7327348376fbd3c89437f20a9e0f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 27 Nov 2024 17:21:00 -0500 Subject: [PATCH 2/5] refactor(CheckMonitoredTrip): Fix comments. --- .../middleware/tripmonitor/jobs/CheckMonitoredTrip.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index c14bc3d14..6bfbac80b 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -674,8 +674,8 @@ public boolean shouldSkipMonitoredTripCheck(boolean persist) throws Exception { // For trips that are snoozed, see if they should be unsnoozed first. if (trip.snoozed) { if (shouldUnsnoozeTrip()) { - // Clear previous journey state and matching itinerary as we want to start afresh. - // This will let + // Clear previous matching itinerary as we want to start afresh. + // The snoozed state will be updated later in the process. previousMatchingItinerary = null; } else { LOG.info("Skipping: Trip is snoozed."); From 68ddb52dd326df740219d64b470bb48dbc00baef Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:22:06 -0500 Subject: [PATCH 3/5] fix(CheckMonitoredTrip): Use last checked timestamp to compute unsnooze conditions. --- .../tripmonitor/jobs/CheckMonitoredTrip.java | 11 +++++--- .../jobs/CheckMonitoredTripTest.java | 28 ++++++++++++------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index 6bfbac80b..9613601b2 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -939,19 +939,22 @@ private String getTripUrl() { /** * Whether a trip should be unsnoozed and monitoring should resume. - * @return true if the current time is after the calendar day (midnight) after the matching trip start day, false otherwise. + * @return true if the current time is after the calendar day (on or after midnight) + * after the matching trip start day, false otherwise. */ public boolean shouldUnsnoozeTrip() { ZoneId otpZoneId = DateTimeUtils.getOtpZoneId(); - var midnightAfterPreviousTrip = ZonedDateTime + var midnightAfterLastChecked = ZonedDateTime .ofInstant( - previousJourneyState.matchingItinerary.startTime.toInstant().plus(1, ChronoUnit.DAYS), + Instant.ofEpochMilli(previousJourneyState.lastCheckedEpochMillis).plus(1, ChronoUnit.DAYS), otpZoneId ) .withHour(0) .withMinute(0) .withSecond(0); - return DateTimeUtils.nowAsZonedDateTime(otpZoneId).isAfter(midnightAfterPreviousTrip); + ZonedDateTime now = DateTimeUtils.nowAsZonedDateTime(otpZoneId); + // Include equal or after midnight as true. + return !now.isBefore(midnightAfterLastChecked); } } diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 0d4051090..5729568c6 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -749,7 +749,7 @@ private void assertCheckMonitoredTrip( @ParameterizedTest @MethodSource("createCanUnsnoozeTripCases") - void canUnsnoozeTrip(ZonedDateTime time, boolean shouldUnsnooze) throws Exception { + void canUnsnoozeTrip(ZonedDateTime lastCheckedTime, ZonedDateTime clockTime, boolean shouldUnsnooze) throws Exception { MonitoredTrip monitoredTrip = PersistenceTestUtils.createMonitoredTrip( user.id, OtpTestUtils.OTP2_DISPATCHER_PLAN_RESPONSE.clone(), @@ -759,10 +759,14 @@ void canUnsnoozeTrip(ZonedDateTime time, boolean shouldUnsnooze) throws Exceptio monitoredTrip.id = UUID.randomUUID().toString(); // Mark trip as snoozed monitoredTrip.snoozed = true; + // Set monitored days to Tuesday only. + monitoredTrip.monday = false; + monitoredTrip.tuesday = true; + Persistence.monitoredTrips.create(monitoredTrip); // Mock the current time - DateTimeUtils.useFixedClockAt(time); + DateTimeUtils.useFixedClockAt(clockTime); // After snoozed trip is over, trip checks on that trip should not be skipped CheckMonitoredTrip check = new CheckMonitoredTrip(monitoredTrip, this::mockOtpPlanResponse); @@ -771,10 +775,8 @@ void canUnsnoozeTrip(ZonedDateTime time, boolean shouldUnsnooze) throws Exceptio JourneyState journeyState = monitoredTrip.journeyState; journeyState.targetDate = "2020-06-09"; journeyState.tripStatus = TripStatus.TRIP_UPCOMING; - journeyState.lastCheckedEpochMillis = Instant - .ofEpochMilli(monitoredTrip.itinerary.startTime.getTime()) - .minus(10, ChronoUnit.MINUTES) - .toEpochMilli(); + // Set last-checked-time + journeyState.lastCheckedEpochMillis = lastCheckedTime.toInstant().toEpochMilli(); journeyState.matchingItinerary = monitoredTrip.itinerary; check.previousJourneyState = journeyState; check.previousMatchingItinerary = monitoredTrip.itinerary; @@ -791,12 +793,18 @@ void canUnsnoozeTrip(ZonedDateTime time, boolean shouldUnsnooze) throws Exceptio private static Stream createCanUnsnoozeTripCases() { // (Trips from above response above starts on Tuesday, June 9, 2020 at 8:40am and ends at 8:58am.) + ZonedDateTime tuesday = noonMonday8June2020.withDayOfMonth(9).withHour(0).withMinute(0).withSecond(0); + ZonedDateTime wednesday = tuesday.withDayOfMonth(10); + return Stream.of( - // At 9:00am on Tuesday, June 9, 2020 (right after trip ends), snoozed trip should remain snoozed. - Arguments.of(noonMonday8June2020.withDayOfMonth(9).withHour(9).withMinute(0), false), - // At 00:00am on Wednesday, June 10, 2020, snoozed trip should be unsnoozed + // Trip snoozed at 8:00am on Tuesday, June 9, 2020, should remain snoozed right after trip ends at 9:00am. + Arguments.of(tuesday.withHour(8), tuesday.withHour(9), false), + // Trip snoozed at 8:00am on Tuesday, June 9, 2020, should unsnooze at 00:00am on Wednesday, June 10, 2020, // but it is too early for the trip to be analyzed again. - Arguments.of(noonMonday8June2020.withDayOfMonth(10).withHour(0).withMinute(0), true) + Arguments.of(tuesday.withHour(8), wednesday, true), + // Trip snoozed on Monday, June 8, 2020 (a day before the trip starts), should unsnooze at 00:00 on + // Tuesday, June 9, 2020. + Arguments.of(noonMonday8June2020, tuesday, true) ); } } From 54c93ae7ba8e1ed43da84a4f766b1cd6d8139d47 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:31:56 -0500 Subject: [PATCH 4/5] test(CheckMonitoredTripTest): Adjust comments. --- .../middleware/tripmonitor/jobs/CheckMonitoredTripTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 5729568c6..799b89438 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -771,7 +771,7 @@ void canUnsnoozeTrip(ZonedDateTime lastCheckedTime, ZonedDateTime clockTime, boo // After snoozed trip is over, trip checks on that trip should not be skipped CheckMonitoredTrip check = new CheckMonitoredTrip(monitoredTrip, this::mockOtpPlanResponse); - // Add artifacts of prior monitoring (e.g. monitoring was active until a few minutes before trip start) + // Add artifacts of prior monitoring (e.g. monitoring was active until a few minutes before trip snooze) JourneyState journeyState = monitoredTrip.journeyState; journeyState.targetDate = "2020-06-09"; journeyState.tripStatus = TripStatus.TRIP_UPCOMING; From 69d0b8352b78e1f75e469a79a4311129c025d619 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 3 Dec 2024 08:45:06 -0500 Subject: [PATCH 5/5] refactor(CheckMonitoredTripTest): Adjust comments and fix typo. --- .../tripmonitor/jobs/CheckMonitoredTripTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java index 799b89438..80d90ff00 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -792,18 +792,18 @@ void canUnsnoozeTrip(ZonedDateTime lastCheckedTime, ZonedDateTime clockTime, boo } private static Stream createCanUnsnoozeTripCases() { - // (Trips from above response above starts on Tuesday, June 9, 2020 at 8:40am and ends at 8:58am.) + // (Trips for these tests start on Tuesday, June 9, 2020 at 8:40am and ends at 8:58am.) ZonedDateTime tuesday = noonMonday8June2020.withDayOfMonth(9).withHour(0).withMinute(0).withSecond(0); ZonedDateTime wednesday = tuesday.withDayOfMonth(10); return Stream.of( // Trip snoozed at 8:00am on Tuesday, June 9, 2020, should remain snoozed right after trip ends at 9:00am. Arguments.of(tuesday.withHour(8), tuesday.withHour(9), false), - // Trip snoozed at 8:00am on Tuesday, June 9, 2020, should unsnooze at 00:00am on Wednesday, June 10, 2020, - // but it is too early for the trip to be analyzed again. + // Trip snoozed at 8:00am on Tuesday, June 9, 2020, should unsnooze at 12:00am (midnight) on + // Wednesday, June 10, 2020, but it is too early for the trip to be analyzed again. Arguments.of(tuesday.withHour(8), wednesday, true), - // Trip snoozed on Monday, June 8, 2020 (a day before the trip starts), should unsnooze at 00:00 on - // Tuesday, June 9, 2020. + // Trip snoozed on Monday, June 8, 2020 (a day before the trip starts), should unsnooze at 12:00am (midnight) + // on Tuesday, June 9, 2020. Arguments.of(noonMonday8June2020, tuesday, true) ); }