Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate delete companion #278

Merged
merged 5 commits into from
Dec 19, 2024
Merged

Propagate delete companion #278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f44faf2
fix(OtpUser): Delete primary traveler references upon user deletion.
binh-dam-ibigroup Dec 5, 2024
cb120c5
fix(OtpUser): Delete companion/observer references upon user deletion.
binh-dam-ibigroup Dec 6, 2024
528d8bb
docs(TrustedCompanion): Fix typo.
binh-dam-ibigroup Dec 16, 2024
501dcca
refactor(OtpUser): Remove redundant comment.
binh-dam-ibigroup Dec 19, 2024
9beaf93
Merge branch 'dev' into propagate-delete-companion
binh-dam-ibigroup Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions src/main/java/org/opentripplanner/middleware/models/OtpUser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.apache.logging.log4j.util.Strings;
import org.opentripplanner.middleware.auth.Auth0Users;
import org.opentripplanner.middleware.auth.RequestingUser;
Expand All @@ -22,7 +23,11 @@
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;

/**
* This represents a user of an OpenTripPlanner instance (typically of the standard OTP UI/otp-react-redux).
Expand Down Expand Up @@ -144,20 +149,32 @@ 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);
}
}

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably a case here for just deleting the trip if the primary traveler is no longer travelling. Perhaps that would start to complicate things though?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation. Could preventing tracking if a companion is set but no primary traveler is defined be a better way to handle that?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think deleting the trip makes sense if there is no primary traveler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created issue #282 to track that.

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

Persistence.monitoredTrips
.getFiltered(Filters.eq("observers.email", email))
.forEach(trip -> removeObserver(this, trip));

return Persistence.otpUsers.removeById(this.id);
}

Expand Down
58 changes: 58 additions & 0 deletions src/main/java/org/opentripplanner/middleware/tripmonitor/TrustedCompanion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -21,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;
Expand Down Expand Up @@ -248,6 +250,62 @@ 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;
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 got invalidated, false otherwise.
*/
public static boolean invalidateRelatedUsers(String email, Collection<RelatedUser> relatedUsers) {
boolean hasChanged = false;
for (RelatedUser relatedUser : relatedUsers) {
hasChanged |= invalidateRelatedUser(email, relatedUser);
}
return hasChanged;
}

/**
* Retrieve the mobility profile for a dependent providing the requesting user is a trusted companion.
*/
Expand Down
41 changes: 41 additions & 0 deletions src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,6 +32,8 @@

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;
import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled;
Expand Down Expand Up @@ -241,9 +244,47 @@ void canRemoveRelatedUserOnDelete() {
nickname
));
Persistence.otpUsers.replace(dependentUserThree.id, dependentUserThree);

// Create a monitored trip with dependentUserThree as primary traveler, relatedUserThree as companion.
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 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);
}

/**
Expand Down
Loading