Skip to content

Commit 38e9557

Browse files
evansirokyevansiroky
committed
refactor(trip monitor): refactor trip monitor timing
Refs #70
1 parent c293bdc commit 38e9557

File tree

17 files changed

+436
-248
lines changed

17 files changed

+436
-248
lines changed

‎configurations/test/env.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ MONGO_DB_NAME: otp_middleware_test
2222
#MONGO_PROTOCOL: mongodb+srv
2323
#MONGO_USER: user
2424

25-
OTP_API_ROOT: http://otp-server.example.com/otp
26-
OTP_SERVER: http://otp-server.example.com/otp
27-
OTP_SERVER_PLAN_END_POINT: /routers/default/plan
25+
OTP_API_ROOT: http://localhost:8080/otp
26+
OTP_SERVER: http://localhost:8080/otp
27+
OTP_PLAN_ENDPOINT: /routers/default/plan
2828
MAXIMUM_PERMITTED_MONITORED_TRIPS: 5
2929

3030
# Uncomment and provide info for running disabled notification tests.

‎src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ private T createOrUpdate(Request req, Response res) {
329329
logMessageAndHalt(req, HttpStatus.FORBIDDEN_403, String.format("Requesting user not authorized to update %s.", className));
330330
}
331331
// Update last updated value.
332-
object.lastUpdated = new Date();
332+
object.lastUpdated = DateTimeUtils.nowAsDate();
333333
// Pin the date created to pre-existing value.
334334
object.dateCreated = preExistingObject.dateCreated;
335335
// Validate that ID in JSON body matches ID param. TODO add test

‎src/main/java/org/opentripplanner/middleware/controllers/api/OtpRequestProcessor.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import org.opentripplanner.middleware.models.TripSummary;
1010
import org.opentripplanner.middleware.otp.OtpDispatcher;
1111
import org.opentripplanner.middleware.otp.OtpDispatcherResponse;
12-
import org.opentripplanner.middleware.otp.response.Response;
12+
import org.opentripplanner.middleware.otp.response.OtpResponse;
1313
import org.opentripplanner.middleware.persistence.Persistence;
1414
import org.opentripplanner.middleware.utils.DateTimeUtils;
1515
import org.opentripplanner.middleware.utils.HttpUtils;
@@ -25,7 +25,6 @@
2525
import static javax.ws.rs.core.MediaType.APPLICATION_XML;
2626
import static org.opentripplanner.middleware.auth.Auth0Connection.isAuthHeaderPresent;
2727
import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText;
28-
import static org.opentripplanner.middleware.otp.OtpDispatcher.OTP_API_ROOT;
2928
import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt;
3029

3130
/**
@@ -139,7 +138,7 @@ private static void handlePlanTripResponse(Request request, OtpDispatcherRespons
139138
if (!storeTripHistory) {
140139
LOG.debug("User does not want trip history stored");
141140
} else {
142-
Response otpResponse = otpDispatcherResponse.getResponse();
141+
OtpResponse otpResponse = otpDispatcherResponse.getResponse();
143142
if (otpResponse == null) {
144143
LOG.warn("OTP response is null, cannot save trip history for user!");
145144
} else {

‎src/main/java/org/opentripplanner/middleware/models/JourneyState.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ public JourneyState(MonitoredTrip monitoredTrip) {
3535
/**
3636
* Timestamp checking the last time a journey was checked.
3737
*/
38-
public long lastChecked;
38+
public long lastCheckedMillis;
3939

4040
public Set<TripMonitorNotification> lastNotifications = new HashSet<>();
4141

42-
public long lastNotificationTime;
42+
public long lastNotificationTimeMillis;
4343

4444
public Set<LocalizedAlert> lastSeenAlerts = new HashSet<>();
4545

