diff --git a/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java b/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java index 080a1535284..99e9063418c 100644 --- a/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java +++ b/src/ext/java/org/opentripplanner/ext/siri/SiriTimetableSnapshotSource.java @@ -11,13 +11,9 @@ import java.time.LocalDate; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.locks.ReentrantLock; import javax.annotation.Nullable; -import org.opentripplanner.framework.time.CountdownTimer; import org.opentripplanner.model.Timetable; 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; @@ -32,6 +28,7 @@ import org.opentripplanner.updater.spi.UpdateError; import org.opentripplanner.updater.spi.UpdateResult; import org.opentripplanner.updater.spi.UpdateSuccess; +import org.opentripplanner.updater.trip.AbstractTimetableSnapshotSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import uk.org.siri.siri20.EstimatedTimetableDeliveryStructure; @@ -42,20 +39,10 @@ * necessary to provide planning threads a consistent constant view of a graph with real-time data at * a specific point in time. */ -public class SiriTimetableSnapshotSource implements TimetableSnapshotProvider { +public class SiriTimetableSnapshotSource extends AbstractTimetableSnapshotSource { private static final Logger LOG = LoggerFactory.getLogger(SiriTimetableSnapshotSource.class); - /** - * The working copy of the timetable snapshot. Should not be visible to routing threads. Should - * only be modified by a thread that holds a lock on {@link #bufferLock}. All public methods that - * might modify this buffer will correctly acquire the lock. - */ - private final TimetableSnapshot buffer = new TimetableSnapshot(); - /** - * Lock to indicate that buffer is in use - */ - private final ReentrantLock bufferLock = new ReentrantLock(true); /** * Use a id generator to generate TripPattern ids for new TripPatterns created by RealTime * updates. @@ -69,66 +56,22 @@ public class SiriTimetableSnapshotSource implements TimetableSnapshotProvider { private final TransitModel transitModel; private final TransitService transitService; - private final TransitLayerUpdater transitLayerUpdater; - - /** - * If a timetable snapshot is requested less than this number of milliseconds after the previous - * snapshot, just return the same one. Throttles the potentially resource-consuming task of - * duplicating a TripPattern -> Timetable map and indexing the new Timetables. - */ - protected CountdownTimer snapshotFrequencyThrottle; - - /** - * The last committed snapshot that was handed off to a routing thread. This snapshot may be given - * to more than one routing thread if the maximum snapshot frequency is exceeded. - */ - private volatile TimetableSnapshot snapshot = null; - - /** Should expired real-time data be purged from the graph. */ - private final boolean purgeExpiredData; - - protected LocalDate lastPurgeDate = null; public SiriTimetableSnapshotSource( TimetableSnapshotSourceParameters parameters, TransitModel transitModel ) { + super( + transitModel.getTransitLayerUpdater(), + parameters, + () -> LocalDate.now(transitModel.getTimeZone()) + ); this.transitModel = transitModel; this.transitService = new DefaultTransitService(transitModel); - this.transitLayerUpdater = transitModel.getTransitLayerUpdater(); - this.snapshotFrequencyThrottle = new CountdownTimer(parameters.maxSnapshotFrequency()); - this.purgeExpiredData = parameters.purgeExpiredData(); this.tripPatternCache = new SiriTripPatternCache(tripPatternIdGenerator, transitService::getPatternForTrip); transitModel.initTimetableSnapshotProvider(this); - - // Force commit so that snapshot initializes - commitTimetableSnapshot(true); - } - - /** - * @return an up-to-date snapshot mapping TripPatterns to Timetables. This snapshot and the - * timetable objects it references are guaranteed to never change, so the requesting thread is - * provided a consistent view of all TripTimes. The routing thread need only release its reference - * to the snapshot to release resources. - */ - public TimetableSnapshot getTimetableSnapshot() { - TimetableSnapshot snapshotToReturn; - - // Try to get a lock on the buffer - if (bufferLock.tryLock()) { - // Make a new snapshot if necessary - try { - commitTimetableSnapshot(false); - return snapshot; - } finally { - bufferLock.unlock(); - } - } - // No lock could be obtained because there is either a snapshot commit busy or updates - // are applied at this moment, just return the current snapshot - return snapshot; } /** @@ -152,12 +95,9 @@ public UpdateResult applyEstimatedTimetable( return UpdateResult.empty(); } - // Acquire lock on buffer - bufferLock.lock(); - List> results = new ArrayList<>(); - try { + withLock(() -> { if (fullDataset) { // Remove all updates from the buffer buffer.clear(feedId); @@ -175,19 +115,9 @@ public UpdateResult applyEstimatedTimetable( LOG.debug("message contains {} trip updates", updates.size()); - // Make a snapshot after each message in anticipation of incoming requests - // Purge data if necessary (and force new snapshot if anything was purged) - // Make sure that the public (locking) getTimetableSnapshot function is not called. - if (purgeExpiredData) { - final boolean modified = purgeExpiredData(); - commitTimetableSnapshot(modified); - } else { - commitTimetableSnapshot(false); - } - } finally { - // Always release lock - bufferLock.unlock(); - } + purgeAndCommit(); + }); + return UpdateResult.ofResults(results); } @@ -249,31 +179,13 @@ private boolean shouldAddNewTrip( return entityResolver.resolveTrip(vehicleJourney) == null; } - private void commitTimetableSnapshot(final boolean force) { - if (force || snapshotFrequencyThrottle.timeIsUp()) { - if (force || buffer.isDirty()) { - LOG.debug("Committing {}", buffer); - snapshot = buffer.commit(transitLayerUpdater, force); - - // We only reset the timer when the snapshot is updated. This will cause the first - // update to be committed after a silent period. This should not have any effect in - // a busy updater. It is however useful when manually testing the updater. - snapshotFrequencyThrottle.restart(); - } else { - LOG.debug("Buffer was unchanged, keeping old snapshot."); - } - } else { - LOG.debug("Snapshot frequency exceeded. Reusing snapshot {}", snapshot); - } - } - /** * Get the latest timetable for TripPattern for a given service date. *

* Snapshot timetable is used as source if initialised, trip patterns scheduled timetable if not. */ private Timetable getCurrentTimetable(TripPattern tripPattern, LocalDate serviceDate) { - TimetableSnapshot timetableSnapshot = snapshot; + TimetableSnapshot timetableSnapshot = getTimetableSnapshot(); if (timetableSnapshot != null) { return timetableSnapshot.resolve(tripPattern, serviceDate); } @@ -429,19 +341,4 @@ private boolean removePreviousRealtimeUpdate(final Trip trip, final LocalDate se return success; } - - private boolean purgeExpiredData() { - final LocalDate today = LocalDate.now(transitModel.getTimeZone()); - final LocalDate previously = today.minusDays(2); // Just to be safe... - - if (lastPurgeDate != null && lastPurgeDate.compareTo(previously) > 0) { - return false; - } - - LOG.debug("purging expired real-time data"); - - lastPurgeDate = previously; - - return buffer.purgeExpiredData(previously); - } } diff --git a/src/main/java/org/opentripplanner/model/TimetableSnapshot.java b/src/main/java/org/opentripplanner/model/TimetableSnapshot.java index 136d70fd7d9..e1dfd5681c0 100644 --- a/src/main/java/org/opentripplanner/model/TimetableSnapshot.java +++ b/src/main/java/org/opentripplanner/model/TimetableSnapshot.java @@ -397,6 +397,13 @@ public void setPatternsForStop(SetMultimap patternsFo this.patternsForStop = patternsForStop; } + /** + * Does this snapshot contain any realtime data or is it completely empty? + */ + public boolean isEmpty() { + return dirtyTimetables.isEmpty() && timetables.isEmpty() && realtimeAddedTripPattern.isEmpty(); + } + /** * Clear timetable for all patterns matching the provided feed id. * diff --git a/src/main/java/org/opentripplanner/updater/trip/AbstractTimetableSnapshotSource.java b/src/main/java/org/opentripplanner/updater/trip/AbstractTimetableSnapshotSource.java new file mode 100644 index 00000000000..3df4f2aba59 --- /dev/null +++ b/src/main/java/org/opentripplanner/updater/trip/AbstractTimetableSnapshotSource.java @@ -0,0 +1,196 @@ +package org.opentripplanner.updater.trip; + +import java.time.LocalDate; +import java.util.Objects; +import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Supplier; +import org.opentripplanner.framework.time.CountdownTimer; +import org.opentripplanner.model.TimetableSnapshot; +import org.opentripplanner.model.TimetableSnapshotProvider; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.TransitLayerUpdater; +import org.opentripplanner.updater.TimetableSnapshotSourceParameters; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A base class for which abstracts away locking, updating, committing and purging of the timetable snapshot. + * In order to keep code reviews easier this is an intermediate stage and will be refactored further. + * In particular the following refactorings are planned: + * + * - use composition instead of inheritance + * - make the buffer private to this class and add an API for its access + * - create only one "snapshot manager" per transit model that is shared between Siri/GTFS-RT updaters + */ +public class AbstractTimetableSnapshotSource implements TimetableSnapshotProvider { + + private static final Logger LOG = LoggerFactory.getLogger(AbstractTimetableSnapshotSource.class); + private final TransitLayerUpdater transitLayerUpdater; + /** + * Lock to indicate that buffer is in use + */ + private final ReentrantLock bufferLock = new ReentrantLock(true); + + /** + * The working copy of the timetable snapshot. Should not be visible to routing threads. Should + * only be modified by a thread that holds a lock on {@link #bufferLock}. All public methods that + * might modify this buffer will correctly acquire the lock. + */ + protected final TimetableSnapshot buffer = new TimetableSnapshot(); + + /** + * The working copy of the timetable snapshot. Should not be visible to routing threads. Should + * only be modified by a thread that holds a lock on {@link #bufferLock}. All public methods that + * might modify this buffer will correctly acquire the lock. By design, only one thread should + * ever be writing to this buffer. + * TODO RT_AB: research and document why this lock is needed since only one thread should ever be + * writing to this buffer. One possible reason may be a need to suspend writes while indexing + * and swapping out the buffer. But the original idea was to make a new copy of the buffer + * before re-indexing it. While refactoring or rewriting parts of this system, we could throw + * an exception if a writing section is entered by more than one thread. + */ + private volatile TimetableSnapshot snapshot = null; + + /** + * If a timetable snapshot is requested less than this number of milliseconds after the previous + * snapshot, just return the same one. Throttles the potentially resource-consuming task of + * duplicating a TripPattern -> Timetable map and indexing the new Timetables. + */ + private final CountdownTimer snapshotFrequencyThrottle; + + /** + * Should expired real-time data be purged from the graph. + * TODO RT_AB: Clarify exactly what "purge" means and in what circumstances would one turn it off. + */ + private final boolean purgeExpiredData; + /** + * We inject a provider to retrieve the current service-date(now). This enables us to unit-test + * the purgeExpiredData feature. + */ + private final Supplier localDateNow; + + private LocalDate lastPurgeDate = null; + + /** + * + * @param localDateNow This supplier allows you to inject a custom lambda to override what is + * considered 'today'. This is useful for unit testing. + */ + public AbstractTimetableSnapshotSource( + TransitLayerUpdater transitLayerUpdater, + TimetableSnapshotSourceParameters parameters, + Supplier localDateNow + ) { + this.transitLayerUpdater = transitLayerUpdater; + this.snapshotFrequencyThrottle = new CountdownTimer(parameters.maxSnapshotFrequency()); + this.purgeExpiredData = parameters.purgeExpiredData(); + this.localDateNow = Objects.requireNonNull(localDateNow); + // Force commit so that snapshot initializes + commitTimetableSnapshot(true); + } + + /** + * @return an up-to-date snapshot mapping TripPatterns to Timetables. This snapshot and the + * timetable objects it references are guaranteed to never change, so the requesting thread is + * provided a consistent view of all TripTimes. The routing thread need only release its reference + * to the snapshot to release resources. + */ + public final TimetableSnapshot getTimetableSnapshot() { + // Try to get a lock on the buffer + if (bufferLock.tryLock()) { + // Make a new snapshot if necessary + try { + commitTimetableSnapshot(false); + return snapshot; + } finally { + bufferLock.unlock(); + } + } + // No lock could be obtained because there is either a snapshot commit busy or updates + // are applied at this moment, just return the current snapshot + return snapshot; + } + + /** + * Request a commit of the timetable snapshot. + *

+ * If there are no updates buffered up or not enough time has elapsed, the existing snapshot + * is returned. + * + * @param force Force the committing of a new snapshot even if the above conditions are not met. + */ + public final void commitTimetableSnapshot(final boolean force) { + if (force || snapshotFrequencyThrottle.timeIsUp()) { + if (force || buffer.isDirty()) { + LOG.debug("Committing {}", buffer); + snapshot = buffer.commit(transitLayerUpdater, force); + + // We only reset the timer when the snapshot is updated. This will cause the first + // update to be committed after a silent period. This should not have any effect in + // a busy updater. It is however useful when manually testing the updater. + snapshotFrequencyThrottle.restart(); + } else { + LOG.debug("Buffer was unchanged, keeping old snapshot."); + } + } else { + LOG.debug("Snapshot frequency exceeded. Reusing snapshot {}", snapshot); + } + } + + /** + * Make a snapshot after each message in anticipation of incoming requests. + * Purge data if necessary (and force new snapshot if anything was purged). + * Make sure that the public (locking) getTimetableSnapshot function is not called. + */ + protected void purgeAndCommit() { + if (purgeExpiredData) { + final boolean modified = purgeExpiredData(); + commitTimetableSnapshot(modified); + } else { + commitTimetableSnapshot(false); + } + } + + /** + * Remove realtime data from previous service dates from the snapshot. This is useful so that + * instances that run for multiple days don't accumulate a lot of realtime data for past + * dates which would increase memory consumption. + * If your OTP instances are restarted throughout the day, this is less useful and can be + * turned off. + */ + protected final boolean purgeExpiredData() { + final LocalDate today = localDateNow.get(); + // TODO: Base this on numberOfDaysOfLongestTrip for tripPatterns + final LocalDate previously = today.minusDays(2); // Just to be safe... + + // Purge data only if we have changed date + if (lastPurgeDate != null && lastPurgeDate.compareTo(previously) >= 0) { + return false; + } + + LOG.debug("Purging expired realtime data"); + + lastPurgeDate = previously; + + return buffer.purgeExpiredData(previously); + } + + protected final LocalDate localDateNow() { + return localDateNow.get(); + } + + /** + * Execute a {@code Runnable} with a locked snapshot buffer and release the lock afterwards. While + * the action of locking and unlocking is not complicated to do for calling code, this method + * exists so that the lock instance is a private field. + */ + protected final void withLock(Runnable action) { + bufferLock.lock(); + + try { + action.run(); + } finally { + // Always release lock + bufferLock.unlock(); + } + } +} diff --git a/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java b/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java index 9cc0cab52a9..a530f348a00 100644 --- a/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java +++ b/src/main/java/org/opentripplanner/updater/trip/TimetableSnapshotSource.java @@ -21,7 +21,6 @@ import com.google.transit.realtime.GtfsRealtime.TripUpdate.StopTimeUpdate; import de.mfdz.MfdzRealtimeExtensions; import java.text.ParseException; -import java.time.Duration; import java.time.LocalDate; import java.time.ZoneId; import java.util.ArrayList; @@ -30,7 +29,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; import javax.annotation.Nonnull; import org.opentripplanner.framework.i18n.I18NString; @@ -40,9 +38,6 @@ import org.opentripplanner.gtfs.mapping.TransitModeMapper; import org.opentripplanner.model.StopTime; import org.opentripplanner.model.Timetable; -import org.opentripplanner.model.TimetableSnapshot; -import org.opentripplanner.model.TimetableSnapshotProvider; -import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.TransitLayerUpdater; import org.opentripplanner.transit.model.basic.TransitMode; import org.opentripplanner.transit.model.framework.DataValidationException; import org.opentripplanner.transit.model.framework.Deduplicator; @@ -76,7 +71,7 @@ * necessary to provide planning threads a consistent constant view of a graph with realtime data at * a specific point in time. */ -public class TimetableSnapshotSource implements TimetableSnapshotProvider { +public class TimetableSnapshotSource extends AbstractTimetableSnapshotSource { private static final Logger LOG = LoggerFactory.getLogger(TimetableSnapshotSource.class); @@ -85,63 +80,16 @@ public class TimetableSnapshotSource implements TimetableSnapshotProvider { */ private static final long MAX_ARRIVAL_DEPARTURE_TIME = 48 * 60 * 60; - /** - * The working copy of the timetable snapshot. Should not be visible to routing threads. Should - * only be modified by a thread that holds a lock on {@link #bufferLock}. All public methods that - * might modify this buffer will correctly acquire the lock. By design, only one thread should - * ever be writing to this buffer. - * TODO RT_AB: research and document why this lock is needed since only one thread should ever be - * writing to this buffer. One possible reason may be a need to suspend writes while indexing - * and swapping out the buffer. But the original idea was to make a new copy of the buffer - * before re-indexing it. While refactoring or rewriting parts of this system, we could throw - * an exception if a writing section is entered by more than one thread. - */ - private final TimetableSnapshot buffer = new TimetableSnapshot(); - - /** Lock to indicate that buffer is in use. */ - private final ReentrantLock bufferLock = new ReentrantLock(true); - /** A synchronized cache of trip patterns added to the graph due to GTFS-realtime messages. */ private final TripPatternCache tripPatternCache = new TripPatternCache(); private final ZoneId timeZone; private final TransitEditorService transitService; - private final TransitLayerUpdater transitLayerUpdater; - - /** - * If a timetable snapshot is requested less than this number of milliseconds after the previous - * snapshot, just return the same one. Throttles the potentially resource-consuming task of - * duplicating a TripPattern → Timetable map and indexing the new Timetables. - */ - private final Duration maxSnapshotFrequency; - - /** - * The last committed snapshot that was handed off to a routing thread. This snapshot may be given - * to more than one routing thread if the maximum snapshot frequency is exceeded. - */ - private volatile TimetableSnapshot snapshot = null; - - /** - * Should expired real-time data be purged from the graph. - * TODO RT_AB: Clarify exactly what "purge" means and in what circumstances would one turn it off. - */ - private final boolean purgeExpiredData; - - protected LocalDate lastPurgeDate = null; - - /** Epoch time in milliseconds at which the last snapshot was generated. */ - protected long lastSnapshotTime = -1; private final Deduplicator deduplicator; private final Map serviceCodes; - /** - * We inject a provider to retrieve the current service-date(now). This enables us to unit-test - * the purgeExpiredData feature. - */ - private final Supplier localDateNow; - public TimetableSnapshotSource( TimetableSnapshotSourceParameters parameters, TransitModel transitModel @@ -158,45 +106,16 @@ public TimetableSnapshotSource( TransitModel transitModel, Supplier localDateNow ) { + super(transitModel.getTransitLayerUpdater(), parameters, localDateNow); this.timeZone = transitModel.getTimeZone(); this.transitService = new DefaultTransitService(transitModel); - this.transitLayerUpdater = transitModel.getTransitLayerUpdater(); this.deduplicator = transitModel.getDeduplicator(); this.serviceCodes = transitModel.getServiceCodes(); - this.maxSnapshotFrequency = parameters.maxSnapshotFrequency(); - this.purgeExpiredData = parameters.purgeExpiredData(); - this.localDateNow = localDateNow; // Inject this into the transit model transitModel.initTimetableSnapshotProvider(this); } - /** - * @return an up-to-date snapshot mapping TripPatterns to Timetables. This snapshot and the - * timetable objects it references are guaranteed to never change, so the requesting thread is - * provided a consistent view of all TripTimes. The routing thread need only release its reference - * to the snapshot to release resources. - */ - public TimetableSnapshot getTimetableSnapshot() { - TimetableSnapshot snapshotToReturn; - - // Try to get a lock on the buffer - if (bufferLock.tryLock()) { - // Make a new snapshot if necessary - try { - snapshotToReturn = getTimetableSnapshot(false); - } finally { - bufferLock.unlock(); - } - } else { - // No lock could be obtained because there is either a snapshot commit busy or updates - // are applied at this moment, just return the current snapshot - snapshotToReturn = snapshot; - } - - return snapshotToReturn; - } - /** * Method to apply a trip update list to the most recent version of the timetable snapshot. A * GTFS-RT feed is always applied against a single static feed (indicated by feedId). @@ -223,13 +142,10 @@ public UpdateResult applyTripUpdates( return UpdateResult.empty(); } - // Acquire lock on buffer - bufferLock.lock(); - Map failuresByRelationship = new HashMap<>(); List> results = new ArrayList<>(); - try { + withLock(() -> { if (fullDataset) { // Remove all updates from the buffer buffer.clear(feedId); @@ -272,7 +188,7 @@ public UpdateResult applyTripUpdates( } else { // TODO: figure out the correct service date. For the special case that a trip // starts for example at 40:00, yesterday would probably be a better guess. - serviceDate = localDateNow.get(); + serviceDate = localDateNow(); } uIndex += 1; @@ -328,19 +244,8 @@ public UpdateResult applyTripUpdates( } } - // Make a snapshot after each message in anticipation of incoming requests - // Purge data if necessary (and force new snapshot if anything was purged) - // Make sure that the public (locking) getTimetableSnapshot function is not called. - if (purgeExpiredData) { - final boolean modified = purgeExpiredData(); - getTimetableSnapshot(modified); - } else { - getTimetableSnapshot(false); - } - } finally { - // Always release lock - bufferLock.unlock(); - } + purgeAndCommit(); + }); var updateResult = UpdateResult.ofResults(results); @@ -370,22 +275,6 @@ private static void logUpdateResult( }); } - private TimetableSnapshot getTimetableSnapshot(final boolean force) { - final long now = System.currentTimeMillis(); - if (force || now - lastSnapshotTime > maxSnapshotFrequency.toMillis()) { - if (force || buffer.isDirty()) { - LOG.debug("Committing {}", buffer); - snapshot = buffer.commit(transitLayerUpdater, force); - } else { - LOG.debug("Buffer was unchanged, keeping old snapshot."); - } - lastSnapshotTime = System.currentTimeMillis(); - } else { - LOG.debug("Snapshot frequency exceeded. Reusing snapshot {}", snapshot); - } - return snapshot; - } - /** * Determine how the trip update should be handled. * @@ -1144,23 +1033,6 @@ private Result handleCanceledTrip( return Result.success(UpdateSuccess.noWarnings()); } - private boolean purgeExpiredData() { - final LocalDate today = localDateNow.get(); - // TODO: Base this on numberOfDaysOfLongestTrip for tripPatterns - final LocalDate previously = today.minusDays(2); // Just to be safe... - - // Purge data only if we have changed date - if (lastPurgeDate != null && lastPurgeDate.compareTo(previously) >= 0) { - return false; - } - - LOG.debug("purging expired realtime data"); - - lastPurgeDate = previously; - - return buffer.purgeExpiredData(previously); - } - /** * Retrieve a trip pattern given a trip id. * diff --git a/src/test/java/org/opentripplanner/GtfsTest.java b/src/test/java/org/opentripplanner/GtfsTest.java index 99f78c18e33..6f14a6db662 100644 --- a/src/test/java/org/opentripplanner/GtfsTest.java +++ b/src/test/java/org/opentripplanner/GtfsTest.java @@ -13,6 +13,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.InputStream; +import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Collections; @@ -207,7 +208,9 @@ protected void setUp() throws Exception { serverContext = TestServerContext.createServerContext(graph, transitModel); timetableSnapshotSource = new TimetableSnapshotSource( - TimetableSnapshotSourceParameters.DEFAULT.withPurgeExpiredData(false), + TimetableSnapshotSourceParameters.DEFAULT + .withPurgeExpiredData(true) + .withMaxSnapshotFrequency(Duration.ZERO), transitModel ); alertPatchServiceImpl = new TransitAlertServiceImpl(transitModel); diff --git a/src/test/java/org/opentripplanner/updater/trip/TimetableSnapshotSourceTest.java b/src/test/java/org/opentripplanner/updater/trip/TimetableSnapshotSourceTest.java index 8c28290de66..eeb7368091c 100644 --- a/src/test/java/org/opentripplanner/updater/trip/TimetableSnapshotSourceTest.java +++ b/src/test/java/org/opentripplanner/updater/trip/TimetableSnapshotSourceTest.java @@ -7,7 +7,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.updater.spi.UpdateError.UpdateErrorType.NO_SERVICE_ON_DATE; @@ -97,15 +96,6 @@ public void testGetSnapshot() throws InvalidProtocolBufferException { final TimetableSnapshot snapshot = updater.getTimetableSnapshot(); assertNotNull(snapshot); assertSame(snapshot, updater.getTimetableSnapshot()); - - updater.applyTripUpdates( - TRIP_MATCHER_NOOP, - REQUIRED_NO_DATA, - fullDataset, - List.of(TripUpdate.parseFrom(cancellation)), - feedId - ); - assertSame(snapshot, updater.getTimetableSnapshot()); } @Test @@ -140,11 +130,7 @@ public void testHandleCanceledTrip() throws InvalidProtocolBufferException { final int tripIndex = pattern.getScheduledTimetable().getTripIndex(tripId); final int tripIndex2 = pattern.getScheduledTimetable().getTripIndex(tripId2); - var updater = new TimetableSnapshotSource( - TimetableSnapshotSourceParameters.DEFAULT, - transitModel, - () -> SERVICE_DATE - ); + var updater = defaultUpdater(); updater.applyTripUpdates( TRIP_MATCHER_NOOP, @@ -231,7 +217,7 @@ public void invalidTripId() { tripUpdateBuilder.setTrip(tripDescriptorBuilder); var tripUpdate = tripUpdateBuilder.build(); - updater.applyTripUpdates( + var result = updater.applyTripUpdates( TRIP_MATCHER_NOOP, REQUIRED_NO_DATA, fullDataset, @@ -239,8 +225,7 @@ public void invalidTripId() { feedId ); - var snapshot = updater.getTimetableSnapshot(); - assertNull(snapshot); + assertEquals(0, result.successful()); }); } @@ -611,7 +596,8 @@ public void invalidTripDate() { // THEN final TimetableSnapshot snapshot = updater.getTimetableSnapshot(); - assertNull(snapshot); + assertTrue(snapshot.isEmpty()); + assertFalse(snapshot.isDirty()); assertEquals(1, result.failed()); var errors = result.failures(); assertEquals(1, errors.get(NO_SERVICE_ON_DATE).size()); @@ -1065,7 +1051,7 @@ public void repeatedlyAddedTripWithNewRoute() { @Nonnull private TimetableSnapshotSource defaultUpdater() { return new TimetableSnapshotSource( - TimetableSnapshotSourceParameters.DEFAULT, + new TimetableSnapshotSourceParameters(Duration.ZERO, true), transitModel, () -> SERVICE_DATE ); @@ -1145,6 +1131,7 @@ public void testPurgeExpiredData( List.of(tripUpdateYesterday), feedId ); + updater.commitTimetableSnapshot(true); final TimetableSnapshot snapshotA = updater.getTimetableSnapshot();