From 8b159d4d67dd873b350e3966b4768a8ba4e6265f Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Sun, 3 Dec 2017 18:46:31 -0500 Subject: [PATCH 01/18] Prepare next development iteration 4.1.0-SNAPSHOT --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 39b97e35..c7873d9e 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.conveyal analyst - 4.0.0-SNAPSHOT + 4.1.0-SNAPSHOT From b330ee5dd2faef3400a29cc0c99b743d81d9264a Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Mon, 4 Dec 2017 09:01:24 -0500 Subject: [PATCH 02/18] fix(stops): Account for 0 stops when converting to r5 to prevent index out of bounds exceptions --- .../com/conveyal/taui/models/AddTripPattern.java | 1 + .../com/conveyal/taui/models/ModificationStop.java | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/main/java/com/conveyal/taui/models/AddTripPattern.java b/src/main/java/com/conveyal/taui/models/AddTripPattern.java index 8a8c58c2..670ef7b6 100644 --- a/src/main/java/com/conveyal/taui/models/AddTripPattern.java +++ b/src/main/java/com/conveyal/taui/models/AddTripPattern.java @@ -47,6 +47,7 @@ public AddTrips toR5 () { at.comment = name; at.bidirectional = bidirectional; + List stops = ModificationStop.getStopsFromSegments(segments); at.frequencies = timetables.stream().map(tt -> tt.toR5(stops)).collect(Collectors.toList()); at.stops = ModificationStop.toSpec(stops); diff --git a/src/main/java/com/conveyal/taui/models/ModificationStop.java b/src/main/java/com/conveyal/taui/models/ModificationStop.java index 89ae9209..f0ab629c 100644 --- a/src/main/java/com/conveyal/taui/models/ModificationStop.java +++ b/src/main/java/com/conveyal/taui/models/ModificationStop.java @@ -33,6 +33,10 @@ static List getStopsFromSegments (List segments) { Stack stops = new Stack<>(); CoordinateReferenceSystem crs = DefaultGeographicCRS.WGS84; + if (segments == null || segments.size() == 0) { + return new ArrayList<>(); + } + Segment firstSegment = segments.get(0); Coordinate firstStopCoord = firstSegment.geometry.getCoordinates()[0]; ModificationStop firstStop = new ModificationStop(firstStopCoord, firstSegment.fromStopId, 0); @@ -93,6 +97,10 @@ static List getStopsFromSegments (List segments) { } static int[] getDwellTimes (List stops, Integer[] dwellTimes, int defaultDwellTime) { + if (stops == null || stops.size() == 0) { + return new int[0]; + } + int[] stopDwellTimes = new int[stops.size()]; int realStopIndex = 0; @@ -111,6 +119,10 @@ static int[] getDwellTimes (List stops, Integer[] dwellTimes, } static int[] getHopTimes (List stops, int[] segmentSpeeds) { + if (stops == null || stops.size() == 0) { + return new int[0]; + } + int[] hopTimes = new int[stops.size() - 1]; ModificationStop lastStop = stops.get(0); From 6f843811fe93632865255e618b1ac788f6377a88 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Mon, 4 Dec 2017 10:35:45 -0500 Subject: [PATCH 03/18] fix(stops): Fix stop generation code that could cause ArrayIndexOutOfBounds exceptions --- .../taui/models/ModificationStop.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/ModificationStop.java b/src/main/java/com/conveyal/taui/models/ModificationStop.java index f0ab629c..80b3241c 100644 --- a/src/main/java/com/conveyal/taui/models/ModificationStop.java +++ b/src/main/java/com/conveyal/taui/models/ModificationStop.java @@ -15,6 +15,7 @@ class ModificationStop { private static double MIN_SPACING_PERCENTAGE = 0.25; + private static int DEFAULT_SEGMENT_SPEED = 15; StopSpec stop; double distanceFromStart; @@ -38,9 +39,10 @@ static List getStopsFromSegments (List segments) { } Segment firstSegment = segments.get(0); - Coordinate firstStopCoord = firstSegment.geometry.getCoordinates()[0]; - ModificationStop firstStop = new ModificationStop(firstStopCoord, firstSegment.fromStopId, 0); - stops.add(firstStop); + + if (firstSegment.stopAtStart) { + stops.add(new ModificationStop(firstSegment.geometry.getCoordinates()[0], firstSegment.fromStopId, 0)); + } double distanceToLastStop = 0; double distanceToLineSegmentStart = 0; @@ -78,11 +80,13 @@ static List getStopsFromSegments (List segments) { distanceToLineSegmentStart += distanceThisLineSegment; } - if (segment.toStopId != null) { + if (segment.stopAtEnd) { // If the last auto-generated stop was too close, pop it - ModificationStop lastStop = stops.peek(); - if (lastStop.stop.id == null && (distanceToLineSegmentStart - distanceToLastStop) / spacing < MIN_SPACING_PERCENTAGE) { - stops.pop(); + if (stops.size() > 0) { + ModificationStop lastStop = stops.peek(); + if (lastStop.stop.id == null && (distanceToLineSegmentStart - distanceToLastStop) / spacing < MIN_SPACING_PERCENTAGE) { + stops.pop(); + } } Coordinate endCoord = coords[coords.length - 1]; @@ -119,7 +123,7 @@ static int[] getDwellTimes (List stops, Integer[] dwellTimes, } static int[] getHopTimes (List stops, int[] segmentSpeeds) { - if (stops == null || stops.size() == 0) { + if (stops == null || stops.size() < 2) { return new int[0]; } @@ -127,10 +131,12 @@ static int[] getHopTimes (List stops, int[] segmentSpeeds) { ModificationStop lastStop = stops.get(0); int realStopIndex = 0; - for (int i = 1; i < stops.size(); i++) { - ModificationStop stop = stops.get(i); + for (int i = 0; i < hopTimes.length; i++) { + ModificationStop stop = stops.get(i + 1); double hopDistance = stop.distanceFromStart - lastStop.distanceFromStart; - hopTimes[i - 1] = (int) (hopDistance / (segmentSpeeds[realStopIndex] * 1000) * 3000); + + int segmentSpeed = segmentSpeeds.length > realStopIndex ? segmentSpeeds[realStopIndex] : DEFAULT_SEGMENT_SPEED; + hopTimes[i] = (int) (hopDistance / (segmentSpeed * 1000) * 3000); if (stop.stop.id != null) { realStopIndex++; From a4649b910fb566f9c625db10bc694a48a3955fb1 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Mon, 4 Dec 2017 15:35:08 -0500 Subject: [PATCH 04/18] fix(stops): Stops sent to r5 with ids should not have coordinates --- .../taui/models/ModificationStop.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/ModificationStop.java b/src/main/java/com/conveyal/taui/models/ModificationStop.java index 80b3241c..b94086ef 100644 --- a/src/main/java/com/conveyal/taui/models/ModificationStop.java +++ b/src/main/java/com/conveyal/taui/models/ModificationStop.java @@ -17,17 +17,26 @@ class ModificationStop { private static double MIN_SPACING_PERCENTAGE = 0.25; private static int DEFAULT_SEGMENT_SPEED = 15; - StopSpec stop; - double distanceFromStart; + private StopSpec stop; + private double distanceFromStart; - ModificationStop(Coordinate c, String id, double distanceFromStart) { + private ModificationStop(Coordinate c, String id, double distanceFromStart) { this.stop = new StopSpec(c.x, c.y); this.stop.id = id; this.distanceFromStart = distanceFromStart; } static List toSpec (List stops) { - return stops.stream().map(s -> s.stop).collect(Collectors.toList()); + return stops + .stream() + .map(s -> { + if (s.stop.id == null){ + return s.stop; + } else { + return new StopSpec(s.stop.id); + } + }) + .collect(Collectors.toList()); } static List getStopsFromSegments (List segments) { @@ -110,7 +119,7 @@ static int[] getDwellTimes (List stops, Integer[] dwellTimes, int realStopIndex = 0; for (int i = 0; i < stops.size(); i++) { String id = stops.get(i).stop.id; - if (id == null || dwellTimes == null) { + if (id == null || dwellTimes == null || dwellTimes.length <= realStopIndex) { stopDwellTimes[i] = defaultDwellTime; } else { Integer specificDwellTime = dwellTimes[realStopIndex]; From 0f9bde2583b00827a0a06185b7c0e647a16e7d3b Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Mon, 4 Dec 2017 15:45:46 -0500 Subject: [PATCH 05/18] refactor(modification-stops): Only generate the StopSpecs when sending to r5 --- .../taui/models/ModificationStop.java | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/ModificationStop.java b/src/main/java/com/conveyal/taui/models/ModificationStop.java index b94086ef..f18d3272 100644 --- a/src/main/java/com/conveyal/taui/models/ModificationStop.java +++ b/src/main/java/com/conveyal/taui/models/ModificationStop.java @@ -17,28 +17,39 @@ class ModificationStop { private static double MIN_SPACING_PERCENTAGE = 0.25; private static int DEFAULT_SEGMENT_SPEED = 15; - private StopSpec stop; + private Coordinate coordinate; + private String id; private double distanceFromStart; private ModificationStop(Coordinate c, String id, double distanceFromStart) { - this.stop = new StopSpec(c.x, c.y); - this.stop.id = id; + this.coordinate = c; + this.id = id; this.distanceFromStart = distanceFromStart; } + /** + * Create the StopSpec types required by r5 + * @param stops + * @return + */ static List toSpec (List stops) { return stops .stream() .map(s -> { - if (s.stop.id == null){ - return s.stop; + if (s.id == null){ + return new StopSpec(s.coordinate.x, s.coordinate.y); } else { - return new StopSpec(s.stop.id); + return new StopSpec(s.id); } }) .collect(Collectors.toList()); } + /** + * We don't just use `StopSpec`s here because we need to keep the `distanceFromStart` for generating hop times. + * @param segments Modification segments + * @return ModificationStop[] + */ static List getStopsFromSegments (List segments) { Stack stops = new Stack<>(); CoordinateReferenceSystem crs = DefaultGeographicCRS.WGS84; @@ -93,7 +104,7 @@ static List getStopsFromSegments (List segments) { // If the last auto-generated stop was too close, pop it if (stops.size() > 0) { ModificationStop lastStop = stops.peek(); - if (lastStop.stop.id == null && (distanceToLineSegmentStart - distanceToLastStop) / spacing < MIN_SPACING_PERCENTAGE) { + if (lastStop.id == null && (distanceToLineSegmentStart - distanceToLastStop) / spacing < MIN_SPACING_PERCENTAGE) { stops.pop(); } } @@ -118,7 +129,7 @@ static int[] getDwellTimes (List stops, Integer[] dwellTimes, int realStopIndex = 0; for (int i = 0; i < stops.size(); i++) { - String id = stops.get(i).stop.id; + String id = stops.get(i).id; if (id == null || dwellTimes == null || dwellTimes.length <= realStopIndex) { stopDwellTimes[i] = defaultDwellTime; } else { @@ -147,7 +158,7 @@ static int[] getHopTimes (List stops, int[] segmentSpeeds) { int segmentSpeed = segmentSpeeds.length > realStopIndex ? segmentSpeeds[realStopIndex] : DEFAULT_SEGMENT_SPEED; hopTimes[i] = (int) (hopDistance / (segmentSpeed * 1000) * 3000); - if (stop.stop.id != null) { + if (stop.id != null) { realStopIndex++; } From ab0dcf21d054c761a86b05975033e71992fe09f2 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Mon, 4 Dec 2017 16:43:56 -0500 Subject: [PATCH 06/18] fix(auth): Actually throw the errors on authentication --- src/main/java/com/conveyal/taui/AnalysisServer.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/conveyal/taui/AnalysisServer.java b/src/main/java/com/conveyal/taui/AnalysisServer.java index 10a59797..0a7b7eb4 100644 --- a/src/main/java/com/conveyal/taui/AnalysisServer.java +++ b/src/main/java/com/conveyal/taui/AnalysisServer.java @@ -150,14 +150,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,22 +167,22 @@ 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) 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 From 047e8a8d89e6849844bddbe6f38430f5290193b0 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Tue, 5 Dec 2017 10:22:19 -0500 Subject: [PATCH 07/18] fix(modes): Create empty enum sets when a mode type doesn't exist --- .../conveyal/taui/models/AnalysisRequest.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java index 9246aaa3..8d5c44c4 100644 --- a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java +++ b/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; @@ -146,15 +146,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 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); + } + } } From 21397bfa2f22bd225a71a1c3402e71c86e5d08aa Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Tue, 5 Dec 2017 10:23:42 -0500 Subject: [PATCH 08/18] feat(log): Log each API request and error response with the user and group who requested it --- .../com/conveyal/taui/AnalysisServer.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/conveyal/taui/AnalysisServer.java b/src/main/java/com/conveyal/taui/AnalysisServer.java index 0a7b7eb4..80a4947d 100644 --- a/src/main/java/com/conveyal/taui/AnalysisServer.java +++ b/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 given, set accessGroup and email automatically"); // hardwire group name if we're working offline req.attribute("accessGroup", "OFFLINE"); req.attribute("email", "analysis@conveyal.com"); } + + // 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."); @@ -190,10 +191,10 @@ public static void handleAuthentication (Request req, Response res) { 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(); From bbd417ef5630d49405d42ce19d725725c99ce3ab Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Tue, 5 Dec 2017 10:42:58 -0500 Subject: [PATCH 09/18] refactor(message): Update the "No Auth0" log message to be more clear --- src/main/java/com/conveyal/taui/AnalysisServer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/taui/AnalysisServer.java b/src/main/java/com/conveyal/taui/AnalysisServer.java index 80a4947d..6b47da3a 100644 --- a/src/main/java/com/conveyal/taui/AnalysisServer.java +++ b/src/main/java/com/conveyal/taui/AnalysisServer.java @@ -84,7 +84,7 @@ public static void main (String... args) { if (AnalysisServerConfig.auth0ClientId != null && AnalysisServerConfig.auth0Secret != null) { handleAuthentication(req, res); } else { - LOG.warn("No Auth0 credentials given, set accessGroup and email automatically"); + 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", "analysis@conveyal.com"); From aa88226ae28dee368ea396448435417ca8e9f902 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Wed, 6 Dec 2017 09:49:33 -0500 Subject: [PATCH 10/18] fix(analysis): Fix the CRC creation of the modifications checksum It incorrectly used Object::toString, switched to use JsonUtilities::objectToJsonBytes --- .../java/com/conveyal/taui/models/AnalysisRequest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java index 8d5c44c4..281fa4ae 100644 --- a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java @@ -9,6 +9,8 @@ import com.conveyal.r5.common.JsonUtilities; import com.conveyal.taui.persistence.Persistence; import com.mongodb.QueryBuilder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.time.LocalDate; import java.util.ArrayList; @@ -19,6 +21,7 @@ import java.util.zip.CRC32; public class AnalysisRequest { + private static final Logger LOG = LoggerFactory.getLogger(AnalysisRequest.class); private static int ZOOM = 9; public String projectId; @@ -83,14 +86,15 @@ 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 + // Generate a checksum of all the modifications CRC32 crc = new CRC32(); - crc.update(modifications.stream().map(Modification::toString).collect(Collectors.joining("-")).getBytes()); + modifications.stream().map(JsonUtilities::objectToJsonBytes).forEach(crc::update); crc.update(JsonUtilities.objectToJsonBytes(this)); + 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; From cf7b2d9046410d75dd34d59d5cd82a8d8c3264fe Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Thu, 7 Dec 2017 09:22:39 -0500 Subject: [PATCH 11/18] refactor(models): Add clarifying comments and convert the list directly to bytes instead of each ind --- .../java/com/conveyal/taui/models/AnalysisRequest.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java index 281fa4ae..56d612a7 100644 --- a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java @@ -9,8 +9,6 @@ import com.conveyal.r5.common.JsonUtilities; import com.conveyal.taui.persistence.Persistence; import com.mongodb.QueryBuilder; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.time.LocalDate; import java.util.ArrayList; @@ -21,7 +19,6 @@ import java.util.zip.CRC32; public class AnalysisRequest { - private static final Logger LOG = LoggerFactory.getLogger(AnalysisRequest.class); private static int ZOOM = 9; public String projectId; @@ -86,9 +83,10 @@ public AnalysisTask populateTask (AnalysisTask task, Project project) { modifications = modificationsForProject(project.accessGroup, projectId, variantIndex); } - // Generate a checksum of all the modifications + // Generate a checksum of all the modifications and request data to uniquely identify this job for the broker + // and the workers. CRC32 crc = new CRC32(); - modifications.stream().map(JsonUtilities::objectToJsonBytes).forEach(crc::update); + crc.update(JsonUtilities.objectToJsonBytes(modifications)); crc.update(JsonUtilities.objectToJsonBytes(this)); long crcValue = crc.getValue(); From ea15a87c7c4149348a269a7d9eb3a4da912a3e52 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Thu, 7 Dec 2017 15:38:29 -0500 Subject: [PATCH 12/18] fix(grid): Fix png exporting for grids Not going to look at "blame" here for fear it's me --- src/main/java/com/conveyal/taui/grids/GridExporter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/taui/grids/GridExporter.java b/src/main/java/com/conveyal/taui/grids/GridExporter.java index b9099a76..bcd78919 100644 --- a/src/main/java/com/conveyal/taui/grids/GridExporter.java +++ b/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); From 480e29c3f258271360951abf51a03059a609a1f2 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Thu, 7 Dec 2017 16:16:48 -0500 Subject: [PATCH 13/18] refactor(exceptions): Replace generic exceptions with AnalysisServerExceptions --- .../analysis/RegionalAnalysisManager.java | 3 ++- .../OpportunityDatasetsController.java | 4 ++-- .../RegionalAnalysisController.java | 3 ++- .../java/com/conveyal/taui/models/Bundle.java | 5 +++-- .../com/conveyal/taui/models/Project.java | 4 +++- .../java/com/conveyal/taui/models/Region.java | 3 ++- .../taui/models/RegionalAnalysis.java | 3 ++- .../conveyal/taui/persistence/MongoMap.java | 6 +++--- .../taui/persistence/OSMPersistence.java | 3 ++- .../taui/persistence/TiledAccessGrid.java | 21 ++++++++++--------- 10 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/conveyal/taui/analysis/RegionalAnalysisManager.java b/src/main/java/com/conveyal/taui/analysis/RegionalAnalysisManager.java index e5ef97cb..9f005695 100644 --- a/src/main/java/com/conveyal/taui/analysis/RegionalAnalysisManager.java +++ b/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 --git a/src/main/java/com/conveyal/taui/controllers/OpportunityDatasetsController.java b/src/main/java/com/conveyal/taui/controllers/OpportunityDatasetsController.java index b85dc306..728163c9 100644 --- a/src/main/java/com/conveyal/taui/controllers/OpportunityDatasetsController.java +++ b/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 writeOpportunityDatasetToS3(Map result = wrappedCollection.removeById((String) key); LOG.info(result.toString()); if (result.getN() == 0) { - throw new RuntimeException(String.format("The data for _id %s does not exist", key)); + throw AnalysisServerException.NotFound(String.format("The data for _id %s does not exist", key)); } return null; diff --git a/src/main/java/com/conveyal/taui/persistence/OSMPersistence.java b/src/main/java/com/conveyal/taui/persistence/OSMPersistence.java index 73cbc0c5..1127ab0e 100644 --- a/src/main/java/com/conveyal/taui/persistence/OSMPersistence.java +++ b/src/main/java/com/conveyal/taui/persistence/OSMPersistence.java @@ -3,6 +3,7 @@ import com.conveyal.osmlib.OSM; import com.conveyal.osmlib.OSMCache; import com.conveyal.taui.AnalysisServerConfig; +import com.conveyal.taui.AnalysisServerException; import com.conveyal.taui.models.Bounds; import com.conveyal.taui.util.HttpUtil; import com.google.common.io.ByteStreams; @@ -38,7 +39,7 @@ public static OSM retrieveOSMFromVexForBounds(Bounds bounds, String key) throws res = HttpUtil.httpClient.execute(get); if (res.getStatusLine().getStatusCode() != 200) { - throw new Exception("Could not retrieve OSM. " + res.getStatusLine()); + throw AnalysisServerException.Unknown("Could not retrieve OSM. " + res.getStatusLine()); } InputStream is = res.getEntity().getContent(); diff --git a/src/main/java/com/conveyal/taui/persistence/TiledAccessGrid.java b/src/main/java/com/conveyal/taui/persistence/TiledAccessGrid.java index c4aa7474..01cc38dc 100644 --- a/src/main/java/com/conveyal/taui/persistence/TiledAccessGrid.java +++ b/src/main/java/com/conveyal/taui/persistence/TiledAccessGrid.java @@ -6,6 +6,7 @@ import com.conveyal.r5.analyst.cluster.AccessGridWriter; import com.conveyal.r5.util.S3Util; import com.conveyal.taui.AnalysisServerConfig; +import com.conveyal.taui.AnalysisServerException; import com.conveyal.taui.util.JsonUtil; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; @@ -150,7 +151,7 @@ protected double computeValueForOrigin(int x, int y, int[] valuesThisOrigin, int TILE_SIZE, valuesThisOrigin.length); } catch (IOException e) { - throw new RuntimeException(e); + throw AnalysisServerException.Unknown(e); } }); @@ -166,7 +167,7 @@ protected double computeValueForOrigin(int x, int y, int[] valuesThisOrigin, int try { writer.writePixel(x % TILE_SIZE, y % TILE_SIZE, valuesThisOrigin); } catch (IOException e) { - throw new RuntimeException(e); + throw AnalysisServerException.Unknown(e); } return 0; @@ -190,7 +191,7 @@ protected double computeValueForOrigin(int x, int y, int[] valuesThisOrigin, int S3Util.s3.putObject(bucketName, key, input, metaData); } catch (IOException e) { - throw new RuntimeException(e); + throw AnalysisServerException.Unknown(e); } }); @@ -207,7 +208,7 @@ protected double computeValueForOrigin(int x, int y, int[] valuesThisOrigin, int LOG.info("Done saving tiled access grid to S3"); } catch (IOException e) { - throw new RuntimeException(e); + throw AnalysisServerException.Unknown(e); } } @@ -218,7 +219,7 @@ private void read () { this.header = JsonUtil.objectMapper.readValue(is, Header.class); is.close(); } catch (IOException e) { - throw new RuntimeException(e); + throw AnalysisServerException.Unknown(e); } } @@ -240,13 +241,13 @@ public int[] getGridCoordinates (int x, int y) { for (int i = 0; i < 8; i++) header[i] = tile.readByte(); String headerStr = new String(header); - if (!"ACCESSGR".equals(headerStr)) throw new IllegalStateException("Tile is not in Access Grid format!"); - if (tile.readInt() != 0) throw new IllegalStateException("Invalid access grid tile version!"); + if (!"ACCESSGR".equals(headerStr)) throw AnalysisServerException.BadRequest("Tile is not in Access Grid format!"); + if (tile.readInt() != 0) throw AnalysisServerException.BadRequest("Invalid access grid tile version!"); tile.seek(24); // seek to width int width = readIntLittleEndian(tile); int height = readIntLittleEndian(tile); - if (width != TILE_SIZE || height != TILE_SIZE) throw new IllegalStateException("Invalid access grid tile size!"); + if (width != TILE_SIZE || height != TILE_SIZE) throw AnalysisServerException.BadRequest("Invalid access grid tile size!"); long nValuesPerPixel = readIntLittleEndian(tile); // 4 bytes per value @@ -262,7 +263,7 @@ public int[] getGridCoordinates (int x, int y) { return values; } catch (IOException | ExecutionException e) { - throw new RuntimeException(e); + throw AnalysisServerException.Unknown(e); } } @@ -279,7 +280,7 @@ public static TiledAccessGrid get (String bucketName, String key) { try { return cache.get(compositeKey); } catch (ExecutionException e) { - throw new RuntimeException(e); + throw AnalysisServerException.Unknown(e); } } From b8f06124c4666a23cb9b715af140e3231e60dd08 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Thu, 7 Dec 2017 17:02:50 -0500 Subject: [PATCH 14/18] fix(bookmarks): Use AnalysisRequest for creating Bookmarks instead of R5's ProfileRequest --- src/main/java/com/conveyal/taui/models/Bookmark.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/Bookmark.java b/src/main/java/com/conveyal/taui/models/Bookmark.java index 0768d3cf..a8a6f7d1 100644 --- a/src/main/java/com/conveyal/taui/models/Bookmark.java +++ b/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; From 48130f96d7a84d03bc65bf6bfbc6fd2522d383fd Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Mon, 11 Dec 2017 11:50:03 -0500 Subject: [PATCH 15/18] refactor(analysis): Exclude request parameters from modification checksum --- src/main/java/com/conveyal/taui/models/AnalysisRequest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java index 56d612a7..4fe5e2a6 100644 --- a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java @@ -83,11 +83,9 @@ public AnalysisTask populateTask (AnalysisTask task, Project project) { modifications = modificationsForProject(project.accessGroup, projectId, variantIndex); } - // Generate a checksum of all the modifications and request data to uniquely identify this job for the broker - // and the workers. + // Generate a checksum of the modifications to uniquely identify this dataset for the broker and the workers. CRC32 crc = new CRC32(); crc.update(JsonUtilities.objectToJsonBytes(modifications)); - crc.update(JsonUtilities.objectToJsonBytes(this)); long crcValue = crc.getValue(); task.scenario = new Scenario(); From f2803c15d1aa92ad525aa4f6763028dc48240a64 Mon Sep 17 00:00:00 2001 From: ansons Date: Mon, 11 Dec 2017 12:09:58 -0500 Subject: [PATCH 16/18] refactor(comment): clarify modifications checksum --- src/main/java/com/conveyal/taui/models/AnalysisRequest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java index 4fe5e2a6..b725e241 100644 --- a/src/main/java/com/conveyal/taui/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/taui/models/AnalysisRequest.java @@ -83,7 +83,9 @@ public AnalysisTask populateTask (AnalysisTask task, Project project) { modifications = modificationsForProject(project.accessGroup, projectId, variantIndex); } - // Generate a checksum of the modifications to uniquely identify this dataset for the broker and the workers. + // 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(JsonUtilities.objectToJsonBytes(modifications)); long crcValue = crc.getValue(); From 65b4b1965111112b81f9cd85915ed53375e43fa9 Mon Sep 17 00:00:00 2001 From: Anson Stewart Date: Mon, 11 Dec 2017 14:03:36 -0500 Subject: [PATCH 17/18] Prepare 4.0.1 release --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f0c6da1c..2a781264 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.conveyal analyst - 4.1.1 + 4.0.1 From 60152336a8ab975743203d99721453f6f884dbf3 Mon Sep 17 00:00:00 2001 From: Anson Stewart Date: Mon, 11 Dec 2017 14:08:29 -0500 Subject: [PATCH 18/18] Prepare 4.1.1 release --- pom.xml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 2a781264..89145f52 100644 --- a/pom.xml +++ b/pom.xml @@ -150,9 +150,7 @@ com.conveyal - - - + r5 4.0.0-20171109.183530-3