Merge pull request #330 from conveyal/dev

Fix Release Sept 29, 2021

.github/workflows/maven.yml

@@ -29,11 +29,11 @@ jobs:
       uses: actions/setup-java@v1
       with:
         java-version: 1.8
-    # Install node 12 for running maven-semantic-release.
-    - name: Use Node.js 12.x
+    # Install node 14+ for running maven-semantic-release.
+    - name: Use Node.js 14.x
       uses: actions/setup-node@v1
       with:
-        node-version: 12.x
+        node-version: 14.x
     - name: Install maven-semantic-release
       # FIXME: Enable cache for node packages (add package.json?)
       run: |

src/main/java/com/conveyal/gtfs/loader/LineContext.java

@@ -1,5 +1,7 @@
 package com.conveyal.gtfs.loader;
 
+import static com.conveyal.gtfs.loader.JdbcGtfsLoader.POSTGRES_NULL_TEXT;
+
 /**
  * Wrapper class that provides access to row values and line context (e.g., line number) for a particular row of GTFS
  * data.
@@ -31,10 +33,15 @@ public String getValueForRow(int columnIndex) {
 
     /**
      * Overloaded method to provide value for the current line for a particular field.
+     * @param fieldName The name of the GTFS field/column to retrieve
+     * @return The value for the field, if found, or POSTGRES_NULL_TEXT if the field is not found,
+     *         consistent with cases where the value is null in the postgres database.
      */
     public String getValueForRow(String fieldName) {
         int fieldIndex = Field.getFieldIndex(fields, fieldName);
-        return rowDataWithLineNumber[fieldIndex + 1];
+        return fieldIndex >= 0
+            ? rowDataWithLineNumber[fieldIndex + 1]
+            : POSTGRES_NULL_TEXT;
     }
 
     /**

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

@@ -90,7 +90,7 @@ public void loadOneRow() throws IOException {
             st.pickup_type    = getIntField("pickup_type", false, 0, 3); // TODO add ranges as parameters
             st.drop_off_type  = getIntField("drop_off_type", false, 0, 3);
             st.continuous_pickup = getIntField("continuous_pickup", true, 0, 3);
-            st.continuous_pickup = getIntField("continuous_drop_off", true, 0, 3);
+            st.continuous_drop_off = getIntField("continuous_drop_off", true, 0, 3);
             st.shape_dist_traveled = getDoubleField("shape_dist_traveled", false, 0D, Double.MAX_VALUE); // FIXME using both 0 and NaN for "missing", define DOUBLE_MISSING
             st.timepoint      = getIntField("timepoint", false, 0, 1, INT_MISSING);
             st.feed           = null; // this could circular-serialize the whole feed

src/test/java/com/conveyal/gtfs/TestUtils.java

@@ -1,5 +1,6 @@
 package com.conveyal.gtfs;
 
+import com.conveyal.gtfs.error.NewGTFSErrorType;
 import org.apache.commons.io.IOUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -185,4 +186,30 @@ public static void assertThatSqlCountQueryYieldsExpectedCount(DataSource dataSou
         );
     }
 
+    /**
+     * Check that the test feed has expected number of errors for the provided values.
+     */
+    public static void checkFeedHasExpectedNumberOfErrors(
+        String testNamespace,
+        DataSource testDataSource,
+        NewGTFSErrorType errorType,
+        String entityType,
+        String lineNumber,
+        String entityId,
+        String badValue,
+        int expectedNumberOfErrors
+    ) {
+        String sql = String.format("select count(*) from %s.errors where error_type = '%s' and entity_type = '%s' and line_number = '%s'",
+            testNamespace,
+            errorType,
+            entityType,
+            lineNumber);
+
+        if (entityId != null) sql += String.format(" and entity_id = '%s'", entityId);
+        if (badValue != null) sql += String.format(" and bad_value = '%s'", badValue);
+
+        assertThatSqlCountQueryYieldsExpectedCount(testDataSource, sql, expectedNumberOfErrors);
+    }
+
+
 }

