From ef7bf4f3b4563a712a7247f06524d66ac1de5a25 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Mon, 10 Oct 2022 17:53:33 -0700 Subject: [PATCH 1/6] Initial transfers.txt trip=>route reference validation. --- .../validator/TransfersRelationValidator.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java new file mode 100644 index 0000000000..57d86f1181 --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java @@ -0,0 +1,86 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import java.util.Optional; +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; +import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; +import org.mobilitydata.gtfsvalidator.table.GtfsTrip; +import org.mobilitydata.gtfsvalidator.table.GtfsTripTableContainer; + +@GtfsValidator +public class TransfersRelationValidator extends SingleEntityValidator { + + private final GtfsTripTableContainer tripsContainer; + + @Inject + public TransfersRelationValidator(GtfsTripTableContainer tripsContainer) { + this.tripsContainer = tripsContainer; + } + + @Override + public void validate(GtfsTransfer entity, NoticeContainer noticeContainer) { + if (entity.hasFromTripId()) { + Optional optTrip = tripsContainer.byTripId(entity.fromTripId()); + if (optTrip.isPresent()) { + GtfsTrip trip = optTrip.get(); + if (entity.hasFromRouteId()) { + if (!trip.routeId().equals(entity.fromRouteId())) { + noticeContainer.addValidationNotice( + new TransferTripReferenceNotice( + entity.csvRowNumber(), + GtfsTransferTableLoader.FROM_TRIP_ID_FIELD_NAME, + entity.fromTripId(), + GtfsTransferTableLoader.FROM_ROUTE_ID_FIELD_NAME, + entity.fromRouteId())); + } + } + } + } + + if (entity.hasToTripId()) { + Optional optTrip = tripsContainer.byTripId(entity.toTripId()); + if (optTrip.isPresent()) { + GtfsTrip trip = optTrip.get(); + if (entity.hasToRouteId()) { + if (!trip.routeId().equals(entity.toRouteId())) { + noticeContainer.addValidationNotice( + new TransferTripReferenceNotice( + entity.csvRowNumber(), + GtfsTransferTableLoader.TO_TRIP_ID_FIELD_NAME, + entity.toTripId(), + GtfsTransferTableLoader.TO_ROUTE_ID_FIELD_NAME, + entity.toRouteId())); + } + } + } + } + } + + public static class TransferTripReferenceNotice extends ValidationNotice { + + private final long csvRowNumber; + + private final String tripFieldName; + private final String tripFieldValue; + private final String referenceFieldName; + private final String referenceFieldValue; + + public TransferTripReferenceNotice( + long csvRowNumber, + String tripFieldName, + String tripFieldValue, + String referenceFieldName, + String referenceFieldValue) { + super(SeverityLevel.ERROR); + this.csvRowNumber = csvRowNumber; + this.tripFieldName = tripFieldName; + this.tripFieldValue = tripFieldValue; + this.referenceFieldName = referenceFieldName; + this.referenceFieldValue = referenceFieldValue; + } + } +} From a0cc7187742626e68bb8857cd3c6071aa1faac8b Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Mon, 10 Oct 2022 19:30:14 -0700 Subject: [PATCH 2/6] Use a FileValidator to support injection. --- .../validator/TransfersRelationValidator.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java index 57d86f1181..f2aa1911fb 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java @@ -7,22 +7,32 @@ import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; import org.mobilitydata.gtfsvalidator.table.GtfsTrip; import org.mobilitydata.gtfsvalidator.table.GtfsTripTableContainer; @GtfsValidator -public class TransfersRelationValidator extends SingleEntityValidator { +public class TransfersRelationValidator extends FileValidator { + private final GtfsTransferTableContainer transfersContainer; private final GtfsTripTableContainer tripsContainer; @Inject - public TransfersRelationValidator(GtfsTripTableContainer tripsContainer) { + public TransfersRelationValidator( + GtfsTransferTableContainer transfersContainer, GtfsTripTableContainer tripsContainer) { + this.transfersContainer = transfersContainer; this.tripsContainer = tripsContainer; } @Override - public void validate(GtfsTransfer entity, NoticeContainer noticeContainer) { + public void validate(NoticeContainer noticeContainer) { + for (GtfsTransfer transfer : transfersContainer.getEntities()) { + validateEntity(transfer, noticeContainer); + } + } + + public void validateEntity(GtfsTransfer entity, NoticeContainer noticeContainer) { if (entity.hasFromTripId()) { Optional optTrip = tripsContainer.byTripId(entity.fromTripId()); if (optTrip.isPresent()) { From 3a3041d35eedde273b9926f38513d1ae2d470bc2 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Tue, 11 Oct 2022 10:00:39 -0700 Subject: [PATCH 3/6] Add trip=>stop reference checks. --- .../validator/TransfersRelationValidator.java | 96 -------------- .../TransfersTripReferenceValidator.java | 124 ++++++++++++++++++ 2 files changed, 124 insertions(+), 96 deletions(-) delete mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java deleted file mode 100644 index f2aa1911fb..0000000000 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersRelationValidator.java +++ /dev/null @@ -1,96 +0,0 @@ -package org.mobilitydata.gtfsvalidator.validator; - -import java.util.Optional; -import javax.inject.Inject; -import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; -import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; -import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; -import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; -import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; -import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; -import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; -import org.mobilitydata.gtfsvalidator.table.GtfsTrip; -import org.mobilitydata.gtfsvalidator.table.GtfsTripTableContainer; - -@GtfsValidator -public class TransfersRelationValidator extends FileValidator { - - private final GtfsTransferTableContainer transfersContainer; - private final GtfsTripTableContainer tripsContainer; - - @Inject - public TransfersRelationValidator( - GtfsTransferTableContainer transfersContainer, GtfsTripTableContainer tripsContainer) { - this.transfersContainer = transfersContainer; - this.tripsContainer = tripsContainer; - } - - @Override - public void validate(NoticeContainer noticeContainer) { - for (GtfsTransfer transfer : transfersContainer.getEntities()) { - validateEntity(transfer, noticeContainer); - } - } - - public void validateEntity(GtfsTransfer entity, NoticeContainer noticeContainer) { - if (entity.hasFromTripId()) { - Optional optTrip = tripsContainer.byTripId(entity.fromTripId()); - if (optTrip.isPresent()) { - GtfsTrip trip = optTrip.get(); - if (entity.hasFromRouteId()) { - if (!trip.routeId().equals(entity.fromRouteId())) { - noticeContainer.addValidationNotice( - new TransferTripReferenceNotice( - entity.csvRowNumber(), - GtfsTransferTableLoader.FROM_TRIP_ID_FIELD_NAME, - entity.fromTripId(), - GtfsTransferTableLoader.FROM_ROUTE_ID_FIELD_NAME, - entity.fromRouteId())); - } - } - } - } - - if (entity.hasToTripId()) { - Optional optTrip = tripsContainer.byTripId(entity.toTripId()); - if (optTrip.isPresent()) { - GtfsTrip trip = optTrip.get(); - if (entity.hasToRouteId()) { - if (!trip.routeId().equals(entity.toRouteId())) { - noticeContainer.addValidationNotice( - new TransferTripReferenceNotice( - entity.csvRowNumber(), - GtfsTransferTableLoader.TO_TRIP_ID_FIELD_NAME, - entity.toTripId(), - GtfsTransferTableLoader.TO_ROUTE_ID_FIELD_NAME, - entity.toRouteId())); - } - } - } - } - } - - public static class TransferTripReferenceNotice extends ValidationNotice { - - private final long csvRowNumber; - - private final String tripFieldName; - private final String tripFieldValue; - private final String referenceFieldName; - private final String referenceFieldValue; - - public TransferTripReferenceNotice( - long csvRowNumber, - String tripFieldName, - String tripFieldValue, - String referenceFieldName, - String referenceFieldValue) { - super(SeverityLevel.ERROR); - this.csvRowNumber = csvRowNumber; - this.tripFieldName = tripFieldName; - this.tripFieldValue = tripFieldValue; - this.referenceFieldName = referenceFieldName; - this.referenceFieldValue = referenceFieldValue; - } - } -} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java new file mode 100644 index 0000000000..ff8a1d2fa0 --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java @@ -0,0 +1,124 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import java.util.List; +import java.util.Optional; +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; +import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTime; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; +import org.mobilitydata.gtfsvalidator.table.GtfsTrip; +import org.mobilitydata.gtfsvalidator.table.GtfsTripTableContainer; + +@GtfsValidator +public class TransfersTripReferenceValidator extends FileValidator { + + private final GtfsTransferTableContainer transfersContainer; + private final GtfsTripTableContainer tripsContainer; + private final GtfsStopTimeTableContainer stopTimeContainer; + + @Inject + public TransfersTripReferenceValidator( + GtfsTransferTableContainer transfersContainer, + GtfsTripTableContainer tripsContainer, + GtfsStopTimeTableContainer stopTimeContainer) { + this.transfersContainer = transfersContainer; + this.tripsContainer = tripsContainer; + this.stopTimeContainer = stopTimeContainer; + } + + @Override + public void validate(NoticeContainer noticeContainer) { + for (GtfsTransfer transfer : transfersContainer.getEntities()) { + validateEntity(transfer, noticeContainer); + } + } + + public void validateEntity(GtfsTransfer entity, NoticeContainer noticeContainer) { + validateTripReferences( + entity, + GtfsTransferTableLoader.FROM_TRIP_ID_FIELD_NAME, + optional(entity.hasFromTripId(), entity.fromTripId()), + GtfsTransferTableLoader.FROM_ROUTE_ID_FIELD_NAME, + optional(entity.hasFromRouteId(), entity.fromRouteId()), + GtfsTransferTableLoader.FROM_STOP_ID_FIELD_NAME, + optional(entity.hasFromStopId(), entity.fromStopId()), + noticeContainer); + validateTripReferences( + entity, + GtfsTransferTableLoader.TO_TRIP_ID_FIELD_NAME, + optional(entity.hasToTripId(), entity.toTripId()), + GtfsTransferTableLoader.TO_ROUTE_ID_FIELD_NAME, + optional(entity.hasToRouteId(), entity.toRouteId()), + GtfsTransferTableLoader.TO_STOP_ID_FIELD_NAME, + optional(entity.hasToStopId(), entity.toStopId()), + noticeContainer); + } + + void validateTripReferences( + GtfsTransfer entity, + String tripFieldName, + Optional tripId, + String routeFieldName, + Optional routeId, + String stopFieldName, + Optional stopId, + NoticeContainer noticeContainer) { + if (tripId.isEmpty()) { + return; + } + Optional optTrip = tripsContainer.byTripId(tripId.get()); + if (optTrip.isEmpty()) { + return; + } + GtfsTrip trip = optTrip.get(); + if (routeId.isPresent()) { + if (!trip.routeId().equals(routeId.get())) { + noticeContainer.addValidationNotice( + new TransferTripReferenceNotice( + entity.csvRowNumber(), tripFieldName, tripId.get(), routeFieldName, routeId.get())); + } + } + if (stopId.isPresent()) { + List stopTimes = stopTimeContainer.byTripId(tripId.get()); + if (!stopTimes.stream().anyMatch((st) -> st.stopId().equals(stopId.get()))) { + noticeContainer.addValidationNotice( + new TransferTripReferenceNotice( + entity.csvRowNumber(), tripFieldName, tripId.get(), stopFieldName, stopId.get())); + } + } + } + + private static Optional optional(boolean hasValue, T value) { + return hasValue ? Optional.of(value) : Optional.empty(); + } + + public static class TransferTripReferenceNotice extends ValidationNotice { + + private final long csvRowNumber; + + private final String tripFieldName; + private final String tripFieldValue; + private final String referenceFieldName; + private final String referenceFieldValue; + + public TransferTripReferenceNotice( + long csvRowNumber, + String tripFieldName, + String tripFieldValue, + String referenceFieldName, + String referenceFieldValue) { + super(SeverityLevel.ERROR); + this.csvRowNumber = csvRowNumber; + this.tripFieldName = tripFieldName; + this.tripFieldValue = tripFieldValue; + this.referenceFieldName = referenceFieldName; + this.referenceFieldValue = referenceFieldValue; + } + } +} From ddc4937adc6a8293fa60734ed4510d24129d2872 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Tue, 11 Oct 2022 15:12:43 -0700 Subject: [PATCH 4/6] More transfers.txt validation. --- .../validator/TransfersStopTypeValidator.java | 84 +++++++++++++++++++ .../TransfersTripReferenceValidator.java | 56 +++++++++++-- 2 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java new file mode 100644 index 0000000000..bb53d35506 --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java @@ -0,0 +1,84 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import java.util.Optional; +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; +import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; +import org.mobilitydata.gtfsvalidator.table.GtfsStop; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; + +@GtfsValidator +public class TransfersStopTypeValidator extends FileValidator { + private final GtfsTransferTableContainer transfersContainer; + private final GtfsStopTableContainer stopsContainer; + + @Inject + public TransfersStopTypeValidator( + GtfsTransferTableContainer transfersContainer, GtfsStopTableContainer stopsContainer) { + this.transfersContainer = transfersContainer; + this.stopsContainer = stopsContainer; + } + + @Override + public void validate(NoticeContainer noticeContainer) { + for (GtfsTransfer entity : transfersContainer.getEntities()) { + validateEntity(entity, noticeContainer); + } + } + + public void validateEntity(GtfsTransfer entity, NoticeContainer noticeContainer) { + validateStopType( + entity, + GtfsTransferTableLoader.FROM_STOP_ID_FIELD_NAME, + entity.fromStopId(), + noticeContainer); + validateStopType( + entity, GtfsTransferTableLoader.TO_STOP_ID_FIELD_NAME, entity.toStopId(), noticeContainer); + } + + private void validateStopType( + GtfsTransfer entity, String stopIdFieldName, String stopId, NoticeContainer noticeContainer) { + Optional optStop = stopsContainer.byStopId(stopId); + if (optStop.isEmpty()) { + // Foreign key reference is validated elsewhere. + return; + } + + GtfsLocationType locationType = optStop.get().locationType(); + if (!isValidTransferStopType(locationType)) { + noticeContainer.addValidationNotice( + new TransfersStopLocationTypeNotice( + entity.csvRowNumber(), stopIdFieldName, locationType)); + } + } + + private static boolean isValidTransferStopType(GtfsLocationType locationType) { + switch (locationType) { + case STOP: + case STATION: + return true; + default: + return false; + } + } + + public static final class TransfersStopLocationTypeNotice extends ValidationNotice { + private final long csvRowNumber; + private final String stopIdFieldName; + private final int locationType; + + public TransfersStopLocationTypeNotice( + long csvRowNumber, String stopIdFieldName, GtfsLocationType locationType) { + super(SeverityLevel.ERROR); + this.csvRowNumber = csvRowNumber; + this.stopIdFieldName = stopIdFieldName; + this.locationType = locationType.getNumber(); + } + } +} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java index ff8a1d2fa0..45ef1a5037 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java @@ -1,5 +1,6 @@ package org.mobilitydata.gtfsvalidator.validator; +import com.google.common.collect.ImmutableSet; import java.util.List; import java.util.Optional; import javax.inject.Inject; @@ -7,6 +8,9 @@ import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.notice.SeverityLevel; import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; +import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; +import org.mobilitydata.gtfsvalidator.table.GtfsStop; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; import org.mobilitydata.gtfsvalidator.table.GtfsStopTime; import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableContainer; import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; @@ -21,15 +25,18 @@ public class TransfersTripReferenceValidator extends FileValidator { private final GtfsTransferTableContainer transfersContainer; private final GtfsTripTableContainer tripsContainer; private final GtfsStopTimeTableContainer stopTimeContainer; + private final GtfsStopTableContainer stopsContainer; @Inject public TransfersTripReferenceValidator( GtfsTransferTableContainer transfersContainer, GtfsTripTableContainer tripsContainer, - GtfsStopTimeTableContainer stopTimeContainer) { + GtfsStopTimeTableContainer stopTimeContainer, + GtfsStopTableContainer stopsContainer) { this.transfersContainer = transfersContainer; this.tripsContainer = tripsContainer; this.stopTimeContainer = stopTimeContainer; + this.stopsContainer = stopsContainer; } @Override @@ -74,6 +81,7 @@ void validateTripReferences( } Optional optTrip = tripsContainer.byTripId(tripId.get()); if (optTrip.isEmpty()) { + // The foreign key reference is validated elsewhere. return; } GtfsTrip trip = optTrip.get(); @@ -85,12 +93,32 @@ void validateTripReferences( } } if (stopId.isPresent()) { - List stopTimes = stopTimeContainer.byTripId(tripId.get()); - if (!stopTimes.stream().anyMatch((st) -> st.stopId().equals(stopId.get()))) { - noticeContainer.addValidationNotice( - new TransferTripReferenceNotice( - entity.csvRowNumber(), tripFieldName, tripId.get(), stopFieldName, stopId.get())); - } + validateTripStopReference( + entity, tripFieldName, tripId, stopFieldName, stopId.get(), noticeContainer); + } + } + + private void validateTripStopReference( + GtfsTransfer entity, + String tripFieldName, + Optional tripId, + String stopFieldName, + String stopId, + NoticeContainer noticeContainer) { + Optional optStop = stopsContainer.byStopId(stopId); + if (optStop.isEmpty()) { + // The foreign key reference is validated elsewhere. + return; + } + ImmutableSet stops = expandStationIfNeeded(optStop.get()); + ImmutableSet ids = + stops.stream().map(GtfsStop::stopId).collect(ImmutableSet.toImmutableSet()); + + List stopTimes = stopTimeContainer.byTripId(tripId.get()); + if (!stopTimes.stream().anyMatch((st) -> ids.contains(st.stopId()))) { + noticeContainer.addValidationNotice( + new TransferTripReferenceNotice( + entity.csvRowNumber(), tripFieldName, tripId.get(), stopFieldName, stopId)); } } @@ -98,10 +126,20 @@ private static Optional optional(boolean hasValue, T value) { return hasValue ? Optional.of(value) : Optional.empty(); } - public static class TransferTripReferenceNotice extends ValidationNotice { + private ImmutableSet expandStationIfNeeded(GtfsStop stop) { + if (stop.locationType() == GtfsLocationType.STOP) { + return ImmutableSet.of(stop); + } else if (stop.locationType() == GtfsLocationType.STATION) { + List stops = stopsContainer.byParentStation(stop.stopId()); + return ImmutableSet.copyOf(stops); + } else { + // Invalid stop location types are validated elsewhere. + return ImmutableSet.of(); + } + } + public static class TransferTripReferenceNotice extends ValidationNotice { private final long csvRowNumber; - private final String tripFieldName; private final String tripFieldValue; private final String referenceFieldName; From 46e6e81ccd722b1426c148c6d3966e595cabcff6 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Thu, 13 Oct 2022 16:15:15 -0700 Subject: [PATCH 5/6] Adding comments, unit-tests, and documentation for new validators. --- RULES.md | 64 +++++++++ .../validator/TransfersStopTypeValidator.java | 32 ++++- .../TransfersTripReferenceValidator.java | 85 ++++++++++-- .../TransfersStopTypeValidatorTest.java | 76 +++++++++++ .../TransfersTripReferenceValidatorTest.java | 122 ++++++++++++++++++ 5 files changed, 358 insertions(+), 21 deletions(-) create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidatorTest.java diff --git a/RULES.md b/RULES.md index 266dea9d02..378ee7447e 100644 --- a/RULES.md +++ b/RULES.md @@ -85,6 +85,9 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`. | [`stop_time_with_arrival_before_previous_departure_time`](#stop_time_with_arrival_before_previous_departure_time) | Backwards time travel between stops in `stop_times.txt` | | [`stop_time_with_only_arrival_or_departure_time`](#stop_time_with_only_arrival_or_departure_time) | Missing `stop_times.arrival_time` or `stop_times.departure_time`. | | [`stop_without_zone_id`](#stop_without_zone_id) | Stop without value for `stops.zone_id`. | +| [`transfer_with_invalid_stop_location_type`](#transfer_with_invalid_stop_location_type) | A stop id field from GTFS file `transfers.txt` references a stop that has a `location_type` other than 0 or 1 (aka Stop/Platform or Station). | +| [`transfer_with_invalid_trip_and_route`](#transfer_with_invalid_trip_and_route) | A trip id field from GTFS file `transfers.txt` references a route that does not match its `trips.txt` `route_id`. | +| [`transfer_with_invalid_trip_and_stop`](#transfer_with_invalid_trip_and_stop) | A trip id field from GTFS file `transfers.txt` references a stop that is not included in the referenced trip's stop-times. | | [`translation_foreign_key_violation`](#translation_foreign_key_violation) | An entity with the given `record_id` and `record_sub_id` cannot be found in the referenced table. | | [`translation_unexpected_value`](#translation_unexpected_value) | A field in a translations row has value but must be empty. | | [`wrong_parent_location_type`](#wrong_parent_location_type) | Incorrect type of the parent location. | @@ -1455,6 +1458,67 @@ If `fare_rules.txt` is provided, and `fare_rules.txt` uses at least one column a + + +### transfer_with_invalid_stop_location_type + +A `from_stop_id` or `to_stop_id` field from GTFS file `transfers.txt` references a stop that has a `location_type` other than 0 or 1 (aka Stop/Platform or Station). + +#### References +* [transfers.txt specification](http://gtfs.org/reference/static/#transferstxt) + +
+#### Notice fields description +| Field name | Description | Type | +|---------------------|---------------------------------------------------------------------------|--------| +| `csvRowNumber` | The row number from `transfers.txt` for the faulty entry. | long | +| `stopIdFieldName` | The name of the stop id field (e.g. `from_stop_id`) referencing the stop. | String | +| `stopId` | The referenced stop id. | String | +| `locationTypeValue` | The numeric value of the invalid location type. | int | +| `locationTypeName` | The name of the invalid location type. | String | +
+ + + +### transfer_with_invalid_trip_and_route + +A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references a route that does not match its `trips.txt` `route_id`. + +#### References +* [transfers.txt specification](http://gtfs.org/reference/static/#transferstxt) + +
+#### Notice fields description +| Field name | Description | Type | +|-------------------|------------------------------------------------------------------------------|--------| +| `csvRowNumber` | The row number from `transfers.txt` for the faulty entry. | long | +| `tripFieldName` | The name of the trip id field (e.g. `from_trip_id`) referencing a trip. | String | +| `tripId` | The referenced trip id. | String | +| `routeFieldName` | The name of the route id field (e.g. `from_route_id`) referencing the route. | String | +| `routeId` | The referenced route id. | String | +| `expectedRouteId` | The expected route id from `trips.txt`. | String | +
+ + + +### transfer_with_invalid_trip_and_stop + +A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references a stop that is not included in the referenced trip's stop-times. + +#### References +* [transfers.txt specification](http://gtfs.org/reference/static/#transferstxt) + +
+#### Notice fields description +| Field name | Description | Type | +|-----------------|----------------------------------------------------------------------------|--------| +| `csvRowNumber` | The row number from `transfers.txt` for the faulty entry. | long | +| `tripFieldName` | The name of the trip id field (e.g. `from_trip_id`) referencing a trip. | String | +| `tripId` | The referenced trip id. | String | +| `stopFieldName` | The name of the stop id field (e.g. `stop_route_id`) referencing the stop. | String | +| `stopId` | The referenced stop id. | String | +
+ ### translation_foreign_key_violation diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java index bb53d35506..c4a38c7399 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java @@ -13,6 +13,9 @@ import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; +/** + * Validates that {@code transfers.from_stop_id} and {@code to_stop_id} reference stops or stations. + */ @GtfsValidator public class TransfersStopTypeValidator extends FileValidator { private final GtfsTransferTableContainer transfersContainer; @@ -53,8 +56,8 @@ private void validateStopType( GtfsLocationType locationType = optStop.get().locationType(); if (!isValidTransferStopType(locationType)) { noticeContainer.addValidationNotice( - new TransfersStopLocationTypeNotice( - entity.csvRowNumber(), stopIdFieldName, locationType)); + new TransferWithInvalidStopLocationTypeNotice( + entity.csvRowNumber(), stopIdFieldName, stopId, locationType)); } } @@ -68,17 +71,32 @@ private static boolean isValidTransferStopType(GtfsLocationType locationType) { } } - public static final class TransfersStopLocationTypeNotice extends ValidationNotice { + /** + * A `from_stop_id` or `to_stop_id` field from GTFS file `transfers.txt` references a stop that + * has a `location_type` other than 0 or 1 (aka Stop/Platform or Station). + * + *

Severity: {@code SeverityLevel.ERROR} + */ + public static final class TransferWithInvalidStopLocationTypeNotice extends ValidationNotice { + // The row number from `transfers.txt` for the faulty entry. private final long csvRowNumber; + // The name of the stop id field (e.g. `from_stop_id`) referencing the stop. private final String stopIdFieldName; - private final int locationType; + // The referenced stop id. + private final String stopId; + // The numeric value of the invalid location type. + private final int locationTypeValue; + // The name of the invalid location type. + private String locationTypeName; - public TransfersStopLocationTypeNotice( - long csvRowNumber, String stopIdFieldName, GtfsLocationType locationType) { + public TransferWithInvalidStopLocationTypeNotice( + long csvRowNumber, String stopIdFieldName, String stopId, GtfsLocationType locationType) { super(SeverityLevel.ERROR); this.csvRowNumber = csvRowNumber; this.stopIdFieldName = stopIdFieldName; - this.locationType = locationType.getNumber(); + this.stopId = stopId; + this.locationTypeValue = locationType.getNumber(); + this.locationTypeName = locationType.toString(); } } } diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java index 45ef1a5037..a2ff52df55 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidator.java @@ -19,6 +19,10 @@ import org.mobilitydata.gtfsvalidator.table.GtfsTrip; import org.mobilitydata.gtfsvalidator.table.GtfsTripTableContainer; +/** + * Validates that if a transfers.txt entry references a trip, then any corresponding route reference + * or stop reference for the transfer are actually associated with the trip. + */ @GtfsValidator public class TransfersTripReferenceValidator extends FileValidator { @@ -88,8 +92,13 @@ void validateTripReferences( if (routeId.isPresent()) { if (!trip.routeId().equals(routeId.get())) { noticeContainer.addValidationNotice( - new TransferTripReferenceNotice( - entity.csvRowNumber(), tripFieldName, tripId.get(), routeFieldName, routeId.get())); + new TransferWithInvalidTripAndRouteNotice( + entity.csvRowNumber(), + tripFieldName, + tripId.get(), + routeFieldName, + routeId.get(), + trip.routeId())); } } if (stopId.isPresent()) { @@ -117,7 +126,7 @@ private void validateTripStopReference( List stopTimes = stopTimeContainer.byTripId(tripId.get()); if (!stopTimes.stream().anyMatch((st) -> ids.contains(st.stopId()))) { noticeContainer.addValidationNotice( - new TransferTripReferenceNotice( + new TransferWithInvalidTripAndStopNotice( entity.csvRowNumber(), tripFieldName, tripId.get(), stopFieldName, stopId)); } } @@ -138,25 +147,73 @@ private ImmutableSet expandStationIfNeeded(GtfsStop stop) { } } - public static class TransferTripReferenceNotice extends ValidationNotice { + /** + * A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references a route that + * does not match its `trips.txt` `route_id`. + * + *

Severity: {@code SeverityLevel.ERROR} + */ + public static class TransferWithInvalidTripAndRouteNotice extends ValidationNotice { + // The row number from `transfers.txt` for the faulty entry. private final long csvRowNumber; + // The name of the trip id field (e.g. `from_trip_id`) referencing a trip. private final String tripFieldName; - private final String tripFieldValue; - private final String referenceFieldName; - private final String referenceFieldValue; + // The referenced trip id. + private final String tripId; + // The name of the route id field (e.g. `from_route_id`) referencing the route. + private final String routeFieldName; + // The referenced route id. + private final String routeId; + // The expected route id from `trips.txt`. + private final String expectedRouteId; - public TransferTripReferenceNotice( + public TransferWithInvalidTripAndRouteNotice( long csvRowNumber, String tripFieldName, - String tripFieldValue, - String referenceFieldName, - String referenceFieldValue) { + String tripId, + String routeFieldName, + String routeId, + String expectedRouteId) { super(SeverityLevel.ERROR); this.csvRowNumber = csvRowNumber; this.tripFieldName = tripFieldName; - this.tripFieldValue = tripFieldValue; - this.referenceFieldName = referenceFieldName; - this.referenceFieldValue = referenceFieldValue; + this.tripId = tripId; + this.routeFieldName = routeFieldName; + this.routeId = routeId; + this.expectedRouteId = expectedRouteId; + } + } + + /** + * A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references a stop that is + * not included in the referenced trip's stop-times. + * + *

Severity: {@code SeverityLevel.ERROR} + */ + public static class TransferWithInvalidTripAndStopNotice extends ValidationNotice { + // The row number from `transfers.txt` for the faulty entry. + private final long csvRowNumber; + // The name of the trip id field (e.g. `from_trip_id`) referencing a trip. + private final String tripFieldName; + // The referenced trip id. + private final String tripId; + // The name of the stop id field (e.g. `stop_route_id`) referencing the stop. + private final String stopFieldName; + // The referenced stop id. + private final String stopId; + + public TransferWithInvalidTripAndStopNotice( + long csvRowNumber, + String tripFieldName, + String tripId, + String stopFieldName, + String stopId) { + super(SeverityLevel.ERROR); + this.csvRowNumber = csvRowNumber; + this.tripFieldName = tripFieldName; + this.tripId = tripId; + this.stopFieldName = stopFieldName; + this.stopId = stopId; } } } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java new file mode 100644 index 0000000000..06dccbfd8b --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java @@ -0,0 +1,76 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; +import org.mobilitydata.gtfsvalidator.table.GtfsStop.Builder; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferType; +import org.mobilitydata.gtfsvalidator.validator.TransfersStopTypeValidator.TransferWithInvalidStopLocationTypeNotice; + +public class TransfersStopTypeValidatorTest { + + private NoticeContainer noticeContainer = new NoticeContainer(); + + @Test + public void testStopToStationTransfer() { + // Transfers between a stop and a station are allowed. + GtfsStopTableContainer stops = + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new Builder().setStopId("s0").setLocationType(GtfsLocationType.STOP).build(), + new Builder().setStopId("s1").setLocationType(GtfsLocationType.STATION).build()), + noticeContainer); + GtfsTransferTableContainer transfers = + GtfsTransferTableContainer.forEntities( + ImmutableList.of( + new GtfsTransfer.Builder() + .setFromStopId("s0") + .setToStopId("s1") + .setTransferType(GtfsTransferType.RECOMMENDED) + .build()), + noticeContainer); + + new TransfersStopTypeValidator(transfers, stops).validate(noticeContainer); + + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void testEntranceToGenericNodeTransfer() { + // Transfers between an entrance and a generic pathway node are NOT allowed. + GtfsStopTableContainer stops = + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new Builder().setStopId("s0").setLocationType(GtfsLocationType.ENTRANCE).build(), + new Builder() + .setStopId("s1") + .setLocationType(GtfsLocationType.GENERIC_NODE) + .build()), + noticeContainer); + GtfsTransferTableContainer transfers = + GtfsTransferTableContainer.forEntities( + ImmutableList.of( + new GtfsTransfer.Builder() + .setCsvRowNumber(2) + .setFromStopId("s0") + .setToStopId("s1") + .setTransferType(GtfsTransferType.RECOMMENDED) + .build()), + noticeContainer); + + new TransfersStopTypeValidator(transfers, stops).validate(noticeContainer); + + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + new TransferWithInvalidStopLocationTypeNotice( + 2, "from_stop_id", "s0", GtfsLocationType.ENTRANCE), + new TransferWithInvalidStopLocationTypeNotice( + 2, "to_stop_id", "s1", GtfsLocationType.GENERIC_NODE)); + } +} diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidatorTest.java new file mode 100644 index 0000000000..f541be2e17 --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidatorTest.java @@ -0,0 +1,122 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; +import org.mobilitydata.gtfsvalidator.table.GtfsStop; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTime; +import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferType; +import org.mobilitydata.gtfsvalidator.table.GtfsTrip.Builder; +import org.mobilitydata.gtfsvalidator.table.GtfsTripTableContainer; +import org.mobilitydata.gtfsvalidator.validator.TransfersTripReferenceValidator.TransferWithInvalidTripAndRouteNotice; +import org.mobilitydata.gtfsvalidator.validator.TransfersTripReferenceValidator.TransferWithInvalidTripAndStopNotice; + +public class TransfersTripReferenceValidatorTest { + + private NoticeContainer noticeContainer = new NoticeContainer(); + + @Test + public void testValidTripReferences() { + // Trips with valid stop and route references. For the second trip, the transfer references + // the parent station for a stop reference. + GtfsTripTableContainer trips = + GtfsTripTableContainer.forEntities( + ImmutableList.of( + new Builder().setTripId("t0").setRouteId("r0").build(), + new Builder().setTripId("t1").setRouteId("r1").build()), + noticeContainer); + GtfsStopTableContainer stops = + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new GtfsStop.Builder().setStopId("s0").build(), + new GtfsStop.Builder().setStopId("s1_stop").setParentStation("s1_station").build(), + new GtfsStop.Builder() + .setStopId("s1_station") + .setLocationType(GtfsLocationType.STATION) + .build()), + noticeContainer); + GtfsStopTimeTableContainer stopTimes = + GtfsStopTimeTableContainer.forEntities( + ImmutableList.of( + new GtfsStopTime.Builder().setTripId("t0").setStopId("s0").build(), + new GtfsStopTime.Builder().setTripId("t1").setStopId("s1_stop").build()), + noticeContainer); + GtfsTransferTableContainer transfers = + GtfsTransferTableContainer.forEntities( + ImmutableList.of( + new GtfsTransfer.Builder() + .setCsvRowNumber(2) + .setFromStopId("s0") + .setFromRouteId("r0") + .setFromTripId("t0") + .setToStopId("s1_station") + .setToRouteId("r1") + .setToTripId("t1") + .setTransferType(GtfsTransferType.IMPOSSIBLE) + .build()), + noticeContainer); + + new TransfersTripReferenceValidator(transfers, trips, stopTimes, stops) + .validate(noticeContainer); + + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void testInvalidTripReferences() { + // Trips with invalid stop and route references. For the from-trip, the route id reference is + // invalid. For to-trip, the stop id reference doesn't match the stop-times associated with the + // trip. + GtfsTripTableContainer trips = + GtfsTripTableContainer.forEntities( + ImmutableList.of( + new Builder().setTripId("t0").setRouteId("r0").build(), + new Builder().setTripId("t1").setRouteId("r1").build()), + noticeContainer); + GtfsStopTableContainer stops = + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new GtfsStop.Builder().setStopId("s0").build(), + new GtfsStop.Builder().setStopId("s1").build(), + new GtfsStop.Builder().setStopId("s2").build()), + noticeContainer); + GtfsStopTimeTableContainer stopTimes = + GtfsStopTimeTableContainer.forEntities( + ImmutableList.of( + new GtfsStopTime.Builder().setTripId("t0").setStopId("s0").build(), + new GtfsStopTime.Builder().setTripId("t1").setStopId("s1").build()), + noticeContainer); + GtfsTransferTableContainer transfers = + GtfsTransferTableContainer.forEntities( + ImmutableList.of( + new GtfsTransfer.Builder() + .setCsvRowNumber(2) + .setFromStopId("s0") + // This is not the expected route id. + .setFromRouteId("DNE") + .setFromTripId("t0") + // This stop is not associated with the trip's stop-times. + .setToStopId("s2") + .setToRouteId("r1") + .setToTripId("t1") + .setTransferType(GtfsTransferType.IMPOSSIBLE) + .build()), + noticeContainer); + + new TransfersTripReferenceValidator(transfers, trips, stopTimes, stops) + .validate(noticeContainer); + + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + new TransferWithInvalidTripAndRouteNotice( + 2, "from_trip_id", "t0", "from_route_id", "DNE", "r0"), + new TransferWithInvalidTripAndStopNotice(2, "to_trip_id", "t1", "to_stop_id", "s2")); + } +} From 4da408eb99d8efc570aa9ba97003d59e07c25253 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Thu, 13 Oct 2022 16:53:32 -0700 Subject: [PATCH 6/6] Table formatting. --- RULES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RULES.md b/RULES.md index 378ee7447e..92698a19c5 100644 --- a/RULES.md +++ b/RULES.md @@ -1468,6 +1468,7 @@ A `from_stop_id` or `to_stop_id` field from GTFS file `transfers.txt` references * [transfers.txt specification](http://gtfs.org/reference/static/#transferstxt)

+ #### Notice fields description | Field name | Description | Type | |---------------------|---------------------------------------------------------------------------|--------| @@ -1476,6 +1477,7 @@ A `from_stop_id` or `to_stop_id` field from GTFS file `transfers.txt` references | `stopId` | The referenced stop id. | String | | `locationTypeValue` | The numeric value of the invalid location type. | int | | `locationTypeName` | The name of the invalid location type. | String | +
@@ -1488,6 +1490,7 @@ A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references * [transfers.txt specification](http://gtfs.org/reference/static/#transferstxt)
+ #### Notice fields description | Field name | Description | Type | |-------------------|------------------------------------------------------------------------------|--------| @@ -1497,6 +1500,7 @@ A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references | `routeFieldName` | The name of the route id field (e.g. `from_route_id`) referencing the route. | String | | `routeId` | The referenced route id. | String | | `expectedRouteId` | The expected route id from `trips.txt`. | String | +
@@ -1509,6 +1513,7 @@ A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references * [transfers.txt specification](http://gtfs.org/reference/static/#transferstxt)
+ #### Notice fields description | Field name | Description | Type | |-----------------|----------------------------------------------------------------------------|--------| @@ -1517,6 +1522,7 @@ A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references | `tripId` | The referenced trip id. | String | | `stopFieldName` | The name of the stop id field (e.g. `stop_route_id`) referencing the stop. | String | | `stopId` | The referenced stop id. | String | +