diff --git a/RULES.md b/RULES.md index 3ef9211289..7a1b317c50 100644 --- a/RULES.md +++ b/RULES.md @@ -144,12 +144,12 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`. ## Table of INFOS -| Notice code | Description | -|-----------------------------------------------------------------------|-----------------------------------------------| -| [`platform_without_parent_station`](#platform_without_parent_station) | A platform has no `parent_station` field set. | -| [`unknown_column`](#unknown_column) | A column name is unknown. | -| [`unknown_file`](#unknown_file) | A file is unknown. | - +| Notice code | Description | +|-----------------------------------------------------------------------|--------------------------------------------------------------------------------------------------| +| [`platform_without_parent_station`](#platform_without_parent_station) | A platform has no `parent_station` field set. | +| [`unknown_column`](#unknown_column) | A column name is unknown. | +| [`unknown_file`](#unknown_file) | A file is unknown. | +| [`unused_parent_station`](#unused_parent_station) | A stop has `location_type` STATION (1) but does not appear in any stop's `parent_station` field. | @@ -2880,6 +2880,26 @@ A file is unknown. + + +### unused_parent_station + +A stop has `location_type` 1 (station) but does not appear in the +`parent_station` field for any stop with `location_type` 0 +(stop/platform) in `stops.txt`. + +#### Notice fields description +| Field name | Description | Type | +|------------------|----------------------------------------|------- | +| `csvRowNumber` | The row number of the faulty record. | Int | +| `stopId` | The id of the faulty stop. | String | +| `stopName` | The name of the faulty stop. | String | + +#### Affected files +* [`stops.txt`](http://gtfs.org/reference/static#stopstxt) + + + # More details - SYSTEM ERRORS diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentLocationTypeValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java similarity index 70% rename from main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentLocationTypeValidator.java rename to main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java index 093aa4c92f..a7c9d796a6 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentLocationTypeValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java @@ -16,8 +16,11 @@ package org.mobilitydata.gtfsvalidator.validator; import static org.mobilitydata.gtfsvalidator.notice.SeverityLevel.ERROR; +import static org.mobilitydata.gtfsvalidator.notice.SeverityLevel.INFO; +import java.util.HashSet; import java.util.Optional; +import java.util.Set; import javax.inject.Inject; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.FileRefs; @@ -31,17 +34,21 @@ import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeSchema; /** - * Validates `location_type` of the referenced `parent_station`. + * Validates parent stations are used and have the correct `location_type`. * - *

Generated notice: {@link WrongParentLocationTypeNotice}. + *

Generated notices: + * + *

*/ @GtfsValidator -public class ParentLocationTypeValidator extends FileValidator { - +public class ParentStationValidator extends FileValidator { private final GtfsStopTableContainer stopTable; @Inject - ParentLocationTypeValidator(GtfsStopTableContainer stopTable) { + ParentStationValidator(GtfsStopTableContainer stopTable) { this.stopTable = stopTable; } @@ -60,7 +67,15 @@ private GtfsLocationType expectedParentLocationType(GtfsLocationType locationTyp @Override public void validate(NoticeContainer noticeContainer) { + Set stations = new HashSet<>(); + Set stationsWithStops = new HashSet(); + for (GtfsStop location : stopTable.getEntities()) { + if (location.locationType() == GtfsLocationType.STATION) { + stations.add(location.stopId()); + // Stations must not have parent_station; this is validated elsewhere. + continue; + } if (!location.hasParentStation()) { continue; } @@ -69,6 +84,9 @@ public void validate(NoticeContainer noticeContainer) { // Broken reference is reported in another rule. continue; } + if (location.locationType() == GtfsLocationType.STOP) { + stationsWithStops.add(location.parentStation()); + } GtfsStop parentLocation = optionalParentLocation.get(); GtfsLocationType expected = expectedParentLocationType(location.locationType()); if (expected != GtfsLocationType.UNRECOGNIZED && parentLocation.locationType() != expected) { @@ -85,6 +103,20 @@ public void validate(NoticeContainer noticeContainer) { expected.getNumber())); } } + + stations.removeAll(stationsWithStops); + for (String station : stations) { + Optional optionalStationStop = stopTable.byStopId(station); + if (optionalStationStop.isEmpty()) { + // This should never happen because we just populated the set stations + // with actual stations. + continue; + } + GtfsStop stationStop = optionalStationStop.get(); + noticeContainer.addValidationNotice( + new UnusedParentStationNotice( + stationStop.csvRowNumber(), stationStop.stopId(), stationStop.stopName())); + } } /** @@ -156,4 +188,27 @@ static class WrongParentLocationTypeNotice extends ValidationNotice { this.expectedLocationType = expectedLocationType; } } + + /** + * Unused parent station. + * + *

A stop has `location_type` STATION (1) but does not appear in any stop's `parent_station`. + */ + @GtfsValidationNotice(severity = INFO, files = @FileRefs({GtfsStopSchema.class})) + static class UnusedParentStationNotice extends ValidationNotice { + /** The row number of the faulty record. */ + private final int csvRowNumber; + + /** The id of the faulty stop. */ + private final String stopId; + + /** The name of the faulty stop. */ + private final String stopName; + + UnusedParentStationNotice(int csvRowNumber, String stopId, String stopName) { + this.csvRowNumber = csvRowNumber; + this.stopId = stopId; + this.stopName = stopName; + } + } } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentLocationTypeValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidatorTest.java similarity index 67% rename from main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentLocationTypeValidatorTest.java rename to main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidatorTest.java index 12ee8b4c5f..9543d859d8 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentLocationTypeValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidatorTest.java @@ -28,15 +28,16 @@ import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; import org.mobilitydata.gtfsvalidator.table.GtfsStop; import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; -import org.mobilitydata.gtfsvalidator.validator.ParentLocationTypeValidator.WrongParentLocationTypeNotice; +import org.mobilitydata.gtfsvalidator.validator.ParentStationValidator.UnusedParentStationNotice; +import org.mobilitydata.gtfsvalidator.validator.ParentStationValidator.WrongParentLocationTypeNotice; @RunWith(JUnit4.class) -public class ParentLocationTypeValidatorTest { +public class ParentStationValidatorTest { private List validateChildAndParent( GtfsLocationType childType, GtfsLocationType parentType) { NoticeContainer noticeContainer = new NoticeContainer(); - new ParentLocationTypeValidator( + new ParentStationValidator( GtfsStopTableContainer.forEntities( ImmutableList.of( new GtfsStop.Builder() @@ -59,7 +60,7 @@ private List validateChildAndParent( private List validateNoParent(GtfsLocationType locationType) { NoticeContainer noticeContainer = new NoticeContainer(); - new ParentLocationTypeValidator( + new ParentStationValidator( GtfsStopTableContainer.forEntities( ImmutableList.of( new GtfsStop.Builder() @@ -76,7 +77,7 @@ private List validateNoParent(GtfsLocationType locationType) { public void stopParent() { assertThat(validateChildAndParent(GtfsLocationType.STOP, GtfsLocationType.STATION)).isEmpty(); assertThat(validateChildAndParent(GtfsLocationType.STOP, GtfsLocationType.ENTRANCE)) - .contains( + .containsExactly( new WrongParentLocationTypeNotice( 1, "child", @@ -92,9 +93,10 @@ public void stopParent() { @Test public void entranceParent() { assertThat(validateChildAndParent(GtfsLocationType.ENTRANCE, GtfsLocationType.STATION)) - .isEmpty(); + // No issue with the types, but the station has no stop (just an entrance). + .containsExactly(new UnusedParentStationNotice(2, "parent", "Parent location")); assertThat(validateChildAndParent(GtfsLocationType.ENTRANCE, GtfsLocationType.STOP)) - .contains( + .containsExactly( new WrongParentLocationTypeNotice( 1, "child", @@ -110,9 +112,10 @@ public void entranceParent() { @Test public void genericNodeParent() { assertThat(validateChildAndParent(GtfsLocationType.GENERIC_NODE, GtfsLocationType.STATION)) - .isEmpty(); + // No issue with the types, but the station has no stop. + .containsExactly(new UnusedParentStationNotice(2, "parent", "Parent location")); assertThat(validateChildAndParent(GtfsLocationType.GENERIC_NODE, GtfsLocationType.STOP)) - .contains( + .containsExactly( new WrongParentLocationTypeNotice( 1, "child", @@ -130,33 +133,65 @@ public void boardingAreaParent() { assertThat(validateChildAndParent(GtfsLocationType.BOARDING_AREA, GtfsLocationType.STOP)) .isEmpty(); assertThat(validateChildAndParent(GtfsLocationType.BOARDING_AREA, GtfsLocationType.STATION)) - .contains( - new WrongParentLocationTypeNotice( - 1, - "child", - "Child location", - GtfsLocationType.BOARDING_AREA.getNumber(), - 2, - "parent", - "Parent location", - GtfsLocationType.STATION.getNumber(), - GtfsLocationType.STOP.getNumber())); + .containsExactlyElementsIn( + ImmutableList.of( + new WrongParentLocationTypeNotice( + 1, + "child", + "Child location", + GtfsLocationType.BOARDING_AREA.getNumber(), + 2, + "parent", + "Parent location", + GtfsLocationType.STATION.getNumber(), + GtfsLocationType.STOP.getNumber()), + new UnusedParentStationNotice(2, "parent", "Parent location"))); } @Test public void noParent() { - // ParentLocationTypeValidator ignores rows without parent_station. + // ParentStationValidator ignores non-stations without parent_station. assertThat(validateNoParent(GtfsLocationType.STOP)).isEmpty(); - assertThat(validateNoParent(GtfsLocationType.STATION)).isEmpty(); assertThat(validateNoParent(GtfsLocationType.ENTRANCE)).isEmpty(); assertThat(validateNoParent(GtfsLocationType.GENERIC_NODE)).isEmpty(); assertThat(validateNoParent(GtfsLocationType.BOARDING_AREA)).isEmpty(); } + @Test + public void unusedStation() { + NoticeContainer noticeContainer = new NoticeContainer(); + new ParentStationValidator( + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new GtfsStop.Builder() + .setCsvRowNumber(1) + .setStopId("child") + .setStopName("Child location") + .setLocationType(GtfsLocationType.STOP) + .setParentStation("used_station") + .build(), + new GtfsStop.Builder() + .setCsvRowNumber(2) + .setStopId("unused_station") + .setStopName("Unused station") + .setLocationType(GtfsLocationType.STATION) + .build(), + new GtfsStop.Builder() + .setCsvRowNumber(3) + .setStopId("used_station") + .setStopName("Used station") + .setLocationType(GtfsLocationType.STATION) + .build()), + noticeContainer)) + .validate(noticeContainer); + assertThat(noticeContainer.getValidationNotices()) + .containsExactly(new UnusedParentStationNotice(2, "unused_station", "Unused station")); + } + @Test public void foreignKeyViolation_handledGracefully() { NoticeContainer noticeContainer = new NoticeContainer(); - new ParentLocationTypeValidator( + new ParentStationValidator( GtfsStopTableContainer.forEntities( ImmutableList.of( new GtfsStop.Builder()