diff --git a/src/main/java/org/opentripplanner/middleware/i18n/Message.java b/src/main/java/org/opentripplanner/middleware/i18n/Message.java index 260c2fb09..fa43c1532 100644 --- a/src/main/java/org/opentripplanner/middleware/i18n/Message.java +++ b/src/main/java/org/opentripplanner/middleware/i18n/Message.java @@ -19,10 +19,10 @@ public enum Message { ACCEPT_DEPENDENT_EMAIL_SUBJECT, ACCEPT_DEPENDENT_EMAIL_MANAGE, ACCEPT_DEPENDENT_ERROR, - ARRIVED_AND_MODE_CHANGE_NOTIFICATION, ARRIVED_NOTIFICATION, DEPARTED_NOTIFICATION, LABEL_AND_CONTENT, + MODE_CHANGE_NOTIFICATION, SMS_STOP_NOTIFICATIONS, TRIP_EMAIL_SUBJECT, TRIP_EMAIL_SUBJECT_FOR_USER, diff --git a/src/main/java/org/opentripplanner/middleware/models/LegTransitionNotification.java b/src/main/java/org/opentripplanner/middleware/models/LegTransitionNotification.java index 731aac664..a3ce72350 100644 --- a/src/main/java/org/opentripplanner/middleware/models/LegTransitionNotification.java +++ b/src/main/java/org/opentripplanner/middleware/models/LegTransitionNotification.java @@ -16,7 +16,7 @@ import java.util.Set; import static com.mongodb.client.model.Filters.eq; -import static org.opentripplanner.middleware.tripmonitor.jobs.NotificationType.ARRIVED_AND_MODE_CHANGE_NOTIFICATION; +import static org.opentripplanner.middleware.tripmonitor.jobs.NotificationType.MODE_CHANGE_NOTIFICATION; import static org.opentripplanner.middleware.tripmonitor.jobs.NotificationType.ARRIVED_NOTIFICATION; import static org.opentripplanner.middleware.tripmonitor.jobs.NotificationType.DEPARTED_NOTIFICATION; import static org.opentripplanner.middleware.triptracker.TravelerLocator.hasRequiredTransitLeg; @@ -54,9 +54,9 @@ public LegTransitionNotification( private TripMonitorNotification createTripMonitorNotification(NotificationType notificationType) { String body; switch (notificationType) { - case ARRIVED_AND_MODE_CHANGE_NOTIFICATION: + case MODE_CHANGE_NOTIFICATION: body = String.format( - Message.ARRIVED_AND_MODE_CHANGE_NOTIFICATION.get(observerLocale), + Message.MODE_CHANGE_NOTIFICATION.get(observerLocale), travelerName, travelerPosition.expectedLeg.to.name ); @@ -103,9 +103,13 @@ public static Set getLegTransitionNotifyUsers(MonitoredTrip trip) { } /** - * If a traveler is on schedule and on either a walk or transit leg check for possible leg transition notification. + * If a traveler is on the route (not deviated), check for possible leg transition notification. */ - public static void checkForLegTransition(TripStatus tripStatus, TravelerPosition travelerPosition, MonitoredTrip trip) { + public static void checkForLegTransition( + TripStatus tripStatus, + TravelerPosition travelerPosition, + MonitoredTrip trip + ) { if ( hasRequiredTripStatus(tripStatus) && (hasRequiredWalkLeg(travelerPosition) || hasRequiredTransitLeg(travelerPosition)) @@ -122,14 +126,14 @@ public static void checkForLegTransition(TripStatus tripStatus, TravelerPosition } /** - * Depending on the traveler's proximity to the start/end of a leg return the appropriate notification type. + * Depending on the traveler's proximity to the start/end of a leg, return the appropriate notification type. */ private static NotificationType getLegTransitionNotificationType(TravelerPosition travelerPosition) { if (isAtStartOfLeg(travelerPosition)) { return DEPARTED_NOTIFICATION; } else if (isApproachingEndOfLeg(travelerPosition)) { if (hasModeChanged(travelerPosition)) { - return ARRIVED_AND_MODE_CHANGE_NOTIFICATION; + return MODE_CHANGE_NOTIFICATION; } return ARRIVED_NOTIFICATION; } diff --git a/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java b/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java index 5a6d8f256..d854d9246 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java +++ b/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java @@ -118,16 +118,21 @@ public static TripMonitorNotification createInitialReminderNotification( ); } + /** + * Checks for equality excluding the parent {@link Model} class. + */ @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; - if (!super.equals(o)) return false; TripMonitorNotification that = (TripMonitorNotification) o; return type == that.type && Objects.equals(body, that.body); } + /** + * Creates a hash code from fields in this class only excluding fields within the parent {@link Model} class. + */ @Override public int hashCode() { - return Objects.hash(super.hashCode(), type, body); + return Objects.hash(type, body); } -} +} \ No newline at end of file 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 0a70fedff..7f2c56dd3 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -53,9 +53,7 @@ public class CheckMonitoredTrip implements Runnable { private static final Logger LOG = LoggerFactory.getLogger(CheckMonitoredTrip.class); - private final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); - - private final String OTP_UI_NAME = ConfigUtils.getConfigPropertyAsText("OTP_UI_NAME"); + public boolean IS_TEST = false; public static final int MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS = ConfigUtils.getConfigPropertyAsInt("MAXIMUM_MONITORED_TRIP_ITINERARY_CHECKS", 3); @@ -268,6 +266,8 @@ public void processLegTransition(NotificationType notificationType, TravelerPosi sendNotifications(observer); } }); + + updateMonitoredTrip(); } /** @@ -590,11 +590,8 @@ private void sendNotifications(OtpUser otpUser) { } // TODO: better handle below when one of the following fails - if (successEmail || successPush || successSms) { + if (successEmail || successPush || successSms || IS_TEST) { notificationTimestampMillis = DateTimeUtils.currentTimeMillis(); - // Prevent repeated notifications by saving successfully sent notifications. - trip.journeyState.lastNotifications.addAll(notifications); - Persistence.monitoredTrips.replace(trip.id, trip); } } @@ -934,6 +931,8 @@ private boolean updateMonitoredTrip() { // Update notification time if notification successfully sent. if (notificationTimestampMillis != -1) { journeyState.lastNotificationTimeMillis = notificationTimestampMillis; + // Prevent repeated notifications by saving successfully sent notifications. + journeyState.lastNotifications.addAll(notifications); } trip.journeyState = journeyState; Persistence.monitoredTrips.replace(trip.id, trip); diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/NotificationType.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/NotificationType.java index 4a40911cc..62d0fa422 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/NotificationType.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/NotificationType.java @@ -12,7 +12,7 @@ public enum NotificationType { ALERT_FOUND, ITINERARY_NOT_FOUND, INITIAL_REMINDER, - ARRIVED_AND_MODE_CHANGE_NOTIFICATION, + MODE_CHANGE_NOTIFICATION, DEPARTED_NOTIFICATION, ARRIVED_NOTIFICATION } \ No newline at end of file diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index df70ee760..c44410aec 100644 --- a/src/main/resources/Message.properties +++ b/src/main/resources/Message.properties @@ -4,9 +4,9 @@ ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = Accept trusted companion ACCEPT_DEPENDENT_EMAIL_SUBJECT = Trusted companion request ACCEPT_DEPENDENT_EMAIL_MANAGE = Manage settings ACCEPT_DEPENDENT_ERROR = Unable to accept trusted companion. -ARRIVED_AND_MODE_CHANGE_NOTIFICATION = %s has arrived at transit stop %s. ARRIVED_NOTIFICATION = %s has arrived at %s. LABEL_AND_CONTENT = %s: %s +MODE_CHANGE_NOTIFICATION = %s has arrived at transit stop %s. DEPARTED_NOTIFICATION = %s has departed %s. SMS_STOP_NOTIFICATIONS = To stop receiving notifications, reply STOP. TRIP_EMAIL_SUBJECT = %s Notification diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index ac826b816..312b3d596 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -4,9 +4,9 @@ ACCEPT_DEPENDENT_EMAIL_LINK_TEXT = Accepter la demande ACCEPT_DEPENDENT_EMAIL_SUBJECT = Demande d'accompagnateur ACCEPT_DEPENDENT_EMAIL_MANAGE = Gérez vos préférences ACCEPT_DEPENDENT_ERROR = La demande d'accompagnateur n'a pas été reçue. -ARRIVED_AND_MODE_CHANGE_NOTIFICATION = %s TODO %s. ARRIVED_NOTIFICATION = TODO %s. LABEL_AND_CONTENT = %s\u00A0: %s +MODE_CHANGE_NOTIFICATION = %s TODO %s. DEPARTED_NOTIFICATION = TODO %s. SMS_STOP_NOTIFICATIONS = Pour arrêter ces notifications, envoyez STOP. TRIP_EMAIL_SUBJECT = Notification pour %s diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 28389f6a1..1f9ad76cd 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2376,7 +2376,7 @@ definitions: - "ALERT_FOUND" - "ITINERARY_NOT_FOUND" - "INITIAL_REMINDER" - - "ARRIVED_AND_MODE_CHANGE_NOTIFICATION" + - "MODE_CHANGE_NOTIFICATION" - "DEPARTED_NOTIFICATION" - "ARRIVED_NOTIFICATION" body: diff --git a/src/test/java/org/opentripplanner/middleware/models/LegTransitionNotificationTest.java b/src/test/java/org/opentripplanner/middleware/models/LegTransitionNotificationTest.java index eb2191f43..8d96ee713 100644 --- a/src/test/java/org/opentripplanner/middleware/models/LegTransitionNotificationTest.java +++ b/src/test/java/org/opentripplanner/middleware/models/LegTransitionNotificationTest.java @@ -29,7 +29,7 @@ class LegTransitionNotificationTest extends OtpMiddlewareTestEnvironment { private static OtpUser observer; @BeforeAll - public static void setup() { + public static void setup() throws Exception { primary = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-primary-user")); companion = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-companion-user")); observer = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-observer-user")); @@ -46,14 +46,13 @@ void testLegTransitionNotifications( NotificationType notificationType, String travelerName, TravelerPosition travelerPosition, - Locale locale, String message ) { TripMonitorNotification notification = new LegTransitionNotification( travelerName, notificationType, travelerPosition, - locale + Locale.US ).tripMonitorNotification; assertNotNull(notification); assertEquals(message, notification.body); @@ -61,7 +60,6 @@ void testLegTransitionNotifications( private static Stream createLegTransitionNotificationTestCases() throws Exception { String travelerName = "Obi-Wan"; - Locale locale = Locale.US; Itinerary itinerary = createDefaultItinerary(); Leg expectedLeg = itinerary.legs.get(1); Coordinates expectedLegDestinationCoords = new Coordinates(expectedLeg.to); @@ -69,14 +67,13 @@ private static Stream createLegTransitionNotificationTestCases() thro Coordinates nextLegDepartureCoords = new Coordinates(nextLeg.from); return Stream.of( Arguments.of( - NotificationType.ARRIVED_AND_MODE_CHANGE_NOTIFICATION, + NotificationType.MODE_CHANGE_NOTIFICATION, travelerName, new TravelerPosition.Builder() .setExpectedLeg(expectedLeg) .setNextLeg(nextLeg) .setCurrentPosition(expectedLegDestinationCoords) .build(), - locale, "Obi-Wan has arrived at transit stop Pioneer Square South MAX Station." ), Arguments.of( @@ -87,7 +84,6 @@ private static Stream createLegTransitionNotificationTestCases() thro .setNextLeg(nextLeg) .setCurrentPosition(nextLegDepartureCoords) .build(), - locale, "Obi-Wan has departed Providence Park MAX Station." ), Arguments.of( @@ -98,7 +94,6 @@ private static Stream createLegTransitionNotificationTestCases() thro .setNextLeg(nextLeg) .setCurrentPosition(expectedLegDestinationCoords) .build(), - locale, "Obi-Wan has arrived at Pioneer Square South MAX Station." ) ); @@ -110,16 +105,11 @@ void testLegTransitionNotifyUsers( String tripOwnerUserId, Set expectedUsers ) { - - RelatedUser relatedUser = new RelatedUser(); - relatedUser.email = observer.email; - relatedUser.status = RelatedUser.RelatedUserStatus.CONFIRMED; - MonitoredTrip trip = new MonitoredTrip(); trip.userId = tripOwnerUserId; trip.primary = new MobilityProfileLite(primary); trip.companion = new RelatedUser(companion.email, RelatedUser.RelatedUserStatus.CONFIRMED); - trip.observers.add(relatedUser); + trip.observers.add(new RelatedUser(observer.email, RelatedUser.RelatedUserStatus.CONFIRMED)); Set users = LegTransitionNotification.getLegTransitionNotifyUsers(trip); assertNotNull(users); 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 80d90ff00..99b1e02ad 100644 --- a/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTripTest.java @@ -10,7 +10,11 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.middleware.models.ItineraryExistence; +import org.opentripplanner.middleware.models.MobilityProfileLite; +import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.models.TrackedJourney; +import org.opentripplanner.middleware.otp.response.Leg; +import org.opentripplanner.middleware.testutils.ApiTestUtils; import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; import org.opentripplanner.middleware.testutils.OtpTestUtils; import org.opentripplanner.middleware.testutils.PersistenceTestUtils; @@ -23,6 +27,8 @@ import org.opentripplanner.middleware.otp.response.OtpResponse; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.tripmonitor.TripStatus; +import org.opentripplanner.middleware.triptracker.TravelerPosition; +import org.opentripplanner.middleware.utils.Coordinates; import org.opentripplanner.middleware.utils.DateTimeUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,6 +50,7 @@ import static org.hamcrest.text.MatchesPattern.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -807,4 +814,48 @@ private static Stream createCanUnsnoozeTripCases() { Arguments.of(noonMonday8June2020, tuesday, true) ); } -} + + @Test + void testDuplicateNotifications() throws Exception { + OtpUser observer = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-observer-user")); + + MonitoredTrip monitoredTrip = PersistenceTestUtils.createMonitoredTrip( + user.id, + OtpTestUtils.OTP2_DISPATCHER_PLAN_RESPONSE.clone(), + true, + OtpTestUtils.createDefaultJourneyState() + ); + monitoredTrip.primary = new MobilityProfileLite(user); + monitoredTrip.observers.add(new RelatedUser(observer.email, RelatedUser.RelatedUserStatus.CONFIRMED)); + Persistence.monitoredTrips.replace(monitoredTrip.id, monitoredTrip); + + Leg expectedLeg = monitoredTrip.itinerary.legs.get(1); + Coordinates expectedLegDestinationCoords = new Coordinates(expectedLeg.to); + Leg nextLeg = monitoredTrip.itinerary.legs.get(2); + + TravelerPosition travelerPosition = new TravelerPosition.Builder() + .setExpectedLeg(expectedLeg) + .setNextLeg(nextLeg) + .setCurrentPosition(expectedLegDestinationCoords) + .build(); + + triggerCheckMonitoredTrip(monitoredTrip, travelerPosition); + MonitoredTrip updated = Persistence.monitoredTrips.getById(monitoredTrip.id); + assertNotEquals(-1, updated.journeyState.lastNotificationTimeMillis); + long previousLastNotificationTimeMillis = updated.journeyState.lastNotificationTimeMillis; + + triggerCheckMonitoredTrip(monitoredTrip, travelerPosition); + updated = Persistence.monitoredTrips.getById(monitoredTrip.id); + assertEquals(previousLastNotificationTimeMillis, updated.journeyState.lastNotificationTimeMillis); + + Persistence.monitoredTrips.removeById(monitoredTrip.id); + Persistence.otpUsers.removeById(observer.id); + } + + private void triggerCheckMonitoredTrip(MonitoredTrip monitoredTrip, TravelerPosition travelerPosition) throws CloneNotSupportedException { + CheckMonitoredTrip checkMonitoredTrip = new CheckMonitoredTrip(monitoredTrip, this::mockOtpPlanResponse); + checkMonitoredTrip.IS_TEST = true; + checkMonitoredTrip.targetZonedDateTime = monitoredTrip.tripZonedDateTime(DateTimeUtils.nowAsLocalDate()); + checkMonitoredTrip.processLegTransition(NotificationType.MODE_CHANGE_NOTIFICATION, travelerPosition); + } +} \ No newline at end of file