Skip to content

Commit 0b27d57

Browse files
Revert "fix(GtfsPlusValidation): Report empty GTFS+ rows in separate validation issue."
This reverts commit 9c64053.
1 parent 9c64053 commit 0b27d57

File tree

3 files changed

+11
-49
lines changed

3 files changed

+11
-49
lines changed

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

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import com.fasterxml.jackson.databind.JsonNode;
1010
import com.fasterxml.jackson.databind.node.ArrayNode;
1111
import org.apache.commons.io.input.BOMInputStream;
12-
import org.apache.logging.log4j.util.Strings;
1312
import org.slf4j.Logger;
1413
import org.slf4j.LoggerFactory;
1514

@@ -151,44 +150,34 @@ private static void validateTable(
151150
// Iterate over each row and validate each field value.
152151
int rowIndex = 0;
153152
int rowsWithWrongNumberOfColumns = 0;
154-
int emptyRows = 0;
155153
while (csvReader.readRecord()) {
156154
// First, check that row has the correct number of fields.
157155
int recordColumnCount = csvReader.getColumnCount();
156+
if (recordColumnCount != fieldsFound.length) {
157+
rowsWithWrongNumberOfColumns++;
158+
}
159+
// Validate each value in row. Note: we iterate over the fields and not values because a row may be missing
160+
// columns, but we still want to validate that missing value (e.g., if it is missing a required field).
158161
String[] rowValues = csvReader.getValues();
159-
if (recordColumnCount == 1 && Strings.isBlank(rowValues[0])) {
160-
// If row is empty (technically, the row has one column with a blank value),
161-
// report that as such (and skip validating column values).
162-
emptyRows++;
163-
} else {
164-
if (recordColumnCount != fieldsFound.length) {
165-
rowsWithWrongNumberOfColumns++;
166-
}
167-
// Validate each value in row. Note: we iterate over the fields and not values because a row may be missing
168-
// columns, but we still want to validate that missing value (e.g., if it is missing a required field).
169-
for (int f = 0; f < fieldsFound.length; f++) {
170-
// If value exists for index, use that. Otherwise, default to null to avoid out of bounds exception.
171-
String val = f < recordColumnCount ? rowValues[f] : null;
172-
validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, fieldsFound[f], gtfsFeed);
173-
}
162+
for (int f = 0; f < fieldsFound.length; f++) {
163+
// If value exists for index, use that. Otherwise, default to null to avoid out of bounds exception.
164+
String val = f < recordColumnCount ? rowValues[f] : null;
165+
validateTableValue(issues, tableId, rowIndex, rowValues, val, fieldsFound, fieldsFound[f], gtfsFeed);
174166
}
175167
rowIndex++;
176168
}
177169
csvReader.close();
178170

179-
// Add issues for wrong number of columns and for empty rows after processing all rows.
171+
// Add issue for wrong number of columns after processing all rows.
180172
// Note: We considered adding an issue for each row, but opted for the single error approach because there's no
181173
// concept of a row-level issue in the UI right now. So we would potentially need to add that to the UI
182174
// somewhere. Also, there's the trouble of reporting the issue at the row level, but not really giving the user
183-
// a great way to resolve the issue in the GTFS+ editor. Essentially, all rows with the wrong number of
175+
// a great way to resolve the issue in the GTFS+ editor. Essentially, all of the rows with the wrong number of
184176
// columns can be resolved simply by clicking the "Save and Revalidate" button -- so the resolution is more at
185177
// the table level than the row level (like, for example, a bad value for a field would be).
186178
if (rowsWithWrongNumberOfColumns > 0) {
187179
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.)"));
188180
}
189-
if (emptyRows > 0) {
190-
issues.add(new ValidationIssue(tableId, null, -1, emptyRows + " row(s) are empty. (File may need to be edited manually.)"));
191-
}
192181
}
193182

194183
/** Determine if a GTFS+ spec field is required. */

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

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ public class GtfsPlusValidationTest extends UnitTest {
3232
private static final Logger LOG = LoggerFactory.getLogger(MergeFeedsJobTest.class);
3333
private static FeedVersion bartVersion1;
3434
private static FeedVersion bartVersion1WithQuotedValues;
35-
private static FeedVersion lavtaVersion1;
3635
private static Project project;
3736
private static JsonNode routeAttributesFieldsNode;
3837

@@ -56,11 +55,6 @@ public static void setUp() throws IOException {
5655
routeAttributesFieldsNode = Objects.requireNonNull(
5756
GtfsPlusValidation.findNode(DataManager.gtfsPlusConfig, "id", "route_attributes")
5857
).get("fields");
59-
60-
FeedSource lavta = new FeedSource("LAVTA");
61-
lavta.projectId = project.id;
62-
Persistence.feedSources.create(lavta);
63-
lavtaVersion1 = createFeedVersionFromGtfsZip(lavta, "lavta-cal-attributes.zip");
6458
}
6559

6660
@AfterAll
@@ -144,25 +138,4 @@ void canBuildRouteSubcategoryToCategoryMap() {
144138
)
145139
), equalTo("3"));
146140
}
147-
148-
@Test
149-
void shouldReportEmptyRows() throws Exception {
150-
// An empty row should be reported as such (separately from a row with incorrect number of columns).
151-
152-
LOG.info("Validating GTFS+ with an empty row");
153-
GtfsPlusValidation validation = GtfsPlusValidation.validate(lavtaVersion1.id);
154-
// Expect one GTFS+ issue of type "empty row".
155-
assertThat(
156-
"Should have one GTFS+ validation issue on the calendar_attributes table",
157-
validation.issues.size(), equalTo(1)
158-
);
159-
assertThat(
160-
"Should have the validation issue on the calendar_attributes table",
161-
validation.issues.get(0).tableId, equalTo("calendar_attributes")
162-
);
163-
assertThat(
164-
"Should have the GTFS+ 'empty row' validation issue on the calendar_attributes table",
165-
validation.issues.get(0).description, equalTo("1 row(s) are empty. (File may need to be edited manually.)")
166-
);
167-
}
168141
}
Binary file not shown.

0 commit comments

Comments
 (0)