Merge branch 'dev-2.x' into rename_repositories

# Conflicts: # application/src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java # application/src/test/java/org/opentripplanner/model/TimetableSnapshotTest.java
opentripplanner · Oct 16, 2024 · 183d3f2 · 183d3f2
2 parents a8a1730 + 3a216cb
commit 183d3f2
Show file tree

Hide file tree

Showing 22 changed files with 347 additions and 245 deletions.
diff --git a/.github/workflows/cibuild.yml b/.github/workflows/cibuild.yml
@@ -108,7 +108,7 @@ jobs:
         if: github.event_name == 'pull_request'
 
       - name: Install Python dependencies
-        run: pip install -r doc/user/requirements.txt
+        run: pip install --break-system-packages -r doc/user/requirements.txt
 
 
       - name: Build main documentation
@@ -179,7 +179,7 @@ jobs:
       - uses: actions/checkout@v4
       - uses: actions/setup-node@v4
         with:
-          node-version: 16
+          node-version: 22
       - name: Run code generator
         working-directory: application/src/main/java/org/opentripplanner/apis/gtfs/generated
         run: |

diff --git a/.github/workflows/performance-test.yml b/.github/workflows/performance-test.yml
@@ -88,17 +88,18 @@ jobs:
       - name: Build graph
         if: matrix.profile == 'core' || github.ref == 'refs/heads/dev-2.x'
         run: |
-          cp target/otp-*-SNAPSHOT-shaded.jar otp.jar
+          cp application/target/otp-*-SNAPSHOT-shaded.jar otp.jar
           java -Xmx32G -jar otp.jar --build --save test/performance/${{ matrix.location }}/
 
       - name: Run speed test
         if: matrix.profile == 'core' || github.ref == 'refs/heads/dev-2.x'
+        working-directory: application
         env:
           PERFORMANCE_INFLUX_DB_PASSWORD: ${{ secrets.PERFORMANCE_INFLUX_DB_PASSWORD }}
           SPEEDTEST_LOCATION: ${{ matrix.location }}
           MAVEN_OPTS: "-Xmx50g -XX:StartFlightRecording=delay=${{ matrix.jfr-delay }},duration=30m,filename=${{ matrix.location}}-speed-test.jfr -Dmaven.repo.local=/home/lenni/.m2/repository/"
         run: |
-          mvn exec:java -Dexec.mainClass="org.opentripplanner.transit.speed_test.SpeedTest" -Dexec.classpathScope=test -Dexec.args="--dir=test/performance/${{ matrix.location }} -p md -n ${{ matrix.iterations }} -i 3 -0" -P prettierSkip
+          mvn exec:java -Dexec.mainClass="org.opentripplanner.transit.speed_test.SpeedTest" -Dexec.classpathScope=test -Dexec.args="--dir=../test/performance/${{ matrix.location }} -p md -n ${{ matrix.iterations }} -i 3 -0" -P prettierSkip
 
       - name: Archive travel results file
         if: matrix.profile == 'core' || github.ref == 'refs/heads/dev-2.x'

diff --git a/.github/workflows/prune-container-images.yml b/.github/workflows/prune-container-images.yml
@@ -18,5 +18,5 @@ jobs:
         run: |
           # remove all snapshot container images that have not been pulled for over a year
           # --keep-semver makes sure that any image with a x.y.z version scheme is unaffected by this
-          pip install prune-container-repo==0.0.4
+          pip install prune-container-repo==0.0.4 --break-system-packages
           prune-container-repo -u ${CONTAINER_REGISTRY_USER} -r ${CONTAINER_REPO} --days=365 --keep-semver --activate
diff --git a/application/pom.xml b/application/pom.xml
@@ -405,39 +405,6 @@
             </testResource>
         </testResources>
         <plugins>
-            <plugin>
-                <groupId>org.codehaus.mojo</groupId>
-                <artifactId>build-helper-maven-plugin</artifactId>
-                <version>3.6.0</version>
-                <executions>
-                    <execution>
-                        <id>build-helper-generate-sources</id>
-                        <phase>generate-sources</phase>
-                        <goals>
-                            <goal>add-source</goal>
-                        </goals>
-                        <configuration>
-                            <sources>
-                                <source>src/main/java</source>
-                                <source>src/ext/java</source>
-                            </sources>
-                        </configuration>
-                    </execution>
-                    <execution>
-                        <id>build-helper-generate-test-sources</id>
-                        <phase>generate-test-sources</phase>
-                        <goals>
-                            <goal>add-test-source</goal>
-                        </goals>
-                        <configuration>
-                            <sources>
-                                <source>src/test/java</source>
-                                <source>src/ext-test/java</source>
-                            </sources>
-                        </configuration>
-                    </execution>
-                </executions>
-            </plugin>
             <plugin>
                 <groupId>org.codehaus.mojo</groupId>
                 <artifactId>build-helper-maven-plugin</artifactId>

diff --git a/...est/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java b/...est/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java
@@ -12,10 +12,14 @@
 import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationRepository;
 import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationService;
 import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup;
