Merge remote-tracking branch 'upstream/dev-2.x' into flex-duration-fa…

…ctors

client-next/package.json

@@ -33,12 +33,12 @@
     "@graphql-codegen/introspection": "4.0.3",
     "@parcel/watcher": "2.4.1",
     "@testing-library/react": "15.0.2",
-    "@types/react": "18.2.73",
-    "@types/react-dom": "18.2.23",
-    "@typescript-eslint/eslint-plugin": "7.5.0",
-    "@typescript-eslint/parser": "7.5.0",
+    "@types/react": "18.2.79",
+    "@types/react-dom": "18.2.25",
+    "@typescript-eslint/eslint-plugin": "7.7.0",
+    "@typescript-eslint/parser": "7.7.0",
     "@vitejs/plugin-react": "4.2.1",
-    "@vitest/coverage-v8": "1.4.0",
+    "@vitest/coverage-v8": "1.5.0",
     "eslint": "8.57.0",
     "eslint-config-prettier": "9.1.0",
     "eslint-plugin-import": "2.29.1",
@@ -48,8 +48,8 @@
     "eslint-plugin-react-refresh": "0.4.6",
     "jsdom": "24.0.0",
     "prettier": "3.2.5",
-    "typescript": "5.4.3",
-    "vite": "5.2.7",
-    "vitest": "1.4.0"
+    "typescript": "5.4.5",
+    "vite": "5.2.8",
+    "vitest": "1.5.0"
   }
 }

docs/Changelog.md

