Skip to content

Commit

Permalink
feat: Flex - Removed required field false positive for stop_id + adde…
Browse files Browse the repository at this point in the history
…d Foreign key violation for location_groups_id (#1834)

Removed required field false positive for stop_id + added Foreign key violation for location_groups_id (#1834)
  • Loading branch information
jcpitre authored Sep 13, 2024
1 parent 014bac0 commit cdcf921
Show file tree
Hide file tree
Showing 6 changed files with 240 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright 2024 MobilityData
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mobilitydata.gtfsvalidator.notice;

import static org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.SectionRef.FILE_REQUIREMENTS;
import static org.mobilitydata.gtfsvalidator.notice.SeverityLevel.ERROR;

import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice;
import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.SectionRefs;

/**
* A stop_time entry has more than one geographical id defined.
*
* <p>In stop_times.txt, you can have only one of stop_id, location_group_id or location_id defined
* for given entry.
*/
@GtfsValidationNotice(severity = ERROR, sections = @SectionRefs(FILE_REQUIREMENTS))
public class ForbiddenGeographyIdNotice extends ValidationNotice {

/** The row of the faulty record. */
private final int csvRowNumber;

/** The sThe id that already exists. */
private final String stopId;

/** The id that already exists. */
private final String locationGroupId;

/** The id that already exists. */
private final String locationId;

public ForbiddenGeographyIdNotice(
int csvRowNumber, String stopId, String locationGroupId, String locationId) {
this.csvRowNumber = csvRowNumber;
this.stopId = stopId;
this.locationGroupId = locationGroupId;
this.locationId = locationId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,17 @@ public interface GtfsStopTimeSchema extends GtfsEntity {

@FieldType(FieldTypeEnum.ID)
@Index
@Required
@ConditionallyRequired
@ForeignKey(table = "stops.txt", field = "stop_id")
String stopId();

@FieldType(FieldTypeEnum.ID)
@ConditionallyRequired
@ForeignKey(table = "location_groups.txt", field = "location_group_id")
String locationGroupId();

@FieldType(FieldTypeEnum.ID)
@ConditionallyRequired
String locationId();

@PrimaryKey(isSequenceUsedForSorting = true, translationRecordIdType = RECORD_SUB_ID)
Expand All @@ -67,6 +73,10 @@ public interface GtfsStopTimeSchema extends GtfsEntity {
@CachedField
String stopHeadsign();

GtfsTime startPickupDropOffWindow();

GtfsTime endPickupDropOffWindow();

GtfsPickupDropOff pickupType();

GtfsPickupDropOff dropOffType();
Expand All @@ -83,4 +93,12 @@ public interface GtfsStopTimeSchema extends GtfsEntity {
@DefaultValue("1")
@RecommendedColumn
GtfsStopTimeTimepoint timepoint();

@FieldType(FieldTypeEnum.ID)
@ForeignKey(table = "booking_rules.txt", field = "booking_rule_id")
String pickupBookingRuleId();

@FieldType(FieldTypeEnum.ID)
@ForeignKey(table = "booking_rules.txt", field = "booking_rule_id")
String dropOffBookingRuleId();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright 2024 MobilityData
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.mobilitydata.gtfsvalidator.validator;

import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator;
import org.mobilitydata.gtfsvalidator.notice.ForbiddenGeographyIdNotice;
import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice;
import org.mobilitydata.gtfsvalidator.notice.NoticeContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTime;

/**
* Validates that only one of stop_id, location_group_id or location_id is defined in a given record
* of stop_times.txt
*
* <p>Generated notice: {@link MissingRequiredFieldNotice}.
*
* <p>Generated notice: {@link ForbiddenGeographyIdNotice}.
*/
@GtfsValidator
public class StopTimesGeographyIdPresenceValidator extends SingleEntityValidator<GtfsStopTime> {

@Override
public void validate(GtfsStopTime stopTime, NoticeContainer noticeContainer) {
int presenceCount = 0;
if (stopTime.hasStopId()) {
presenceCount++;
}
if (stopTime.hasLocationGroupId()) {
presenceCount++;
}

if (stopTime.hasLocationId()) {
presenceCount++;
}

if (presenceCount == 0) {
// None of the 3 geography IDs are present, but we need at least stop_id
noticeContainer.addValidationNotice(
new MissingRequiredFieldNotice(
GtfsStopTime.FILENAME, stopTime.csvRowNumber(), GtfsStopTime.STOP_ID_FIELD_NAME));
} else if (presenceCount > 1) {
// More than one geography ID is present, but only one is allowed
noticeContainer.addValidationNotice(
new ForbiddenGeographyIdNotice(
stopTime.csvRowNumber(),
stopTime.stopId(),
stopTime.locationGroupId(),
stopTime.locationId()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ public void testNoticeClassFieldNames() {
"fileNameA",
"fileNameB",
"pathwayMode",
"isBidirectional");
"isBidirectional",
"locationGroupId",
"locationId");
}

private static List<String> discoverValidationNoticeFieldNames() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* Copyright 2020 Google LLC, MobilityData IO
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.mobilitydata.gtfsvalidator.validator;

import static com.google.common.truth.Truth.assertThat;

import java.util.List;
import org.junit.Test;
import org.mobilitydata.gtfsvalidator.notice.ForbiddenGeographyIdNotice;
import org.mobilitydata.gtfsvalidator.notice.MissingRequiredFieldNotice;
import org.mobilitydata.gtfsvalidator.notice.NoticeContainer;
import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTime;

public class StopTimesGeographyIdPresenceValidatorTest {

@Test
public void NoGeographyIdShouldGenerateMissingRequiredFieldNotice() {
assertThat(validationNoticesFor(new GtfsStopTime.Builder().setCsvRowNumber(2).build()))
.containsExactly(new MissingRequiredFieldNotice("stop_times.txt", 2, "stop_id"));
}

@Test
public void OneGeographyIdShouldGenerateNothing() {
assertThat(
validationNoticesFor(
new GtfsStopTime.Builder().setCsvRowNumber(2).setStopId("stop_id").build()))
.isEmpty();
assertThat(
validationNoticesFor(
new GtfsStopTime.Builder().setCsvRowNumber(2).setLocationGroupId("id").build()))
.isEmpty();
assertThat(
validationNoticesFor(
new GtfsStopTime.Builder().setCsvRowNumber(2).setLocationId("id").build()))
.isEmpty();
}

@Test
public void MultipleGeographyIdShouldGenerateNotice() {
assertThat(
validationNoticesFor(
new GtfsStopTime.Builder()
.setStopId("stop_id")
.setLocationGroupId("location_group_id")
.setCsvRowNumber(2)
.build()))
.containsExactly(new ForbiddenGeographyIdNotice(2, "stop_id", "location_group_id", ""));

assertThat(
validationNoticesFor(
new GtfsStopTime.Builder()
.setStopId("stop_id")
.setLocationId("location_id")
.setCsvRowNumber(2)
.build()))
.containsExactly(new ForbiddenGeographyIdNotice(2, "stop_id", "", "location_id"));

assertThat(
validationNoticesFor(
new GtfsStopTime.Builder()
.setLocationGroupId("location_group_id")
.setLocationId("location_id")
.setCsvRowNumber(2)
.build()))
.containsExactly(new ForbiddenGeographyIdNotice(2, "", "location_group_id", "location_id"));

assertThat(
validationNoticesFor(
new GtfsStopTime.Builder()
.setStopId("stop_id")
.setLocationGroupId("location_group_id")
.setLocationId("location_id")
.setCsvRowNumber(2)
.build()))
.containsExactly(
new ForbiddenGeographyIdNotice(2, "stop_id", "location_group_id", "location_id"));
}

private List<ValidationNotice> validationNoticesFor(GtfsStopTime entity) {
StopTimesGeographyIdPresenceValidator validator = new StopTimesGeographyIdPresenceValidator();
NoticeContainer noticeContainer = new NoticeContainer();
validator.validate(entity, noticeContainer);
return noticeContainer.getValidationNotices();
}
}
4 changes: 2 additions & 2 deletions scripts/queue_runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ do
ID=$(jq '.id' <<< "$item")
URL=$(jq '.url' <<< "$item")
path_name=${ID//\"/}
java -Xmx10G -Xms8G -jar gtfs-validator-snapshot/gtfs-validator*.jar --url $URL --output_base $OUTPUT_BASE/output/$path_name --validation_report_name latest.json --system_errors_report_name latest_errors.json --skip_validator_update
java -Xmx12G -Xms8G -jar gtfs-validator-snapshot/gtfs-validator*.jar --url $URL --output_base $OUTPUT_BASE/output/$path_name --validation_report_name latest.json --system_errors_report_name latest_errors.json --skip_validator_update
if [ "$master" = "--include-master" ];
then
java -Xmx10G -Xms8G -jar gtfs-validator-master/gtfs-validator*.jar --url $URL --output_base $OUTPUT_BASE/output/$path_name --validation_report_name reference.json --system_errors_report_name reference_errors.json --skip_validator_update
java -Xmx12G -Xms8G -jar gtfs-validator-master/gtfs-validator*.jar --url $URL --output_base $OUTPUT_BASE/output/$path_name --validation_report_name reference.json --system_errors_report_name reference_errors.json --skip_validator_update
fi;
wait
done

0 comments on commit cdcf921

Please sign in to comment.