Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,31 @@ public RealTimeRaptorTransitDataUpdater(TimetableRepository timetableRepository)
this.timetableRepository = timetableRepository;
}

/// Updates the real-time [RaptorTransitData] to use the modified timetables.
Copy link
Member

Choose a reason for hiding this comment

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

Can you make this proper Javadoc, please?

Copy link
Member

Choose a reason for hiding this comment

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

Or is this now valid Markdown-ish Javadoc?

Copy link
Member

Choose a reason for hiding this comment

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

Can you use the correct Javadoc syntax here please?

Copy link
Member

Choose a reason for hiding this comment

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

We discussed this while you were gone. Apparently IntelliJ IDEA already supports this new markdown format and then no one was really against using it. However, we didn't discuss it too much and not sure what is the way forward with it.

Copy link
Member

Choose a reason for hiding this comment

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

No, it's fine. i was unaware that it will be officially supported. Intellij is the only place I would read Javadoc so I don't care if other tools don't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is Markdown-ish javadoc

///
/// This method bridges the different update approaches:
/// 1. `updatedTimetables` and `timetables` only contains [Timetable]s with real-time
/// updates. This means that removed items are not present.
/// 2. [RaptorTransitData] requires applying the changes to a previous snapshot: adding,
/// updating and removing timetables.
///
/// To support this the method has three tasks:
/// 1. Collect [TripPatternForDate]s which have invalidated data (`oldTripPatternsForDate`).
/// Trips may change in multiple ways and because of that may move between [TripPattern]s. To
/// track a [TripIdAndServiceDate] it's previous state needs to be stored so that all relevant
/// places may be updated.
/// * a trip may have a new (real-time) Timetable, which results in two updated [Timetable]s
/// * a trip may move between scheduled [StopPattern]s and/or real-time [StopPattern]s
/// 2. Collect [TripPatternForDate]s which have valid data (`newTripPatternsForDate`).
/// There are two options:
/// 1. an update was received
/// 2. no update was received, and so the previous updated should be removed. If the update
/// was for a scheduled trip, then the schedule should be restored.
/// 3. Remove the `oldTripPatternsForDate` and add the `newTripPatternsForDate` to the
/// [RaptorTransitData].
///
/// @param updatedTimetables that changed with the current snapshot
/// @param timetables which are affected by real-time updates
public void update(
Collection<Timetable> updatedTimetables,
Map<TripPattern, SortedSet<Timetable>> timetables
Expand Down Expand Up @@ -204,7 +229,8 @@ public void update(
patternsForDate.remove(tripPatternForDate);
}
} else {
LOG.warn("Could not fetch timetable for {}", pattern);
LOG.warn("Could not fetch timetable for {}, removing.", pattern);
patternsForDate.remove(tripPatternForDate);
Copy link
Member

Choose a reason for hiding this comment

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

This is the most important change in the PR. I absolutely support changing this class but I'm having a hard time understanding the consequences of this change: the code is very complex with several collections updating each other.

If you have a better understanding, do you have time to walk me through the code? Maybe @vpaturet is interested as well.

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalInt;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -132,10 +133,26 @@ public static Result<TripTimesPatch, UpdateError> createUpdatedTripTimesFromGtfs
}