@@ -67,13 +67,13 @@ public JourneyState(MonitoredTrip monitoredTrip) {
6767
*/
6868
public void update(CheckMonitoredTrip checkMonitoredTripJob) {
6969
targetDate = checkMonitoredTripJob.targetDate;
70-
lastChecked = DateTimeUtils.currentTimeMillis();
70+
lastCheckedMillis = DateTimeUtils.currentTimeMillis();
7171
matchingItinerary = checkMonitoredTripJob.matchingItinerary;
7272
lastDepartureDelay = checkMonitoredTripJob.departureDelay;
7373
lastArrivalDelay = checkMonitoredTripJob.arrivalDelay;
7474
// Update notification time if notification successfully sent.
75-
if (checkMonitoredTripJob.notificationTimestamp != -1) {
76-
lastNotificationTime = checkMonitoredTripJob.notificationTimestamp;
75+
if (checkMonitoredTripJob.notificationTimestampMillis != -1) {
76+
lastNotificationTimeMillis = checkMonitoredTripJob.notificationTimestampMillis;
7777
}
7878
Persistence.journeyStates.replace(this.id, this);
7979
}

‎src/main/java/org/opentripplanner/middleware/models/Model.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.opentripplanner.middleware.models;
22

33
import org.opentripplanner.middleware.auth.Auth0UserProfile;
4+
import org.opentripplanner.middleware.utils.DateTimeUtils;
45

56
import java.io.Serializable;
67
import java.util.Date;
@@ -16,8 +17,8 @@ public Model () {
1617
// This autogenerates an ID
1718
// this is OK for dump/restore, because the ID will simply be overridden
1819
this.id = UUID.randomUUID().toString();
19-
this.lastUpdated = new Date();
20-
this.dateCreated = new Date();
20+
this.lastUpdated = DateTimeUtils.nowAsDate();
21+
this.dateCreated = DateTimeUtils.nowAsDate();
2122
}
2223
public String id;
2324
public Date lastUpdated;

‎src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java

+18-16
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import org.apache.http.NameValuePair;
44
import org.apache.http.client.utils.URLEncodedUtils;
5+
import org.bson.codecs.pojo.annotations.BsonIgnore;
56
import org.bson.conversions.Bson;
67
import org.opentripplanner.middleware.auth.Auth0UserProfile;
78
import org.opentripplanner.middleware.auth.Permission;
@@ -136,17 +137,22 @@ public class MonitoredTrip extends Model {
136137
*/
137138
public boolean notifyOnItineraryChange = true;
138139

139-
private transient JourneyState journeyState;
140-
141-
private transient List<NameValuePair> parsedParams;
142-
143140
public MonitoredTrip() {
144141
}
145142

146-
public MonitoredTrip(OtpDispatcherResponse otpDispatcherResponse) {
143+
public MonitoredTrip(OtpDispatcherResponse otpDispatcherResponse) throws URISyntaxException {
147144
queryParams = otpDispatcherResponse.requestUri.getQuery();
148145
TripPlan plan = otpDispatcherResponse.getResponse().plan;
149146
itinerary = plan.itineraries.get(0);
147+
148+
// extract trip time from parsed params
149+
List<NameValuePair> parsedParams = getParsedParams();
150+
for (NameValuePair parsedParam : parsedParams) {
151+
if (parsedParam.getName().equals("time")) {
152+
tripTime = parsedParam.getValue();
153+
break;
154+
}
155+
}
150156
initializeFromItinerary();
151157
}
152158

@@ -250,10 +256,8 @@ private Bson tripIdFilter() {
250256
* Get the journey state for this trip.
251257
*/
252258
public JourneyState retrieveJourneyState() {
253-
// first return the journeyState for this trip if it has already been fetched
254-
if (journeyState != null) return journeyState;
255-
// hasn't been fetched, attempt to retrieve from the db
256-
journeyState = Persistence.journeyStates.getOneFiltered(tripIdFilter());
259+
// attempt to retrieve from the db
260+
JourneyState journeyState = Persistence.journeyStates.getOneFiltered(tripIdFilter());
257261
// If journey state does not exist, create and persist.
258262
if (journeyState == null) {
259263
journeyState = new JourneyState(this);
@@ -300,19 +304,14 @@ public boolean delete() {
300304
}
301305

302306
public List<NameValuePair> getParsedParams() throws URISyntaxException {
303-
// use the transient value of parsedParams if it is available
304-
if (parsedParams != null) return parsedParams;
305-
306-
// need to parse the params
307-
parsedParams = URLEncodedUtils.parse(
307+
return URLEncodedUtils.parse(
308308
new URI(String.format("http://example.com/%s", queryParams)),
309309
UTF_8
310310
);
311-
return parsedParams;
312311
}
313312

314313
public boolean isArriveBy() throws URISyntaxException {
315-
for (NameValuePair param : parsedParams) {
314+
for (NameValuePair param : getParsedParams()) {
316315
if (param.getName().equals("arriveBy")) {
317316
return param.getValue().equals("true");
318317
}
@@ -328,6 +327,7 @@ public boolean isArriveBy() throws URISyntaxException {
328327
* use the local time at the destination. Therefore, this method will return the timezone at the destination if this
329328
* trip is an arriveBy trip, or the timezone at the origin if the trip is a depart at trip.
330329
*/
330+
@BsonIgnore
331331
public ZoneId getTimezoneForTargetLocation() throws URISyntaxException {
332332
double lat, lon;
333333
if (isArriveBy()) {
@@ -353,13 +353,15 @@ public ZoneId getTimezoneForTargetLocation() throws URISyntaxException {
353353
/**
354354
* Returns the target hour of the day that the trip is either departing at or arriving by
355355
*/
356+
@BsonIgnore
356357
public int getHour() {
357358
return Integer.valueOf(tripTime.split(":")[0]);
358359
}
359360

360361
/**
361362
* Returns the target minute of the hour that the trip is either departing at or arriving by
362363
*/
364+
@BsonIgnore
363365
public int getMinute() {
364366
return Integer.valueOf(tripTime.split(":")[1]);
365367
}

‎src/main/java/org/opentripplanner/middleware/otp/OtpDispatcher.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import javax.ws.rs.core.UriBuilder;
88
import java.io.IOException;
99
import java.net.URI;
10+
import java.net.URISyntaxException;
1011
import java.net.http.HttpClient;
1112
import java.net.http.HttpRequest;
1213
import java.net.http.HttpResponse;
@@ -87,10 +88,11 @@ private static OtpDispatcherResponse sendOtpRequest(URI uri) {
8788
// Get response from OTP
8889
OtpDispatcherResponse otpDispatcherResponse = null;
8990
try {
91+
LOG.info("Sending request to OTP: {}", uri.toString());
9092
HttpResponse<String> otpResponse = client.send(request, HttpResponse.BodyHandlers.ofString());
9193
otpDispatcherResponse = new OtpDispatcherResponse(otpResponse);
9294
} catch (InterruptedException | IOException e) {
93-
LOG.error("Error requesting OTP data", e);
95+
LOG.error("Error requesting OTP data from {}", uri, e);
9496
}
9597
return otpDispatcherResponse;
9698
}

‎src/main/java/org/opentripplanner/middleware/otp/OtpDispatcherResponse.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33

44
import org.apache.commons.lang3.SerializationUtils;
5-
import org.opentripplanner.middleware.otp.response.Response;
5+
import org.opentripplanner.middleware.otp.response.OtpResponse;
66
import org.opentripplanner.middleware.utils.JsonUtils;
77
import org.slf4j.Logger;
88
import org.slf4j.LoggerFactory;
@@ -55,11 +55,11 @@ public OtpDispatcherResponse(String otpResponse) {
5555
* Response. POJO version of response from an OTP server.
5656
* Do not persist in case these classes change. This should always be re-instantiated from responseBody if needed.
5757
*/
58-
public Response getResponse() {
59-
return JsonUtils.getPOJOFromJSON(responseBody, Response.class);
58+
public OtpResponse getResponse() {
59+
return JsonUtils.getPOJOFromJSON(responseBody, OtpResponse.class);
6060
}
6161

62-
public void setResponse(Response response) {
62+
public void setResponse(OtpResponse response) {
6363
responseBody = JsonUtils.toJson(response);
6464
}
6565

‎src/main/java/org/opentripplanner/middleware/otp/response/Itinerary.java

+14-3
Original file line numberDiff line numberDiff line change
@@ -98,20 +98,31 @@ public void clearAlerts() {
9898
}
9999
}
100100

101+
/**
102+
* This method calculates equality in the context of trip monitoring in order to analyzing equality when
103+
* checking if itineraries match.
104+
*
105+
* FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
106+
*/
101107
@Override
102108
public boolean equals(Object o) {
103109
if (this == o) return true;
104110
if (o == null || getClass() != o.getClass()) return false;
105111
Itinerary itinerary = (Itinerary) o;
106-
return startTime.equals(itinerary.startTime) &&
107-
endTime.equals(itinerary.endTime) &&
112+
return duration.equals(itinerary.duration) &&
108113
Objects.equals(transfers, itinerary.transfers) &&
109114
legs.equals(itinerary.legs);
110115
}
111116

117+
/**
118+
* This method calculates the hash code in the context of trip monitoring in order to analyzing equality when
119+
* checking if itineraries match.
120+
*
121+
* FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
122+
*/
112123
@Override
113124
public int hashCode() {
114-
return Objects.hash(startTime, endTime, transfers, legs);
125+
return Objects.hash(duration, transfers, legs);
115126
}
116127

117128
@Override

‎src/main/java/org/opentripplanner/middleware/otp/response/Leg.java

+8-6
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,15 @@ public class Leg implements Cloneable {
5151
/**
5252
* This method calculates equality in the context of trip monitoring in order to analyzing equality when
5353
* checking if itineraries are the same.
54+
*
55+
* FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
5456
*/
5557
@Override
5658
public boolean equals(Object o) {
5759
if (this == o) return true;
5860
if (o == null || getClass() != o.getClass()) return false;
5961
Leg leg = (Leg) o;
60-
return startTime.equals(leg.startTime) &&
61-
endTime.equals(leg.endTime) &&
62+
return duration.equals(leg.duration) &&
6263
mode.equals(leg.mode) &&
6364
from.equals(leg.from) &&
6465
to.equals(leg.to) &&
@@ -73,14 +74,15 @@ public boolean equals(Object o) {
7374
}
7475

7576
/**
76-
* This method calculates equality in the context of trip monitoring in order to analyzing equality when
77-
* checking if itineraries are the same.
77+
* This method calculates the hash code in the context of trip monitoring in order to analyzing equality when
78+
* checking if itineraries match.
79+
*
80+
* FIXME: maybe don't check duration exactly as it might vary slightly in certain trips
7881
*/
7982
@Override
8083
public int hashCode() {
8184
return Objects.hash(
82-
startTime,
83-
endTime,
85+
duration,
8486
mode,
8587
from,
8688
to,

‎src/main/java/org/opentripplanner/middleware/otp/response/Response.java ‎src/main/java/org/opentripplanner/middleware/otp/response/OtpResponse.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Pare down version of class original produced for OpenTripPlanner.
1212
*/
1313
@JsonIgnoreProperties(ignoreUnknown = true)
14-
public class Response {
14+
public class OtpResponse {
1515

1616
/** A dictionary of the parameters provided in the request that triggered this response. */
1717
public HashMap<String, String> requestParameters;

‎src/main/java/org/opentripplanner/middleware/persistence/TypedPersistence.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.bson.conversions.Bson;
1212
import org.opentripplanner.middleware.bugsnag.BugsnagReporter;
1313
import org.opentripplanner.middleware.models.Model;
14+
import org.opentripplanner.middleware.utils.DateTimeUtils;
1415
import org.slf4j.Logger;
1516
import org.slf4j.LoggerFactory;
1617

@@ -113,7 +114,7 @@ public void replace(String id, T replaceObject) {
113114
*/
114115
public T update(String id, Document updateDocument) {
115116
// Set last updated.
116-
updateDocument.put("lastUpdated", new Date());
117+
updateDocument.put("lastUpdated", DateTimeUtils.nowAsDate());
117118
return mongoCollection.findOneAndUpdate(eq(id), new Document("$set", updateDocument), findOneAndUpdateOptions);
118119
}
119120

0 commit comments

Comments
 (0)