From 84163160fcb0980546052795ba614a1d50331368 Mon Sep 17 00:00:00 2001 From: Brian Ferris Date: Thu, 20 Oct 2022 07:47:08 -0700 Subject: [PATCH] feat: Add validation support for transfers.txt transfer_type 4 and 5. (#1266) * Add validation support for transfers.txt transfer_type 4 and 5. * Update validation of in-seat transfers to match online discussion. * Add notice documentation. Co-authored-by: isabelle-dr <63653518+isabelle-dr@users.noreply.github.com> --- RULES.md | 20 ++ .../validator/ValidatorReference.java | 15 ++ .../table/GtfsTransferSchema.java | 3 + .../table/GtfsTransferTypeEnum.java | 2 + .../validator/TransferDirection.java | 67 ++++++ .../TransfersInSeatTransferTypeValidator.java | 167 +++++++++++++++ .../validator/TransfersStopTypeValidator.java | 25 +-- .../TransfersTripReferenceValidator.java | 115 ++++------ .../validator/TransferDirectionTest.java | 76 +++++++ ...nsfersInSeatTransferTypeValidatorTest.java | 199 ++++++++++++++++++ .../TransfersStopTypeValidatorTest.java | 23 +- .../TransfersTripReferenceValidatorTest.java | 35 ++- 12 files changed, 623 insertions(+), 124 deletions(-) create mode 100644 core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorReference.java create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransferDirection.java create mode 100644 main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidator.java create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransferDirectionTest.java create mode 100644 main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidatorTest.java diff --git a/RULES.md b/RULES.md index 92698a19c5..0d4a2a2601 100644 --- a/RULES.md +++ b/RULES.md @@ -132,6 +132,7 @@ Each Notice is associated with a severity: `INFO`, `WARNING`, `ERROR`. | [`stop_too_far_from_shape`](#stop_too_far_from_shape) | Stop too far from trip shape. | | [`stop_too_far_from_shape_using_user_distance`](#stop_too_far_from_shape_using_user_distance) | Stop time too far from shape. | | [`stop_without_stop_time`](#stop_without_stop_time) | A stop in `stops.txt` is not referenced by any `stop_times.stop_id`. | +| [`transfer_with_suspicious_mid_trip_in_seat`](#transfer_with_suspicious_mid_trip_in_seat) | A trip id field from GTFS file `transfers.txt` with an in-seat transfer type references a stop that is not in the expected position in the trip's stop-times. | | [`translation_unknown_table_name`](#translation_unknown_table_name) | A translation references an unknown or missing GTFS table. | | [`unexpected_enum_value`](#unexpected_enum_value) | An enum has an unexpected value. | | [`unusable_trip`](#unusable_trip) | Trips must have more than one stop to be usable. | @@ -1525,6 +1526,25 @@ A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references + + +### transfer_with_suspicious_mid_trip_in_seat + +A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` with an in-seat transfer type references a stop that is not in the expected position in the trip's stop-times. For in-seat transfers, we expect the stop to be the last stop-time in the trip sequence for `from_stop_id` and the first stop-time for `to_stop_id`. If you are intentionally using this feature to model mid-trip transfers, you can ignore this warning, but be aware that this functionality is still considered to be partially experimental in some interpretations of the spec. + +
+ +#### Notice fields description +| Field name | Description | Type | +|-------------------|---------------------------------------------------------------------------|--------| +| `csvRowNumber` | The row number from `transfers.txt` for the faulty entry. | long | +| `tripIdFieldName` | The name of the trip id field (e.g. `from_trip_id`) referencing a trip. | String | +| `tripId` | The referenced trip id. | String | +| `stopIdFieldName` | The name of the stop id field (e.g. `from_stop_id`) referencing the stop. | String | +| `stopId` | The referenced stop id. | String | + +
+ ### translation_foreign_key_violation diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorReference.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorReference.java new file mode 100644 index 0000000000..e09d54dd82 --- /dev/null +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/ValidatorReference.java @@ -0,0 +1,15 @@ +package org.mobilitydata.gtfsvalidator.validator; + +/** + * This is a no-op class that allows for static references to other validators as a form of + * dependency documentation. + */ +public final class ValidatorReference { + private ValidatorReference() {} + + /** + * A no-op method that allows one to statically document that a particular validation condition is + * handled by another validator. + */ + public static final void validatedElsewhereBy(Class... validator) {} +} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferSchema.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferSchema.java index 781145f354..cb0e5cb4ec 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferSchema.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferSchema.java @@ -18,6 +18,7 @@ import static org.mobilitydata.gtfsvalidator.annotation.TranslationRecordIdType.*; +import org.mobilitydata.gtfsvalidator.annotation.ConditionallyRequired; import org.mobilitydata.gtfsvalidator.annotation.FieldType; import org.mobilitydata.gtfsvalidator.annotation.FieldTypeEnum; import org.mobilitydata.gtfsvalidator.annotation.ForeignKey; @@ -47,11 +48,13 @@ public interface GtfsTransferSchema extends GtfsEntity { int minTransferTime(); @FieldType(FieldTypeEnum.ID) + @ConditionallyRequired @ForeignKey(table = "trips.txt", field = "trip_id") @PrimaryKey(translationRecordIdType = UNSUPPORTED) String fromTripId(); @FieldType(FieldTypeEnum.ID) + @ConditionallyRequired @ForeignKey(table = "trips.txt", field = "trip_id") @PrimaryKey(translationRecordIdType = UNSUPPORTED) String toTripId(); diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferTypeEnum.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferTypeEnum.java index 16c11301dd..141e203a90 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferTypeEnum.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsTransferTypeEnum.java @@ -22,4 +22,6 @@ @GtfsEnumValue(name = "TIMED", value = 1) @GtfsEnumValue(name = "MINIMUM_TIME", value = 2) @GtfsEnumValue(name = "IMPOSSIBLE", value = 3) +@GtfsEnumValue(name = "IN_SEAT_TRANSFER_ALLOWED", value = 4) +@GtfsEnumValue(name = "IN_SEAT_TRANSFER_NOT_ALLOWED", value = 5) public interface GtfsTransferTypeEnum {} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransferDirection.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransferDirection.java new file mode 100644 index 0000000000..b7881a29c9 --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransferDirection.java @@ -0,0 +1,67 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; + +/** + * An enum type, along with various convenience methods, for identifying the direction of transfer + * in a `transfers.txt` entry and accessing associated fields. + */ +public enum TransferDirection { + /** + * The source of the transfer, including fields `from_stop_id`, `from_route_id`, and + * `from_trip_id`. + */ + TRANSFER_FROM, + /** + * The destination of the transfer, including fields `to_stop_id`, `to_route_id`, and + * `to_trip_id`. + */ + TRANSFER_TO; + + public String stopIdFieldName() { + return isFrom() + ? GtfsTransferTableLoader.FROM_STOP_ID_FIELD_NAME + : GtfsTransferTableLoader.TO_STOP_ID_FIELD_NAME; + } + + public String stopId(GtfsTransfer transfer) { + return isFrom() ? transfer.fromStopId() : transfer.toStopId(); + } + + public boolean hasStopId(GtfsTransfer transfer) { + return isFrom() ? transfer.hasFromStopId() : transfer.hasToStopId(); + } + + public String routeIdFieldName() { + return isFrom() + ? GtfsTransferTableLoader.FROM_ROUTE_ID_FIELD_NAME + : GtfsTransferTableLoader.TO_ROUTE_ID_FIELD_NAME; + } + + public boolean hasRouteId(GtfsTransfer transfer) { + return isFrom() ? transfer.hasFromRouteId() : transfer.hasToRouteId(); + } + + public String routeId(GtfsTransfer transfer) { + return isFrom() ? transfer.fromRouteId() : transfer.toRouteId(); + } + + public String tripIdFieldName() { + return isFrom() + ? GtfsTransferTableLoader.FROM_TRIP_ID_FIELD_NAME + : GtfsTransferTableLoader.TO_TRIP_ID_FIELD_NAME; + } + + public boolean hasTripId(GtfsTransfer transfer) { + return isFrom() ? transfer.hasFromTripId() : transfer.hasToTripId(); + } + + public String tripId(GtfsTransfer transfer) { + return isFrom() ? transfer.fromTripId() : transfer.toTripId(); + } + + private boolean isFrom() { + return this == TRANSFER_FROM; + } +} diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidator.java new file mode 100644 index 0000000000..296f3072ca --- /dev/null +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidator.java @@ -0,0 +1,167 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import static org.mobilitydata.gtfsvalidator.validator.ValidatorReference.validatedElsewhereBy; + +import java.util.List; +import java.util.Optional; +import javax.inject.Inject; +import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; +import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice; +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; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableLoader; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferType; +import org.mobilitydata.gtfsvalidator.validator.TransfersStopTypeValidator.TransferWithInvalidStopLocationTypeNotice; + +/** + * Validates that entries in `transfers.txt` with an in-seat transfer type are properly specified. + * + * @see TransfersStopTypeValidator + * @see TransfersTripReferenceValidator + */ +@GtfsValidator +public class TransfersInSeatTransferTypeValidator extends FileValidator { + + private final GtfsTransferTableContainer transfers; + private final GtfsStopTableContainer stops; + private final GtfsStopTimeTableContainer stopTimes; + + @Inject + public TransfersInSeatTransferTypeValidator( + GtfsTransferTableContainer transfers, + GtfsStopTableContainer stops, + GtfsStopTimeTableContainer stopTimes) { + this.transfers = transfers; + this.stops = stops; + this.stopTimes = stopTimes; + } + + @Override + public void validate(NoticeContainer noticeContainer) { + for (GtfsTransfer transfer : transfers.getEntities()) { + validateEntity(transfer, noticeContainer); + } + } + + public void validateEntity(GtfsTransfer transfer, NoticeContainer noticeContainer) { + if (!transfer.hasTransferType()) { + return; + } + if (!isInSeatTransferType(transfer.transferType())) { + return; + } + + for (TransferDirection transferDirection : TransferDirection.values()) { + + // Trip IDs are required for in-seat transfer types. + if (!transferDirection.hasTripId(transfer)) { + noticeContainer.addValidationNotice( + new MissingRequiredFieldNotice( + GtfsTransferTableLoader.FILENAME, + transfer.csvRowNumber(), + transferDirection.tripIdFieldName())); + } + + validateStop(transfer, transferDirection, noticeContainer); + } + } + + private boolean isInSeatTransferType(GtfsTransferType transferType) { + switch (transferType) { + case IN_SEAT_TRANSFER_ALLOWED: + case IN_SEAT_TRANSFER_NOT_ALLOWED: + return true; + default: + return false; + } + } + + private void validateStop( + GtfsTransfer transfer, TransferDirection transferDirection, NoticeContainer noticeContainer) { + String stopId = transferDirection.stopId(transfer); + Optional optStop = stops.byStopId(stopId); + if (optStop.isEmpty()) { + // This foreign key reference is + validatedElsewhereBy( + GtfsTransferFromStopIdForeignKeyValidator.class, + GtfsTransferToStopIdForeignKeyValidator.class); + return; + } + // Per the spec, normally a stop or station location type is required for a transfer entry. + // However, for in-seat transfers, stations are specifically forbidden. + GtfsLocationType locationType = optStop.get().locationType(); + if (locationType == GtfsLocationType.STATION) { + noticeContainer.addValidationNotice( + new TransferWithInvalidStopLocationTypeNotice(transfer, transferDirection, locationType)); + } + + List stopTimesForTrip = stopTimes.byTripId(transferDirection.tripId(transfer)); + if (stopTimesForTrip.isEmpty() + || !stopTimesForTrip.stream().anyMatch((st) -> st.stopId().equals(stopId))) { + // Requiring that a transfer trip's stop-times reference the transfer stop is + validatedElsewhereBy(TransfersTripReferenceValidator.class); + return; + } + + GtfsStopTime transferStop = getInSeatTransferStopTime(stopTimesForTrip, transferDirection); + if (!transferStop.stopId().equals(stopId)) { + noticeContainer.addValidationNotice( + new TransferWithSuspiciousMidTripInSeatNotice(transfer, transferDirection)); + } + } + + private GtfsStopTime getInSeatTransferStopTime( + List stopTimesForTrip, TransferDirection transferDirection) { + switch (transferDirection) { + case TRANSFER_FROM: + return stopTimesForTrip.get(stopTimesForTrip.size() - 1); + case TRANSFER_TO: + return stopTimesForTrip.get(0); + default: + throw new UnsupportedOperationException("Unhandled TransferDirection=" + transferDirection); + } + } + + /** + * A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` with an in-seat transfer + * type references a stop that is not in the expected position in the trip's stop-times. + * + *

For in-seat transfers, we expect the stop to be the last stop-time in the trip sequence for + * `from_stop_id` and the first stop-time for `to_stop_id`. If you are intentionally using this + * feature to model mid-trip transfers, you can ignore this warning, but be aware that this + * functionality is still considered to be partially experimental in some interpretations of the + * spec. + * + *

Severity: {@code SeverityLevel.WARNING} + */ + public static class TransferWithSuspiciousMidTripInSeatNotice 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 tripIdFieldName; + // The referenced trip id. + private final String tripId; + // The name of the stop id field (e.g. `from_stop_id`) referencing the stop. + private final String stopIdFieldName; + // The referenced stop id. + private final String stopId; + + public TransferWithSuspiciousMidTripInSeatNotice( + GtfsTransfer transfer, TransferDirection transferDirection) { + super(SeverityLevel.WARNING); + this.csvRowNumber = transfer.csvRowNumber(); + this.tripIdFieldName = transferDirection.tripIdFieldName(); + this.tripId = transferDirection.tripId(transfer); + this.stopIdFieldName = transferDirection.stopIdFieldName(); + this.stopId = transferDirection.stopId(transfer); + } + } +} 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 c4a38c7399..891a7a6a86 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidator.java @@ -11,7 +11,6 @@ import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; 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. @@ -36,18 +35,13 @@ public void validate(NoticeContainer 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); + validateStopType(entity, TransferDirection.TRANSFER_FROM, noticeContainer); + validateStopType(entity, TransferDirection.TRANSFER_TO, noticeContainer); } private void validateStopType( - GtfsTransfer entity, String stopIdFieldName, String stopId, NoticeContainer noticeContainer) { - Optional optStop = stopsContainer.byStopId(stopId); + GtfsTransfer entity, TransferDirection transferDirection, NoticeContainer noticeContainer) { + Optional optStop = stopsContainer.byStopId(transferDirection.stopId(entity)); if (optStop.isEmpty()) { // Foreign key reference is validated elsewhere. return; @@ -56,8 +50,7 @@ private void validateStopType( GtfsLocationType locationType = optStop.get().locationType(); if (!isValidTransferStopType(locationType)) { noticeContainer.addValidationNotice( - new TransferWithInvalidStopLocationTypeNotice( - entity.csvRowNumber(), stopIdFieldName, stopId, locationType)); + new TransferWithInvalidStopLocationTypeNotice(entity, transferDirection, locationType)); } } @@ -90,11 +83,11 @@ public static final class TransferWithInvalidStopLocationTypeNotice extends Vali private String locationTypeName; public TransferWithInvalidStopLocationTypeNotice( - long csvRowNumber, String stopIdFieldName, String stopId, GtfsLocationType locationType) { + GtfsTransfer transfer, TransferDirection transferDirection, GtfsLocationType locationType) { super(SeverityLevel.ERROR); - this.csvRowNumber = csvRowNumber; - this.stopIdFieldName = stopIdFieldName; - this.stopId = stopId; + this.csvRowNumber = transfer.csvRowNumber(); + this.stopIdFieldName = transferDirection.stopIdFieldName(); + this.stopId = transferDirection.stopId(transfer); 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 a2ff52df55..67bd87914f 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,7 @@ package org.mobilitydata.gtfsvalidator.validator; +import static org.mobilitydata.gtfsvalidator.validator.ValidatorReference.validatedElsewhereBy; + import com.google.common.collect.ImmutableSet; import java.util.List; import java.util.Optional; @@ -15,7 +17,6 @@ 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; @@ -51,90 +52,57 @@ public void validate(NoticeContainer 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); + for (TransferDirection transferDirection : TransferDirection.values()) { + validateTripReferences(entity, transferDirection, noticeContainer); + } } void validateTripReferences( - GtfsTransfer entity, - String tripFieldName, - Optional tripId, - String routeFieldName, - Optional routeId, - String stopFieldName, - Optional stopId, - NoticeContainer noticeContainer) { - if (tripId.isEmpty()) { + GtfsTransfer entity, TransferDirection transferDirection, NoticeContainer noticeContainer) { + if (!transferDirection.hasTripId(entity)) { return; } - Optional optTrip = tripsContainer.byTripId(tripId.get()); + Optional optTrip = tripsContainer.byTripId(transferDirection.tripId(entity)); if (optTrip.isEmpty()) { - // The foreign key reference is validated elsewhere. + // The foreign key reference is + validatedElsewhereBy( + GtfsTransferFromTripIdForeignKeyValidator.class, + GtfsTransferToTripIdForeignKeyValidator.class); return; } GtfsTrip trip = optTrip.get(); - if (routeId.isPresent()) { - if (!trip.routeId().equals(routeId.get())) { + if (transferDirection.hasRouteId(entity)) { + if (!trip.routeId().equals(transferDirection.routeId(entity))) { noticeContainer.addValidationNotice( - new TransferWithInvalidTripAndRouteNotice( - entity.csvRowNumber(), - tripFieldName, - tripId.get(), - routeFieldName, - routeId.get(), - trip.routeId())); + new TransferWithInvalidTripAndRouteNotice(entity, transferDirection, trip.routeId())); } } - if (stopId.isPresent()) { - validateTripStopReference( - entity, tripFieldName, tripId, stopFieldName, stopId.get(), noticeContainer); + if (transferDirection.hasStopId(entity)) { + validateTripStopReference(entity, transferDirection, noticeContainer); } } private void validateTripStopReference( - GtfsTransfer entity, - String tripFieldName, - Optional tripId, - String stopFieldName, - String stopId, - NoticeContainer noticeContainer) { - Optional optStop = stopsContainer.byStopId(stopId); + GtfsTransfer entity, TransferDirection transferDirection, NoticeContainer noticeContainer) { + Optional optStop = stopsContainer.byStopId(transferDirection.stopId(entity)); if (optStop.isEmpty()) { - // The foreign key reference is validated elsewhere. + // The foreign key reference is + validatedElsewhereBy( + GtfsTransferFromStopIdForeignKeyValidator.class, + GtfsTransferToStopIdForeignKeyValidator.class); return; } ImmutableSet stops = expandStationIfNeeded(optStop.get()); ImmutableSet ids = stops.stream().map(GtfsStop::stopId).collect(ImmutableSet.toImmutableSet()); - List stopTimes = stopTimeContainer.byTripId(tripId.get()); + List stopTimes = stopTimeContainer.byTripId(transferDirection.tripId(entity)); if (!stopTimes.stream().anyMatch((st) -> ids.contains(st.stopId()))) { noticeContainer.addValidationNotice( - new TransferWithInvalidTripAndStopNotice( - entity.csvRowNumber(), tripFieldName, tripId.get(), stopFieldName, stopId)); + new TransferWithInvalidTripAndStopNotice(entity, transferDirection)); } } - private static Optional optional(boolean hasValue, T value) { - return hasValue ? Optional.of(value) : Optional.empty(); - } - private ImmutableSet expandStationIfNeeded(GtfsStop stop) { if (stop.locationType() == GtfsLocationType.STOP) { return ImmutableSet.of(stop); @@ -168,18 +136,13 @@ public static class TransferWithInvalidTripAndRouteNotice extends ValidationNoti private final String expectedRouteId; public TransferWithInvalidTripAndRouteNotice( - long csvRowNumber, - String tripFieldName, - String tripId, - String routeFieldName, - String routeId, - String expectedRouteId) { + GtfsTransfer transfer, TransferDirection transferDirection, String expectedRouteId) { super(SeverityLevel.ERROR); - this.csvRowNumber = csvRowNumber; - this.tripFieldName = tripFieldName; - this.tripId = tripId; - this.routeFieldName = routeFieldName; - this.routeId = routeId; + this.csvRowNumber = transfer.csvRowNumber(); + this.tripFieldName = transferDirection.tripIdFieldName(); + this.tripId = transferDirection.tripId(transfer); + this.routeFieldName = transferDirection.routeIdFieldName(); + this.routeId = transferDirection.routeId(transfer); this.expectedRouteId = expectedRouteId; } } @@ -203,17 +166,13 @@ public static class TransferWithInvalidTripAndStopNotice extends ValidationNotic private final String stopId; public TransferWithInvalidTripAndStopNotice( - long csvRowNumber, - String tripFieldName, - String tripId, - String stopFieldName, - String stopId) { + GtfsTransfer transfer, TransferDirection transferDirection) { super(SeverityLevel.ERROR); - this.csvRowNumber = csvRowNumber; - this.tripFieldName = tripFieldName; - this.tripId = tripId; - this.stopFieldName = stopFieldName; - this.stopId = stopId; + this.csvRowNumber = transfer.csvRowNumber(); + this.tripFieldName = transferDirection.tripIdFieldName(); + this.tripId = transferDirection.tripId(transfer); + this.stopFieldName = transferDirection.stopIdFieldName(); + this.stopId = transferDirection.stopId(transfer); } } } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransferDirectionTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransferDirectionTest.java new file mode 100644 index 0000000000..6f965a11fe --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransferDirectionTest.java @@ -0,0 +1,76 @@ +package org.mobilitydata.gtfsvalidator.validator; + +import static com.google.common.truth.Truth.assertThat; +import static org.mobilitydata.gtfsvalidator.validator.TransferDirection.TRANSFER_FROM; +import static org.mobilitydata.gtfsvalidator.validator.TransferDirection.TRANSFER_TO; + +import org.junit.Test; +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransfer.Builder; + +public class TransferDirectionTest { + + @Test + public void testFieldName() { + assertThat(TRANSFER_FROM.stopIdFieldName()).isEqualTo("from_stop_id"); + assertThat(TRANSFER_TO.stopIdFieldName()).isEqualTo("to_stop_id"); + + assertThat(TRANSFER_FROM.routeIdFieldName()).isEqualTo("from_route_id"); + assertThat(TRANSFER_TO.routeIdFieldName()).isEqualTo("to_route_id"); + + assertThat(TRANSFER_FROM.tripIdFieldName()).isEqualTo("from_trip_id"); + assertThat(TRANSFER_TO.tripIdFieldName()).isEqualTo("to_trip_id"); + } + + @Test + public void testHasMethodsForEmptyTransfer() { + GtfsTransfer emptyTransfer = new Builder().build(); + + assertThat(TRANSFER_FROM.hasStopId(emptyTransfer)).isFalse(); + assertThat(TRANSFER_TO.hasStopId(emptyTransfer)).isFalse(); + + assertThat(TRANSFER_FROM.hasRouteId(emptyTransfer)).isFalse(); + assertThat(TRANSFER_TO.hasRouteId(emptyTransfer)).isFalse(); + + assertThat(TRANSFER_FROM.hasTripId(emptyTransfer)).isFalse(); + assertThat(TRANSFER_TO.hasTripId(emptyTransfer)).isFalse(); + } + + @Test + public void testHasMethods() { + assertThat(TRANSFER_FROM.hasStopId(new GtfsTransfer.Builder().setFromStopId("a").build())) + .isTrue(); + assertThat(TRANSFER_TO.hasStopId(new GtfsTransfer.Builder().setToStopId("a").build())).isTrue(); + + assertThat(TRANSFER_FROM.hasRouteId(new GtfsTransfer.Builder().setFromRouteId("a").build())) + .isTrue(); + assertThat(TRANSFER_TO.hasRouteId(new GtfsTransfer.Builder().setToRouteId("a").build())) + .isTrue(); + + assertThat(TRANSFER_FROM.hasTripId(new GtfsTransfer.Builder().setFromTripId("a").build())) + .isTrue(); + assertThat(TRANSFER_TO.hasTripId(new GtfsTransfer.Builder().setToTripId("a").build())).isTrue(); + } + + @Test + public void testIdMethods() { + GtfsTransfer transfer = + new Builder() + .setFromStopId("stopA") + .setFromRouteId("routeA") + .setFromTripId("tripA") + .setToStopId("stopB") + .setToRouteId("routeB") + .setToTripId("tripB") + .build(); + + assertThat(TRANSFER_FROM.stopId(transfer)).isEqualTo("stopA"); + assertThat(TRANSFER_TO.stopId(transfer)).isEqualTo("stopB"); + + assertThat(TRANSFER_FROM.routeId(transfer)).isEqualTo("routeA"); + assertThat(TRANSFER_TO.routeId(transfer)).isEqualTo("routeB"); + + assertThat(TRANSFER_FROM.tripId(transfer)).isEqualTo("tripA"); + assertThat(TRANSFER_TO.tripId(transfer)).isEqualTo("tripB"); + } +} diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidatorTest.java new file mode 100644 index 0000000000..fd0611ae1e --- /dev/null +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersInSeatTransferTypeValidatorTest.java @@ -0,0 +1,199 @@ +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.GtfsTransfer.Builder; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferTableContainer; +import org.mobilitydata.gtfsvalidator.table.GtfsTransferType; +import org.mobilitydata.gtfsvalidator.validator.TransfersInSeatTransferTypeValidator.TransferWithSuspiciousMidTripInSeatNotice; +import org.mobilitydata.gtfsvalidator.validator.TransfersStopTypeValidator.TransferWithInvalidStopLocationTypeNotice; + +public class TransfersInSeatTransferTypeValidatorTest { + + private final NoticeContainer noticeContainer = new NoticeContainer(); + + @Test + public void testValidInSeatTransfer() { + // In-seat transfer between the last stop of the from-trip and the first-stop of the to-trip. + GtfsStopTableContainer stops = + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new GtfsStop.Builder() + .setStopId("s0") + .setLocationType(GtfsLocationType.STOP) + .build(), + new GtfsStop.Builder() + .setStopId("s1") + .setLocationType(GtfsLocationType.STOP) + .build()), + noticeContainer); + GtfsStopTimeTableContainer stopTimes = + GtfsStopTimeTableContainer.forEntities( + ImmutableList.of( + new GtfsStopTime.Builder() + .setTripId("t0") + .setStopId("s?") + .setStopSequence(0) + .build(), + new GtfsStopTime.Builder() + .setTripId("t0") + .setStopId("s0") + .setStopSequence(1) + .build(), + new GtfsStopTime.Builder() + .setTripId("t1") + .setStopId("s1") + .setStopSequence(0) + .build(), + new GtfsStopTime.Builder() + .setTripId("t1") + .setStopId("s?") + .setStopSequence(1) + .build()), + noticeContainer); + GtfsTransferTableContainer transfers = + GtfsTransferTableContainer.forEntities( + ImmutableList.of( + new GtfsTransfer.Builder() + .setCsvRowNumber(2) + .setFromStopId("s0") + .setToStopId("s1") + .setFromTripId("t0") + .setToTripId("t1") + .setTransferType(GtfsTransferType.IN_SEAT_TRANSFER_ALLOWED) + .build()), + noticeContainer); + + new TransfersInSeatTransferTypeValidator(transfers, stops, stopTimes).validate(noticeContainer); + + assertThat(noticeContainer.getValidationNotices()).isEmpty(); + } + + @Test + public void testInvalidInSeatTransferToStation() { + // Invalid: `to_stop_id` refers to a station, which is forbidden for in-seat transfers. + GtfsStopTableContainer stops = + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new GtfsStop.Builder() + .setStopId("s0") + .setLocationType(GtfsLocationType.STOP) + .build(), + new GtfsStop.Builder() + .setStopId("s1") + .setLocationType(GtfsLocationType.STATION) + .build()), + noticeContainer); + GtfsStopTimeTableContainer stopTimes = + GtfsStopTimeTableContainer.forEntities( + ImmutableList.of( + new GtfsStopTime.Builder() + .setTripId("t0") + .setStopId("s?") + .setStopSequence(0) + .build(), + new GtfsStopTime.Builder() + .setTripId("t0") + .setStopId("s0") + .setStopSequence(1) + .build(), + new GtfsStopTime.Builder() + .setTripId("t1") + .setStopId("s1") + .setStopSequence(0) + .build(), + new GtfsStopTime.Builder() + .setTripId("t1") + .setStopId("s?") + .setStopSequence(1) + .build()), + noticeContainer); + GtfsTransfer transfer = + new Builder() + .setCsvRowNumber(2) + .setFromStopId("s0") + .setToStopId("s1") + .setFromTripId("t0") + .setToTripId("t1") + .setTransferType(GtfsTransferType.IN_SEAT_TRANSFER_ALLOWED) + .build(); + GtfsTransferTableContainer transfers = + GtfsTransferTableContainer.forEntities(ImmutableList.of(transfer), noticeContainer); + + new TransfersInSeatTransferTypeValidator(transfers, stops, stopTimes).validate(noticeContainer); + + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + new TransferWithInvalidStopLocationTypeNotice( + transfer, TransferDirection.TRANSFER_TO, GtfsLocationType.STATION)); + } + + @Test + public void testSuspiciousMidTripInSeatTransfer() { + // Invalid: `to_stop_id` refers to a station, which is forbidden for in-seat transfers. + GtfsStopTableContainer stops = + GtfsStopTableContainer.forEntities( + ImmutableList.of( + new GtfsStop.Builder() + .setStopId("s0") + .setLocationType(GtfsLocationType.STOP) + .build(), + new GtfsStop.Builder() + .setStopId("s1") + .setLocationType(GtfsLocationType.STOP) + .build()), + noticeContainer); + GtfsStopTimeTableContainer stopTimes = + GtfsStopTimeTableContainer.forEntities( + ImmutableList.of( + new GtfsStopTime.Builder() + .setTripId("t0") + .setStopId("s0") + .setStopSequence(0) + .build(), + new GtfsStopTime.Builder() + .setTripId("t0") + .setStopId("s?") + .setStopSequence(1) + .build(), + new GtfsStopTime.Builder() + .setTripId("t1") + .setStopId("s?") + .setStopSequence(0) + .build(), + new GtfsStopTime.Builder() + .setTripId("t1") + .setStopId("s1") + .setStopSequence(1) + .build()), + noticeContainer); + GtfsTransfer transfer = + new Builder() + .setCsvRowNumber(2) + .setFromStopId("s0") + .setToStopId("s1") + .setFromTripId("t0") + .setToTripId("t1") + .setTransferType(GtfsTransferType.IN_SEAT_TRANSFER_ALLOWED) + .build(); + GtfsTransferTableContainer transfers = + GtfsTransferTableContainer.forEntities(ImmutableList.of(transfer), noticeContainer); + + new TransfersInSeatTransferTypeValidator(transfers, stops, stopTimes).validate(noticeContainer); + + assertThat(noticeContainer.getValidationNotices()) + .containsExactly( + new TransferWithSuspiciousMidTripInSeatNotice( + transfer, TransferDirection.TRANSFER_FROM), + new TransferWithSuspiciousMidTripInSeatNotice(transfer, TransferDirection.TRANSFER_TO)); + } +} diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java index 06dccbfd8b..0add19d140 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersStopTypeValidatorTest.java @@ -15,7 +15,7 @@ public class TransfersStopTypeValidatorTest { - private NoticeContainer noticeContainer = new NoticeContainer(); + private final NoticeContainer noticeContainer = new NoticeContainer(); @Test public void testStopToStationTransfer() { @@ -53,24 +53,23 @@ public void testEntranceToGenericNodeTransfer() { .setLocationType(GtfsLocationType.GENERIC_NODE) .build()), noticeContainer); + GtfsTransfer transfer = + new GtfsTransfer.Builder() + .setCsvRowNumber(2) + .setFromStopId("s0") + .setToStopId("s1") + .setTransferType(GtfsTransferType.RECOMMENDED) + .build(); GtfsTransferTableContainer transfers = - GtfsTransferTableContainer.forEntities( - ImmutableList.of( - new GtfsTransfer.Builder() - .setCsvRowNumber(2) - .setFromStopId("s0") - .setToStopId("s1") - .setTransferType(GtfsTransferType.RECOMMENDED) - .build()), - noticeContainer); + GtfsTransferTableContainer.forEntities(ImmutableList.of(transfer), noticeContainer); new TransfersStopTypeValidator(transfers, stops).validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) .containsExactly( new TransferWithInvalidStopLocationTypeNotice( - 2, "from_stop_id", "s0", GtfsLocationType.ENTRANCE), + transfer, TransferDirection.TRANSFER_FROM, GtfsLocationType.ENTRANCE), new TransferWithInvalidStopLocationTypeNotice( - 2, "to_stop_id", "s1", GtfsLocationType.GENERIC_NODE)); + transfer, TransferDirection.TRANSFER_TO, 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 index f541be2e17..ecb7bf97f6 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TransfersTripReferenceValidatorTest.java @@ -20,7 +20,7 @@ public class TransfersTripReferenceValidatorTest { - private NoticeContainer noticeContainer = new NoticeContainer(); + private final NoticeContainer noticeContainer = new NoticeContainer(); @Test public void testValidTripReferences() { @@ -93,22 +93,21 @@ public void testInvalidTripReferences() { new GtfsStopTime.Builder().setTripId("t0").setStopId("s0").build(), new GtfsStopTime.Builder().setTripId("t1").setStopId("s1").build()), noticeContainer); + GtfsTransfer transfer = + 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(); 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); + GtfsTransferTableContainer.forEntities(ImmutableList.of(transfer), noticeContainer); new TransfersTripReferenceValidator(transfers, trips, stopTimes, stops) .validate(noticeContainer); @@ -116,7 +115,7 @@ public void testInvalidTripReferences() { 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")); + transfer, TransferDirection.TRANSFER_FROM, "r0"), + new TransferWithInvalidTripAndStopNotice(transfer, TransferDirection.TRANSFER_TO)); } }