Skip to content

Commit

Permalink
refactor(Addressed PR feedback):
Browse files Browse the repository at this point in the history
br648 committed Dec 16, 2024
1 parent 4e788c5 commit 22ed611
Showing 10 changed files with 86 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -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,
Original file line number Diff line number Diff line change
@@ -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<OtpUser> 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;
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion src/main/resources/Message.properties
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion src/main/resources/Message_fr.properties
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion src/main/resources/latest-spark-swagger-output.yaml
Original file line number Diff line number Diff line change
@@ -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:
Original file line number Diff line number Diff line change
@@ -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,37 +46,34 @@ 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);
}

private static Stream<Arguments> 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);
Leg nextLeg = itinerary.legs.get(2);
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<Arguments> 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<Arguments> 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<OtpUser> 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<OtpUser> users = LegTransitionNotification.getLegTransitionNotifyUsers(trip);
assertNotNull(users);
Original file line number Diff line number Diff line change
@@ -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<Arguments> 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);
}
}

0 comments on commit 22ed611

Please sign in to comment.