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.

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

Trip sharing #271

Merged
merged 7 commits into from
Dec 3, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
);
}

/**
* Provides an entity filter for requests on entities that contain a userId field,
* whether the request is made with the userId param or a token.
*/
protected Bson getEntityFilter(OtpUser user) {
return Filters.eq(USER_ID_PARAM, user.id);
}

/**
* HTTP endpoint to get multiple entities based on the user permissions
*/
Expand All @@ -203,7 +211,7 @@ private ResponseList<T> getMany(Request req, Response res) {
if (userId != null) {
OtpUser otpUser = Persistence.otpUsers.getById(userId);
if (requestingUser.canManageEntity(otpUser)) {
return persistence.getResponseList(Filters.eq(USER_ID_PARAM, userId), offset, limit);
return persistence.getResponseList(getEntityFilter(otpUser), offset, limit);
} else {
res.status(HttpStatus.FORBIDDEN_403);
return null;
Expand Down Expand Up @@ -231,7 +239,7 @@ private ResponseList<T> getMany(Request req, Response res) {
} else {
// For all other cases the assumption is that the request is being made by an Otp user and the requested
// entities have a 'userId' parameter. Only entities that match the requesting user id are returned.
return persistence.getResponseList(Filters.eq(USER_ID_PARAM, requestingUser.otpUser.id), offset, limit);
return persistence.getResponseList(getEntityFilter(requestingUser.otpUser), offset, limit);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.eclipse.jetty.http.HttpStatus;
import org.opentripplanner.middleware.models.ItineraryExistence;
import org.opentripplanner.middleware.models.MonitoredTrip;
import org.opentripplanner.middleware.models.OtpUser;
import org.opentripplanner.middleware.persistence.Persistence;
import org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip;
import org.opentripplanner.middleware.tripmonitor.jobs.MonitoredTripLocks;
Expand Down Expand Up @@ -53,6 +54,16 @@ protected void buildEndpoint(ApiEndpoint baseEndpoint) {
super.buildEndpoint(modifiedEndpoint);
}

@Override
protected Bson getEntityFilter(OtpUser user) {
return Filters.or(
Filters.eq(USER_ID_PARAM, user.id),
Filters.eq("primary.userId", user.id),
Filters.eq("companion.email", user.email),
Filters.eq("observers.email", user.email)
br648 marked this conversation as resolved.
Show resolved Hide resolved
);
}

/**
* Before creating a {@link MonitoredTrip}, check that the itinerary associated with the trip exists on the selected
* days of the week. Update the itinerary if everything looks OK, otherwise halt the request.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public class MonitoredTrip extends Model {
*/
public boolean notifyAtLeadingInterval = true;

public RelatedUser primary;
public MobilityProfileLite primary;
public RelatedUser companion;
public List<RelatedUser> observers = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably out of scope, but should any updates to related users be reflected in monitored trips? For instance, if a related user is removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

@binh-dam-ibigroup just this to potentially address if you think it is in scope/important?

Copy link
Contributor

Choose a reason for hiding this comment

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

@binh-dam-ibigroup just this to potentially address if you think it is in scope/important?

@binh-dam-ibigroup I just need your thoughts on this and then I can approve if it is out of scope.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@br648 If you are ok with this, we can do delete propagation in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@binh-dam-ibigroup I think it is out of scope. Will approve.


Expand Down
17 changes: 14 additions & 3 deletions src/main/resources/latest-spark-swagger-output.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1686,10 +1686,10 @@ paths:
"200":
description: "Successful operation"
examples: {}
schema:
$ref: "#/definitions/MobilityProfileLite"
responseSchema:
$ref: "#/definitions/MobilityProfileLite"
schema:
$ref: "#/definitions/MobilityProfileLite"
"400":
description: "The request was not formed properly (e.g., some required parameters\
\ may be missing). See the details of the returned response to determine\
Expand Down Expand Up @@ -2536,6 +2536,17 @@ definitions:
$ref: "#/definitions/Fare"
details:
$ref: "#/definitions/FareDetails"
MobilityProfileLite:
type: "object"
properties:
userId:
type: "string"
mobilityMode:
type: "string"
email:
type: "string"
name:
type: "string"
LocalizedAlert:
type: "object"
properties:
Expand Down Expand Up @@ -2636,7 +2647,7 @@ definitions:
notifyAtLeadingInterval:
type: "boolean"
primary:
$ref: "#/definitions/RelatedUser"
$ref: "#/definitions/MobilityProfileLite"
companion:
$ref: "#/definitions/RelatedUser"
observers:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,29 @@
import org.opentripplanner.middleware.controllers.response.ResponseList;
import org.opentripplanner.middleware.models.AdminUser;
import org.opentripplanner.middleware.models.ItineraryExistence;
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.otp.response.Itinerary;
import org.opentripplanner.middleware.otp.response.Place;
import org.opentripplanner.middleware.persistence.Persistence;
import org.opentripplanner.middleware.testutils.ApiTestUtils;
import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment;
import org.opentripplanner.middleware.testutils.OtpTestUtils;
import org.opentripplanner.middleware.testutils.PersistenceTestUtils;
import org.opentripplanner.middleware.utils.HttpResponseValues;
import org.opentripplanner.middleware.utils.JsonUtils;

import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import static org.junit.jupiter.api.Assertions.assertEquals;
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;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled;
import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled;
Expand Down Expand Up @@ -66,7 +70,6 @@ public class MonitoredTripControllerTest extends OtpMiddlewareTestEnvironment {
private static final String UI_QUERY_PARAMS
= "?fromPlace=fromplace%3A%3A28.556631%2C-81.411781&toPlace=toplace%3A%3A28.545925%2C-81.348609&date=2020-11-13&time=14%3A21&arriveBy=false&mode=WALK%2CBUS%2CRAIL&numItineraries=3";
private static final String DUMMY_STRING = "ABCDxyz";
private static HashMap<String, String> guardianHeaders;

/**
* Create Otp and Admin user accounts. Create Auth0 account for just the Otp users. If
Expand Down Expand Up @@ -146,7 +149,6 @@ void canGetOwnMonitoredTrips() throws Exception {

// Multi Otp user has 'enhanced' admin credentials, still expect only 1 trip to be returned as the scope will
// limit the requesting user to a single 'otp-user' user type.
// TODO: Determine if a separate admin endpoint should be maintained for getting all/combined trips.
assertEquals(1, multiTrips.data.size());

// Get trips for only the multi Otp user by specifying Otp user id.
Expand Down Expand Up @@ -225,4 +227,77 @@ private static void createMonitoredTripForUser(OtpUser otpUser) {

Persistence.monitoredTrips.create(monitoredTrip);
}

@Test
void canGetSharedTrips() throws Exception {
MonitoredTrip ownTrip = new MonitoredTrip();
ownTrip.id = "shared-trips-own-trip";
ownTrip.userId = soloOtpUser.id;

RelatedUser companion = new RelatedUser();
companion.email = "[email protected]";

RelatedUser soloAsCompanion = new RelatedUser();
soloAsCompanion.email = soloOtpUser.email;

MonitoredTrip ownTripWithCompanion = new MonitoredTrip();
ownTripWithCompanion.id = "shared-trips-own-trip-with-companion";
ownTripWithCompanion.companion = companion;
ownTripWithCompanion.userId = soloOtpUser.id;

MonitoredTrip ownTripWithObservers = new MonitoredTrip();
ownTripWithObservers.id = "shared-trips-own-trip-with-observers";
ownTripWithObservers.observers = List.of(companion);
ownTripWithObservers.userId = soloOtpUser.id;

MobilityProfileLite soloAsPrimary = new MobilityProfileLite();
soloAsPrimary.userId = soloOtpUser.id;

MobilityProfileLite otherAsPrimary = new MobilityProfileLite();
otherAsPrimary.userId = multiOtpUser.id;

MonitoredTrip ownTripForDependent = new MonitoredTrip();
ownTripForDependent.id = "shared-trips-own-trip-for-dependent";
ownTripForDependent.primary = otherAsPrimary;
ownTripForDependent.userId = soloOtpUser.id;

MonitoredTrip otherTrip = new MonitoredTrip();
otherTrip.id = "shared-trips-other-trip";
otherTrip.userId = multiOtpUser.id;

MonitoredTrip otherTripWithSoloAsDependent = new MonitoredTrip();
otherTripWithSoloAsDependent.id = "shared-trips-other-trip-solo-primary";
otherTripWithSoloAsDependent.primary = soloAsPrimary;
otherTripWithSoloAsDependent.userId = multiOtpUser.id;

MonitoredTrip otherTripWithSoloAsCompanion = new MonitoredTrip();
otherTripWithSoloAsCompanion.id = "shared-trips-other-trip-solo-companion";
otherTripWithSoloAsCompanion.companion = soloAsCompanion;
otherTripWithSoloAsCompanion.userId = multiOtpUser.id;

MonitoredTrip otherTripWithSoloAsObserver = new MonitoredTrip();
otherTripWithSoloAsObserver.id = "shared-trips-other-trip-solo-observer";
otherTripWithSoloAsObserver.observers = List.of(soloAsCompanion);
otherTripWithSoloAsObserver.userId = multiOtpUser.id;

List<MonitoredTrip> trips = List.of(
ownTrip,
ownTripForDependent,
ownTripWithCompanion,
ownTripWithObservers,
otherTrip,
otherTripWithSoloAsDependent,
otherTripWithSoloAsCompanion,
otherTripWithSoloAsObserver
);
trips.forEach(Persistence.monitoredTrips::create);

List<MonitoredTrip> fetchedTrips = getMonitoredTripsForUser(MONITORED_TRIP_PATH, soloOtpUser).data;
assertEquals(trips.size() - 1, fetchedTrips.size());

Set<String> ids = trips.stream().map(t -> t.id).collect(Collectors.toSet());
ids.remove(otherTrip.id);
Set<String> fetchedIds = fetchedTrips.stream().map(t -> t.id).collect(Collectors.toSet());
assertTrue(ids.containsAll(fetchedIds));
}
}
Loading