[Do not merge] Remove "short" legs from shared stop results

DEVELOPMENT_DECISION_RECORDS.md

@@ -16,7 +16,7 @@ tests. Expect to include some code cleanup as part of all PRs.
 ## Follow-Naming-Conventions
 
 Use established terminology from GTFS, NeTEx or the existing OTP code. Make sure the code is easy
-to read and understand. [Follow naming conventions](CODE_CONVENTIONS.md#naming-conventions) . 
+to read and understand. [Follow naming conventions](doc/dev/decisionrecords/NamingConventions.md#naming-conventions) . 
 
 
 ## Write-Code-Documentation - Use JavaDoc
@@ -32,7 +32,7 @@ notes on `private` members and as inline comments.
 
 **See also**
  - [Developers-Guide > Code comments](doc/user/Developers-Guide.md#code-comments).
- - [Codestyle > Javadoc Guidlines](doc/dev/decisionrecords/Codestyle.md#javadoc-guidlines) - JavaDoc checklist
+ - [Codestyle > Javadoc Guidelines](doc/dev/decisionrecords/Codestyle.md#javadoc-guidlines) - JavaDoc checklist
 
 
 ## Document-Config-and-APIs

doc/user/Developers-Guide.md

@@ -205,7 +205,8 @@ so they are a bit easier to maintain that way. The primary audience is also acti
 that have the code checked out locally.
 
  - [Architecture](https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/ARCHITECTURE.md) 
- - [Code Conventions](https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/CODE_CONVENTIONS.md)
+ - [Code Style](https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/doc/dev/decisionrecords/Codestyle.md)
+ - [Naming Conventions](https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/doc/dev/decisionrecords/NamingConventions.md)
  - [Development Decision Records](https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/DEVELOPMENT_DECISION_RECORDS.md)
 
 

doc/user/requirements.txt

@@ -1,4 +1,4 @@
-mkdocs==1.6.0
-mkdocs-material==9.5.27
+mkdocs==1.6.1
+mkdocs-material==9.5.39
 mike@git+https://github.com/jimporter/mike.git@f0522f245e64687dd18384fbd86b721175711474
 mkdocs-no-sitemap-plugin==0.0.1

pom.xml

@@ -60,13 +60,13 @@
         <!-- Lib versions - keep list sorted on property name -->
         <geotools.version>32.0</geotools.version>
         <google.dagger.version>2.52</google.dagger.version>
-        <jackson.version>2.17.2</jackson.version>
+        <jackson.version>2.18.0</jackson.version>
         <jersey.version>3.1.8</jersey.version>
         <junit.version>5.11.0</junit.version>
         <micrometer.version>1.13.4</micrometer.version>
         <netcdf4.version>5.6.0</netcdf4.version>
         <logback.version>1.5.7</logback.version>
-        <lucene.version>9.11.1</lucene.version>
+        <lucene.version>9.12.0</lucene.version>
         <slf4j.version>2.0.16</slf4j.version>
         <netex-java-model.version>2.0.15</netex-java-model.version>
         <siri-java-model.version>1.27</siri-java-model.version>

renovate.json5

@@ -147,7 +147,8 @@
     {
       "description": "give some projects time to publish a changelog before opening the PR",
       "matchPackagePrefixes": [
-        "com.google.dagger:"
+        "com.google.dagger:",
+        "com.fasterxml.jackson"
       ],
       "minimumReleaseAge": "1 week"
     },

src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNamesTest.java → src/ext-test/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopsTest.java

@@ -12,14 +12,18 @@
 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;
 
-class DecorateConsolidatedStopNamesTest {
+class DecorateConsolidatedStopsTest {
 
   private static final FareProduct fp = new FareProduct(
     id("fp"),
@@ -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 transitModel = TestStopConsolidationModel.buildTransitModel();
-
-    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, transitModel);
-    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 DecorateConsolidatedStops defaultFilter() {
+    var transitModel = TestStopConsolidationModel.buildTransitModel();
+
+    var repo = new DefaultStopConsolidationRepository();
+    repo.addGroups(GROUPS);
+
+    var service = new DefaultStopConsolidationService(repo, transitModel);
+    return new DecorateConsolidatedStops(service);
+  }
 }

src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java

@@ -18,7 +18,7 @@
 import org.apache.lucene.analysis.standard.StandardAnalyzer;
 import org.apache.lucene.codecs.Codec;
 import org.apache.lucene.codecs.PostingsFormat;
-import org.apache.lucene.codecs.lucene99.Lucene99Codec;
+import org.apache.lucene.codecs.lucene912.Lucene912Codec;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field.Store;
 import org.apache.lucene.document.StoredField;
@@ -34,7 +34,7 @@
 import org.apache.lucene.search.FuzzyQuery;
 import org.apache.lucene.search.PrefixQuery;
 import org.apache.lucene.search.TermQuery;
-import org.apache.lucene.search.suggest.document.Completion99PostingsFormat;
+import org.apache.lucene.search.suggest.document.Completion912PostingsFormat;
 import org.apache.lucene.search.suggest.document.CompletionAnalyzer;
 import org.apache.lucene.search.suggest.document.ContextQuery;
 import org.apache.lucene.search.suggest.document.ContextSuggestField;
@@ -200,8 +200,8 @@ private StopCluster toStopCluster(Document document) {
 
   static IndexWriterConfig iwcWithSuggestField(Analyzer analyzer, final Set<String> suggestFields) {
     IndexWriterConfig iwc = new IndexWriterConfig(analyzer);
-    Codec filterCodec = new Lucene99Codec() {
-      final PostingsFormat postingsFormat = new Completion99PostingsFormat();
+    Codec filterCodec = new Lucene912Codec() {
+      final PostingsFormat postingsFormat = new Completion912PostingsFormat();
 
       @Override
       public PostingsFormat getPostingsFormatForField(String field) {

src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStopNames.java → src/ext/java/org/opentripplanner/ext/stopconsolidation/DecorateConsolidatedStops.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;
 
@@ -11,17 +13,19 @@
  * then replaces it with the appropriate, agency-specific stop name. This is so that the physical
  * signage and in-vehicle display matches what OTP returns as a board/alight stop name.
  */
-public class DecorateConsolidatedStopNames implements ItineraryDecorator {
+public class DecorateConsolidatedStops implements ItineraryDecorator {
 
+  private static final int MAX_INTRA_STOP_WALK_DISTANCE_METERS = 15;
   private final StopConsolidationService service;
 
-  public DecorateConsolidatedStopNames(StopConsolidationService service) {
+  public DecorateConsolidatedStops(StopConsolidationService service) {
     this.service = Objects.requireNonNull(service);
   }
 
   @Override
   public void decorate(Itinerary itinerary) {
     replaceConsolidatedStops(itinerary);
+    removeWalkLegs(itinerary);
   }
 
   /**
@@ -51,11 +55,48 @@ 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 removeWalkLegs(Itinerary itinerary) {
+    var legs = new ArrayList<>(itinerary.getLegs());
+    var first = legs.getFirst();
+    if (
+      service.isPartOfConsolidatedStop(first.getTo().stop) &&
+      first.isWalkingLeg() &&
+      first.getDistanceMeters() < MAX_INTRA_STOP_WALK_DISTANCE_METERS
+    ) {
+      legs.removeFirst();
+    }
+    var last = legs.getLast();
+    if (
+      service.isPartOfConsolidatedStop(last.getFrom().stop) &&
+      last.isWalkingLeg() &&
+      last.getDistanceMeters() < MAX_INTRA_STOP_WALK_DISTANCE_METERS
+    ) {
+      legs.removeLast();
+    }
+
+    var transfersRemoved = legs.stream().filter(l -> !isTransferWithinConsolidatedStop(l)).toList();
+
+    itinerary.setLegs(transfersRemoved);
+  }
+
+  private boolean isTransferWithinConsolidatedStop(Leg l) {
+    return (
+      l.isWalkingLeg() &&
+      (l.getDistanceMeters() < MAX_INTRA_STOP_WALK_DISTANCE_METERS) &&
+      service.isPartOfConsolidatedStop(l.getFrom().stop) &&
+      service.isPartOfConsolidatedStop(l.getTo().stop)
+    );
+  }
+
   /**
    * Figures out if the from/to stops are part of a consolidated stop group and therefore
    * some stops need to be replaced.
    * <p>
-   * Please consult the Javadoc of {@link DecorateConsolidatedStopNames#replaceConsolidatedStops(Itinerary)}
+   * Please consult the Javadoc of {@link DecorateConsolidatedStops#replaceConsolidatedStops(Itinerary)}
    * for details of this idiosyncratic business logic and in particular why the logic is not the same
    * for the from/to stops.
    */

src/ext/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationModule.java

@@ -26,7 +26,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 TransitModel transitModel;

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);
 }

src/ext/java/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java

@@ -5,6 +5,7 @@
 import java.util.Optional;
 import java.util.stream.Stream;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opentripplanner.ext.stopconsolidation.StopConsolidationRepository;
 import org.opentripplanner.ext.stopconsolidation.StopConsolidationService;
 import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup;
@@ -68,8 +69,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

src/main/java/org/opentripplanner/apis/gtfs/datafetchers/LegImpl.java

@@ -10,6 +10,7 @@
 import org.opentripplanner.apis.gtfs.generated.GraphQLDataFetchers;
 import org.opentripplanner.apis.gtfs.generated.GraphQLTypes;
 import org.opentripplanner.apis.gtfs.mapping.NumberMapper;
+import org.opentripplanner.apis.gtfs.mapping.RealtimeStateMapper;
 import org.opentripplanner.ext.restapi.mapping.LocalDateMapper;
 import org.opentripplanner.ext.ridehailing.model.RideEstimate;
 import org.opentripplanner.ext.ridehailing.model.RideHailingLeg;
@@ -191,10 +192,10 @@ public DataFetcher<Boolean> realTime() {
   }
 
   @Override
-  public DataFetcher<String> realtimeState() {
+  public DataFetcher<GraphQLTypes.GraphQLRealtimeState> realtimeState() {
     return environment -> {
       var state = getSource(environment).getRealTimeState();
-      return (state != null) ? state.name() : null;
+      return RealtimeStateMapper.map(state);
     };
   }
 

src/main/java/org/opentripplanner/apis/gtfs/datafetchers/StoptimeImpl.java

@@ -3,10 +3,11 @@
 import graphql.schema.DataFetcher;
 import graphql.schema.DataFetchingEnvironment;
 import org.opentripplanner.apis.gtfs.generated.GraphQLDataFetchers;
+import org.opentripplanner.apis.gtfs.generated.GraphQLTypes;
+import org.opentripplanner.apis.gtfs.mapping.RealtimeStateMapper;
 import org.opentripplanner.framework.graphql.GraphQLUtils;
 import org.opentripplanner.model.StopTime;
 import org.opentripplanner.model.TripTimeOnDate;
-import org.opentripplanner.transit.model.timetable.RealTimeState;
 import org.opentripplanner.transit.model.timetable.Trip;
 
 public class StoptimeImpl implements GraphQLDataFetchers.GraphQLStoptime {
@@ -67,11 +68,11 @@ public DataFetcher<Integer> realtimeDeparture() {
   }
 
   @Override
-  public DataFetcher<String> realtimeState() {
+  public DataFetcher<GraphQLTypes.GraphQLRealtimeState> realtimeState() {
     return environment ->
       getSource(environment).isCanceledEffectively()
-        ? RealTimeState.CANCELED.name()
-        : getSource(environment).getRealTimeState().name();
+        ? GraphQLTypes.GraphQLRealtimeState.CANCELED
+        : RealtimeStateMapper.map(getSource(environment).getRealTimeState());
   }
 
   @Override

src/main/java/org/opentripplanner/apis/gtfs/generated/GraphQLDataFetchers.java

@@ -18,6 +18,7 @@
 import org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLBikesAllowed;
 import org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLInputField;
 import org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLOccupancyStatus;
+import org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLRealtimeState;
 import org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLRelativeDirection;
 import org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLRoutingErrorCode;
 import org.opentripplanner.apis.gtfs.generated.GraphQLTypes.GraphQLTransitMode;
@@ -504,7 +505,7 @@ public interface GraphQLLeg {
 
     public DataFetcher<Boolean> realTime();
 
-    public DataFetcher<String> realtimeState();
+    public DataFetcher<GraphQLRealtimeState> realtimeState();
 
     public DataFetcher<Boolean> rentedBike();
 
@@ -1082,7 +1083,7 @@ public interface GraphQLStoptime {
 
     public DataFetcher<Integer> realtimeDeparture();
 
-    public DataFetcher<String> realtimeState();
+    public DataFetcher<GraphQLRealtimeState> realtimeState();
 
     public DataFetcher<Integer> scheduledArrival();