From 33c6bac1423c0f04522d8b511ba41e3d202c66e7 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Mon, 14 Oct 2024 10:59:17 +0100 Subject: [PATCH 1/2] use primary stop object for consolidated stops --- .../DecorateConsolidatedStopNamesTest.java | 2 ++ .../DecorateConsolidatedStopNames.java | 5 ++-- .../DefaultStopConsolidationService.java | 25 ++++++++++++++++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java b/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java index 7f389bc41ec..bc7e7c1cd4a 100644 --- a/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java @@ -57,7 +57,9 @@ void changeNames() { var updatedLeg = itinerary.getLegs().getFirst(); assertEquals(STOP_C.getName(), updatedLeg.getFrom().name); + assertEquals(STOP_C.getId(), updatedLeg.getFrom().stop.getId()); assertEquals(STOP_D.getName(), updatedLeg.getTo().name); + assertEquals(STOP_C.getId(), updatedLeg.getTo().stop.getId()); // Check that the fares were carried over assertEquals(fpu, updatedLeg.fareProducts()); diff --git a/src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java b/src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java index 1806b3e9e32..5cb6b07ae06 100644 --- a/src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java +++ b/src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java @@ -31,8 +31,9 @@ public void decorate(Itinerary itinerary) { * operating the route, so that the name in the result matches the physical signage on the stop. *

* If the leg has a "to" stop that is a primary stop, then we don't want to show the stop that's on - * the signage but what is shown _inside_ the vehicle. That's why we use the agency-specific (aka - * secondary) stop. + * the signage but what is shown _inside_ the vehicle. However, we still want to use the primary ID + * for stop information and alternatives legs. So we copy the agency-specific (aka secondary) stop + * name to the primary stop object. *

* This follows the somewhat idiosyncratic logic of the consolidated stops feature. */ diff --git a/src/ext/java/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java b/src/ext/java/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java index 9f31e366be5..76f22eecdd9 100644 --- a/src/ext/java/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java +++ b/src/ext/java/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java @@ -9,8 +9,13 @@ import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup; import org.opentripplanner.ext.stopconsolidation.model.StopReplacement; +import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.organization.Agency; +import org.opentripplanner.transit.model.site.AreaStop; +import org.opentripplanner.transit.model.site.GroupStop; +import org.opentripplanner.transit.model.site.RegularStop; +import org.opentripplanner.transit.model.site.RegularStopBuilder; import org.opentripplanner.transit.model.site.StopLocation; import org.opentripplanner.transit.service.TransitModel; import org.slf4j.Logger; @@ -95,7 +100,25 @@ private Optional findAgencySpecificStop(StopLocation stop, Agency .flatMap(g -> g.secondaries().stream()) .filter(secondary -> secondary.getFeedId().equals(agency.getId().getFeedId())) .findAny() - .map(id -> transitModel.getStopModel().getRegularStop(id)); + .map(id -> copyStopWithName(stop, transitModel.getStopModel().getRegularStop(id).getName())); + } + + @Nonnull + private StopLocation copyStopWithName(StopLocation primaryStop, I18NString agencySpecificName) { + if (primaryStop instanceof RegularStop) { + return ((RegularStop) primaryStop).copy().withName(agencySpecificName).build(); + } + + if (primaryStop instanceof AreaStop) { + return ((AreaStop) primaryStop).copy().withName(agencySpecificName).build(); + } + + if (primaryStop instanceof GroupStop) { + return ((GroupStop) primaryStop).copy().withName(agencySpecificName).build(); + } + + // should not happen unless new stop types are added + throw new IllegalArgumentException("Unknown stop location type: " + primaryStop); } @Override From 9b3201b2d488448699dedc4280d99a318aefac10 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Mon, 14 Oct 2024 12:50:30 +0100 Subject: [PATCH 2/2] link timetable to modified TripPattern --- .../StopConsolidationModuleTest.java | 2 ++ .../org/opentripplanner/model/Timetable.java | 20 +++++++++++++++++++ .../transit/model/network/TripPattern.java | 4 ++-- .../model/network/TripPatternTest.java | 3 +++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationModuleTest.java b/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationModuleTest.java index 0df495d63d0..01fa0de29eb 100644 --- a/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationModuleTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationModuleTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.ext.stopconsolidation.TestStopConsolidationModel.PATTERN; import static org.opentripplanner.ext.stopconsolidation.TestStopConsolidationModel.STOP_A; import static org.opentripplanner.ext.stopconsolidation.TestStopConsolidationModel.STOP_B; @@ -29,6 +30,7 @@ void replacePatterns() { var modifiedPattern = transitModel.getTripPatternForId(PATTERN.getId()); assertFalse(modifiedPattern.getRoutingTripPattern().getPattern().sameAs(PATTERN)); assertFalse(modifiedPattern.sameAs(PATTERN)); + assertTrue(modifiedPattern.getScheduledTimetable().getPattern().sameAs(modifiedPattern)); var modifiedStop = modifiedPattern .getRoutingTripPattern() diff --git a/src/main/java/org/opentripplanner/model/Timetable.java b/src/main/java/org/opentripplanner/model/Timetable.java index 10c384e6211..22bb5d0e021 100644 --- a/src/main/java/org/opentripplanner/model/Timetable.java +++ b/src/main/java/org/opentripplanner/model/Timetable.java @@ -82,6 +82,17 @@ public Timetable(TripPattern pattern) { this.pattern = tt.pattern; } + /** + * Copy the timetable with a replaced pattern. + * The new pattern should have the same number of stops as the original pattern. + */ + public Timetable(Timetable original, TripPattern newPattern) { + pattern = newPattern; + tripTimes.addAll(original.tripTimes); + frequencyEntries.addAll(original.frequencyEntries); + serviceDate = original.serviceDate; + } + /** @return the index of TripTimes for this trip ID in this particular Timetable */ public int getTripIndex(FeedScopedId tripId) { int ret = 0; @@ -493,4 +504,13 @@ public TripTimes getRepresentativeTripTimes() { return null; } } + + public boolean sameAs(Timetable other) { + return ( + Objects.equals(pattern, other.pattern) && + Objects.equals(tripTimes, other.tripTimes) && + Objects.equals(frequencyEntries, other.frequencyEntries) && + Objects.equals(serviceDate, other.serviceDate) + ); + } } diff --git a/src/main/java/org/opentripplanner/transit/model/network/TripPattern.java b/src/main/java/org/opentripplanner/transit/model/network/TripPattern.java index 57c71d06113..3d703efbf8b 100644 --- a/src/main/java/org/opentripplanner/transit/model/network/TripPattern.java +++ b/src/main/java/org/opentripplanner/transit/model/network/TripPattern.java @@ -134,7 +134,7 @@ public TripPattern(TripPatternBuilder builder) { this.scheduledTimetable = builder.getScheduledTimetable() != null - ? builder.getScheduledTimetable() + ? new Timetable(builder.getScheduledTimetable(), this) : new Timetable(this); this.originalTripPattern = builder.getOriginalTripPattern(); @@ -512,7 +512,7 @@ public boolean sameAs(@Nonnull TripPattern other) { Objects.equals(this.containsMultipleModes, other.containsMultipleModes) && Objects.equals(this.name, other.name) && Objects.equals(this.stopPattern, other.stopPattern) && - Objects.equals(this.scheduledTimetable, other.scheduledTimetable) + this.scheduledTimetable.sameAs(other.scheduledTimetable) ); } diff --git a/src/test/java/org/opentripplanner/transit/model/network/TripPatternTest.java b/src/test/java/org/opentripplanner/transit/model/network/TripPatternTest.java index b2adc7544a3..622b2e3e4a8 100644 --- a/src/test/java/org/opentripplanner/transit/model/network/TripPatternTest.java +++ b/src/test/java/org/opentripplanner/transit/model/network/TripPatternTest.java @@ -68,6 +68,9 @@ void copy() { assertEquals(subject.getHopGeometry(0), copy.getHopGeometry(0)); assertEquals(subject.getHopGeometry(1), copy.getHopGeometry(1)); assertEquals(HOP_GEOMETRIES.get(1), copy.getHopGeometry(1)); + + // The linked timetable should link back to the new pattern + assertSame(copy, copy.getScheduledTimetable().getPattern()); } @Test