+import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopLeg;
 import org.opentripplanner.model.fare.FareProduct;
 import org.opentripplanner.model.fare.FareProductUse;
+import org.opentripplanner.model.plan.Leg;
 import org.opentripplanner.model.plan.Place;
 import org.opentripplanner.model.plan.PlanTestConstants;
+import org.opentripplanner.model.plan.ScheduledTransitLeg;
+import org.opentripplanner.model.plan.StreetLeg;
 import org.opentripplanner.model.plan.TestItineraryBuilder;
 import org.opentripplanner.transit.model.basic.Money;
 
@@ -32,21 +36,18 @@ class DecorateConsolidatedStopNamesTest {
   private static final List<FareProductUse> fpu = List.of(
     new FareProductUse("c1a04702-1fb6-32d4-ba02-483bf68111ed", fp)
   );
+  private static final List<ConsolidatedStopGroup> GROUPS = List.of(
+    new ConsolidatedStopGroup(STOP_C.getId(), List.of(STOP_D.getId()))
+  );
+  private static final Place PLACE_C = Place.forStop(STOP_C);
 
   @Test
   void changeNames() {
-    var timetableRepository = TestStopConsolidationModel.buildTimetableRepository();
-
-    var groups = List.of(new ConsolidatedStopGroup(STOP_C.getId(), List.of(STOP_D.getId())));
-    var repo = new DefaultStopConsolidationRepository();
-    repo.addGroups(groups);
-
-    var service = new DefaultStopConsolidationService(repo, timetableRepository);
-    var filter = new DecorateConsolidatedStopNames(service);
+    var filter = defaultFilter();
 
     var itinerary = TestItineraryBuilder
-      .newItinerary(Place.forStop(STOP_C))
-      .bus(TestStopConsolidationModel.ROUTE, 1, T11_05, T11_12, Place.forStop(STOP_C))
+      .newItinerary(PLACE_C)
+      .bus(TestStopConsolidationModel.ROUTE, 1, T11_05, T11_12, PLACE_C)
       .bus(1, T11_05, T11_12, PlanTestConstants.E)
       .bus(1, T11_05, T11_12, PlanTestConstants.F)
       .build();
@@ -62,4 +63,51 @@ void changeNames() {
     // Check that the fares were carried over
     assertEquals(fpu, updatedLeg.fareProducts());
   }
+
+  @Test
+  void removeTransferAtConsolidatedStop() {
+    final var filter = defaultFilter();
+
+    var itinerary = TestItineraryBuilder
+      .newItinerary(PLACE_C)
+      .bus(TestStopConsolidationModel.ROUTE, 1, T11_05, T11_12, PLACE_C)
+      .walk(1, PLACE_C)
+      .bus(1, T11_05, T11_12, PlanTestConstants.F)
+      .build();
+
+    filter.decorate(itinerary);
+
+    var legs = itinerary.getLegs().stream().map(Leg::getClass).toList();
+    assertEquals(List.of(ConsolidatedStopLeg.class, ScheduledTransitLeg.class), legs);
+  }
+
+  @Test
+  void keepRegularTransfer() {
+    final var filter = defaultFilter();
+
+    var itinerary = TestItineraryBuilder
+      .newItinerary(PLACE_C)
+      .bus(TestStopConsolidationModel.ROUTE, 1, T11_05, T11_12, PLACE_C)
+      .walk(1, PlanTestConstants.E)
+      .bus(1, T11_05, T11_12, PlanTestConstants.F)
+      .build();
+
+    filter.decorate(itinerary);
+
+    var legs = itinerary.getLegs().stream().map(Leg::getClass).toList();
+    assertEquals(
+      List.of(ConsolidatedStopLeg.class, StreetLeg.class, ScheduledTransitLeg.class),
+      legs
+    );
+  }
+
+  private static DecorateConsolidatedStopNames defaultFilter() {
+    var transitModel = TestStopConsolidationModel.buildTransitModel();
+
+    var repo = new DefaultStopConsolidationRepository();
+    repo.addGroups(GROUPS);
+
+    var service = new DefaultStopConsolidationService(repo, transitModel);
+    return new DecorateConsolidatedStopNames(service);
+  }
 }
diff --git a/...src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java b/...src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java
@@ -1,8 +1,10 @@
 package org.opentripplanner.ext.stopconsolidation;
 
+import java.util.ArrayList;
 import java.util.Objects;
 import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopLeg;
 import org.opentripplanner.model.plan.Itinerary;
+import org.opentripplanner.model.plan.Leg;
 import org.opentripplanner.model.plan.ScheduledTransitLeg;
 import org.opentripplanner.routing.algorithm.filterchain.framework.spi.ItineraryDecorator;
 
@@ -13,6 +15,7 @@
  */
 public class DecorateConsolidatedStopNames implements ItineraryDecorator {
 
+  private static final int MAX_INTRA_STOP_WALK_DISTANCE_METERS = 15;
   private final StopConsolidationService service;
 
   public DecorateConsolidatedStopNames(StopConsolidationService service) {
@@ -22,6 +25,7 @@ public DecorateConsolidatedStopNames(StopConsolidationService service) {
   @Override
   public void decorate(Itinerary itinerary) {
     replaceConsolidatedStops(itinerary);
+    removeShortWalkLegs(itinerary);
   }
 
   /**
@@ -51,6 +55,43 @@ private void replaceConsolidatedStops(Itinerary i) {
     });
   }
 
+  /**
+   * Removes walk legs from and to a consolidated stop if they are deemed "short". This means that
+   * they are from a different element of the consolidated stop.
+   */
+  private void removeShortWalkLegs(Itinerary itinerary) {
+    var legs = new ArrayList<>(itinerary.getLegs());
+    var first = legs.getFirst();
+    if (
+      service.isPartOfConsolidatedStop(first.getTo().stop) &&
+        isShortWalkLeg(first)
+    ) {
+      legs.removeFirst();
+    }
+    var last = legs.getLast();
+    if (
+      service.isPartOfConsolidatedStop(last.getFrom().stop) &&
+        isShortWalkLeg(last)
+    ) {
+      legs.removeLast();
+    }
+
+    var transfersRemoved = legs.stream().filter(l -> !isTransferWithinConsolidatedStop(l)).toList();
+
+    itinerary.setLegs(transfersRemoved);
+  }
+
+  private boolean isTransferWithinConsolidatedStop(Leg l) {
+    return isShortWalkLeg(l) &&
+      service.isPartOfConsolidatedStop(l.getFrom().stop) &&
+      service.isPartOfConsolidatedStop(l.getTo().stop);
+  }
+
+  private static boolean isShortWalkLeg(Leg leg) {
+    return leg.isWalkingLeg() &&
+      leg.getDistanceMeters() < MAX_INTRA_STOP_WALK_DISTANCE_METERS;
+  }
+
   /**
    * Figures out if the from/to stops are part of a consolidated stop group and therefore
    * some stops need to be replaced.

diff --git a/...ation/src/ext/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationModule.java b/...ation/src/ext/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationModule.java
@@ -25,7 +25,7 @@
  */
 public class StopConsolidationModule implements GraphBuilderModule {
 
-  private static final Logger LOG = LoggerFactory.getLogger(TripPattern.class);
+  private static final Logger LOG = LoggerFactory.getLogger(StopConsolidationModule.class);
 
   private final StopConsolidationRepository repository;
   private final TimetableRepository timetableRepository;

diff --git a/...tion/src/ext/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationService.java b/...tion/src/ext/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationService.java
@@ -2,6 +2,7 @@
 
 import java.util.List;
 import java.util.Optional;
+import javax.annotation.Nullable;
 import org.opentripplanner.ext.stopconsolidation.model.StopReplacement;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.organization.Agency;
@@ -44,5 +45,5 @@ public interface StopConsolidationService {
    */
   Optional<StopLocation> primaryStop(FeedScopedId id);
 
-  boolean isPartOfConsolidatedStop(StopLocation sl);
+  boolean isPartOfConsolidatedStop(@Nullable StopLocation sl);
 }
diff --git a/...a/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java b/...a/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java
@@ -5,6 +5,7 @@
 import java.util.Optional;
 import java.util.stream.Stream;
 import org.opentripplanner.ext.stopconsolidation.StopConsolidationRepository;
+import javax.annotation.Nullable;
 import org.opentripplanner.ext.stopconsolidation.StopConsolidationService;
 import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup;
 import org.opentripplanner.ext.stopconsolidation.model.StopReplacement;
@@ -67,8 +68,12 @@ public boolean isSecondaryStop(StopLocation stop) {
   }
 
   @Override
-  public boolean isPartOfConsolidatedStop(StopLocation sl) {
-    return isSecondaryStop(sl) || isPrimaryStop(sl);
+  public boolean isPartOfConsolidatedStop(@Nullable StopLocation sl) {
+    if (sl == null) {
+      return false;
+    } else {
+      return isSecondaryStop(sl) || isPrimaryStop(sl);
+    }
   }
 
   @Override

diff --git a/application/src/main/java/org/opentripplanner/apis/gtfs/generated/README.md b/application/src/main/java/org/opentripplanner/apis/gtfs/generated/README.md
@@ -2,7 +2,7 @@
 
 ## Requirements
 
-- NodeJS (version 16 or newer)
+- NodeJS (version 18 or newer)
 
 ## Running
 

diff --git a/application/src/main/java/org/opentripplanner/apis/gtfs/generated/package.json b/application/src/main/java/org/opentripplanner/apis/gtfs/generated/package.json
@@ -11,7 +11,7 @@
   "license": "LGPL-3.0",
   "dependencies": {
     "@graphql-codegen/add": "5.0.3",
-    "@graphql-codegen/cli": "5.0.2",
+    "@graphql-codegen/cli": "5.0.3",
     "@graphql-codegen/java": "4.0.1",
     "@graphql-codegen/java-resolvers": "3.0.0",
     "graphql": "16.9.0"