From f44faf2c83832e657318da63e0599c302c0e3569 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 5 Dec 2024 16:49:02 -0500 Subject: [PATCH 1/4] fix(OtpUser): Delete primary traveler references upon user deletion. --- .../opentripplanner/middleware/models/OtpUser.java | 11 ++++++++++- .../middleware/tripmonitor/TrustedCompanion.java | 14 ++++++++++++++ .../controllers/api/OtpUserControllerTest.java | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 1c9e32a02..ce908aaaf 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonSetter; +import com.mongodb.client.model.Filters; import org.opentripplanner.middleware.auth.Auth0Users; import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.persistence.Persistence; @@ -22,6 +23,7 @@ import java.util.stream.Stream; import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removeDependent; +import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removePrimaryTraveler; /** * This represents a user of an OpenTripPlanner instance (typically of the standard OTP UI/otp-react-redux). @@ -153,10 +155,17 @@ public boolean delete(boolean deleteAuth0User) { } } - // If a dependent, remove relationship with all related users. + // If a dependent user, remove relationship with all related users. for (RelatedUser relatedUser : relatedUsers) { removeDependent(this, relatedUser); } + + // If a dependent user, remove self as the primary traveler in trips created by other users. + // TODO: Should we alert the user who created the trip of the deletion? + Persistence.monitoredTrips + .getFiltered(Filters.eq("primary.userId", id)) + .forEach(trip -> removePrimaryTraveler(this, trip)); + return Persistence.otpUsers.removeById(this.id); } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index cad8bb17b..fb15795d5 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -8,6 +8,7 @@ import org.opentripplanner.middleware.auth.Auth0Connection; import org.opentripplanner.middleware.i18n.Message; import org.opentripplanner.middleware.models.MobilityProfileLite; +import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.persistence.Persistence; @@ -248,6 +249,19 @@ public static void removeDependent(OtpUser dependent, RelatedUser relatedUser) { } } + /** + * Remove the specified user as the primary traveler from the specified trip. + */ + public static void removePrimaryTraveler(OtpUser otpUser, MonitoredTrip trip) { + if (trip.primary != null && otpUser.id.equals(trip.primary.userId)) { + trip.primary = null; + // Recheck existence of record in Mongo in case trip got deleted since last Mongo query. + if (Persistence.monitoredTrips.getById(trip.id) != null) { + Persistence.monitoredTrips.replace(trip.id, trip); + } + } + } + /** * Retrieve the mobility profile for a dependent providing the requesting user is a trusted companion. */ diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index a1d9f0c9e..6d41cf1f1 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -12,6 +12,7 @@ import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.middleware.models.MobilityProfile; import org.opentripplanner.middleware.models.MobilityProfileLite; +import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.persistence.Persistence; @@ -31,6 +32,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled; @@ -241,9 +243,21 @@ void canRemoveRelatedUserOnDelete() { nickname )); Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree); + + // Create a monitored trip with dependentUserThree as primary traveler, relatedUserThree as companion. + MonitoredTrip trip = new MonitoredTrip(); + trip.id = UUID.randomUUID().toString(); + trip.primary = new MobilityProfileLite(dependentUserThree); + trip.companion = new RelatedUser(relatedUserThree.email, RelatedUser.RelatedUserStatus.CONFIRMED, "nickname"); + Persistence.monitoredTrips.create(trip); + dependentUserThree.delete(false); relatedUserThree = Persistence.otpUsers.getById(relatedUserThree.id); assertFalse(relatedUserThree.dependents.contains(dependentUserThree.id)); + + // If a dependent user deletes their profile, delete them from any trip where they are a dependent. + MonitoredTrip updatedTrip = Persistence.monitoredTrips.getById(trip.id); + assertNull(updatedTrip.primary); } /** From cb120c52c71bf94fd499e0c58a2f769293cb063a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 6 Dec 2024 09:15:54 -0500 Subject: [PATCH 2/4] fix(OtpUser): Delete companion/observer references upon user deletion. --- .../middleware/models/OtpUser.java | 22 +++++--- .../tripmonitor/TrustedCompanion.java | 52 +++++++++++++++++-- .../api/OtpUserControllerTest.java | 41 ++++++++++++--- 3 files changed, 98 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index ce908aaaf..787043da7 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -22,7 +22,10 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.invalidateRelatedUsers; +import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removeCompanion; import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removeDependent; +import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removeObserver; import static org.opentripplanner.middleware.tripmonitor.TrustedCompanion.removePrimaryTraveler; /** @@ -145,12 +148,7 @@ public boolean delete(boolean deleteAuth0User) { // If a related user, invalidate relationship with all dependents. for (String userId : dependents) { OtpUser dependent = Persistence.otpUsers.getById(userId); - if (dependent != null) { - for (RelatedUser relatedUser : dependent.relatedUsers) { - if (relatedUser.email.equals(this.email)) { - relatedUser.status = RelatedUser.RelatedUserStatus.INVALID; - } - } + if (dependent != null && invalidateRelatedUsers(email, dependent.relatedUsers)) { Persistence.otpUsers.replace(dependent.id, dependent); } } @@ -166,6 +164,18 @@ public boolean delete(boolean deleteAuth0User) { .getFiltered(Filters.eq("primary.userId", id)) .forEach(trip -> removePrimaryTraveler(this, trip)); + // If a companion user, invalidate relationship in trips where they are companions and observers. + // TODO: Should we alert the user who created the trip of the deletion? + Persistence.monitoredTrips + .getFiltered(Filters.eq("companion.email", email)) + .forEach(trip -> removeCompanion(this, trip)); + + // If a companion user, invalidate relationship in trips where they are companions and observers. + // TODO: Should we alert the user who created the trip of the deletion? + Persistence.monitoredTrips + .getFiltered(Filters.eq("observers.email", email)) + .forEach(trip -> removeObserver(this, trip)); + return Persistence.otpUsers.removeById(this.id); } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index fb15795d5..4f46b7dc6 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -22,6 +22,7 @@ import java.net.URLEncoder; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -255,11 +256,54 @@ public static void removeDependent(OtpUser dependent, RelatedUser relatedUser) { public static void removePrimaryTraveler(OtpUser otpUser, MonitoredTrip trip) { if (trip.primary != null && otpUser.id.equals(trip.primary.userId)) { trip.primary = null; - // Recheck existence of record in Mongo in case trip got deleted since last Mongo query. - if (Persistence.monitoredTrips.getById(trip.id) != null) { - Persistence.monitoredTrips.replace(trip.id, trip); - } + Persistence.monitoredTrips.replace(trip.id, trip); + } + } + + /** + * Remove the specified user as companion from the specified trip. + */ + public static void removeCompanion(OtpUser otpUser, MonitoredTrip trip) { + if (invalidateRelatedUser(otpUser.email, trip.companion)) { + Persistence.monitoredTrips.replace(trip.id, trip); + } + } + + /** + * Remove the specified user as observer from the specified trip. + */ + public static void removeObserver(OtpUser otpUser, MonitoredTrip trip) { + if (invalidateRelatedUsers(otpUser.email, trip.observers)) { + Persistence.monitoredTrips.replace(trip.id, trip); + } + } + + /** + * Helper to invalidate a RelatedUser (companion) relationship. + * @return true if the status changed to invalid, false otherwise. + */ + public static boolean invalidateRelatedUser(String email, RelatedUser relatedUser) { + if ( + relatedUser != null && + email.equals(relatedUser.email) && + relatedUser.status == RelatedUser.RelatedUserStatus.CONFIRMED + ) { + relatedUser.status = RelatedUser.RelatedUserStatus.INVALID; + return true; + } + return false; + } + + /** + * Remove the specified user as observer from the specified trip. + * @return true if one or more related users gor invalidated, false otherwise. + */ + public static boolean invalidateRelatedUsers(String email, Collection relatedUsers) { + boolean hasChanged = false; + for (RelatedUser relatedUser : relatedUsers) { + hasChanged |= invalidateRelatedUser(email, relatedUser); } + return hasChanged; } /** diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index 6d41cf1f1..ac1648efe 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -32,6 +32,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -245,19 +246,45 @@ void canRemoveRelatedUserOnDelete() { Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree); // Create a monitored trip with dependentUserThree as primary traveler, relatedUserThree as companion. - MonitoredTrip trip = new MonitoredTrip(); - trip.id = UUID.randomUUID().toString(); - trip.primary = new MobilityProfileLite(dependentUserThree); - trip.companion = new RelatedUser(relatedUserThree.email, RelatedUser.RelatedUserStatus.CONFIRMED, "nickname"); - Persistence.monitoredTrips.create(trip); + MonitoredTrip tripWithPrimaryTraveler = new MonitoredTrip(); + tripWithPrimaryTraveler.id = UUID.randomUUID().toString(); + tripWithPrimaryTraveler.userId = relatedUserThree.id; + tripWithPrimaryTraveler.primary = new MobilityProfileLite(dependentUserThree); + tripWithPrimaryTraveler.companion = new RelatedUser( + relatedUserThree.email, + RelatedUser.RelatedUserStatus.CONFIRMED, + "nickname" + ); + Persistence.monitoredTrips.create(tripWithPrimaryTraveler); + + // Create a monitored trip with dependentUserThree as companion and observer to relatedUserThree. + // (In practice, a user cannot be both a companion and observer.) + MonitoredTrip tripWithCompanionAndObserver = new MonitoredTrip(); + tripWithCompanionAndObserver.id = UUID.randomUUID().toString(); + tripWithCompanionAndObserver.userId = relatedUserThree.id; + tripWithCompanionAndObserver.companion = new RelatedUser( + dependentUserThree.email, + RelatedUser.RelatedUserStatus.CONFIRMED, + "nickname" + ); + tripWithCompanionAndObserver.observers.add(tripWithCompanionAndObserver.companion); + Persistence.monitoredTrips.create(tripWithCompanionAndObserver); dependentUserThree.delete(false); relatedUserThree = Persistence.otpUsers.getById(relatedUserThree.id); assertFalse(relatedUserThree.dependents.contains(dependentUserThree.id)); // If a dependent user deletes their profile, delete them from any trip where they are a dependent. - MonitoredTrip updatedTrip = Persistence.monitoredTrips.getById(trip.id); - assertNull(updatedTrip.primary); + MonitoredTrip updatedTripWithPrimaryTraveler = Persistence.monitoredTrips.getById(tripWithPrimaryTraveler.id); + assertNull(updatedTripWithPrimaryTraveler.primary); + + // If a companion on a trip deletes their profile, invalidate them from any trip where they are a companion + // or an observer. + MonitoredTrip updatedTripWithCompanionAndObserver = Persistence.monitoredTrips.getById(tripWithCompanionAndObserver.id); + assertNotNull(updatedTripWithCompanionAndObserver.companion); + assertEquals(RelatedUser.RelatedUserStatus.INVALID, updatedTripWithCompanionAndObserver.companion.status); + assertFalse(updatedTripWithCompanionAndObserver.observers.isEmpty()); + assertEquals(RelatedUser.RelatedUserStatus.INVALID, updatedTripWithCompanionAndObserver.observers.get(0).status); } /** From 528d8bbb2e4bbbb431752b5d6e418f96e62ccf34 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 16 Dec 2024 10:28:35 -0500 Subject: [PATCH 3/4] docs(TrustedCompanion): Fix typo. --- .../middleware/tripmonitor/TrustedCompanion.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java index 4f46b7dc6..59c59b1a7 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java @@ -296,7 +296,7 @@ public static boolean invalidateRelatedUser(String email, RelatedUser relatedUse /** * Remove the specified user as observer from the specified trip. - * @return true if one or more related users gor invalidated, false otherwise. + * @return true if one or more related users got invalidated, false otherwise. */ public static boolean invalidateRelatedUsers(String email, Collection relatedUsers) { boolean hasChanged = false; From 501dcca21dc46f0afea02dc68343a83b371f2436 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:07:33 -0500 Subject: [PATCH 4/4] refactor(OtpUser): Remove redundant comment. --- .../java/org/opentripplanner/middleware/models/OtpUser.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 787043da7..e766d685b 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -170,8 +170,6 @@ public boolean delete(boolean deleteAuth0User) { .getFiltered(Filters.eq("companion.email", email)) .forEach(trip -> removeCompanion(this, trip)); - // If a companion user, invalidate relationship in trips where they are companions and observers. - // TODO: Should we alert the user who created the trip of the deletion? Persistence.monitoredTrips .getFiltered(Filters.eq("observers.email", email)) .forEach(trip -> removeObserver(this, trip));