Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add validation support for transfers.txt transfer_type 4 and 5. #1266

Merged
merged 6 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down Expand Up @@ -1525,6 +1526,25 @@ A `from_trip_id` or `to_trip_id` field from GTFS file `transfers.txt` references

</details>

<a name="TransferWithSuspiciousMidTripInSeatNotice"/>

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

<details>

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

</details>

<a name="TranslationForeignKeyViolationNotice"/>

### translation_foreign_key_violation
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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<GtfsStop> 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<GtfsStopTime> 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<GtfsStopTime> 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.
*
* <p>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.
*
* <p>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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<GtfsStop> optStop = stopsContainer.byStopId(stopId);
GtfsTransfer entity, TransferDirection transferDirection, NoticeContainer noticeContainer) {
Optional<GtfsStop> optStop = stopsContainer.byStopId(transferDirection.stopId(entity));
if (optStop.isEmpty()) {
// Foreign key reference is validated elsewhere.
return;
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -90,11 +83,11 @@ public static final class TransferWithInvalidStopLocationTypeNotice extends Vali
private String locationTypeName;

public TransferWithInvalidStopLocationTypeNotice(
isabelle-dr marked this conversation as resolved.
Show resolved Hide resolved
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();
}
Expand Down
Loading