Merge pull request #948 from conveyal/dev

v7.3 Release

build.gradle

@@ -14,6 +14,7 @@ java {
     toolchain {
         languageVersion.set(JavaLanguageVersion.of(21))
     }
+    withSourcesJar()
 }
 
 jar {
@@ -164,7 +165,7 @@ dependencies {
     }
 
     // Database driver.
-    implementation 'org.mongodb:mongo-java-driver:3.11.0'
+    implementation 'org.mongodb:mongodb-driver-legacy:5.2.0'
 
     // Legacy system for storing Java objects, this functionality is now provided by the MongoDB driver itself.
     implementation 'org.mongojack:mongojack:2.10.1'

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

@@ -13,14 +13,13 @@
 import com.conveyal.file.FileUtils;
 import com.conveyal.gtfs.GTFSCache;
 import com.conveyal.gtfs.GTFSFeed;
-import com.conveyal.gtfs.error.GTFSError;
 import com.conveyal.gtfs.error.GeneralError;
 import com.conveyal.gtfs.model.Stop;
 import com.conveyal.gtfs.validator.PostLoadValidator;
 import com.conveyal.osmlib.Node;
 import com.conveyal.osmlib.OSM;
-import com.conveyal.r5.analyst.progress.ProgressInputStream;
 import com.conveyal.r5.analyst.cluster.TransportNetworkConfig;
+import com.conveyal.r5.analyst.progress.ProgressInputStream;
 import com.conveyal.r5.analyst.progress.Task;
 import com.conveyal.r5.streets.OSMCache;
 import com.conveyal.r5.util.ExceptionUtils;
@@ -81,6 +80,7 @@ public BundleController (BackendComponents components) {
     public void registerEndpoints (Service sparkService) {
         sparkService.path("/api/bundle", () -> {
             sparkService.get("", this::getBundles, toJson);
+            sparkService.get("/:_id/config", this::getBundleConfig, toJson);
             sparkService.get("/:_id", this::getBundle, toJson);
             sparkService.post("", this::create, toJson);
             sparkService.put("/:_id", this::update, toJson);
@@ -110,15 +110,13 @@ private Bundle create (Request req, Response res) {
         try {
             bundle.name = files.get("bundleName").get(0).getString("UTF-8");
             bundle.regionId = files.get("regionId").get(0).getString("UTF-8");
-
             if (files.get("osmId") != null) {
                 bundle.osmId = files.get("osmId").get(0).getString("UTF-8");
                 Bundle bundleWithOsm = Persistence.bundles.find(QueryBuilder.start("osmId").is(bundle.osmId).get()).next();
                 if (bundleWithOsm == null) {
                     throw AnalysisServerException.badRequest("Selected OSM does not exist.");
                 }
             }
-
             if (files.get("feedGroupId") != null) {
                 bundle.feedGroupId = files.get("feedGroupId").get(0).getString("UTF-8");
                 Bundle bundleWithFeed = Persistence.bundles.find(QueryBuilder.start("feedGroupId").is(bundle.feedGroupId).get()).next();
@@ -135,6 +133,13 @@ private Bundle create (Request req, Response res) {
                 bundle.feedsComplete = bundleWithFeed.feedsComplete;
                 bundle.totalFeeds = bundleWithFeed.totalFeeds;
             }
+            if (files.get("config") != null) {
+                // Validation by deserializing into a model class instance. Unknown fields are ignored to
+                // allow sending config to custom or experimental workers with features unknown to the backend.
+                // The fields specifying OSM and GTFS IDs are not expected here. They will be ignored and overwritten.
+                String configString = files.get("config").get(0).getString();
+                bundle.config = JsonUtil.objectMapper.readValue(configString, TransportNetworkConfig.class);
+            }
             UserPermissions userPermissions = UserPermissions.from(req);
             bundle.accessGroup = userPermissions.accessGroup;
             bundle.createdBy = userPermissions.email;
@@ -274,15 +279,19 @@ private Bundle create (Request req, Response res) {
         return bundle;
     }
 
+    /** SIDE EFFECTS: This method will change the field bundle.config before writing it. */
     private void writeNetworkConfigToCache (Bundle bundle) throws IOException {
-        TransportNetworkConfig networkConfig = new TransportNetworkConfig();
-        networkConfig.osmId = bundle.osmId;
-        networkConfig.gtfsIds = bundle.feeds.stream().map(f -> f.bundleScopedFeedId).collect(Collectors.toList());
-
+        // If the user specified additional network configuration options, they should already be in bundle.config.
+        // If no custom options were specified, we start with a fresh, empty instance.
+        if (bundle.config == null) {
+            bundle.config = new TransportNetworkConfig();
+        }
+        // This will overwrite and override any inconsistent osm and gtfs IDs that were mistakenly supplied by the user.
+        bundle.config.osmId = bundle.osmId;
+        bundle.config.gtfsIds = bundle.feeds.stream().map(f -> f.bundleScopedFeedId).collect(Collectors.toList());
         String configFileName = bundle._id + ".json";
         File configFile = FileUtils.createScratchFile("json");
-        JsonUtil.objectMapper.writeValue(configFile, networkConfig);
-
+        JsonUtil.objectMapper.writeValue(configFile, bundle.config);
         FileStorageKey key = new FileStorageKey(BUNDLES, configFileName);
         fileStorage.moveIntoStorage(key, configFile);
     }
@@ -312,6 +321,31 @@ private Bundle getBundle (Request req, Response res) {
         return bundle;
     }
 
+    /**
+     * There are two copies of the Bundle/Network config: one in the Bundle entry in the database and one in a JSON
+     * file (obtainable by the workers). This method always reads the one in the file, which has been around longer
+     * and is considered the definitive source of truth. The entry in the database is a newer addition and has only
+     * been around since September 2024.
+     */
+    private TransportNetworkConfig getBundleConfig(Request request, Response res) {
+        // Unfortunately this mimics logic in TransportNetworkCache. Deduplicate in a static utility method?
+        String id = GTFSCache.cleanId(request.params("_id"));
+        FileStorageKey key = new FileStorageKey(BUNDLES, id, "json");
+        File networkConfigFile = fileStorage.getFile(key);
+        if (!networkConfigFile.exists()) {
+            throw AnalysisServerException.notFound("Bundle configuration file could not be found.");
+        }
+
+        // Unlike in the worker, we expect the backend to have a model field for every known network/bundle option.
+        // Therefore, use the default objectMapper that does not tolerate unknown fields.
+        try {
+            return JsonUtil.objectMapper.readValue(networkConfigFile, TransportNetworkConfig.class);
+        } catch (Exception exception) {
+            LOG.error("Exception deserializing stored network config", exception);
+            return null;
+        }
+    }
+
     private Collection<Bundle> getBundles (Request req, Response res) {
         return Persistence.bundles.findPermittedForQuery(req);
     }

src/main/java/com/conveyal/analysis/models/Bundle.java

@@ -5,6 +5,7 @@
 import com.conveyal.gtfs.error.GTFSError;
 import com.conveyal.gtfs.model.FeedInfo;
 import com.conveyal.gtfs.validator.model.Priority;
+import com.conveyal.r5.analyst.cluster.TransportNetworkConfig;
 import com.fasterxml.jackson.annotation.JsonIgnore;
 
 import java.time.LocalDate;
@@ -47,6 +48,11 @@ public class Bundle extends Model implements Cloneable {
     public int feedsComplete;
     public int totalFeeds;
 
+    // The definitive TransportNetworkConfig is a JSON file stored alongside the feeds in file storage. It is
+    // duplicated here to record any additional user-specified options that were supplied when the bundle was created.
+    // It may contain redundant copies of information stored in the outer level Bundle such as OSM and GTFS feed IDs.
+    public TransportNetworkConfig config;
+
     public static String bundleScopeFeedId (String feedId, String feedGroupId) {
         return String.format("%s_%s", feedId, feedGroupId);
     }

src/main/java/com/conveyal/r5/analyst/NetworkPreloader.java

@@ -110,11 +110,13 @@ protected TransportNetwork buildValue(Key key) {
 
         // Get the set of points to which we are measuring travel time. Any smaller sub-grids created here will
         // reference the scenarioNetwork's built-in full-extent pointset, so can reuse its linkage.
-        // TODO handle multiple destination grids.
+        // FIXME handle multiple destination grids.
 
         if (key.destinationGridExtents == null) {
             // Special (and ideally temporary) case for regional freeform destinations, where there is no grid to link.
             // The null destinationGridExtents are created by the WebMercatorExtents#forPointsets else clause.
+            // FIXME there is no grid to link, but there are points and egress tables to make!
+            //  see com.conveyal.r5.analyst.cluster.AnalysisWorkerTask.loadAndValidateDestinationPointSets
             return scenarioNetwork;
         }
 

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

@@ -26,14 +26,14 @@
  * scenarios from the backend instead of from S3.
  *
  * TODO merge this with TransportNetworkCache#resolveScenario into a single multi-level mem/disk/s3 cache.
- * Note that this cache is going to just grow indefinitely in size as a worker receives many iterations of the same
- * scenario - that could be a memory leak. Again multi level caching could releive those worries.
- * It's debatable whether we should be hanging on to scenarios passed with single point requests becuase they may never
- * be used again.
+ * This cache grows in size without bound as a worker receives many iterations of the same scenario.
+ * This is technically a sort of memory leak for long-lived workers. Multi-level caching could relieve those worries.
+ * However, this cache stores only the Scenarios and Modifications, not any large egress tables or linkages.
+ *
+ * It's debatable whether we should be hanging on to scenarios passed with single point requests,
+ * because they may never be used again.
  * Should we just always require a single point task to be sent to the cluster before a regional?
  * That would not ensure the scenario was present on all workers though.
- *
- * Created by abyrd on 2018-10-29
  */
 public class ScenarioCache {
 
@@ -44,7 +44,7 @@ public class ScenarioCache {
     public synchronized void storeScenario (Scenario scenario) {
         Scenario existingScenario = scenariosById.put(scenario.id, scenario);
         if (existingScenario != null) {
-            LOG.debug("Scenario cache already contained a this scenario.");
+            LOG.debug("Scenario cache already contained this scenario.");
         }
     }
 

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

@@ -2,12 +2,8 @@
 
 import com.conveyal.r5.analyst.fare.InRoutingFareCalculator;
 import com.conveyal.r5.analyst.scenario.Modification;
-import com.conveyal.r5.analyst.scenario.RasterCost;
-import com.conveyal.r5.analyst.scenario.ShapefileLts;
 import com.conveyal.r5.profile.StreetMode;
-import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonInclude;
-import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 
 import java.util.List;
 import java.util.Set;
@@ -54,4 +50,11 @@ public class TransportNetworkConfig {
      */
     public Set<StreetMode> buildGridsForModes;
 
+    /**
+     * Specifies which "labeler" to use when setting traversal mode permissions from OSM tags. For now, only
+     * implemented with "sidewalk" to use the SidewalkTraversalPermissionLayer. This should eventually be cleaned up
+     * (specifying different labelers, using enums).
+     */
+    public String traversalPermissionLabeler;
+
 }

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

@@ -39,7 +39,6 @@ public class WorkerStatus {
     public String workerVersion;
     public String workerId;
     public Set<String> networks = new HashSet<>();
-    public Set<String> scenarios = new HashSet<>();
     public double secondsSinceLastPoll;
     public Map<String, Integer> tasksPerMinuteByJobId;
     @JsonUnwrapped(prefix = "ec2")
@@ -86,7 +85,6 @@ public WorkerStatus (AnalysisWorker worker) {
         // networks = worker.networkPreloader.transportNetworkCache.getLoadedNetworkIds();
         // For now we report a single network, even before it's loaded.
         networks = Sets.newHashSet(worker.networkId);
-        scenarios = worker.networkPreloader.transportNetworkCache.getAppliedScenarios();
         ec2 = worker.ec2info;
 
         OperatingSystemMXBean operatingSystemMXBean = ManagementFactory.getOperatingSystemMXBean();

src/main/java/com/conveyal/r5/labeling/SidewalkTraversalPermissionLabeler.java

@@ -0,0 +1,35 @@
+package com.conveyal.r5.labeling;
+
+import com.conveyal.osmlib.Way;
+import com.conveyal.r5.streets.EdgeStore;
+
+/**
+ * Traversal permission labeler that restricts walking on most driving ways (useful for networks with complete
+ * sidewalks). Also includes permissions for the United States (see USTraversalPermissionLabeler).
+ */
+public class SidewalkTraversalPermissionLabeler extends TraversalPermissionLabeler {
+    static {
+        addPermissions("pedestrian", "bicycle=yes");
+        addPermissions("bridleway", "bicycle=yes;foot=yes"); //horse=yes but we don't support horse
+        addPermissions("cycleway", "bicycle=yes;foot=yes");
+        addPermissions("trunk|primary|secondary|tertiary|unclassified|residential|living_street|road|service|track",
+                "access=yes");
+    }
+
+    @Override
+    public RoadPermission getPermissions(Way way) {
+        RoadPermission rp = super.getPermissions(way);
+        if (rp.forward.contains(EdgeStore.EdgeFlag.ALLOWS_CAR) ||
+            rp.forward.contains(EdgeStore.EdgeFlag.NO_THRU_TRAFFIC_CAR) ||
+            rp.backward.contains(EdgeStore.EdgeFlag.ALLOWS_CAR) ||
+            rp.backward.contains(EdgeStore.EdgeFlag.NO_THRU_TRAFFIC_CAR)
+        ) {
+            rp.forward.remove(EdgeStore.EdgeFlag.ALLOWS_PEDESTRIAN);
+            rp.forward.remove(EdgeStore.EdgeFlag.NO_THRU_TRAFFIC_PEDESTRIAN);
+            rp.backward.remove(EdgeStore.EdgeFlag.ALLOWS_PEDESTRIAN);
+            rp.backward.remove(EdgeStore.EdgeFlag.NO_THRU_TRAFFIC_PEDESTRIAN);
+        }
+        return rp;
+    }
+
+}

src/main/java/com/conveyal/r5/streets/StreetLayer.java

@@ -6,12 +6,14 @@
 import com.conveyal.osmlib.OSMEntity;
 import com.conveyal.osmlib.Relation;
 import com.conveyal.osmlib.Way;
+import com.conveyal.r5.analyst.cluster.TransportNetworkConfig;
 import com.conveyal.r5.analyst.scenario.PickupWaitTimes;
 import com.conveyal.r5.api.util.BikeRentalStation;
 import com.conveyal.r5.api.util.ParkRideParking;
 import com.conveyal.r5.common.GeometryUtils;
 import com.conveyal.r5.labeling.LevelOfTrafficStressLabeler;
 import com.conveyal.r5.labeling.RoadPermission;
+import com.conveyal.r5.labeling.SidewalkTraversalPermissionLabeler;
 import com.conveyal.r5.labeling.SpeedLabeler;
 import com.conveyal.r5.labeling.StreetClass;
 import com.conveyal.r5.labeling.TraversalPermissionLabeler;
@@ -132,9 +134,9 @@ public class StreetLayer implements Serializable, Cloneable {
     public TIntObjectMap<ParkRideParking> parkRideLocationsMap;
 
     // TODO these are only needed when building the network, should we really be keeping them here in the layer?
-    //      We should instead have a network builder that holds references to this transient state.
-    // TODO don't hardwire to US
-    private transient TraversalPermissionLabeler permissionLabeler = new USTraversalPermissionLabeler();
+    //      We should instead have a network builder that holds references to this transient state. Note initial
+    //      approach of specifying a TraversalPermissionLabeler in TransportNetworkConfig.
+    private transient TraversalPermissionLabeler permissionLabeler;
     private transient LevelOfTrafficStressLabeler stressLabeler = new LevelOfTrafficStressLabeler();
     private transient TypeOfEdgeLabeler typeOfEdgeLabeler = new TypeOfEdgeLabeler();
     private transient SpeedLabeler speedLabeler;
@@ -207,6 +209,22 @@ public class StreetLayer implements Serializable, Cloneable {
 
     public StreetLayer() {
         speedLabeler = new SpeedLabeler(SpeedConfig.defaultConfig());
+        permissionLabeler = new USTraversalPermissionLabeler();
+    }
+
+    public StreetLayer(TransportNetworkConfig config) {
+        this();
+        if (config != null) {
+            permissionLabeler = switch (config.traversalPermissionLabeler) {
+                case "sidewalk" -> new SidewalkTraversalPermissionLabeler();
+                case null -> new USTraversalPermissionLabeler();
+                default -> throw new IllegalArgumentException(
+                        "Unknown traversal permission labeler: " + config.traversalPermissionLabeler
+                );
+            };
+        } else {
+            permissionLabeler = new USTraversalPermissionLabeler();
+        }
     }
 
     /** Load street layer from an OSM-lib OSM DB */

src/main/java/com/conveyal/r5/streets/StreetRouter.java

@@ -742,7 +742,7 @@ public int getTravelTimeToVertex (int vertexIndex) {
      * fragments from the vertices at either end of the edge up to the destination split point.
      * If no states can be produced return null.
      *
-     * Note that this is only used by the point to point street router, not by LinkedPointSets (which have equivalent
+     * NOTE that this is ONLY USED BY the point to point street router, NOT BY LinkedPointSets (which have equivalent
      * logic in their eval method). The PointSet implementation only needs to produce times, not States. But ideally
      * some common logic can be factored out.
      */