fix(PatternFinder): use LinkedHashMap for patterns and fix snapshots

It appears that there has been a longstanding bug in this part of the code, where stop_times and trips were potentially being misassigned to patterns. This needs an extra set of eyes for verification just to be sure though.
conveyal · Dec 8, 2020 · fd059ce · fd059ce
1 parent e07fa33
commit fd059ce
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 17 deletions.
diff --git a/src/main/java/com/conveyal/gtfs/PatternFinder.java b/src/main/java/com/conveyal/gtfs/PatternFinder.java
@@ -23,6 +23,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -86,7 +87,8 @@ public Map<TripPatternKey, Pattern> createPatternObjects(Map<String, Stop> stopB
         // Make pattern ID one-based to avoid any JS type confusion between an ID of zero vs. null value.
         int nextPatternId = 1;
         // Create an in-memory list of Patterns because we will later rename them before inserting them into storage.
-        Map<TripPatternKey, Pattern> patterns = new HashMap<>();
+        // Use a LinkedHashMap so we can retrieve the entrySets later in the order of insertion.
+        Map<TripPatternKey, Pattern> patterns = new LinkedHashMap<>();
         // TODO assign patterns sequential small integer IDs (may include route)
         for (TripPatternKey key : tripsForPattern.keySet()) {
             Collection<Trip> trips = tripsForPattern.get(key);

diff --git a/...est/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchPatterns-0.json b/...est/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchPatterns-0.json
@@ -5,7 +5,7 @@
       "patterns" : [ {
         "direction_id" : 0,
         "id" : 1,
-        "name" : "2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (1 trips)",
+        "name" : "2 stops from Butler Ln to Child Stop (1 trips)",
         "pattern_id" : "1",
         "pattern_stops" : [ {
           "default_dwell_time" : 0,
@@ -23,16 +23,16 @@
           "timepoint" : null
         }, {
           "default_dwell_time" : 0,
-          "default_travel_time" : 60,
+          "default_travel_time" : 1740,
           "drop_off_type" : 0,
           "id" : 2,
           "pattern_id" : "1",
           "pickup_type" : 0,
           "shape_dist_traveled" : 341.4491961,
           "stop" : [ {
-            "stop_id" : "johv"
+            "stop_id" : "1234"
           } ],
-          "stop_id" : "johv",
+          "stop_id" : "1234",
           "stop_sequence" : 1,
           "timepoint" : null
         } ],
@@ -94,17 +94,17 @@
         "stops" : [ {
           "stop_id" : "4u6g"
         }, {
-          "stop_id" : "johv"
+          "stop_id" : "1234"
         } ],
         "trip_count" : 1,
         "trips" : [ {
-          "trip_id" : "a30277f8-e50a-4a85-9141-b1e0da9d429d"
+          "trip_id" : "frequency-trip"
         } ],
         "use_frequency" : null
       }, {
         "direction_id" : 0,
         "id" : 2,
-        "name" : "2 stops from Butler Ln to Child Stop (1 trips)",
+        "name" : "2 stops from Butler Ln to Scotts Valley Dr & Victor Sq (1 trips)",
         "pattern_id" : "2",
         "pattern_stops" : [ {
           "default_dwell_time" : 0,
@@ -122,16 +122,16 @@
           "timepoint" : null
         }, {
           "default_dwell_time" : 0,
-          "default_travel_time" : 1740,
+          "default_travel_time" : 60,
           "drop_off_type" : 0,
           "id" : 4,
           "pattern_id" : "2",
           "pickup_type" : 0,
           "shape_dist_traveled" : 341.4491961,
           "stop" : [ {
-            "stop_id" : "1234"
+            "stop_id" : "johv"
           } ],
-          "stop_id" : "1234",
+          "stop_id" : "johv",
           "stop_sequence" : 1,
           "timepoint" : null
         } ],
@@ -193,11 +193,11 @@
         "stops" : [ {
           "stop_id" : "4u6g"
         }, {
-          "stop_id" : "1234"
+          "stop_id" : "johv"
         } ],
         "trip_count" : 1,
         "trips" : [ {
-          "trip_id" : "frequency-trip"
+          "trip_id" : "a30277f8-e50a-4a85-9141-b1e0da9d429d"
         } ],
         "use_frequency" : null
       } ]

diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStops-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchStops-0.json
@@ -39,7 +39,7 @@
         "location_type" : 0,
         "parent_station" : null,
         "patterns" : [ {
-          "pattern_id" : "1"
+          "pattern_id" : "2"
         } ],
         "routes" : [ {
           "route_id" : "1"
@@ -83,7 +83,7 @@
         "location_type" : 0,
         "parent_station" : "123",
         "patterns" : [ {
-          "pattern_id" : "2"
+          "pattern_id" : "1"
         } ],
         "routes" : [ {
           "route_id" : "1"

diff --git a/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchTrips-0.json b/src/test/resources/snapshots/com/conveyal/gtfs/graphql/GTFSGraphQLTest/canFetchTrips-0.json
@@ -8,7 +8,7 @@
         "direction_id" : 0,
         "frequencies" : [ ],
         "id" : 2,
-        "pattern_id" : "1",
+        "pattern_id" : "2",
         "route_id" : "1",
         "service_id" : "04100312-8fe1-46a5-a9f2-556f39478f57",
         "shape" : [ {
@@ -83,7 +83,7 @@
           "trip_id" : "frequency-trip"
         } ],
         "id" : 3,
-        "pattern_id" : "2",
+        "pattern_id" : "1",
         "route_id" : "1",
         "service_id" : "04100312-8fe1-46a5-a9f2-556f39478f57",
         "shape" : [ {