@@ -13,6 +13,8 @@ based on merged pull requests. Search GitHub issues and pull requests for smalle
 - Discourage instead of ban cycling on use_sidepath ways and do the same for walking on foot=use_sidepath [#5790](https://github.com/opentripplanner/OpenTripPlanner/pull/5790)
 - Prune islands with mode-less stop vertices [#5782](https://github.com/opentripplanner/OpenTripPlanner/pull/5782)
 - Overwrite default WALK directMode when it is not set in the request, but modes is set [#5779](https://github.com/opentripplanner/OpenTripPlanner/pull/5779)
+- Fix trip duplication in Graph Builder DSJ mapping [#5794](https://github.com/opentripplanner/OpenTripPlanner/pull/5794)
+- Optionally abort startup when unknown configuration parameters were detected [#5676](https://github.com/opentripplanner/OpenTripPlanner/pull/5676)
 [](AUTOMATIC_CHANGELOG_PLACEHOLDER_DO_NOT_REMOVE)
 
 ## 2.5.0 (2024-03-13)

docs/Frontends.md

@@ -32,7 +32,7 @@ While the "classic" (i.e. old) debug frontend is enabled by default as of this w
 // otp-config.json
 {
   "otpFeatures": {
-    "DebugClient": true,
+    "DebugUi": true,
     "SandboxAPIGeocoder": true
   }
 }

docs/Migrating-Configuration.md

@@ -0,0 +1,37 @@
+While OTP's GraphQL APIs are very, very stable even across versions, the JSON configuration schema
+is not. Changes to it are relatively frequent and you can see all PRs that change it with
+the [Github tag 'config change'](https://github.com/opentripplanner/OpenTripPlanner/pulls?q=label%3A%22config+change%22).
+
+### Migrating the JSON configuration
+
+OTP validates the configuration and prints warnings during startup. This means that when you
+migrate to a newer version, you should carefully inspect the logs. If you see messages like
+
+```
+(NodeAdapter.java:170) Unexpected config parameter: 'routingDefaults.stairsReluctance:1.65' in 'router-config.json'
+```
+
+this means there are properties in your configuration that are unknown to OTP and therefore likely
+to be a configuration error, perhaps because the schema was changed. In such a case you should
+consult the [feature](Configuration.md#otp-features), [router](RouterConfiguration.md) or 
+[build configuration documentation](BuildConfiguration.md) to find out what the new schema looks like.
+
+By default, OTP starts up even if unknown configuration parameters exist. This supports forward and backwards
+migration of config parameters independent if the OTP version. This allows the configuration to follow 
+the lifecycle of the environment and still be able to roll back OTP. 
+Combined with the config parameter substitution it also allows using the same configuration in 
+different environments. Tip! Rolling out the configuration before rolling out a new
+version of OTP, ensure you that you are safe and can roll back later. 
+
+An example: you change the location of the graphs, a critical error occurs afterwards and you need to 
+roll back. Any member of the dev-ops team should then be confident that they can roll back without 
+risk - even if the environment changed since the last OTP version was rolled out.
+
+### Aborting on invalid configuration
+
+If you want OTP to abort the startup when encountering unknown configuration parameters, you can add 
+the flag `--abortOnUnknownConfig` to your regular OTP CLI commands.
+
+This should of course be used wisely as it can also accidentally make your production instances refuse to start up.
+For some deployments this is a good solution - especially if the config substitution feature is used to inject
+environment specific information. Using this feature in the graph-build phase is less risky, than in the OTP serve phase.

mkdocs.yml

@@ -76,6 +76,7 @@ nav:
     - Router: 'RouterConfiguration.md'
     - "Route Request": 'RouteRequest.md'
     - "Realtime Updaters": 'UpdaterConfig.md'
+    - "Migrating between versions/builds": 'Migrating-Configuration.md'
 - Features explained:
     - "Routing modes": 'RoutingModes.md'
     - "In-station navigation": 'In-Station-Navigation.md'

pom.xml

@@ -67,7 +67,7 @@
         <netcdf4.version>5.5.3</netcdf4.version>
         <logback.version>1.5.3</logback.version>
         <lucene.version>9.10.0</lucene.version>
-        <slf4j.version>2.0.12</slf4j.version>
+        <slf4j.version>2.0.13</slf4j.version>
         <netex-java-model.version>2.0.15</netex-java-model.version>
         <siri-java-model.version>1.26</siri-java-model.version>
         <jaxb-runtime.version>4.0.5</jaxb-runtime.version>
@@ -674,7 +674,7 @@
         <dependency>
             <groupId>org.entur.gbfs</groupId>
             <artifactId>gbfs-java-model</artifactId>
-            <version>3.0.26</version>
+            <version>3.1.1</version>
         </dependency>
 
         <!-- TESTING -->

src/client/debug-client-preview/index.html

@@ -5,8 +5,8 @@
     <link rel="icon" type="image/svg+xml" href="/img/otp-logo.svg" />
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <title>OTP Debug Client</title>
-    <script type="module" crossorigin src="https://cdn.jsdelivr.net/gh/opentripplanner/debug-client-assets@main/2024/04/2024-04-15T05:43/assets/index-DpEuoXkZ.js"></script>
-    <link rel="stylesheet" crossorigin href="https://cdn.jsdelivr.net/gh/opentripplanner/debug-client-assets@main/2024/04/2024-04-15T05:43/assets/index-CrZjV0cy.css">
+    <script type="module" crossorigin src="https://cdn.jsdelivr.net/gh/opentripplanner/debug-client-assets@main/2024/04/2024-04-16T05:11/assets/index-B9qtLphn.js"></script>
+    <link rel="stylesheet" crossorigin href="https://cdn.jsdelivr.net/gh/opentripplanner/debug-client-assets@main/2024/04/2024-04-16T05:11/assets/index-CrZjV0cy.css">
   </head>
   <body>
     <div id="root"></div>

src/main/java/org/opentripplanner/framework/geometry/GeometryUtils.java

@@ -10,8 +10,6 @@
 import java.util.stream.Stream;
 import org.geojson.GeoJsonObject;
 import org.geojson.LngLatAlt;
-import org.geotools.api.referencing.crs.CoordinateReferenceSystem;
-import org.geotools.referencing.CRS;
 import org.locationtech.jts.algorithm.ConvexHull;
 import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.CoordinateSequence;
@@ -28,30 +26,12 @@
 import org.locationtech.jts.linearref.LengthLocationMap;
 import org.locationtech.jts.linearref.LinearLocation;
 import org.locationtech.jts.linearref.LocationIndexedLine;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 public class GeometryUtils {
 
-  private static final Logger LOG = LoggerFactory.getLogger(GeometryUtils.class);
-
   private static final CoordinateSequenceFactory csf = new PackedCoordinateSequenceFactory();
   private static final GeometryFactory gf = new GeometryFactory(csf);
 
-  /** A shared copy of the WGS84 CRS with longitude-first axis order. */
-  public static final CoordinateReferenceSystem WGS84_XY;
-
-  static {
-    try {
-      WGS84_XY = CRS.getAuthorityFactory(true).createCoordinateReferenceSystem("EPSG:4326");
-    } catch (Exception ex) {
-      LOG.error("Unable to create longitude-first WGS84 CRS", ex);
-      throw new RuntimeException(
-        "Could not create longitude-first WGS84 coordinate reference system."
-      );
-    }
-  }
-
   public static <T> Geometry makeConvexHull(
     Collection<T> collection,
     Function<T, Coordinate> mapToCoordinate

src/main/java/org/opentripplanner/graph_builder/module/ned/ElevationModule.java

@@ -20,13 +20,14 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import org.geotools.api.coverage.Coverage;
 import org.geotools.api.coverage.PointOutsideCoverageException;
+import org.geotools.api.referencing.crs.CoordinateReferenceSystem;
 import org.geotools.api.referencing.operation.TransformException;
 import org.geotools.geometry.Position2D;
+import org.geotools.referencing.CRS;
 import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.Geometry;
 import org.locationtech.jts.geom.impl.PackedCoordinateSequence;
 import org.opentripplanner.framework.geometry.EncodedPolyline;
-import org.opentripplanner.framework.geometry.GeometryUtils;
 import org.opentripplanner.framework.geometry.SphericalDistanceLibrary;
 import org.opentripplanner.framework.lang.IntUtils;
 import org.opentripplanner.framework.logging.ProgressTracker;
@@ -61,6 +62,25 @@
 public class ElevationModule implements GraphBuilderModule {
 
   private static final Logger LOG = LoggerFactory.getLogger(ElevationModule.class);
+  /**
+   * The WGS84 CRS with longitude-first axis order. The first time a CRS lookup is
+   * performed is surprisingly expensive (around 500ms), apparently due to  initializing
+   * an HSQLDB JDBC connection. For this reason, the constant is defined in this
+   * narrower scope rather than a shared utility class, where it was seen to incur the
+   * initialization cost in a broader range of tests than is necessary.
+   */
+  private static final CoordinateReferenceSystem WGS84_XY;
+
+  static {
+    try {
+      WGS84_XY = CRS.getAuthorityFactory(true).createCoordinateReferenceSystem("EPSG:4326");
+    } catch (Exception ex) {
+      LOG.error("Unable to create longitude-first WGS84 CRS", ex);
+      throw new RuntimeException(
+        "Could not create longitude-first WGS84 coordinate reference system."
+      );
+    }
+  }
 
   /** The elevation data to be used in calculating elevations. */
   private final ElevationGridCoverageFactory gridCoverageFactory;
@@ -564,7 +584,7 @@ private double getElevation(Coverage coverage, double x, double y)
       // GeoTIFFs in various projections. Note that GeoTools defaults to strict EPSG axis ordering of (lat, long)
       // for DefaultGeographicCRS.WGS84, but OTP is using (long, lat) throughout and assumes unprojected DEM
       // rasters to also use (long, lat).
-      coverage.evaluate(new Position2D(GeometryUtils.WGS84_XY, x, y), values);
+      coverage.evaluate(new Position2D(WGS84_XY, x, y), values);
     } catch (PointOutsideCoverageException e) {
       nPointsOutsideDEM.incrementAndGet();
       throw e;

src/main/java/org/opentripplanner/netex/mapping/TripPatternMapper.java

@@ -67,7 +67,7 @@ class TripPatternMapper {
 
   private final ReadOnlyHierarchicalMap<String, Route> routeById;
 
-  private final Multimap<String, ServiceJourney> serviceJourniesByPatternId = ArrayListMultimap.create();
+  private final Multimap<String, ServiceJourney> serviceJourneysByPatternId = ArrayListMultimap.create();
 
   private final ReadOnlyHierarchicalMapById<OperatingDay> operatingDayById;
 
@@ -152,12 +152,12 @@ class TripPatternMapper {
     this.serviceJourneyById = serviceJourneyById;
     // Index service journey by pattern id
     for (ServiceJourney sj : serviceJourneyById.localValues()) {
-      this.serviceJourniesByPatternId.put(sj.getJourneyPatternRef().getValue().getRef(), sj);
+      this.serviceJourneysByPatternId.put(sj.getJourneyPatternRef().getValue().getRef(), sj);
     }
   }
 
   Optional<TripPatternMapperResult> mapTripPattern(JourneyPattern_VersionStructure journeyPattern) {
-    Collection<ServiceJourney> serviceJourneys = serviceJourniesByPatternId.get(
+    Collection<ServiceJourney> serviceJourneys = serviceJourneysByPatternId.get(
       journeyPattern.getId()
     );
 
@@ -392,9 +392,12 @@ private Trip mapTrip(
     JourneyPattern_VersionStructure journeyPattern,
     ServiceJourney serviceJourney
   ) {
-    return tripMapper.mapServiceJourney(
-      serviceJourney,
-      () -> findTripHeadsign(journeyPattern, serviceJourney)
+    return deduplicator.deduplicateObject(
+      Trip.class,
+      tripMapper.mapServiceJourney(
+        serviceJourney,
+        () -> findTripHeadsign(journeyPattern, serviceJourney)
+      )
     );
   }
 

src/main/java/org/opentripplanner/standalone/OTPMain.java

@@ -13,6 +13,7 @@
 import org.opentripplanner.raptor.configure.RaptorConfig;
 import org.opentripplanner.routing.graph.SerializedGraphObject;
 import org.opentripplanner.standalone.config.CommandLineParameters;
+import org.opentripplanner.standalone.config.ConfigModel;
 import org.opentripplanner.standalone.configure.ConstructApplication;
 import org.opentripplanner.standalone.configure.LoadApplication;
 import org.opentripplanner.standalone.server.GrizzlyServer;
@@ -113,6 +114,8 @@ private static void startOTPServer(CommandLineParameters cli) {
     var loadApp = new LoadApplication(cli);
     var config = loadApp.config();
 
+    detectUnusedConfigParams(cli, config);
+
     // Validate data sources, command line arguments and config before loading and
     // processing input data to fail early
     loadApp.validateConfigAndDataSources();
@@ -173,6 +176,15 @@ private static void startOTPServer(CommandLineParameters cli) {
     }
   }
 
+  /**
+   * Optionally, check if the config is valid and if not abort the startup process.
+   */
+  private static void detectUnusedConfigParams(CommandLineParameters cli, ConfigModel config) {
+    if (cli.abortOnUnknownConfig) {
+      config.abortOnUnknownParameters();
+    }
+  }
+
   private static void startOtpWebServer(CommandLineParameters params, ConstructApplication app) {
     // Index graph for travel search
     app.transitModel().index();

src/main/java/org/opentripplanner/standalone/config/BuildConfig.java

@@ -726,4 +726,11 @@ public int getSubwayAccessTimeSeconds() {
   public NodeAdapter asNodeAdapter() {
     return root;
   }
+
+  /**
+   * Checks if any unknown or invalid parameters were encountered while loading the configuration.
+   */
+  public boolean hasUnknownParameters() {
+    return root.hasUnknownParameters();
+  }
 }

src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java

@@ -133,6 +133,12 @@ public class CommandLineParameters {
   )
   public boolean visualize;
 
+  @Parameter(
+    names = { "--abortOnUnknownConfig" },
+    description = "Abort the startup if configuration files are found to contain unknown parameters."
+  )
+  public boolean abortOnUnknownConfig = false;
+
   /**
    * The remaining single parameter after the switches is the directory with the configuration
    * files. This directory may contain other files like the graph, input data and report files.

src/main/java/org/opentripplanner/standalone/config/ConfigModel.java

@@ -2,6 +2,7 @@
 
 import com.fasterxml.jackson.databind.node.MissingNode;
 import org.opentripplanner.framework.application.OTPFeature;
+import org.opentripplanner.framework.application.OtpAppException;
 import org.opentripplanner.framework.application.OtpFileNames;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -105,4 +106,24 @@ public static void initializeOtpFeatures(OtpConfig otpConfig) {
     OTPFeature.enableFeatures(otpConfig.otpFeatures);
     OTPFeature.logFeatureSetup();
   }
+
+  /**
+   * Checks if any unknown or invalid parameters were encountered while loading the configuration.
+   * <p>
+   * If so it throws an exception.
+   */
+  public void abortOnUnknownParameters() {
+    if (
+      (
+        otpConfig.hasUnknownParameters() ||
+        buildConfig.hasUnknownParameters() ||
+        routerConfig.hasUnknownParameters()
+      )
+    ) {
+      throw new OtpAppException(
+        "Configuration contains unknown parameters (see above for details). " +
+        "Please fix your configuration or remove --abortOnUnknownConfig from your OTP CLI parameters."
+      );
+    }
+  }
 }

src/main/java/org/opentripplanner/standalone/config/OtpConfig.java

@@ -71,4 +71,11 @@ public OtpConfig(NodeAdapter nodeAdapter, boolean logUnusedParams) {
       root.logAllWarnings(LOG::warn);
     }
   }
+
+  /**
+   * Checks if any unknown or invalid parameters were encountered while loading the configuration.
+   */
+  public boolean hasUnknownParameters() {
+    return root.hasUnknownParameters();
+  }
 }

src/main/java/org/opentripplanner/standalone/config/RouterConfig.java

@@ -150,4 +150,11 @@ public String toString() {
     // Print ONLY the values set, not default values
     return root.toPrettyString();
   }
+
+  /**
+   * Checks if any unknown or invalid parameters were encountered while loading the configuration.
+   */
+  public boolean hasUnknownParameters() {
+    return root.hasUnknownParameters();
+  }
 }

src/main/java/org/opentripplanner/standalone/config/framework/json/NodeAdapter.java

@@ -172,6 +172,13 @@ public void logAllWarnings(Consumer<String> logger) {
     allWarnings().forEach(logger);
   }
 
+  /**
+   * Checks if any unknown or invalid properties were encountered while loading the configuration.
+   */
+  public boolean hasUnknownParameters() {
+    return !unusedParams().isEmpty();
+  }
+
   /**
    * Be careful when using this method - this bypasses the NodeAdaptor, and we loose
    * track of unused parameters and cannot generate documentation for the children.

src/main/java/org/opentripplanner/standalone/config/framework/json/ParameterBuilder.java

@@ -120,10 +120,6 @@ public Boolean asBoolean(boolean defaultValue) {
     return ofOptional(BOOLEAN, defaultValue, JsonNode::asBoolean);
   }
 
-  public Map<String, Boolean> asBooleanMap() {
-    return ofOptionalMap(BOOLEAN, JsonNode::asBoolean);
-  }
-
   /** @throws OtpAppException if parameter is missing. */
   public double asDouble() {
     return ofRequired(DOUBLE).asDouble();