Merge pull request #98 from conveyal/dev

4.0.1 Release

Author: ansoncfit

Diff for: pom.xml

@@ -6,8 +6,7 @@
 
     <groupId>com.conveyal</groupId>
     <artifactId>analyst</artifactId>
-    <version>4.0.0</version>
-
+    <version>4.0.1</version>
     <build>
         <plugins>
             <plugin>

Diff for: src/main/java/com/conveyal/taui/AnalysisServer.java

@@ -81,16 +81,17 @@ public static void main (String... args) {
             // Default is JSON, will be overridden by the few controllers that do not return JSON
             res.type("application/json");
 
-            // Log each API request
-            LOG.info("{} {}", req.requestMethod(), req.pathInfo());
-
-            if (!AnalysisServerConfig.offline) {
+            if (AnalysisServerConfig.auth0ClientId != null && AnalysisServerConfig.auth0Secret != null) {
                 handleAuthentication(req, res);
             } else {
+                LOG.warn("No Auth0 credentials were supplied, setting accessGroup and email to placeholder defaults");
                 // hardwire group name if we're working offline
                 req.attribute("accessGroup", "OFFLINE");
                 req.attribute("email", "[email protected]");
             }
+
+            // Log each API request
+            LOG.info("{} {} by {} of {}", req.requestMethod(), req.pathInfo(), req.attribute("email"), req.attribute("accessGroup"));
         });
 
         // Register all our HTTP request handlers with the Spark HTTP framework.
