Skip to content

Commit

Permalink
feat: Add a rule that stations (location_type 1) must be the parent_s…
Browse files Browse the repository at this point in the history 
…tation of some stop (location_type 0). (#1493)


---------

Co-authored-by: David Gamez <[email protected]>
  • Loading branch information
@bradyhunsaker @davidgamez
bradyhunsaker and davidgamez authored Jun 21, 2023
1 parent 95cd693 commit eada0f4
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 34 deletions.
32 changes: 26 additions & 6 deletions RULES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. |

<a name="SYSTEM_ERRORS"/>

Expand Down Expand Up @@ -2880,6 +2880,26 @@ A file is unknown.

</details>

<a name="UnusedParentStationNotice"/>

### 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)

</details>

# More details - SYSTEM ERRORS

<a name="IOError"/>
Expand Down
65 changes: 60 additions & 5 deletions ...alidator/ParentLocationTypeValidator.java → ...tor/validator/ParentStationValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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`.
*
* <p>Generated notice: {@link WrongParentLocationTypeNotice}.
* <p>Generated notices:
*
* <ul>
* <li>{@link WrongParentLocationTypeNotice}.
* <li>{@link UnusedParentStationNotice}.
* </ul>
*/
@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;
}

Expand All @@ -60,7 +67,15 @@ private GtfsLocationType expectedParentLocationType(GtfsLocationType locationTyp

@Override
public void validate(NoticeContainer noticeContainer) {
Set<String> stations = new HashSet<>();
Set<String> stationsWithStops = new HashSet<String>();

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;
}
Expand All @@ -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) {
Expand All @@ -85,6 +103,20 @@ public void validate(NoticeContainer noticeContainer) {
expected.getNumber()));
}
}

stations.removeAll(stationsWithStops);
for (String station : stations) {
Optional<GtfsStop> 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()));
}
}

/**
Expand Down Expand Up @@ -156,4 +188,27 @@ static class WrongParentLocationTypeNotice extends ValidationNotice {
this.expectedLocationType = expectedLocationType;
}
}

/**
* Unused parent station.
*
* <p>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;
}
}
}
81 changes: 58 additions & 23 deletions ...ator/ParentLocationTypeValidatorTest.java → ...validator/ParentStationValidatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ValidationNotice> validateChildAndParent(
GtfsLocationType childType, GtfsLocationType parentType) {
NoticeContainer noticeContainer = new NoticeContainer();
new ParentLocationTypeValidator(
new ParentStationValidator(
GtfsStopTableContainer.forEntities(
ImmutableList.of(
new GtfsStop.Builder()
Expand All @@ -59,7 +60,7 @@ private List<ValidationNotice> validateChildAndParent(

private List<ValidationNotice> validateNoParent(GtfsLocationType locationType) {
NoticeContainer noticeContainer = new NoticeContainer();
new ParentLocationTypeValidator(
new ParentStationValidator(
GtfsStopTableContainer.forEntities(
ImmutableList.of(
new GtfsStop.Builder()
Expand All @@ -76,7 +77,7 @@ private List<ValidationNotice> 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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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()
Expand Down

0 comments on commit eada0f4

Please sign in to comment.