Fix NullPointerException in stop transfer priority cost vector genera…

…tion opentripplanner#5943 (squashed d6c9f6f..0c914c4)

docs/BuildConfiguration.md

@@ -731,7 +731,7 @@ but we want to calculate the transfers always from OSM data.
 
 **Since version:** `2.3` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"allowed"`   
 **Path:** /gtfsDefaults   
-**Enum values:** `discouraged` | `allowed` | `recommended` | `preferred`
+**Enum values:** `preferred` | `recommended` | `allowed` | `discouraged`
 
 Should there be some preference or aversion for transfers at stops that are part of a station.
 
@@ -980,7 +980,7 @@ but we want to calculate the transfers always from OSM data.
 
 **Since version:** `2.3` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"allowed"`   
 **Path:** /transitFeeds/[0]   
-**Enum values:** `discouraged` | `allowed` | `recommended` | `preferred`
+**Enum values:** `preferred` | `recommended` | `allowed` | `discouraged`
 
 Should there be some preference or aversion for transfers at stops that are part of a station. Overrides the value specified in `gtfsDefaults`.
 

docs/RouterConfiguration.md

@@ -363,7 +363,7 @@ for more info."
 
 **Since version:** `2.0` ∙ **Type:** `enum map of integer` ∙ **Cardinality:** `Optional`   
 **Path:** /transit   
-**Enum keys:** `discouraged` | `allowed` | `recommended` | `preferred`
+**Enum keys:** `preferred` | `recommended` | `allowed` | `discouraged`
 
 Costs for boarding and alighting during transfers at stops with a given transfer priority.
 

src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParameters.java