if (match) {
update.stopHeadsign().ifPresent(x -> builder.withStopHeadsign(index, x));
update.pickup().ifPresent(x -> updatedPickups.put(index, x));
update.dropoff().ifPresent(x -> updatedDropoffs.put(index, x));
update.assignedStopId().ifPresent(x -> replacedStopIndices.put(index, x));
var scheduledStopId = timetable.getPattern().getStop(i).getId().getId();
var scheduledStopHeadsign = tripTimes.getHeadsign(i);
var scheduledPickup = timetable.getPattern().getBoardType(i);
var scheduledDropoff = timetable.getPattern().getAlightType(i);
update
.stopHeadsign()
.filter(x -> !Objects.equals(x, scheduledStopHeadsign))
.ifPresent(x -> builder.withStopHeadsign(index, x));
update
.pickup()
.filter(x -> x != scheduledPickup)
.ifPresent(x -> updatedPickups.put(index, x));
update
.dropoff()
.filter(x -> x != scheduledDropoff)
.ifPresent(x -> updatedDropoffs.put(index, x));
update
.assignedStopId()
.filter(x -> !Objects.equals(x, scheduledStopId))
.ifPresent(x -> replacedStopIndices.put(index, x));

var scheduleRelationship = update.scheduleRelationship();
// Handle each schedule relationship case
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.opentripplanner.transit.model._data;

import java.time.Duration;
import java.util.List;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitTuningParameters;
import org.opentripplanner.routing.api.request.RouteRequest;
import org.opentripplanner.transit.model.site.StopTransferPriority;

class TestTransitTuningParameters implements TransitTuningParameters {

@Override
public boolean enableStopTransferPriority() {
return false;
}

@Override
public Integer stopBoardAlightDuringTransferCost(StopTransferPriority key) {
return 0;
}

@Override
public int transferCacheMaxSize() {
return 0;
}

@Override
public Duration maxSearchWindow() {
return null;
}

@Override
public List<Duration> pagingSearchWindowAdjustments() {
return List.of();
}

@Override
public List<RouteRequest> transferCacheRequests() {
return List.of();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import java.time.ZoneId;
import org.opentripplanner.LocalTimeParser;
import org.opentripplanner.model.TimetableSnapshot;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.RaptorTransitData;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.RaptorTransitDataMapper;
import org.opentripplanner.routing.algorithm.raptoradapter.transit.mappers.RealTimeRaptorTransitDataUpdater;
import org.opentripplanner.transit.service.DefaultTransitService;
import org.opentripplanner.transit.service.TimetableRepository;
import org.opentripplanner.transit.service.TransitService;
Expand Down Expand Up @@ -40,8 +43,14 @@ public static TransitTestEnvironmentBuilder of(LocalDate serviceDate, ZoneId tim
this.timetableRepository = timetableRepository;

this.timetableRepository.index();
this.timetableRepository.setRaptorTransitData(
RaptorTransitDataMapper.map(new TestTransitTuningParameters(), timetableRepository)
);
this.timetableRepository.setRealtimeRaptorTransitData(
new RaptorTransitData(timetableRepository.getRaptorTransitData())
);
this.snapshotManager = new TimetableSnapshotManager(
null,
new RealTimeRaptorTransitDataUpdater(timetableRepository),
TimetableSnapshotParameters.PUBLISH_IMMEDIATELY,
() -> defaultServiceDate
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.opentripplanner.updater.trip.UpdateIncrementality.FULL_DATASET;

import com.google.transit.realtime.GtfsRealtime;
import java.time.LocalDate;
import java.util.List;
import org.opentripplanner.transit.model._data.TransitTestEnvironment;
import org.opentripplanner.updater.spi.UpdateResult;
Expand All @@ -20,7 +21,7 @@ public class GtfsRtTestHelper {
this.gtfsAdapter = new GtfsRealTimeTripUpdateAdapter(
transitTestEnvironment.timetableRepository(),
transitTestEnvironment.timetableSnapshotManager(),
() -> transitTestEnvironment.defaultServiceDate()
transitTestEnvironment::defaultServiceDate
);
}

Expand All @@ -32,13 +33,29 @@ public TripUpdateBuilder tripUpdateScheduled(String tripId) {
return tripUpdate(tripId, GtfsRealtime.TripDescriptor.ScheduleRelationship.SCHEDULED);
}

public TripUpdateBuilder tripUpdateScheduled(String tripId, LocalDate serviceDate) {
return tripUpdate(
tripId,
serviceDate,
GtfsRealtime.TripDescriptor.ScheduleRelationship.SCHEDULED
);
}

public TripUpdateBuilder tripUpdate(
String tripId,
GtfsRealtime.TripDescriptor.ScheduleRelationship scheduleRelationship
) {
return tripUpdate(tripId, transitTestEnvironment.defaultServiceDate(), scheduleRelationship);
}

public TripUpdateBuilder tripUpdate(
String tripId,
LocalDate serviceDate,
GtfsRealtime.TripDescriptor.ScheduleRelationship scheduleRelationship
) {
return new TripUpdateBuilder(
tripId,
transitTestEnvironment.defaultServiceDate(),
serviceDate,
scheduleRelationship,
transitTestEnvironment.timeZone()
);
Expand All @@ -55,6 +72,10 @@ public UpdateResult applyTripUpdate(
return applyTripUpdates(List.of(update), incrementality);
}

public UpdateResult applyTripUpdates(List<GtfsRealtime.TripUpdate> updates) {
return applyTripUpdates(updates, FULL_DATASET);
}

public UpdateResult applyTripUpdates(
List<GtfsRealtime.TripUpdate> updates,
UpdateIncrementality incrementality
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ public interface RealtimeTestConstants {
String STOP_B_ID = "B";
String STOP_C_ID = "C";
String STOP_D_ID = "D";
String STOP_E_ID = "E";
}
Loading
Loading