Skip to content

Commit

Permalink
Move GTFS field names from table loader to entity classes (#1274)
Browse files Browse the repository at this point in the history
E.g., use GtfsStop.STOP_ID_FIELD_NAME instead of
GtfsStopTableLoader.STOP_ID_FIELD_NAME.

Motivation

1. Table loader classes should be only used for loading data tables.
   They should not be used by validator classes.
2. Our final goal is to stop generating table loader classes and instead
   have a manually written classe that uses GTFS schema as a parameter. 
   GTFS field names have to be generated, so we move them to entity classes
   that are generated anyway.
  • Loading branch information
aababilov committed Oct 22, 2022
1 parent 8416316 commit 35bf825
Show file tree
Hide file tree
Showing 18 changed files with 132 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
import org.mobilitydata.gtfsvalidator.table.GtfsAgency;
import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableLoader;

/**
* Validates that all agencies have the same timezone and language and that agency_id field is set
Expand Down Expand Up @@ -63,7 +62,7 @@ public void validate(NoticeContainer noticeContainer) {
new MissingRequiredFieldNotice(
agencyTable.gtfsFilename(),
agency.csvRowNumber(),
GtfsAgencyTableLoader.AGENCY_ID_FIELD_NAME));
GtfsAgency.AGENCY_ID_FIELD_NAME));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@
import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator;
import org.mobilitydata.gtfsvalidator.notice.ForeignKeyViolationNotice;
import org.mobilitydata.gtfsvalidator.notice.NoticeContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsCalendar;
import org.mobilitydata.gtfsvalidator.table.GtfsCalendarDate;
import org.mobilitydata.gtfsvalidator.table.GtfsCalendarDateTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsCalendarDateTableLoader;
import org.mobilitydata.gtfsvalidator.table.GtfsCalendarTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsCalendarTableLoader;
import org.mobilitydata.gtfsvalidator.table.GtfsTrip;
import org.mobilitydata.gtfsvalidator.table.GtfsTripTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsTripTableLoader;

/**
* Validates that service_id field in "trips.txt" references a valid service_id in "calendar.txt" or
Expand Down Expand Up @@ -57,10 +56,10 @@ public void validate(NoticeContainer noticeContainer) {
if (!hasReferencedKey(childKey, calendarContainer, calendarDateContainer)) {
noticeContainer.addValidationNotice(
new ForeignKeyViolationNotice(
GtfsTripTableLoader.FILENAME,
GtfsTripTableLoader.SERVICE_ID_FIELD_NAME,
GtfsCalendarTableLoader.FILENAME + " or " + GtfsCalendarDateTableLoader.FILENAME,
GtfsCalendarTableLoader.SERVICE_ID_FIELD_NAME,
GtfsTrip.FILENAME,
GtfsTrip.SERVICE_ID_FIELD_NAME,
GtfsCalendar.FILENAME + " or " + GtfsCalendarDate.FILENAME,
GtfsCalendar.SERVICE_ID_FIELD_NAME,
childKey,
trip.csvRowNumber()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

package org.mobilitydata.gtfsvalidator.validator;

import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.ARRIVAL_TIME_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.DEPARTURE_TIME_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.ARRIVAL_TIME_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.DEPARTURE_TIME_FIELD_NAME;

import com.google.common.collect.Multimaps;
import java.util.List;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
import org.mobilitydata.gtfsvalidator.table.GtfsPathway;
import org.mobilitydata.gtfsvalidator.table.GtfsPathwayTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsPathwayTableLoader;
import org.mobilitydata.gtfsvalidator.table.GtfsStop;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer;

Expand Down Expand Up @@ -54,15 +53,9 @@ public class PathwayEndpointTypeValidator extends FileValidator {
public void validate(NoticeContainer noticeContainer) {
for (GtfsPathway pathway : pathwayTable.getEntities()) {
checkEndpoint(
pathway,
GtfsPathwayTableLoader.FROM_STOP_ID_FIELD_NAME,
pathway.fromStopId(),
noticeContainer);
pathway, GtfsPathway.FROM_STOP_ID_FIELD_NAME, pathway.fromStopId(), noticeContainer);
checkEndpoint(
pathway,
GtfsPathwayTableLoader.TO_STOP_ID_FIELD_NAME,
pathway.toStopId(),
noticeContainer);
pathway, GtfsPathway.TO_STOP_ID_FIELD_NAME, pathway.toStopId(), noticeContainer);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTime;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader;
import org.mobilitydata.gtfsvalidator.type.GtfsTime;

/**
Expand Down Expand Up @@ -64,8 +63,8 @@ public void validate(NoticeContainer noticeContainer) {
stopTime.tripId(),
stopTime.stopSequence(),
hasArrival
? GtfsStopTimeTableLoader.ARRIVAL_TIME_FIELD_NAME
: GtfsStopTimeTableLoader.DEPARTURE_TIME_FIELD_NAME));
? GtfsStopTime.ARRIVAL_TIME_FIELD_NAME
: GtfsStopTime.DEPARTURE_TIME_FIELD_NAME));
}
if (hasArrival
&& previousDepartureRow != -1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTime;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTimepoint;

/**
Expand All @@ -49,7 +48,7 @@ public class TimepointTimeValidator extends FileValidator {

@Override
public void validate(NoticeContainer noticeContainer) {
if (!stopTimes.hasColumn(GtfsStopTimeTableLoader.TIMEPOINT_FIELD_NAME)) {
if (!stopTimes.hasColumn(GtfsStopTime.TIMEPOINT_FIELD_NAME)) {
// legacy datasets do not use timepoint column in stop_times.txt as a result:
// - this should be flagged;
// - but also no notice regarding the absence of arrival_time or departure_time should be
Expand All @@ -65,12 +64,12 @@ public void validate(NoticeContainer noticeContainer) {
if (!stopTime.hasArrivalTime()) {
noticeContainer.addValidationNotice(
new StopTimeTimepointWithoutTimesNotice(
stopTime, GtfsStopTimeTableLoader.ARRIVAL_TIME_FIELD_NAME));
stopTime, GtfsStopTime.ARRIVAL_TIME_FIELD_NAME));
}
if (!stopTime.hasDepartureTime()) {
noticeContainer.addValidationNotice(
new StopTimeTimepointWithoutTimesNotice(
stopTime, GtfsStopTimeTableLoader.DEPARTURE_TIME_FIELD_NAME));
stopTime, GtfsStopTime.DEPARTURE_TIME_FIELD_NAME));
}
}
}
Expand Down Expand Up @@ -128,7 +127,7 @@ static class MissingTimepointColumnNotice extends ValidationNotice {

MissingTimepointColumnNotice() {
super(SeverityLevel.WARNING);
this.filename = GtfsStopTimeTableLoader.FILENAME;
this.filename = GtfsStopTime.FILENAME;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
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
Expand All @@ -20,9 +19,7 @@ public enum TransferDirection {
TRANSFER_TO;

public String stopIdFieldName() {
return isFrom()
? GtfsTransferTableLoader.FROM_STOP_ID_FIELD_NAME
: GtfsTransferTableLoader.TO_STOP_ID_FIELD_NAME;
return isFrom() ? GtfsTransfer.FROM_STOP_ID_FIELD_NAME : GtfsTransfer.TO_STOP_ID_FIELD_NAME;
}

public String stopId(GtfsTransfer transfer) {
Expand All @@ -34,9 +31,7 @@ public boolean hasStopId(GtfsTransfer transfer) {
}

public String routeIdFieldName() {
return isFrom()
? GtfsTransferTableLoader.FROM_ROUTE_ID_FIELD_NAME
: GtfsTransferTableLoader.TO_ROUTE_ID_FIELD_NAME;
return isFrom() ? GtfsTransfer.FROM_ROUTE_ID_FIELD_NAME : GtfsTransfer.TO_ROUTE_ID_FIELD_NAME;
}

public boolean hasRouteId(GtfsTransfer transfer) {
Expand All @@ -48,9 +43,7 @@ public String routeId(GtfsTransfer transfer) {
}

public String tripIdFieldName() {
return isFrom()
? GtfsTransferTableLoader.FROM_TRIP_ID_FIELD_NAME
: GtfsTransferTableLoader.TO_TRIP_ID_FIELD_NAME;
return isFrom() ? GtfsTransfer.FROM_TRIP_ID_FIELD_NAME : GtfsTransfer.TO_TRIP_ID_FIELD_NAME;
}

public boolean hasTripId(GtfsTransfer transfer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,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.GtfsTransferType;
import org.mobilitydata.gtfsvalidator.validator.TransfersStopTypeValidator.TransferWithInvalidStopLocationTypeNotice;

Expand Down Expand Up @@ -65,7 +64,7 @@ public void validateEntity(GtfsTransfer transfer, NoticeContainer noticeContaine
if (!transferDirection.hasTripId(transfer)) {
noticeContainer.addValidationNotice(
new MissingRequiredFieldNotice(
GtfsTransferTableLoader.FILENAME,
GtfsTransfer.FILENAME,
transfer.csvRowNumber(),
transferDirection.tripIdFieldName()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.mobilitydata.gtfsvalidator.table.GtfsTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsTranslation;
import org.mobilitydata.gtfsvalidator.table.GtfsTranslationTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsTranslationTableLoader;

/**
* Validates that translations are provided in accordance with GTFS Specification.
Expand All @@ -54,7 +53,7 @@ public class TranslationFieldAndReferenceValidator extends FileValidator {
@Override
public void validate(NoticeContainer noticeContainer) {
// The legacy Google translation format does not define `translations.table_name` field.
if (!translationTable.getHeader().hasColumn(GtfsTranslationTableLoader.TABLE_NAME_FIELD_NAME)) {
if (!translationTable.getHeader().hasColumn(GtfsTranslation.TABLE_NAME_FIELD_NAME)) {
// Skip validation if legacy Google translation format is detected.
return;
}
Expand All @@ -81,25 +80,25 @@ private boolean validateStandardRequiredFields(NoticeContainer noticeContainer)
if (!translation.hasFieldName()) {
noticeContainer.addValidationNotice(
new MissingRequiredFieldNotice(
GtfsTranslationTableLoader.FILENAME,
GtfsTranslation.FILENAME,
translation.csvRowNumber(),
GtfsTranslationTableLoader.FIELD_NAME_FIELD_NAME));
GtfsTranslation.FIELD_NAME_FIELD_NAME));
isValid = false;
}
if (!translation.hasLanguage()) {
noticeContainer.addValidationNotice(
new MissingRequiredFieldNotice(
GtfsTranslationTableLoader.FILENAME,
GtfsTranslation.FILENAME,
translation.csvRowNumber(),
GtfsTranslationTableLoader.LANGUAGE_FIELD_NAME));
GtfsTranslation.LANGUAGE_FIELD_NAME));
isValid = false;
}
if (!translation.hasTableName()) {
noticeContainer.addValidationNotice(
new MissingRequiredFieldNotice(
GtfsTranslationTableLoader.FILENAME,
GtfsTranslation.FILENAME,
translation.csvRowNumber(),
GtfsTranslationTableLoader.TABLE_NAME_FIELD_NAME));
GtfsTranslation.TABLE_NAME_FIELD_NAME));
isValid = false;
}
}
Expand All @@ -112,16 +111,12 @@ private void validateTranslation(GtfsTranslation translation, NoticeContainer no
if (translation.hasRecordId()) {
noticeContainer.addValidationNotice(
new TranslationUnexpectedValueNotice(
translation,
GtfsTranslationTableLoader.RECORD_ID_FIELD_NAME,
translation.recordId()));
translation, GtfsTranslation.RECORD_ID_FIELD_NAME, translation.recordId()));
}
if (translation.hasRecordSubId()) {
noticeContainer.addValidationNotice(
new TranslationUnexpectedValueNotice(
translation,
GtfsTranslationTableLoader.RECORD_SUB_ID_FIELD_NAME,
translation.recordSubId()));
translation, GtfsTranslation.RECORD_SUB_ID_FIELD_NAME, translation.recordSubId()));
}
}
Optional<GtfsTableContainer<?>> parentTable =
Expand All @@ -146,14 +141,14 @@ private void validateReferenceIntegrity(
translation,
translation.hasRecordId(),
keyColumnNames.size() >= 1,
GtfsTranslationTableLoader.RECORD_ID_FIELD_NAME,
GtfsTranslation.RECORD_ID_FIELD_NAME,
translation.recordId(),
noticeContainer)
|| isMissingOrUnexpectedField(
translation,
translation.hasRecordSubId(),
keyColumnNames.size() >= 2,
GtfsTranslationTableLoader.RECORD_SUB_ID_FIELD_NAME,
GtfsTranslation.RECORD_SUB_ID_FIELD_NAME,
translation.recordSubId(),
noticeContainer)) {
return;
Expand Down Expand Up @@ -187,7 +182,7 @@ private static boolean isMissingOrUnexpectedField(
} else {
noticeContainer.addValidationNotice(
new MissingRequiredFieldNotice(
GtfsTranslationTableLoader.FILENAME, translation.csvRowNumber(), fieldName));
GtfsTranslation.FILENAME, translation.csvRowNumber(), fieldName));
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.mobilitydata.gtfsvalidator.table.GtfsAgencyTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsRoute;
import org.mobilitydata.gtfsvalidator.table.GtfsRouteTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsRouteTableLoader;

/**
* Checks that agency_id field in "routes.txt" is defined for every row if there is more than 1
Expand Down Expand Up @@ -52,9 +51,7 @@ public void validate(NoticeContainer noticeContainer) {
if (!route.hasAgencyId()) {
noticeContainer.addValidationNotice(
new MissingRequiredFieldNotice(
routeTable.gtfsFilename(),
route.csvRowNumber(),
GtfsRouteTableLoader.AGENCY_ID_FIELD_NAME));
routeTable.gtfsFilename(), route.csvRowNumber(), GtfsRoute.AGENCY_ID_FIELD_NAME));
}
// No need to check reference integrity because it is done by a validator generated from
// @ForeignKey annotation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@
package org.mobilitydata.gtfsvalidator.validator;

import static com.google.common.truth.Truth.assertThat;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTableLoader.STOP_ID_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.ARRIVAL_TIME_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.CONTINUOUS_DROP_OFF_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.CONTINUOUS_PICKUP_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.DEPARTURE_TIME_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.DROP_OFF_TYPE_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.PICKUP_TYPE_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.SHAPE_DIST_TRAVELED_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.STOP_HEADSIGN_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.STOP_SEQUENCE_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.TIMEPOINT_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableLoader.TRIP_ID_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStop.STOP_ID_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.ARRIVAL_TIME_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.CONTINUOUS_DROP_OFF_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.CONTINUOUS_PICKUP_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.DEPARTURE_TIME_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.DROP_OFF_TYPE_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.PICKUP_TYPE_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.SHAPE_DIST_TRAVELED_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.STOP_HEADSIGN_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.STOP_SEQUENCE_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.TIMEPOINT_FIELD_NAME;
import static org.mobilitydata.gtfsvalidator.table.GtfsStopTime.TRIP_ID_FIELD_NAME;

import java.util.ArrayList;
import java.util.List;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ private static JavaFile generateValidator(GtfsFileDescriptor fileDescriptor) {
.addStatement("$T currency = entity.$L()", Currency.class, currencyField.name())
.beginControlFlow("if (amount.scale() != currency.getDefaultFractionDigits())")
.addStatement(
"noticeContainer.addValidationNotice(new $T(\"$L\", \"$L\", entity.csvRowNumber(), amount))",
"noticeContainer.addValidationNotice(new $T(\"$L\", \"$L\", entity.csvRowNumber(),"
+ " amount))",
InvalidCurrencyAmountNotice.class,
fileDescriptor.filename(),
amountField.name())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,19 +144,20 @@ private static CodeBlock generateNoticeContext(
GtfsFieldDescriptor startField,
GtfsFieldDescriptor endField,
StartEndRangeNoticeType noticeType) {
TypeName tableLoaderTypeName = new GtfsEntityClasses(fileDescriptor).tableLoaderTypeName();
TypeName gtfsEntityTypeName =
new GtfsEntityClasses(fileDescriptor).entityImplementationTypeName();
CodeBlock.Builder block =
CodeBlock.builder().add("$T.FILENAME, entity.csvRowNumber(), ", tableLoaderTypeName);
CodeBlock.builder().add("$T.FILENAME, entity.csvRowNumber(), ", gtfsEntityTypeName);
if (fileDescriptor.hasSingleColumnPrimaryKey()) {
block.add("entity.$L(), ", fileDescriptor.getSingleColumnPrimaryKey().name());
}
block.add("$T.$L, ", tableLoaderTypeName, FieldNameConverter.fieldNameField(startField.name()));
block.add("$T.$L, ", gtfsEntityTypeName, FieldNameConverter.fieldNameField(startField.name()));
if (noticeType.equals(StartEndRangeNoticeType.OUT_OF_ORDER)) {
block.add("entity.$L().toString(), ", startField.name());
}
block.add(
"$T.$L, entity.$L().toString()",
tableLoaderTypeName,
gtfsEntityTypeName,
FieldNameConverter.fieldNameField(endField.name()),
endField.name());
return block.build();
Expand Down
Loading

0 comments on commit 35bf825

Please sign in to comment.