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

Cleanup trip times - part B #5525

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private static TripPattern delay(TripPattern pattern1, int seconds) {
}

private static TripTimes delay(TripTimes tt, int seconds) {
var delayed = tt.copyOfScheduledTimes();
var delayed = tt.copyScheduledTimes();
IntStream
.range(0, delayed.getNumStops())
.forEach(i -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import org.opentripplanner.transit.model.site.RegularStop;
import org.opentripplanner.transit.model.site.Station;
import org.opentripplanner.transit.model.timetable.OccupancyStatus;
import org.opentripplanner.transit.model.timetable.RealTimeTripTimes;
import org.opentripplanner.transit.model.timetable.Trip;
import org.opentripplanner.transit.model.timetable.TripTimes;
import org.opentripplanner.transit.model.timetable.TripTimesFactory;
import org.opentripplanner.transit.service.StopModel;
import uk.org.siri.siri20.OccupancyEnumeration;
Expand All @@ -46,7 +46,7 @@ public class TimetableHelperTest {
ZoneIds.CET
);

private TripTimes tripTimes;
private RealTimeTripTimes tripTimes;

@BeforeEach
public void setUp() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.opentripplanner.framework.time.ServiceDateUtils;
import org.opentripplanner.model.StopTime;
import org.opentripplanner.transit.model.basic.TransitMode;
import org.opentripplanner.transit.model.framework.DataValidationException;
import org.opentripplanner.transit.model.framework.FeedScopedId;
import org.opentripplanner.transit.model.framework.Result;
import org.opentripplanner.transit.model.network.Route;
Expand All @@ -27,11 +28,11 @@
import org.opentripplanner.transit.model.organization.Operator;
import org.opentripplanner.transit.model.site.RegularStop;
import org.opentripplanner.transit.model.timetable.RealTimeState;
import org.opentripplanner.transit.model.timetable.RealTimeTripTimes;
import org.opentripplanner.transit.model.timetable.Trip;
import org.opentripplanner.transit.model.timetable.TripTimes;
import org.opentripplanner.transit.model.timetable.TripTimesFactory;
import org.opentripplanner.transit.service.TransitModel;
import org.opentripplanner.updater.spi.TripTimesValidationMapper;
import org.opentripplanner.updater.spi.DataValidationExceptionMapper;
import org.opentripplanner.updater.spi.UpdateError;
import org.rutebanken.netex.model.BusSubmodeEnumeration;
import org.rutebanken.netex.model.RailSubmodeEnumeration;
Expand Down Expand Up @@ -202,14 +203,14 @@ Result<TripUpdate, UpdateError> build() {
.withStopPattern(stopPattern)
.build();

TripTimes tripTimes = TripTimesFactory.tripTimes(
RealTimeTripTimes tripTimes = TripTimesFactory.tripTimes(
trip,
aimedStopTimes,
transitModel.getDeduplicator()
);
tripTimes.setServiceCode(transitModel.getServiceCodes().get(trip.getServiceId()));
pattern.add(tripTimes);
TripTimes updatedTripTimes = tripTimes.copyOfScheduledTimes();
RealTimeTripTimes updatedTripTimes = tripTimes.copyScheduledTimes();

// Loop through calls again and apply updates
for (int stopSequence = 0; stopSequence < calls.size(); stopSequence++) {
Expand All @@ -231,9 +232,10 @@ Result<TripUpdate, UpdateError> build() {
}

/* Validate */
var validityResult = updatedTripTimes.validateNonIncreasingTimes();
if (validityResult.isPresent()) {
return TripTimesValidationMapper.toResult(tripId, validityResult.get());
try {
updatedTripTimes.validateNonIncreasingTimes();
} catch (DataValidationException e) {
return DataValidationExceptionMapper.toResult(e);
}

// Adding trip to index necessary to include values in graphql-queries
Expand Down
t2gran marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@
import java.util.Set;
import org.opentripplanner.ext.siri.mapper.PickDropMapper;
import org.opentripplanner.framework.time.ServiceDateUtils;
import org.opentripplanner.transit.model.framework.FeedScopedId;
import org.opentripplanner.transit.model.framework.DataValidationException;
import org.opentripplanner.transit.model.framework.Result;
import org.opentripplanner.transit.model.network.StopPattern;
import org.opentripplanner.transit.model.network.TripPattern;
import org.opentripplanner.transit.model.site.RegularStop;
import org.opentripplanner.transit.model.site.StopLocation;
import org.opentripplanner.transit.model.timetable.RealTimeState;
import org.opentripplanner.transit.model.timetable.RealTimeTripTimes;
import org.opentripplanner.transit.model.timetable.TripTimes;
import org.opentripplanner.updater.spi.TripTimesValidationMapper;
import org.opentripplanner.updater.spi.DataValidationExceptionMapper;
import org.opentripplanner.updater.spi.UpdateError;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -94,7 +95,7 @@ public ModifiedTripBuilder(
* in form the SIRI-ET update.
*/
public Result<TripUpdate, UpdateError> build() {
TripTimes newTimes = existingTripTimes.copyOfScheduledTimes();
RealTimeTripTimes newTimes = existingTripTimes.copyScheduledTimes();

StopPattern stopPattern = createStopPattern(pattern, calls, entityResolver);

Expand All @@ -114,24 +115,24 @@ public Result<TripUpdate, UpdateError> build() {
newTimes.setRealTimeState(RealTimeState.MODIFIED);
}

var error = newTimes.validateNonIncreasingTimes();
final FeedScopedId id = newTimes.getTrip().getId();
if (error.isPresent()) {
var updateError = error.get();
// TODO - Handle DataValidationException at the outemost level(pr trip)
try {
newTimes.validateNonIncreasingTimes();
} catch (DataValidationException e) {
LOG.info(
"Invalid SIRI-ET data for trip {} - TripTimes are non-increasing after applying SIRI delay propagation at stop index {}",
id,
updateError.stopIndex()
"Invalid SIRI-ET data for trip {} - TripTimes failed to validate after applying SIRI delay propagation. {}",
newTimes.getTrip().getId(),
e.getMessage()
);
return TripTimesValidationMapper.toResult(id, updateError);
return DataValidationExceptionMapper.toResult(e);
}

int numStopsInUpdate = newTimes.getNumStops();
int numStopsInPattern = pattern.numberOfStops();
if (numStopsInUpdate != numStopsInPattern) {
LOG.info(
"Invalid SIRI-ET data for trip {} - Inconsistent number of updated stops ({}) and stops in pattern ({})",
id,
newTimes.getTrip().getId(),
numStopsInUpdate,
numStopsInPattern
);
Expand All @@ -145,7 +146,7 @@ public Result<TripUpdate, UpdateError> build() {
/**
* Applies real-time updates from the calls into newTimes.
*/
private void applyUpdates(TripTimes newTimes) {
private void applyUpdates(RealTimeTripTimes newTimes) {
ZonedDateTime startOfService = ServiceDateUtils.asStartOfService(serviceDate, zoneId);
Set<CallWrapper> alreadyVisited = new HashSet<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@
import org.opentripplanner.model.TimetableSnapshot;
import org.opentripplanner.model.TimetableSnapshotProvider;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.TransitLayerUpdater;
import org.opentripplanner.transit.model.framework.DataValidationException;
import org.opentripplanner.transit.model.framework.Result;
import org.opentripplanner.transit.model.network.TripPattern;
import org.opentripplanner.transit.model.timetable.RealTimeTripTimes;
import org.opentripplanner.transit.model.timetable.Trip;
import org.opentripplanner.transit.model.timetable.TripTimes;
import org.opentripplanner.transit.service.DefaultTransitService;
import org.opentripplanner.transit.service.TransitModel;
import org.opentripplanner.transit.service.TransitService;
import org.opentripplanner.updater.TimetableSnapshotSourceParameters;
import org.opentripplanner.updater.spi.DataValidationExceptionMapper;
import org.opentripplanner.updater.spi.UpdateError;
import org.opentripplanner.updater.spi.UpdateResult;
import org.opentripplanner.updater.spi.UpdateSuccess;
Expand Down Expand Up @@ -215,6 +218,8 @@ private Result<UpdateSuccess, UpdateError> apply(

/* commit */
return addTripToGraphAndBuffer(result.successValue());
} catch (DataValidationException e) {
return DataValidationExceptionMapper.toResult(e);
} catch (Exception e) {
LOG.warn(
"{} EstimatedJourney {} failed.",
Expand Down Expand Up @@ -395,7 +400,7 @@ private boolean markScheduledTripAsDeleted(Trip trip, final LocalDate serviceDat
if (tripTimes == null) {
LOG.warn("Could not mark scheduled trip as deleted {}", trip.getId());
} else {
final TripTimes newTripTimes = tripTimes.copyOfScheduledTimes();
final RealTimeTripTimes newTripTimes = tripTimes.copyScheduledTimes();
newTripTimes.deleteTrip();
buffer.update(pattern, newTripTimes, serviceDate);
success = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.opentripplanner.ext.siri.mapper.OccupancyMapper;
import org.opentripplanner.framework.i18n.NonLocalizedString;
import org.opentripplanner.framework.time.ServiceDateUtils;
import org.opentripplanner.transit.model.timetable.TripTimes;
import org.opentripplanner.transit.model.timetable.RealTimeTripTimes;
import uk.org.siri.siri20.NaturalLanguageStringStructure;
import uk.org.siri.siri20.OccupancyEnumeration;

Expand Down Expand Up @@ -52,7 +52,7 @@ private static int handleMissingRealtime(int... times) {

public static void applyUpdates(
ZonedDateTime departureDate,
TripTimes tripTimes,
RealTimeTripTimes tripTimes,
int index,
boolean isLastStop,
boolean isJourneyPredictionInaccurate,
Expand Down

This file was deleted.

37 changes: 37 additions & 0 deletions src/main/java/org/opentripplanner/framework/error/OtpError.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.opentripplanner.framework.error;

import java.util.Locale;

/**
* A generic representation of an error. The error should have a code used to group the same
* type of errors together. To avoid filling up memory with error strings during graph build
* we store errors in memory "decomposed". The {@link #messageTemplate()} and
* {@link #messageArguments()} is used to construct the message. Use the {@link Locale#ROOT}
* when constructing the message - we only support english with SI formatting.
*/
public interface OtpError {
/**
* An error code used to identify the error type. This is NOT an enum, but feel free
* to use an inum in the implementation, then use the enum NAME as the code or
* enum TYPE:NAME. Then name should be unique within OTP.
*/
String errorCode();

/**
* The string template used to create a human-readable error message. Use the
* {@link String#format(Locale, String, Object...)} format.
*/
String messageTemplate();

/**
* The arguments to inject into the message.
*/
Object[] messageArguments();

/**
* Construct a message.
*/
default String message() {
return String.format(Locale.ROOT, messageTemplate(), messageArguments());
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.graph_builder.issue.api;

import java.util.List;
import org.opentripplanner.framework.error.OtpError;

/**
* This service is used to store issued during data import. When the import is complete
Expand All @@ -19,6 +20,9 @@ public interface DataImportIssueStore {
/** Add an issue to the issue report. */
void add(DataImportIssue issue);

/** Add an issue to the issue report. */
void add(OtpError issue);

/** Add an issue to the issue report without the need of creating an issue class. */
void add(String type, String message);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.graph_builder.issue.api;

import java.util.List;
import org.opentripplanner.framework.error.OtpError;

/**
* A no-op implementation of the issue store, convenient for unit testing. No issues are
Expand All @@ -11,6 +12,9 @@ class NoopDataImportIssueStore implements DataImportIssueStore {
@Override
public void add(DataImportIssue issue) {}

@Override
public void add(OtpError issue) {}

@Override
public void add(String type, String message) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import jakarta.inject.Singleton;
import java.util.ArrayList;
import java.util.List;
import org.opentripplanner.framework.error.OtpError;
import org.opentripplanner.graph_builder.issue.api.DataImportIssue;
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
import org.opentripplanner.graph_builder.issue.api.Issue;
Expand Down Expand Up @@ -30,6 +31,11 @@ public void add(DataImportIssue issue) {
}
}

@Override
public void add(OtpError issue) {
add(issue.errorCode(), issue.messageTemplate(), issue.messageArguments());
}

@Override
public void add(String type, String message) {
add(Issue.issue(type, message));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@
import java.util.Map;
import java.util.Set;
import org.opentripplanner.ext.flex.trip.FlexTrip;
import org.opentripplanner.framework.logging.ProgressTracker;
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
import org.opentripplanner.graph_builder.issues.TripDegenerate;
import org.opentripplanner.graph_builder.issues.TripUndefinedService;
import org.opentripplanner.graph_builder.module.geometry.GeometryProcessor;
import org.opentripplanner.model.Frequency;
import org.opentripplanner.model.StopTime;
import org.opentripplanner.model.impl.OtpTransitServiceBuilder;
import org.opentripplanner.transit.model.framework.DataValidationException;
import org.opentripplanner.transit.model.framework.Deduplicator;
import org.opentripplanner.transit.model.framework.FeedScopedId;
import org.opentripplanner.transit.model.network.Route;
Expand Down Expand Up @@ -47,7 +49,6 @@ public class GenerateTripPatternsOperation {
private final Multimap<StopPattern, TripPattern> tripPatterns;
private final ListMultimap<Trip, Frequency> frequenciesForTrip = ArrayListMultimap.create();

private int tripCount = 0;
private int freqCount = 0;
private int scheduledCount = 0;

Expand All @@ -70,17 +71,21 @@ public void run() {
collectFrequencyByTrip();

final Collection<Trip> trips = transitDaoBuilder.getTripsById().values();
final int tripsSize = trips.size();
var progressLogger = ProgressTracker.track("build trip patterns", 50_000, trips.size());
LOG.info(progressLogger.startMessage());

/* Loop over all trips, handling each one as a frequency-based or scheduled trip. */
for (Trip trip : trips) {
if (++tripCount % 100000 == 0) {
LOG.debug("build trip patterns {}/{}", tripCount, tripsSize);
try {
buildTripPatternForTrip(trip);
//noinspection Convert2MethodRef
progressLogger.step(m -> LOG.info(m));
} catch (DataValidationException e) {
issueStore.add(e.error());
}

buildTripPatternForTrip(trip);
}

LOG.info(progressLogger.completeMessage());
LOG.info(
"Added {} frequency-based and {} single-trip timetable entries.",
freqCount,
Expand Down
Loading
Loading