Merge pull request ibi-group#473 from ibi-group/gtfsplus-handle-empty-rows

Report empty GTFS+ rows in separate validation issue

Author: binh-dam-ibigroup

src/main/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidation.java

@@ -9,6 +9,7 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import org.apache.commons.io.input.BOMInputStream;
+import org.apache.logging.log4j.util.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -150,34 +151,44 @@ private static void validateTable(
         // Iterate over each row and validate each field value.
         int rowIndex = 0;
         int rowsWithWrongNumberOfColumns = 0;
+        int emptyRows = 0;
         while (csvReader.readRecord()) {
             // First, check that row has the correct number of fields.
             int recordColumnCount = csvReader.getColumnCount();
-            if (recordColumnCount != fieldsFound.length) {
-                rowsWithWrongNumberOfColumns++;
-            }
-            // Validate each value in row. Note: we iterate over the fields and not values because a row may be missing
-            // columns, but we still want to validate that missing value (e.g., if it is missing a required field).
             String[] rowValues = csvReader.getValues();
-            for (int f = 0; f < fieldsFound.length; f++) {
-                // If value exists for index, use that. Otherwise, default to null to avoid out of bounds exception.
-                String val = f < recordColumnCount ? rowValues[f] : null;
-                validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, fieldsFound[f], gtfsFeed);
+            if (recordColumnCount == 1 && Strings.isBlank(rowValues[0])) {
+                // If row is empty (technically, the row has one column with a blank value),
+                // report that as such (and skip validating column values).
+                emptyRows++;
+            } else {
+                if (recordColumnCount != fieldsFound.length) {
+                    rowsWithWrongNumberOfColumns++;
+                }
+                // Validate each value in row. Note: we iterate over the fields and not values because a row may be missing
+                // columns, but we still want to validate that missing value (e.g., if it is missing a required field).
+                for (int f = 0; f < fieldsFound.length; f++) {
+                    // If value exists for index, use that. Otherwise, default to null to avoid out of bounds exception.
+                    String val = f < recordColumnCount ? rowValues[f] : null;
+                    validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, fieldsFound[f], gtfsFeed);
+                }
             }
             rowIndex++;
         }
         csvReader.close();
 
-        // Add issue for wrong number of columns after processing all rows.
+        // Add issues for wrong number of columns and for empty rows after processing all rows.
         // Note: We considered adding an issue for each row, but opted for the single error approach because there's no
         // concept of a row-level issue in the UI right now. So we would potentially need to add that to the UI
         // somewhere. Also, there's the trouble of reporting the issue at the row level, but not really giving the user
-        // a great way to resolve the issue in the GTFS+ editor. Essentially, all of the rows with the wrong number of
+        // a great way to resolve the issue in the GTFS+ editor. Essentially, all rows with the wrong number of
         // columns can be resolved simply by clicking the "Save and Revalidate" button -- so the resolution is more at
         // the table level than the row level (like, for example, a bad value for a field would be).
         if (rowsWithWrongNumberOfColumns > 0) {
             issues.add(new ValidationIssue(tableId, null, -1, rowsWithWrongNumberOfColumns + " row(s) do not contain the same number of fields as there are headers. (File may need to be edited manually.)"));
         }
+        if (emptyRows > 0) {
+            issues.add(new ValidationIssue(tableId, null, -1, emptyRows + " row(s) are empty. (File may need to be edited manually.)"));
+        }
     }
 
     /** Determine if a GTFS+ spec field is required. */

src/test/java/com/conveyal/datatools/manager/gtfsplus/GtfsPlusValidationTest.java

@@ -32,6 +32,7 @@ public class GtfsPlusValidationTest extends UnitTest {
     private static final Logger LOG = LoggerFactory.getLogger(MergeFeedsJobTest.class);
     private static FeedVersion bartVersion1;
     private static FeedVersion bartVersion1WithQuotedValues;
+    private static FeedVersion lavtaVersion1;
     private static Project project;
     private static JsonNode routeAttributesFieldsNode;
 
@@ -55,6 +56,11 @@ public static void setUp() throws IOException {
         routeAttributesFieldsNode = Objects.requireNonNull(
                 GtfsPlusValidation.findNode(DataManager.gtfsPlusConfig, "id", "route_attributes")
             ).get("fields");
+
+        FeedSource lavta = new FeedSource("LAVTA");
+        lavta.projectId = project.id;
+        Persistence.feedSources.create(lavta);
+        lavtaVersion1 = createFeedVersionFromGtfsZip(lavta, "lavta-cal-attributes.zip");
     }
 
     @AfterAll
@@ -138,4 +144,25 @@ void canBuildRouteSubcategoryToCategoryMap() {
                 )
             ), equalTo("3"));
     }
+
+    @Test
+    void shouldReportEmptyRows() throws Exception {
+        // An empty row should be reported as such (separately from a row with incorrect number of columns).
+
+        LOG.info("Validating GTFS+ with an empty row");
+        GtfsPlusValidation validation = GtfsPlusValidation.validate(lavtaVersion1.id);
+        // Expect one GTFS+ issue of type "empty row".
+        assertThat(
+            "Should have one GTFS+ validation issue on the calendar_attributes table",
+            validation.issues.size(), equalTo(1)
+        );
+        assertThat(
+            "Should have the validation issue on the calendar_attributes table",
+            validation.issues.get(0).tableId, equalTo("calendar_attributes")
+        );
+        assertThat(
+            "Should have the GTFS+ 'empty row' validation issue on the calendar_attributes table",
+            validation.issues.get(0).description, equalTo("1 row(s) are empty. (File may need to be edited manually.)")
+        );
+    }
 }