src/test/java/com/conveyal/gtfs/loader/ConditionallyRequiredTest.java

@@ -15,38 +15,51 @@
 
 import static com.conveyal.gtfs.GTFS.load;
 import static com.conveyal.gtfs.GTFS.validate;
-import static com.conveyal.gtfs.TestUtils.assertThatSqlCountQueryYieldsExpectedCount;
+import static com.conveyal.gtfs.TestUtils.checkFeedHasExpectedNumberOfErrors;
 import static com.conveyal.gtfs.error.NewGTFSErrorType.CONDITIONALLY_REQUIRED;
 import static com.conveyal.gtfs.error.NewGTFSErrorType.REFERENTIAL_INTEGRITY;
 import static com.conveyal.gtfs.error.NewGTFSErrorType.AGENCY_ID_REQUIRED_FOR_MULTI_AGENCY_FEEDS;
 
 public class ConditionallyRequiredTest {
-    private static String testDBName;
-    private static DataSource testDataSource;
-    private static String testNamespace;
+    private static String VTADBName;
+    private static DataSource VTADataSource;
+    private static String VTANamespace;
+
+    private static String triDeltaDBName;
+    private static DataSource triDeltaDataSource;
+    private static String triDeltaNamespace;
 
     @BeforeAll
     public static void setUpClass() throws IOException {
-        // Create a new database
-        testDBName = TestUtils.generateNewDB();
-        String dbConnectionUrl = String.format("jdbc:postgresql://localhost/%s", testDBName);
-        testDataSource = TestUtils.createTestDataSource(dbConnectionUrl);
-        // load feed into db
-        String zipFileName = TestUtils.zipFolderFiles(
-            "real-world-gtfs-feeds/VTA-gtfs-conditionally-required-checks",
-            true);
-        FeedLoadResult feedLoadResult = load(zipFileName, testDataSource);
-        testNamespace = feedLoadResult.uniqueIdentifier;
-        validate(testNamespace, testDataSource);
+        VTADBName = TestUtils.generateNewDB();
+        VTADataSource = TestUtils.createTestDataSource(String.format("jdbc:postgresql://localhost/%s", VTADBName));
+        VTANamespace = loadFeedAndValidate(VTADataSource, "real-world-gtfs-feeds/VTA-gtfs-conditionally-required-checks");
+
+        triDeltaDBName = TestUtils.generateNewDB();
+        triDeltaDataSource = TestUtils.createTestDataSource(String.format("jdbc:postgresql://localhost/%s", triDeltaDBName));
+        triDeltaNamespace = loadFeedAndValidate(triDeltaDataSource, "real-world-gtfs-feeds/tri-delta-fare-rules");
+    }
+
+    /**
+     * Load feed from zip file into a database and validate.
+     */
+    private static String loadFeedAndValidate(DataSource dataSource, String zipFolderName) throws IOException {
+        String zipFileName = TestUtils.zipFolderFiles(zipFolderName,  true);
+        FeedLoadResult feedLoadResult = load(zipFileName, dataSource);
+        String namespace = feedLoadResult.uniqueIdentifier;
+        validate(namespace, dataSource);
+        // return name space.
+        return namespace;
     }
 
     @AfterAll
     public static void tearDownClass() {
-        TestUtils.dropDB(testDBName);
+        TestUtils.dropDB(VTADBName);
+        TestUtils.dropDB(triDeltaDBName);
     }
 
     @Test
-    public void stopTimeTableMissingConditionallyRequiredArrivalDepartureTimes() {
+    void stopTimeTableMissingConditionallyRequiredArrivalDepartureTimes() {
         checkFeedHasOneError(
             CONDITIONALLY_REQUIRED,
             "StopTime",
@@ -58,7 +71,7 @@ public void stopTimeTableMissingConditionallyRequiredArrivalDepartureTimes() {
 
     @ParameterizedTest
     @MethodSource("createStopTableChecks")
-    public void stopTableConditionallyRequiredTests(
+    void stopTableConditionallyRequiredTests(
         NewGTFSErrorType errorType,
         String entityType,
         String lineNumber,
@@ -82,7 +95,7 @@ private static Stream<Arguments> createStopTableChecks() {
 
     @ParameterizedTest
     @MethodSource("createTranslationTableChecks")
-    public void translationTableConditionallyRequiredTests(
+    void translationTableConditionallyRequiredTests(
         String entityType,
         String lineNumber,
         String entityId,
@@ -100,7 +113,7 @@ private static Stream<Arguments> createTranslationTableChecks() {
     }
 
     @Test
-    public void agencyTableMissingConditionallyRequiredAgencyId() {
+    void agencyTableMissingConditionallyRequiredAgencyId() {
         checkFeedHasOneError(
             AGENCY_ID_REQUIRED_FOR_MULTI_AGENCY_FEEDS,
             "Agency",
@@ -110,7 +123,7 @@ public void agencyTableMissingConditionallyRequiredAgencyId() {
     }
 
     @Test
-    public void tripTableMissingConditionallyRequiredShapeId() {
+    void tripTableMissingConditionallyRequiredShapeId() {
         checkFeedHasOneError(
             CONDITIONALLY_REQUIRED,
             "Trip",
@@ -120,8 +133,27 @@ public void tripTableMissingConditionallyRequiredShapeId() {
         );
     }
 
+    /**
+     * If an optional column is missing from an imported GTFS feed
+     * (e.g. if the "contains_id" column is missing from the "fare_rules" table),
+     * then, referential integrity errors should not be triggered.
+     */
+    @Test
+    void shouldNotTriggerRefIntegrityErrorForMissingOptionalColumn() {
+        checkFeedHasExpectedNumberOfErrors(
+            triDeltaNamespace,
+            triDeltaDataSource,
+            REFERENTIAL_INTEGRITY,
+            "FareRule",
+            "2",
+            "1",
+            null,
+            0
+        );
+    }
+
     @Test
-    public void routeTableMissingConditionallyRequiredAgencyId() {
+    void routeTableMissingConditionallyRequiredAgencyId() {
         checkFeedHasOneError(
             AGENCY_ID_REQUIRED_FOR_MULTI_AGENCY_FEEDS,
             "Route",
@@ -132,7 +164,7 @@ public void routeTableMissingConditionallyRequiredAgencyId() {
     }
 
     @Test
-    public void fareAttributeTableMissingConditionallyRequiredAgencyId() {
+    void fareAttributeTableMissingConditionallyRequiredAgencyId() {
         checkFeedHasOneError(
             AGENCY_ID_REQUIRED_FOR_MULTI_AGENCY_FEEDS,
             "FareAttribute",
@@ -143,18 +175,18 @@ public void fareAttributeTableMissingConditionallyRequiredAgencyId() {
     }
 
     /**
-     * Check that the test feed has exactly one error for the provided values.
+     * Check that a test feed has exactly one error for the provided values.
      */
     private void checkFeedHasOneError(NewGTFSErrorType errorType, String entityType, String lineNumber, String entityId, String badValue) {
-        String sql = String.format("select count(*) from %s.errors where error_type = '%s' and entity_type = '%s' and line_number = '%s'",
-            testNamespace,
+        checkFeedHasExpectedNumberOfErrors(
+            VTANamespace,
+            VTADataSource,
             errorType,
             entityType,
-            lineNumber);
-
-        if (entityId != null) sql += String.format(" and entity_id = '%s'", entityId);
-        if (badValue != null) sql += String.format(" and bad_value = '%s'", badValue);
-
-        assertThatSqlCountQueryYieldsExpectedCount(testDataSource, sql,1);
+            lineNumber,
+            entityId,
+            badValue,
+            1
+        );
     }
 }

src/test/java/com/conveyal/gtfs/loader/GtfsFlexTest.java

@@ -0,0 +1,61 @@
+package com.conveyal.gtfs.loader;
+
+import com.conveyal.gtfs.TestUtils;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import javax.sql.DataSource;
+import java.io.IOException;
+import java.util.stream.Stream;
+
+import static com.conveyal.gtfs.GTFS.load;
+import static com.conveyal.gtfs.GTFS.validate;
+import static com.conveyal.gtfs.TestUtils.assertThatSqlCountQueryYieldsExpectedCount;
+
+/**
+ * Load in a GTFS feed with GTFS flex features, and ensure all needed fields are imported correctly.
+ * TODO: update feed to use more features, and test for these.
+ */
+public class GtfsFlexTest {
+    private static String testDBName;
+    private static DataSource testDataSource;
+    private static String testNamespace;
+
+    @BeforeAll
+    public static void setUpClass() throws IOException {
+        // Create a new database
+        testDBName = TestUtils.generateNewDB();
+        String dbConnectionUrl = String.format("jdbc:postgresql://localhost/%s", testDBName);
+        testDataSource = TestUtils.createTestDataSource(dbConnectionUrl);
+        // load feed into db
+        String zipFileName = TestUtils.zipFolderFiles(
+            "real-world-gtfs-feeds/washington-park-shuttle-with-flex-additions",
+            true);
+        FeedLoadResult feedLoadResult = load(zipFileName, testDataSource);
+        testNamespace = feedLoadResult.uniqueIdentifier;
+        validate(testNamespace, testDataSource);
+    }
+
+    @ParameterizedTest
+    @MethodSource("createContinuousPickupAndDropOffChecks")
+    void continuousPickupAndDropOffTests(String namespace, String field, int value, int expectedCount) {
+        String query = String.format("select count(*) from %s.stop_times where %s = '%s'",
+                namespace,
+                field,
+                value);
+        assertThatSqlCountQueryYieldsExpectedCount(testDataSource, query, expectedCount);
+    }
+
+    private static Stream<Arguments> createContinuousPickupAndDropOffChecks() {
+        return Stream.of(
+                Arguments.of(testNamespace, "continuous_pickup", 0, 3),
+                Arguments.of(testNamespace, "continuous_pickup", 1, 5),
+                Arguments.of(testNamespace, "continuous_pickup", 2, 1),
+                Arguments.of(testNamespace, "continuous_drop_off", 0, 2),
+                Arguments.of(testNamespace, "continuous_drop_off", 1, 5),
+                Arguments.of(testNamespace, "continuous_drop_off", 2, 2)
+        );
+    }
+}

src/test/java/com/conveyal/gtfs/loader/LineContextTest.java

@@ -0,0 +1,33 @@
+package com.conveyal.gtfs.loader;
+
+import org.junit.jupiter.api.Test;
+
+import static com.conveyal.gtfs.loader.JdbcGtfsLoader.POSTGRES_NULL_TEXT;
+import static com.conveyal.gtfs.loader.Requirement.OPTIONAL;
+import static com.conveyal.gtfs.loader.Requirement.REQUIRED;
+import static com.conveyal.gtfs.loader.Table.FARE_ATTRIBUTES;
+import static com.conveyal.gtfs.loader.Table.ROUTES;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class LineContextTest {
+    /**
+     * Emulates querying a field in a GTFS record for which the column is absent in the source GTFS.
+     * Here we query the optional "contains_id" field that is missing in the "fare_rules" table.
+     */
+    @Test
+    void shouldReturnPostgresNullTextForMissingField() {
+        // Line number, fare_id, route_id
+        // (Optional field "contains_id" is missing.)
+        String[] rowDataWithLineNumber = new String[] {"2", "1", "300"};
+        Table table = Table.FARE_RULES;
+
+        // Here, only list fields that are loaded from the GTFS feed, as happens during execution.
+        Field[] fields = new Field[] {
+            new StringField("fare_id", REQUIRED).isReferenceTo(FARE_ATTRIBUTES),
+            new StringField("route_id", OPTIONAL).isReferenceTo(ROUTES),
+        };
+        LineContext lineContext = new LineContext(table, fields, rowDataWithLineNumber, 2);
+
+        assertEquals(POSTGRES_NULL_TEXT, lineContext.getValueForRow("contains_id"));
+    }
+}

src/test/resources/fake-agency-mtc-long-fields/fare_attributes.txt

@@ -1,2 +1,2 @@
-fare_id,price,currency_type,payment_method,transfers,transfer_duration
-route_based_fare,1.23,USD,0,0,0
+fare_id,agency_id,price,currency_type,payment_method,transfers,transfer_duration
+route_based_fare,1,1.23,USD,0,0,0

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/agency.txt

@@ -0,0 +1,2 @@
+agency_id,agency_name,agency_url,agency_phone,agency_timezone,agency_lang
+1,"Tri Delta Transit","http://trideltatransit.com","(925) 754-4040","America/Los_Angeles","en"

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/calendar.txt

@@ -0,0 +1,2 @@
+service_id,monday,tuesday,wednesday,thursday,friday,saturday,sunday,start_date,end_date
+1,1,1,1,1,1,1,1,20210828,20221231

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/fare_attributes.txt

@@ -0,0 +1,2 @@
+fare_id,price,currency_type,payment_method,transfers,transfer_duration,agency_id
+"1",2.00,USD,0,0,,1

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/fare_rules.txt

@@ -0,0 +1,2 @@
+fare_id,route_id
+"1","300"

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/routes.txt

@@ -0,0 +1,2 @@
+route_id,agency_id,route_short_name,route_long_name,route_type,route_color,route_text_color,route_url
+"300",1,"300","Brentwood Park & Ride / Antioch BART",3,800080,FFFFFF,"http://12.155.17.20/RTT/Public/Schedule.aspx?RouteNo=300"

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/shapes.txt

@@ -0,0 +1,3 @@
+shape_id,shape_pt_lon,shape_pt_lat,shape_pt_sequence
+27,-121.780946,37.996068,1
+27,-121.698770,37.933812,2

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/stop_times.txt

@@ -0,0 +1,6 @@
+trip_id,arrival_time,departure_time,stop_id,stop_sequence,stop_headsign,timepoint
+1720,17:46:00,17:46:00,722,1,,1
+1720,17:59:00,17:59:00,33,2,,1
+1720,18:02:01,18:02:01,443,3,,0
+1720,18:20:06,18:20:06,421,4,,0
+1720,18:24:00,18:24:00,610,5,,1

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/stops.txt

@@ -0,0 +1,6 @@
+stop_id,stop_code,stop_name,stop_lat,stop_lon,stop_url,zone_id
+33,813644,"Main St & Empire Ave",37.997839,-121.730492,"http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=33",
+421,810930,"Brentwood Blvd & Village Dr",37.941524,-121.696341,"http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=421",
+443,811043,"Main St & Norcross Ln",37.997716,-121.716376,"http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=443",
+610,811425,"Brentwood Park n Ride",37.933838,-121.698755,"http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=610",
+722,817754,"Antioch BART",37.995572,-121.778137,"http://12.155.17.20/RTT/Public/RoutePositionET.aspx?PlatformTag=722",

src/test/resources/real-world-gtfs-feeds/tri-delta-fare-rules/trips.txt

@@ -0,0 +1,2 @@
+route_id,service_id,trip_id,trip_headsign,direction_id,block_id,shape_id
+"300",1,1720,"Eastbound Brentwood Park & Ride",0,167,27

src/test/resources/real-world-gtfs-feeds/washington-park-shuttle-with-flex-additions/agency.txt

@@ -0,0 +1,2 @@
+agency_id,agency_url,agency_lang,agency_name,agency_phone,agency_timezone,agency_fare_url
+501,http://ewpshuttle.org/,en,Washington Park Shuttle,503-319-0999,America/Los_Angeles,http://explorewashingtonpark.org/getting-here/#shuttle