@@ -24,9 +24,6 @@ public record GtfsFeedParameters(
   implements DataSourceConfig {
   public static final boolean DEFAULT_REMOVE_REPEATED_STOPS = true;
 
-  public static final StopTransferPriority DEFAULT_STATION_TRANSFER_PREFERENCE =
-    StopTransferPriority.ALLOWED;
-
   public static final boolean DEFAULT_DISCARD_MIN_TRANSFER_TIMES = false;
 
   public static final boolean DEFAULT_BLOCK_BASED_INTERLINING = true;

src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParametersBuilder.java

@@ -18,7 +18,7 @@ public class GtfsFeedParametersBuilder {
 
   public GtfsFeedParametersBuilder() {
     this.removeRepeatedStops = GtfsFeedParameters.DEFAULT_REMOVE_REPEATED_STOPS;
-    this.stationTransferPreference = GtfsFeedParameters.DEFAULT_STATION_TRANSFER_PREFERENCE;
+    this.stationTransferPreference = StopTransferPriority.defaultValue();
     this.discardMinTransferTimes = GtfsFeedParameters.DEFAULT_DISCARD_MIN_TRANSFER_TIMES;
     this.blockBasedInterlining = GtfsFeedParameters.DEFAULT_BLOCK_BASED_INTERLINING;
     this.maxInterlineDistance = GtfsFeedParameters.DEFAULT_MAX_INTERLINE_DISTANCE;

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java

@@ -19,6 +19,11 @@ static List<List<Transfer>> mapTransfers(StopModel stopModel, TransitModel trans
 
     for (int i = 0; i < stopModel.stopIndexSize(); ++i) {
       var stop = stopModel.stopByIndex(i);
+
+      if (stop == null) {
+        continue;
+      }
+
       ArrayList<Transfer> list = new ArrayList<>();
 
       for (PathTransfer pathTransfer : transitModel.getTransfersByStop(stop)) {

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java

@@ -190,12 +190,21 @@ static int[] createStopBoardAlightTransferCosts(
     if (!tuningParams.enableStopTransferPriority()) {
       return null;
     }
+    int defaultCost = RaptorCostConverter.toRaptorCost(
+      tuningParams.stopBoardAlightDuringTransferCost(StopTransferPriority.defaultValue())
+    );
     int[] stopBoardAlightTransferCosts = new int[stops.stopIndexSize()];
 
     for (int i = 0; i < stops.stopIndexSize(); ++i) {
-      StopTransferPriority priority = stops.stopByIndex(i).getPriority();
-      int domainCost = tuningParams.stopBoardAlightDuringTransferCost(priority);
-      stopBoardAlightTransferCosts[i] = RaptorCostConverter.toRaptorCost(domainCost);
+      // There can be holes in the stop index, so we need to account for 'null' here.
+      var stop = stops.stopByIndex(i);
+      if (stop == null) {
+        stopBoardAlightTransferCosts[i] = defaultCost;
+      } else {
+        var priority = stop.getPriority();
+        int domainCost = tuningParams.stopBoardAlightDuringTransferCost(priority);
+        stopBoardAlightTransferCosts[i] = RaptorCostConverter.toRaptorCost(domainCost);
+      }
     }
     return stopBoardAlightTransferCosts;
   }

src/main/java/org/opentripplanner/transit/model/site/RegularStop.java

@@ -127,7 +127,9 @@ public Point getGeometry() {
   @Override
   @Nonnull
   public StopTransferPriority getPriority() {
-    return isPartOfStation() ? getParentStation().getPriority() : StopTransferPriority.ALLOWED;
+    return isPartOfStation()
+      ? getParentStation().getPriority()
+      : StopTransferPriority.defaultValue();
   }
 
   @Override

src/main/java/org/opentripplanner/transit/model/site/Station.java

@@ -30,8 +30,6 @@ public class Station
   extends AbstractTransitEntity<Station, StationBuilder>
   implements StopLocationsGroup, LogInfo {
 
-  public static final StopTransferPriority DEFAULT_PRIORITY = StopTransferPriority.ALLOWED;
-
   private final I18NString name;
   private final String code;
   private final I18NString description;
@@ -52,7 +50,8 @@ public class Station
     // Required fields
     this.name = Objects.requireNonNull(builder.getName());
     this.coordinate = Objects.requireNonNull(builder.getCoordinate());
-    this.priority = Objects.requireNonNullElse(builder.getPriority(), DEFAULT_PRIORITY);
+    this.priority =
+      Objects.requireNonNullElse(builder.getPriority(), StopTransferPriority.defaultValue());
     this.transfersNotAllowed = builder.isTransfersNotAllowed();
 
     // Optional fields

src/main/java/org/opentripplanner/transit/model/site/StopLocation.java

@@ -142,7 +142,7 @@ default ZoneId getTimeZone() {
 
   @Nonnull
   default StopTransferPriority getPriority() {
-    return StopTransferPriority.ALLOWED;
+    return StopTransferPriority.defaultValue();
   }
 
   boolean isPartOfSameStationAs(StopLocation alternativeStop);

src/main/java/org/opentripplanner/transit/model/site/StopTransferPriority.java

@@ -9,31 +9,35 @@
  */
 public enum StopTransferPriority {
   /**
-   * Block transfers from/to this stop. In OTP this is not a definitive block, just a huge penalty
-   * is added to the cost function.
+   * Preferred place to transfer, strongly recommended.
    * <p>
-   * NeTEx equivalent is NO_INTERCHANGE.
+   * NeTEx equivalent is PREFERRED_INTERCHANGE.
    */
-  DISCOURAGED,
-
+  PREFERRED,
+  /**
+   * Recommended stop place.
+   * <p>
+   * NeTEx equivalent is RECOMMENDED_INTERCHANGE.
+   */
+  RECOMMENDED,
   /**
    * Allow transfers from/to this stop. This is the default.
    * <p>
    * NeTEx equivalent is INTERCHANGE_ALLOWED.
    */
   ALLOWED,
-
   /**
-   * Recommended stop place.
+   * Block transfers from/to this stop. In OTP this is not a definitive block, just a huge penalty
+   * is added to the cost function.
    * <p>
-   * NeTEx equivalent is RECOMMENDED_INTERCHANGE.
+   * NeTEx equivalent is NO_INTERCHANGE.
    */
-  RECOMMENDED,
+  DISCOURAGED;
 
   /**
-   * Preferred place to transfer, strongly recommended.
-   * <p>
-   * NeTEx equivalent is PREFERRED_INTERCHANGE.
+   * The {@link #ALLOWED} is used as default value in cases where the value is not set.
    */
-  PREFERRED,
+  public static StopTransferPriority defaultValue() {
+    return ALLOWED;
+  }
 }

src/main/java/org/opentripplanner/transit/service/StopModel.java

@@ -162,6 +162,7 @@ public Collection<GroupStop> listGroupStops() {
     return groupStopById.values();
   }
 
+  @Nullable
   public StopLocation stopByIndex(int stopIndex) {
     return index.stopByIndex(stopIndex);
   }

src/main/java/org/opentripplanner/transit/service/StopModelIndex.java

@@ -1,8 +1,11 @@
 package org.opentripplanner.transit.service;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
+import javax.annotation.Nullable;
 import org.locationtech.jts.geom.Envelope;
 import org.opentripplanner.framework.collection.CollectionsView;
 import org.opentripplanner.framework.geometry.HashGridSpatialIndex;
@@ -12,6 +15,9 @@
 import org.opentripplanner.transit.model.site.RegularStop;
 import org.opentripplanner.transit.model.site.Station;
 import org.opentripplanner.transit.model.site.StopLocation;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
 
 /**
  * Indexed access to Stop entities.
@@ -20,6 +26,8 @@
  */
 class StopModelIndex {
 
+  private static final Logger LOG = LoggerFactory.getLogger(StopModelIndex.class);
+
   private final HashGridSpatialIndex<RegularStop> regularStopSpatialIndex = new HashGridSpatialIndex<>();
   private final Map<Station, MultiModalStation> multiModalStationForStations = new HashMap<>();
   private final HashGridSpatialIndex<AreaStop> locationIndex = new HashGridSpatialIndex<>();
@@ -58,6 +66,8 @@ class StopModelIndex {
     // Trim the sizes of the indices
     regularStopSpatialIndex.compact();
     locationIndex.compact();
+
+    logHolesInIndex();
   }
 
   /**
@@ -71,6 +81,7 @@ MultiModalStation getMultiModalStationForStation(Station station) {
     return multiModalStationForStations.get(station);
   }
 
+  @Nullable
   StopLocation stopByIndex(int index) {
     return stopsByIndex[index];
   }
@@ -82,4 +93,19 @@ int stopIndexSize() {
   Collection<AreaStop> findAreaStops(Envelope envelope) {
     return locationIndex.query(envelope);
   }
+
+  /**
+   * A small number of holes in the stop-index is ok, but if there are many, it will affect
+   * the Raptor performance.
+   */
+  private void logHolesInIndex() {
+    int c = (int) Arrays.stream(stopsByIndex).filter(Objects::isNull).count();
+    if (c > 0) {
+      double p = (100.0 * c) / stopsByIndex.length;
+      // Log this as warning if more than 5% of the space is null
+      LOG
+        .atLevel(p >= 5.0 ? Level.WARN : Level.INFO)
+        .log("The stop index contains holes in it. {} of {} is null.", c, stopsByIndex.length);
+    }
+  }
 }