@@ -123,23 +124,23 @@ public static void main (String... args) {
 
         exception(AnalysisServerException.class, (e, request, response) -> {
             AnalysisServerException ase = ((AnalysisServerException) e);
-            AnalysisServer.respondToException(response, ase, ase.type.name(), ase.message, ase.httpCode);
+            AnalysisServer.respondToException(ase, request, response, ase.type.name(), ase.message, ase.httpCode);
         });
 
         exception(IOException.class, (e, request, response) -> {
-            AnalysisServer.respondToException(response, e, "BAD_REQUEST", e.getMessage(), 400);
+            AnalysisServer.respondToException(e, request, response, "BAD_REQUEST", e.getMessage(), 400);
         });
 
         exception(FileUploadException.class, (e, request, response) -> {
-            AnalysisServer.respondToException(response, e, "BAD_REQUEST", e.getMessage(), 400);
+            AnalysisServer.respondToException(e, request, response, "BAD_REQUEST", e.getMessage(), 400);
         });
 
         exception(NullPointerException.class, (e, request, response) -> {
-            AnalysisServer.respondToException(response, e, "UNKNOWN", e.getMessage(), 400);
+            AnalysisServer.respondToException(e, request, response, "UNKNOWN", e.getMessage(), 400);
         });
 
         exception(RuntimeException.class, (e, request, response) -> {
-            AnalysisServer.respondToException(response, e, "RUNTIME", e.getMessage(), 400);
+            AnalysisServer.respondToException(e, request, response, "RUNTIME", e.getMessage(), 400);
         });
 
         LOG.info("Conveyal Analysis server is ready.");
@@ -150,14 +151,14 @@ public static void handleAuthentication (Request req, Response res) {
 
         // authorization required
         if (auth == null || auth.isEmpty()) {
-            AnalysisServerException.Unauthorized("You must be logged in.");
+            throw AnalysisServerException.Unauthorized("You must be logged in.");
         }
 
         // make sure it's properly formed
         String[] authComponents = auth.split(" ");
 
         if (authComponents.length != 2 || !"bearer".equals(authComponents[0].toLowerCase())) {
-            AnalysisServerException.Unknown("Authorization header is malformed: " + auth);
+            throw AnalysisServerException.Unknown("Authorization header is malformed: " + auth);
         }
 
         // validate the JWT
@@ -167,33 +168,33 @@ public static void handleAuthentication (Request req, Response res) {
         try {
             jwt = verifier.verify(authComponents[1]);
         } catch (Exception e) {
-            AnalysisServerException.Forbidden("Login failed to verify with our authorization provider. " + e.getMessage());
+            throw AnalysisServerException.Forbidden("Login failed to verify with our authorization provider. " + e.getMessage());
         }
 
         if (!jwt.containsKey("analyst")) {
-            AnalysisServerException.Forbidden("Access denied. User does not have access to Analysis.");
+            throw AnalysisServerException.Forbidden("Access denied. User does not have access to Analysis.");
         }
 
         String group = null;
         try {
             group = (String) ((Map<String, Object>) jwt.get("analyst")).get("group");
         } catch (Exception e) {
-            AnalysisServerException.Forbidden("Access denied. User is not associated with any group. " + e.getMessage());
+            throw AnalysisServerException.Forbidden("Access denied. User is not associated with any group. " + e.getMessage());
         }
 
         if (group == null) {
-            AnalysisServerException.Forbidden("Access denied. User is not associated with any group.");
+            throw AnalysisServerException.Forbidden("Access denied. User is not associated with any group.");
         }
 
         // attributes to be used on models
         req.attribute("accessGroup", group);
         req.attribute("email", jwt.get("email"));
     }
 
-    public static void respondToException(Response response, Exception e, String type, String message, int code) {
+    public static void respondToException(Exception e, Request request, Response response, String type, String message, int code) {
         String stack = ExceptionUtils.getStackTrace(e);
 
-        LOG.error("Server exception thrown, type: {}, message: {}", type, message);
+        LOG.error("{} {} -> {} {} by {} of {}", type, message, request.requestMethod(), request.pathInfo(), request.attribute("email"), request.attribute("accessGroup"));
         LOG.error(stack);
 
         JSONObject body = new JSONObject();

Diff for: src/main/java/com/conveyal/taui/analysis/RegionalAnalysisManager.java

@@ -12,6 +12,7 @@
 import com.conveyal.r5.analyst.cluster.RegionalTask;
 import com.conveyal.r5.analyst.scenario.Scenario;
 import com.conveyal.taui.AnalysisServerConfig;
+import com.conveyal.taui.AnalysisServerException;
 import com.conveyal.taui.models.RegionalAnalysis;
 import com.conveyal.taui.persistence.TiledAccessGrid;
 import com.conveyal.taui.util.HttpUtil;
@@ -116,7 +117,7 @@ public static void enqueue (RegionalAnalysis regionalAnalysis) {
                 }
             } catch (IOException e) {
                 LOG.error("error enqueueing requests", e);
-                throw new RuntimeException("error enqueueing requests", e);
+                throw AnalysisServerException.Unknown(e);
             }
 
             consumer.registerJob(templateTask,

Diff for: src/main/java/com/conveyal/taui/controllers/OpportunityDatasetsController.java

@@ -283,7 +283,7 @@ private static Object downloadOpportunityDataset (Request req, Response res) thr
         if (!s3.doesObjectExist(BUCKET, String.format("%s.%s", gridPath, format))) {
             // if this grid is not on S3 in the requested format, try to get the .grid format
             if (!s3.doesObjectExist(BUCKET, String.format("%s.grid", gridPath))) {
-                throw new IllegalArgumentException("This grid does not exist.");
+                throw AnalysisServerException.NotFound("This grid does not exist.");
             } else {
                 // get the grid and convert it to the requested format
                 S3Object s3Grid = s3.getObject(BUCKET, String.format("%s.grid", gridPath));
@@ -327,7 +327,7 @@ private static List<Region.OpportunityDataset> writeOpportunityDatasetToS3(Map<S
                 status.status = Status.ERROR;
                 status.message = e.getMessage();
                 status.completed();
-                throw new RuntimeException(e);
+                throw AnalysisServerException.Unknown(e);
             }
 
             Region.OpportunityDataset opportunityDataset = new Region.OpportunityDataset();

Diff for: src/main/java/com/conveyal/taui/controllers/RegionalAnalysisController.java

@@ -7,6 +7,7 @@
 import com.conveyal.r5.analyst.SelectingGridReducer;
 import com.conveyal.r5.analyst.cluster.RegionalTask;
 import com.conveyal.taui.AnalysisServerConfig;
+import com.conveyal.taui.AnalysisServerException;
 import com.conveyal.taui.analysis.RegionalAnalysisManager;
 import com.conveyal.taui.grids.GridExporter;
 import com.conveyal.taui.models.AnalysisRequest;
@@ -95,7 +96,7 @@ public static Object getPercentile (Request req, Response res) throws IOExceptio
                 // Andrew Owen style average instantaneous accessibility
                 // The samples stored in the access grid are samples of instantaneous accessibility at different minutes
                 // and Monte Carlo draws, average them together
-                throw new IllegalArgumentException("Old-style instantaneous-accessibility regional analyses are no longer supported");
+                throw AnalysisServerException.BadRequest("Old-style instantaneous-accessibility regional analyses are no longer supported");
             } else {
                 // This is accessibility given x percentile travel time, the first sample is the point estimate
                 // computed using all monte carlo draws, and subsequent samples are bootstrap replications. Return the

Diff for: src/main/java/com/conveyal/taui/grids/GridExporter.java

@@ -92,7 +92,7 @@ public static void writeToS3(Grid grid, AmazonS3 s3, String bucket, String key,
             try {
                 if ("grid".equals(format)) {
                     grid.write(new GZIPOutputStream(pos));
-                } else if ("png".equals("format")) {
+                } else if ("png".equals(format)) {
                     grid.writePng(pos);
                 } else if ("tiff".equals(format)) {
                     grid.writeGeotiff(pos);

Diff for: src/main/java/com/conveyal/taui/models/AddTripPattern.java

@@ -47,6 +47,7 @@ public AddTrips toR5 () {
         at.comment = name;
 
         at.bidirectional = bidirectional;
+
         List<ModificationStop> stops = ModificationStop.getStopsFromSegments(segments);
         at.frequencies = timetables.stream().map(tt -> tt.toR5(stops)).collect(Collectors.toList());
         at.stops = ModificationStop.toSpec(stops);

Diff for: src/main/java/com/conveyal/taui/models/AnalysisRequest.java

@@ -28,7 +28,6 @@ public class AnalysisRequest {
     // All analyses parameters
     public String accessModes;
     public float bikeSpeed;
-    public float carSpeed;
     public LocalDate date;
     public String directModes;
     public String egressModes;
@@ -43,6 +42,7 @@ public class AnalysisRequest {
 
     // Parameters that aren't currently configurable in the UI
     public int bikeTrafficStress = 4;
+    public float carSpeed = 20;
     public int maxWalkTime = 20;
     public int maxBikeTime = 20;
     public int maxCarTime = 45;
@@ -83,14 +83,16 @@ public AnalysisTask populateTask (AnalysisTask task, Project project) {
             modifications = modificationsForProject(project.accessGroup, projectId, variantIndex);
         }
 
-        // No idea how long this operation takes or if it is actually necessary
+        // The CRC is appended to the scenario ID to identify a unique revision of the scenario (still denoted here
+        // as variant) allowing the worker to cache and reuse networks built by applying that exact revision of the
+        // scenario to a base network.
         CRC32 crc = new CRC32();
-        crc.update(modifications.stream().map(Modification::toString).collect(Collectors.joining("-")).getBytes());
-        crc.update(JsonUtilities.objectToJsonBytes(this));
+        crc.update(JsonUtilities.objectToJsonBytes(modifications));
+        long crcValue = crc.getValue();
 
         task.scenario = new Scenario();
         // TODO figure out why we use both
-        task.jobId = String.format("%s-%s-%s", projectId, variantIndex, crc.getValue());
+        task.jobId = String.format("%s-%s-%s", projectId, variantIndex, crcValue);
         task.scenario.id = task.scenarioId = task.jobId;
         task.scenario.modifications = modifications;
 
@@ -146,15 +148,21 @@ public AnalysisTask populateTask (AnalysisTask task, Project project) {
             task.maxTripDurationMinutes = maxTripDurationMinutes;
         }
 
-        task.accessModes = EnumSet.copyOf(Arrays.stream(accessModes.split(","))
-                .map(LegMode::valueOf).collect(Collectors.toList()));
-        task.directModes = EnumSet.copyOf(Arrays.stream(directModes.split(","))
-                .map(LegMode::valueOf).collect(Collectors.toList()));
-        task.egressModes = EnumSet.copyOf(Arrays.stream(egressModes.split(","))
-                .map(LegMode::valueOf).collect(Collectors.toList()));
-        task.transitModes = EnumSet.copyOf(Arrays.stream(transitModes.split(","))
-                .map(TransitModes::valueOf).collect(Collectors.toList()));
+        task.accessModes = getEnumSetFromString(accessModes);
+        task.directModes = getEnumSetFromString(directModes);
+        task.egressModes = getEnumSetFromString(egressModes);
+        task.transitModes = transitModes != null && !"".equals(transitModes)
+                ? EnumSet.copyOf(Arrays.stream(transitModes.split(",")).map(TransitModes::valueOf).collect(Collectors.toList()))
+                : EnumSet.noneOf(TransitModes.class);
 
         return task;
     }
+
+    private EnumSet<LegMode> getEnumSetFromString (String s) {
+        if (s != null && !"".equals(s)) {
+            return EnumSet.copyOf(Arrays.stream(s.split(",")).map(LegMode::valueOf).collect(Collectors.toList()));
+        } else {
+            return EnumSet.noneOf(LegMode.class);
+        }
+    }
 }

Diff for: src/main/java/com/conveyal/taui/models/Bookmark.java

@@ -1,12 +1,10 @@
 package com.conveyal.taui.models;
 
-import com.conveyal.r5.profile.ProfileRequest;
-
 /**
  * A bookmark represents "frozen" settings for single point results.
  */
 public class Bookmark extends Model {
-    public ProfileRequest profileRequest;
+    public AnalysisRequest profileRequest;
 
     public int isochroneCutoff;
 

Diff for: src/main/java/com/conveyal/taui/models/Bundle.java

@@ -6,6 +6,7 @@
 import com.conveyal.gtfs.GTFSFeed;
 import com.conveyal.r5.analyst.cluster.BundleManifest;
 import com.conveyal.taui.AnalysisServerConfig;
+import com.conveyal.taui.AnalysisServerException;
 import com.conveyal.taui.util.JsonUtil;
 
 import java.io.File;
@@ -62,7 +63,7 @@ public Bundle clone () {
         try {
             return (Bundle) super.clone();
         } catch (CloneNotSupportedException e) {
-            throw new RuntimeException(e);
+            throw AnalysisServerException.Unknown(e);
         }
     }
 
@@ -93,7 +94,7 @@ public FeedSummary clone () {
             try {
                 return (FeedSummary) super.clone();
             } catch (CloneNotSupportedException e) {
-                throw new RuntimeException(e);
+                throw AnalysisServerException.Unknown(e);
             }
         }
     }