Merge remote-tracking branch 'origin/dev'

Author: abyrd

LICENSE

@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2020-2023 Conveyal
+Copyright (c) 2020-2025 Conveyal LLC
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

build.gradle

@@ -3,7 +3,7 @@ plugins {
     id 'application'
     id 'maven-publish'
     id 'com.palantir.git-version' version '2.0.0'
-    id 'com.github.johnrengelman.shadow' version '8.1.1'
+    id 'com.gradleup.shadow' version '9.0.1'
 }
 
 group = 'com.conveyal'
@@ -26,7 +26,7 @@ jar {
     manifest {
         attributes 'Automatic-Module-Name': 'com.conveyal.r5',
             'Main-Class': 'com.conveyal.analysis.BackendMain',
-            'Build-Jdk-Spec': targetCompatibility.getMajorVersion(),
+            'Build-Jdk-Spec': java.toolchain.languageVersion,
             'Implementation-Title': 'Conveyal Analysis Backend',
             'Implementation-Vendor': 'Conveyal LLC',
             'Implementation-Version': project.version
@@ -270,5 +270,9 @@ dependencies {
     // Although rarely used it should be low-impact: it is a test-only dependency with no transitive dependenices.
     testImplementation('org.jfree:jfreechart:1.5.1')
 
+    // Load the test launcher, no longer automatic in Gradle 9:
+    // https://github.com/gradle/gradle/issues/34512
+    testRuntimeOnly "org.junit.platform:junit-platform-launcher"
+
 }
 

src/main/java/com/conveyal/analysis/SelectingGridReducer.java

@@ -3,6 +3,7 @@
 import com.conveyal.r5.analyst.Grid;
 import com.google.common.io.LittleEndianDataInputStream;
 
+import java.io.BufferedInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.zip.GZIPInputStream;
@@ -33,8 +34,10 @@ public SelectingGridReducer(int index) {
         this.index = index;
     }
 
+    private static final int BUFSIZE = 32 * 1024;
+
     public Grid compute (InputStream rawInput) throws IOException {
-        LittleEndianDataInputStream input = new LittleEndianDataInputStream(new GZIPInputStream(rawInput));
+        LittleEndianDataInputStream input = new LittleEndianDataInputStream(new BufferedInputStream(new GZIPInputStream(rawInput, BUFSIZE), BUFSIZE));
 
         char[] header = new char[8];
         for (int i = 0; i < 8; i++) {

src/main/java/com/conveyal/analysis/controllers/RegionalAnalysisController.java

@@ -60,6 +60,9 @@
 import static com.conveyal.analysis.util.JsonUtil.toJson;
 import static com.conveyal.file.FileCategory.BUNDLES;
 import static com.conveyal.file.FileCategory.RESULTS;
+import static com.conveyal.file.FileStorageFormat.GEOTIFF;
+import static com.conveyal.file.FileStorageFormat.GRID;
+import static com.conveyal.file.FileStorageFormat.PNG;
 import static com.conveyal.file.UrlWithHumanName.filenameCleanString;
 import static com.conveyal.r5.transit.TransportNetworkCache.getScenarioFilename;
 import static com.google.common.base.Preconditions.checkArgument;
@@ -161,10 +164,10 @@ private RegionalAnalysis deleteRegionalAnalysis (Request req, Response res) {
         return analysis;
     }
 
-    private int getIntQueryParameter (Request req, String parameterName, int defaultValue) {
+    private int getIntQueryParameter (Request req, String parameterName) {
         String paramValue = req.queryParams(parameterName);
         if (paramValue == null) {
-            return defaultValue;
+            throw new IllegalArgumentException("Must provide query parameter " + parameterName);
         }
         try {
             return Integer.parseInt(paramValue);
@@ -294,21 +297,21 @@ private Object getAllRegionalResults (Request req, Response res) throws IOExcept
         }
         // File did not exist. Create it in the background and ask caller to request it later.
         filesBeingPrepared.add(zippedResultsKey.path);
-        Task task = Task.create("Zip all geotiffs for regional analysis " + analysis.name)
+        Task task = Task.create("Preparing regional results archive (hit download again when complete)")
             .forUser(userPermissions)
             .withAction(progressListener -> {
                 int nSteps = analysis.destinationPointSetIds.length * analysis.cutoffsMinutes.length *
                         analysis.travelTimePercentiles.length * 2 + 1;
                 progressListener.beginTask("Creating and archiving geotiffs...", nSteps);
                 // Iterate over all dest, cutoff, percentile combinations and generate one geotiff for each combination.
                 List<HumanKey> humanKeys = new ArrayList<>();
+                GridResultType gridResultType = determineGridResultType(analysis);
                 for (String destinationPointSetId : analysis.destinationPointSetIds) {
                     OpportunityDataset destinations = getDestinations(destinationPointSetId, userPermissions);
-                    // TODO handle dual access
-                    for (int cutoffMinutes : analysis.cutoffsMinutes) {
+                    for (int threshold : getValidThresholds(analysis)) {
                         for (int percentile : analysis.travelTimePercentiles) {
                             HumanKey gridKey = getSingleCutoffGrid(
-                                    analysis, destinations, cutoffMinutes, percentile, GridResultType.ACCESS, FileStorageFormat.GEOTIFF
+                                analysis, destinations, threshold, percentile, gridResultType, GEOTIFF
                             );
                             humanKeys.add(gridKey);
                             progressListener.increment();
@@ -384,59 +387,17 @@ private UrlWithHumanName getRegionalResults (Request req, Response res) throws I
         // expected to have no gridded results and cleanly return a 404?
         final String regionalAnalysisId = req.params("_id");
         FileStorageFormat format = FileStorageFormat.valueOf(req.params("format").toUpperCase());
-        if (!FileStorageFormat.GRID.equals(format) && !FileStorageFormat.PNG.equals(format) && !FileStorageFormat.GEOTIFF.equals(format)) {
-            throw AnalysisServerException.badRequest("Format \"" + format + "\" is invalid. Request format must be \"grid\", \"png\", or \"geotiff\".");
+        if (!List.of(GRID, PNG, GEOTIFF).contains(format)) {
+            throw AnalysisServerException.badRequest("Parameter 'format' must be one of [grid, png, geotiff].");
         }
         final UserPermissions userPermissions = UserPermissions.from(req);
         RegionalAnalysis analysis = getAnalysis(regionalAnalysisId, userPermissions);
-
-        // TODO handle a regional analysis that includes both regular accessibility and dual access results.
-        GridResultType gridResultType = analysis.request.includeTemporalDensity ? GridResultType.DUAL_ACCESS : GridResultType.ACCESS;
-
-        // If a query parameter is supplied, range check it, otherwise use the middle value in the list.
-        int threshold;
-        if (gridResultType.equals(GridResultType.DUAL_ACCESS)) {
-            int nThresholds = analysis.request.dualAccessThresholds.length;
-            int[] thresholds = analysis.request.dualAccessThresholds;
-            checkState(nThresholds > 0, "Regional analysis has no dual access thresholds.");
-            threshold = getIntQueryParameter(req, "threshold", thresholds[nThresholds / 2]);
-            checkArgument(new TIntArrayList(thresholds).contains(threshold),
-                    "Dual access thresholds for this regional analysis must be taken from this list: (%s)",
-                    Ints.join(", ", thresholds)
-            );
-        } else {
-            // Handle newer regional analyses with multiple cutoffs in an array.
-            // The cutoff variable holds the actual cutoff in minutes, not the position in the array of cutoffs.
-            checkState(analysis.cutoffsMinutes != null, "Regional analysis has no cutoffs.");
-            int nCutoffs = analysis.cutoffsMinutes.length;
-            checkState(nCutoffs > 0, "Regional analysis has no cutoffs.");
-            threshold = getIntQueryParameter(req, "threshold", analysis.cutoffsMinutes[nCutoffs / 2]);
-            checkArgument(new TIntArrayList(analysis.cutoffsMinutes).contains(threshold),
-                    "Travel time cutoff for this regional analysis must be taken from this list: (%s)",
-                    Ints.join(", ", analysis.cutoffsMinutes)
-            );
-        }
-
-        // If a query parameter is supplied, range check it, otherwise use the middle value in the list.
-        // The percentile variable holds the actual percentile (25, 50, 95) not the position in the array.
-        int nPercentiles = analysis.travelTimePercentiles.length;
-        checkState(nPercentiles > 0, "Regional analysis has no percentiles.");
-        int percentile = getIntQueryParameter(req, "percentile", analysis.travelTimePercentiles[nPercentiles / 2]);
-        checkArgument(new TIntArrayList(analysis.travelTimePercentiles).contains(percentile),
-                "Percentile for this regional analysis must be taken from this list: (%s)",
-                Ints.join(", ", analysis.travelTimePercentiles));
-
-        // Handle regional analyses with multiple destination pointsets per analysis.
-        int nGrids = analysis.destinationPointSetIds.length;
-        checkState(nGrids > 0, "Regional analysis has no grids.");
-        String destinationPointSetId = req.queryParams("destinationPointSetId");
-        if (destinationPointSetId == null) {
-            destinationPointSetId = analysis.destinationPointSetIds[0];
-        }
-        checkArgument(Arrays.asList(analysis.destinationPointSetIds).contains(destinationPointSetId),
-                "Destination gridId must be one of: %s",
-                String.join(",", analysis.destinationPointSetIds));
-
+        GridResultType gridResultType = determineGridResultType(analysis);
+        // The threshold parameter holds the value in minutes, not the position in the array of thresholds.
+        int threshold = getAndValidateIntParameter(req, "threshold", getValidThresholds(analysis));
+        int percentile = getAndValidateIntParameter(req, "percentile", analysis.travelTimePercentiles);
+        String destinationPointSetId = getAndValidateStringParameter(
+              req, "destinationPointSetId", analysis.destinationPointSetIds);
         // We started implementing the ability to retrieve and display partially completed analyses.
         // We eventually decided these should not be available here at the same endpoint as complete, immutable results.
         if (broker.findJob(regionalAnalysisId) != null) {
@@ -449,6 +410,41 @@ private UrlWithHumanName getRegionalResults (Request req, Response res) throws I
         return fileStorage.getJsonUrl(gridKey.storageKey, gridKey.humanName);
     }
 
+    private int[] getValidThresholds (RegionalAnalysis analysis) {
+        return switch (determineGridResultType(analysis)) {
+            case ACCESS -> analysis.cutoffsMinutes;
+            case DUAL_ACCESS -> analysis.request.dualAccessThresholds;
+        };
+    }
+
+    // This assumes each set of regional analysis results has only primal or dual access, not both.
+    // TODO handle regional analyses that include both regular accessibility and dual access results.
+    private GridResultType determineGridResultType (RegionalAnalysis analysis) {
+        return analysis.request.includeTemporalDensity ? GridResultType.DUAL_ACCESS : GridResultType.ACCESS;
+    }
+
+    /// Get the value for a given query parameter name, check that it's non-null and can be parsed
+    /// as an integer, and check that the value is present in an array of valid values.
+    private int getAndValidateIntParameter (Request req, String parameterName, int[] allowedValues) {
+        int value = getIntQueryParameter(req, parameterName);
+        checkState(allowedValues != null && allowedValues.length > 0, "Lacking values for " + parameterName);
+        checkArgument(Ints.contains(allowedValues, value), "Parameter '%s' must be one of: %s",
+              parameterName, Arrays.toString(allowedValues));
+        return value;
+    }
+
+    /// Should behave identically to getAndValidateIntParameter, but for Strings.
+    private String getAndValidateStringParameter (Request req, String parameterName, String[] allowedValues) {
+        checkState(allowedValues != null && allowedValues.length > 0, "Lacking values for " + parameterName);
+        String value = req.queryParams(parameterName);
+        if (value == null || value.isEmpty()) {
+            throw new IllegalArgumentException("Must provide query parameter " + parameterName);
+        }
+        checkArgument(List.of(allowedValues).contains(value), "Parameter '%s' must be one of: %s",
+              parameterName, Arrays.toString(allowedValues));
+        return value;
+    }
+
     private Object getCsvResults (Request req, Response res) {
         final String regionalAnalysisId = req.params("_id");
         final CsvResultType resultType = CsvResultType.valueOf(req.params("resultType").toUpperCase());

src/main/java/com/conveyal/gtfs/model/Transfer.java

@@ -34,7 +34,7 @@ public void loadOneRow() throws IOException {
             tr.sourceFileLine    = row;
             tr.from_stop_id      = getStringField("from_stop_id", true);
             tr.to_stop_id        = getStringField("to_stop_id", true);
-            tr.transfer_type     = getIntField("transfer_type", true, 0, 3);
+            tr.transfer_type     = getIntField("transfer_type", true, 0, 5);
             tr.min_transfer_time = getIntField("min_transfer_time", false, 0, Integer.MAX_VALUE);
             tr.from_route_id     = getStringField("from_route_id", false);
             tr.to_route_id       = getStringField("to_route_id", false);
@@ -43,10 +43,12 @@ public void loadOneRow() throws IOException {
 
             getRefField("from_stop_id", true, feed.stops);
             getRefField("to_stop_id", true, feed.stops);
-            getRefField("from_route_id", false, feed.routes);
-            getRefField("to_route_id", false, feed.routes);
-            getRefField("from_trip_id", false, feed.trips);
-            getRefField("to_trip_id", false, feed.trips);
+            // We do not validate referential integrity of these fields because they are not
+            // consumed by R5, and some prominent feeds contain thousands of such errors.
+            // getRefField("from_route_id", false, feed.routes);
+            // getRefField("to_route_id", false, feed.routes);
+            // getRefField("from_trip_id", false, feed.trips);
+            // getRefField("to_trip_id", false, feed.trips);
 
             // row number used as an arbitrary unique string to give MapDB a key.
             feed.transfers.put(Long.toString(row), tr);

src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java

@@ -25,13 +25,16 @@
 @JsonInclude(JsonInclude.Include.NON_NULL)
 public class TransportNetworkConfig {
 
-    /** ID of the OSM file, for use with OSMCache */
+    /** ID of the OSM file. Used as a key for fetching from OSMCache. */
     public String osmId;
 
-    /** IDs of the GTFS files, for use with GTFSCache */
+    /** IDs of the GTFS files. Used as keys for fetching from GTFSCache. */
     public List<String> gtfsIds;
 
-    /** The fare calculator for analysis, if any. TODO this is not yet wired up to TransportNetwork.setFareCalculator. */
+    /**
+     * The fare calculator for analysis, if any.
+     * TODO this is not yet wired up to TransportNetwork.setFareCalculator.
+     */
     public InRoutingFareCalculator analysisFareCalculator;
 
     /** A list of _R5_ modifications to apply during network build. May be null. */
@@ -57,4 +60,57 @@ public class TransportNetworkConfig {
      */
     public String traversalPermissionLabeler;
 
+    /**
+     * Whether to save detailed trip shapes from GTFS (e.g., for Conveyal Taui sites or the Network Viewer).
+     * If false, straight line segments between stops will be used in visualizations.
+     */
+    public boolean saveShapes;
+
+    /**
+     * How to handle stop-to-stop transfers in GTFS transfers.txt, and how to combine them with OSM-derived transfers.
+     * OSM_ONLY is the default, but STOP_TO_PATTERN should generally provide better results where GTFS data is good.
+     * In some cases path results may be strange or incorrect, but the quality of travel times should be no worse than
+     * the default OSM_ONLY option. For example: a large station in which subway platforms are separated by long walk
+     * times specified in GTFS but no times specified for transfers between those platforms and nearby bus stops.
+     *
+     * Currently we only handle stop-to-stop transfers in GTFS transfers.txt. Other more specific transfers types like
+     * route-to-route and trip-to-trip are not compatible with our current routing approach, so will be ignored with
+     * a warning. We also only import transfer type 2 ("minimum amount of time between arrival and departure to ensure
+     * a connection"). For any option except OSM_ONLY, these GTFS transfers override and replace any OSM street distance
+     * calculations. Although the GTFS spec text says "minimum amount of time", implying that other information like OSM
+     * routing could make times longer, we believe the proper interpretation is that transfers.txt provides a typical
+     * safe amount of time needed to walk between the two stops, which would only be made worse by comparing with OSM.
+     *
+     * Additional considerations:
+     * Strangely, BOARD_SLACK_SECONDS appears to only be used in classes for displaying paths, not for routing.
+     * We currently apply a hard lower limit of 60 seconds between alighting and boarding.
+     * Should this be configurable or interact with the transfer entries?
+     */
+    public TransferConfig transfers;
+
+    public enum TransferConfig {
+        /// Find transfers only by searching through the OSM street network, ignore GTFS transfers
+        OSM_ONLY,
+        /// Load transfers only from GTFS transfers.txt, do not use the OSM street network
+        GTFS_ONLY,
+        /// Use OSM where GTFS does not provide a transfer from a given stop to a given trip pattern
+        STOP_TO_PATTERN,
+        /// Find transfers via streets for any pair of stops not connected by a GTFS transfer
+        STOP_PAIR,
+    }
+
+    /**
+     * Steepest allowable slope for traversal. If a way has an "incline" tag  (e.g., from OSW or GATIS rather than
+     * a typical OSM source) with an absolute value that exceeds this limit, custom TraversalPermissionLabelers can
+     * remove permissions. Currently implemented only for pedestrians.
+     */
+    public Double maxIncline;
+
+    /**
+     * Whether to exclude pedestrian traversal of ways with highway=stairs tags and nodes with kerb=raised tags. This
+     * option should generally be used with detailed sidewalk networks and a TraversalPermissionLabeler that forces
+     * use of sidewalks (i.e., disallows walking on roadways).
+     */
+    public boolean stepFree;
+
 }