From 34017dea4c2416548f2f6abab03b62f8565fc5eb Mon Sep 17 00:00:00 2001 From: Jingsi Lu Date: Thu, 10 Oct 2024 17:28:07 -0400 Subject: [PATCH 1/6] modified the validate method to create an exception for the rule --- .../LocationHasStopTimesValidator.java | 35 ++++++++--- .../LocationHasStopTimesValidatorTest.java | 60 +++++++++++++------ 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java index 26af7774e9..b3de5bca7a 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java @@ -18,20 +18,16 @@ import static org.mobilitydata.gtfsvalidator.notice.SeverityLevel.ERROR; import static org.mobilitydata.gtfsvalidator.notice.SeverityLevel.WARNING; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.inject.Inject; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.FileRefs; import org.mobilitydata.gtfsvalidator.annotation.GtfsValidator; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.notice.ValidationNotice; -import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; -import org.mobilitydata.gtfsvalidator.table.GtfsStop; -import org.mobilitydata.gtfsvalidator.table.GtfsStopSchema; -import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; -import org.mobilitydata.gtfsvalidator.table.GtfsStopTime; -import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeSchema; -import org.mobilitydata.gtfsvalidator.table.GtfsStopTimeTableContainer; +import org.mobilitydata.gtfsvalidator.table.*; /** * Checks that stops and only stops have stop times. @@ -48,19 +44,40 @@ public class LocationHasStopTimesValidator extends FileValidator { private final GtfsStopTimeTableContainer stopTimeTable; + private final GtfsLocationGroupStopsTableContainer locationGroupStopTable; + @Inject LocationHasStopTimesValidator( - GtfsStopTableContainer stopTable, GtfsStopTimeTableContainer stopTimeTable) { + GtfsStopTableContainer stopTable, + GtfsStopTimeTableContainer stopTimeTable, + GtfsLocationGroupStopsTableContainer locationGroupStopTable) { this.stopTable = stopTable; this.stopTimeTable = stopTimeTable; + this.locationGroupStopTable = locationGroupStopTable; } @Override public void validate(NoticeContainer noticeContainer) { + Set stopIdsInStopTimesandLocationGroupStops = new HashSet<>(); + Set locationGroupIdsInStopTimes = new HashSet<>(); + + for (GtfsStopTime stopTime : stopTimeTable.getEntities()) { + stopIdsInStopTimesandLocationGroupStops.add(stopTime.stopId()); + locationGroupIdsInStopTimes.add(stopTime.locationGroupId()); + } + + for (String locationGroupId : locationGroupIdsInStopTimes) { + for (GtfsLocationGroupStops locationGroupStop : locationGroupStopTable.getEntities()) { + if (locationGroupStop.locationGroupId().equals(locationGroupId)) { + stopIdsInStopTimesandLocationGroupStops.add(locationGroupStop.stopId()); + } + } + } + for (GtfsStop stop : stopTable.getEntities()) { List stopTimes = stopTimeTable.byStopId(stop.stopId()); if (stop.locationType().equals(GtfsLocationType.STOP)) { - if (stopTimes.isEmpty()) { + if (stopTimes.isEmpty() && !stopIdsInStopTimesandLocationGroupStops.contains(stop.stopId())) { noticeContainer.addValidationNotice(new StopWithoutStopTimeNotice(stop)); } } else if (!stopTimes.isEmpty()) { diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java index aaa72f13e9..c584b18712 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java @@ -28,11 +28,7 @@ import org.junit.runners.JUnit4; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; 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.*; import org.mobilitydata.gtfsvalidator.validator.LocationHasStopTimesValidator.LocationWithUnexpectedStopTimeNotice; import org.mobilitydata.gtfsvalidator.validator.LocationHasStopTimesValidator.StopWithoutStopTimeNotice; @@ -40,11 +36,14 @@ public final class LocationHasStopTimesValidatorTest { private static List generateNotices( - List stops, List stopTimes) { + List stops, + List stopTimes, + List locationGroupStops) { NoticeContainer noticeContainer = new NoticeContainer(); new LocationHasStopTimesValidator( GtfsStopTableContainer.forEntities(stops, noticeContainer), - GtfsStopTimeTableContainer.forEntities(stopTimes, noticeContainer)) + GtfsStopTimeTableContainer.forEntities(stopTimes, noticeContainer), + GtfsLocationGroupStopsTableContainer.forEntities(locationGroupStops, noticeContainer)) .validate(noticeContainer); return noticeContainer.getValidationNotices(); } @@ -62,45 +61,68 @@ private static GtfsStopTime createStopTimeFor(GtfsStop stop) { return new GtfsStopTime.Builder().setStopId(stop.stopId()).build(); } - @Test - public void stopWithStopTime_yieldsNoNotice() { - GtfsStop stop = createLocation(STOP); - assertThat(generateNotices(ImmutableList.of(stop), ImmutableList.of(createStopTimeFor(stop)))) - .isEmpty(); + private static GtfsLocationGroupStops createLocationGroupStops() { + return new GtfsLocationGroupStops.Builder() + .setCsvRowNumber(2) + .setLocationGroupId("locationGroupId") + .setStopId("stopId") + .build(); } @Test - public void unusedStop_yieldsNotice() { + public void stopWithStopTime_yieldsNoNotice() { GtfsStop stop = createLocation(STOP); - assertThat(generateNotices(ImmutableList.of(stop), ImmutableList.of())) - .containsExactly(new StopWithoutStopTimeNotice(stop)); + assertThat( + generateNotices( + ImmutableList.of(stop), + ImmutableList.of(createStopTimeFor(stop)), + ImmutableList.of(createLocationGroupStops()))) + .isEmpty(); } @Test public void stationWithStopTime_yieldsNotice() { GtfsStop location = createLocation(STATION); GtfsStopTime stopTime = createStopTimeFor(location); - assertThat(generateNotices(ImmutableList.of(location), ImmutableList.of(stopTime))) + assertThat( + generateNotices( + ImmutableList.of(location), + ImmutableList.of(stopTime), + ImmutableList.of(createLocationGroupStops()))) .containsExactly(new LocationWithUnexpectedStopTimeNotice(location, stopTime)); } @Test public void stationWithoutStopTime_yieldsNoNotice() { GtfsStop location = createLocation(STATION); - assertThat(generateNotices(ImmutableList.of(location), ImmutableList.of())).isEmpty(); + assertThat( + generateNotices( + ImmutableList.of(location), + ImmutableList.of(), + ImmutableList.of(createLocationGroupStops()))) + .isEmpty(); } @Test public void entranceWithStopTime_yieldsNotice() { GtfsStop location = createLocation(ENTRANCE); GtfsStopTime stopTime = createStopTimeFor(location); - assertThat(generateNotices(ImmutableList.of(location), ImmutableList.of(stopTime))) + assertThat( + generateNotices( + ImmutableList.of(location), + ImmutableList.of(stopTime), + ImmutableList.of(createLocationGroupStops()))) .containsExactly(new LocationWithUnexpectedStopTimeNotice(location, stopTime)); } @Test public void entranceWithoutStopTime_yieldsNoNotice() { GtfsStop location = createLocation(ENTRANCE); - assertThat(generateNotices(ImmutableList.of(location), ImmutableList.of())).isEmpty(); + assertThat( + generateNotices( + ImmutableList.of(location), + ImmutableList.of(), + ImmutableList.of(createLocationGroupStops()))) + .isEmpty(); } } From 3aee26f7755503660cc03a570484f53d48adc517 Mon Sep 17 00:00:00 2001 From: Jingsi Lu Date: Thu, 10 Oct 2024 17:42:14 -0400 Subject: [PATCH 2/6] changed from and to or --- .../gtfsvalidator/validator/LocationHasStopTimesValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java index b3de5bca7a..46a60add31 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java @@ -77,7 +77,7 @@ public void validate(NoticeContainer noticeContainer) { for (GtfsStop stop : stopTable.getEntities()) { List stopTimes = stopTimeTable.byStopId(stop.stopId()); if (stop.locationType().equals(GtfsLocationType.STOP)) { - if (stopTimes.isEmpty() && !stopIdsInStopTimesandLocationGroupStops.contains(stop.stopId())) { + if (stopTimes.isEmpty() || !stopIdsInStopTimesandLocationGroupStops.contains(stop.stopId())) { noticeContainer.addValidationNotice(new StopWithoutStopTimeNotice(stop)); } } else if (!stopTimes.isEmpty()) { From 918d11232502abeaf4a1ab7b44b0b75e89a57ecd Mon Sep 17 00:00:00 2001 From: Jingsi Lu Date: Fri, 11 Oct 2024 15:30:05 -0400 Subject: [PATCH 3/6] should be and --- .../validator/LocationHasStopTimesValidator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java index 46a60add31..5a7bdc4c45 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java @@ -77,8 +77,8 @@ public void validate(NoticeContainer noticeContainer) { for (GtfsStop stop : stopTable.getEntities()) { List stopTimes = stopTimeTable.byStopId(stop.stopId()); if (stop.locationType().equals(GtfsLocationType.STOP)) { - if (stopTimes.isEmpty() || !stopIdsInStopTimesandLocationGroupStops.contains(stop.stopId())) { - noticeContainer.addValidationNotice(new StopWithoutStopTimeNotice(stop)); + if (stopTimes.isEmpty() && !stopIdsInStopTimesandLocationGroupStops.contains(stop.stopId())) { + noticeContainer.addValidationNotice(new StopWithoutStopTimeNotice(stop)); } } else if (!stopTimes.isEmpty()) { noticeContainer.addValidationNotice( From a18bf57963c97d82a31760a661b37e35eb95cfc9 Mon Sep 17 00:00:00 2001 From: Jingsi Lu Date: Fri, 11 Oct 2024 15:55:25 -0400 Subject: [PATCH 4/6] updated class comments and added tests --- .../validator/LocationHasStopTimesValidator.java | 9 ++++++--- .../LocationHasStopTimesValidatorTest.java | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java index 5a7bdc4c45..c61158d0b1 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java @@ -34,7 +34,9 @@ * *
    *
  • every stop (or platform) should have stop times; - *
  • every non-stop location (station, entrance etc) may not have stop times. + *
  • every non-stop location (station, entrance etc) may not have stop times; + *
  • if a stop is part of a location group referenced in stop_times.txt, it should not trigger a + * warning. *
*/ @GtfsValidator @@ -77,8 +79,9 @@ public void validate(NoticeContainer noticeContainer) { for (GtfsStop stop : stopTable.getEntities()) { List stopTimes = stopTimeTable.byStopId(stop.stopId()); if (stop.locationType().equals(GtfsLocationType.STOP)) { - if (stopTimes.isEmpty() && !stopIdsInStopTimesandLocationGroupStops.contains(stop.stopId())) { - noticeContainer.addValidationNotice(new StopWithoutStopTimeNotice(stop)); + if (stopTimes.isEmpty() + && !stopIdsInStopTimesandLocationGroupStops.contains(stop.stopId())) { + noticeContainer.addValidationNotice(new StopWithoutStopTimeNotice(stop)); } } else if (!stopTimes.isEmpty()) { noticeContainer.addValidationNotice( diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java index c584b18712..1b16db18ec 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java @@ -48,15 +48,19 @@ private static List generateNotices( return noticeContainer.getValidationNotices(); } - private static GtfsStop createLocation(GtfsLocationType locationType) { + private static GtfsStop createLocation(GtfsLocationType locationType, String stopId) { return new GtfsStop.Builder() .setCsvRowNumber(2) - .setStopId("location1") + .setStopId(stopId) .setStopName("Location 1") .setLocationType(locationType) .build(); } + private static GtfsStop createLocation(GtfsLocationType locationType) { + return createLocation(locationType, "location1"); + } + private static GtfsStopTime createStopTimeFor(GtfsStop stop) { return new GtfsStopTime.Builder().setStopId(stop.stopId()).build(); } @@ -80,6 +84,13 @@ public void stopWithStopTime_yieldsNoNotice() { .isEmpty(); } + @Test + public void stopWithoutStopTime_yieldsNotice() { + GtfsStop location = createLocation(STOP, "stopId"); + assertThat(generateNotices(ImmutableList.of(location), ImmutableList.of(), ImmutableList.of())) + .containsExactly(new StopWithoutStopTimeNotice(location)); + } + @Test public void stationWithStopTime_yieldsNotice() { GtfsStop location = createLocation(STATION); From 74ffef9d77fd0d29129a99435fb8d1534187f96a Mon Sep 17 00:00:00 2001 From: Jingsi Lu Date: Thu, 17 Oct 2024 15:30:17 -0400 Subject: [PATCH 5/6] optimized code --- .../validator/LocationHasStopTimesValidator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java index c61158d0b1..f0fd871fc8 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java @@ -69,10 +69,10 @@ public void validate(NoticeContainer noticeContainer) { } for (String locationGroupId : locationGroupIdsInStopTimes) { - for (GtfsLocationGroupStops locationGroupStop : locationGroupStopTable.getEntities()) { - if (locationGroupStop.locationGroupId().equals(locationGroupId)) { - stopIdsInStopTimesandLocationGroupStops.add(locationGroupStop.stopId()); - } + List locationGroupStops = + locationGroupStopTable.byLocationGroupId(locationGroupId); + for (var locationGroupStop : locationGroupStops) { + stopIdsInStopTimesandLocationGroupStops.add(locationGroupStop.stopId()); } } From bb76519ebbcf79ed867c1fbab48cac1b4191faad Mon Sep 17 00:00:00 2001 From: Jingsi Lu Date: Thu, 17 Oct 2024 15:55:01 -0400 Subject: [PATCH 6/6] added null checks --- .../validator/LocationHasStopTimesValidator.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java index f0fd871fc8..282faa293d 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java @@ -64,8 +64,12 @@ public void validate(NoticeContainer noticeContainer) { Set locationGroupIdsInStopTimes = new HashSet<>(); for (GtfsStopTime stopTime : stopTimeTable.getEntities()) { - stopIdsInStopTimesandLocationGroupStops.add(stopTime.stopId()); - locationGroupIdsInStopTimes.add(stopTime.locationGroupId()); + if (!stopTime.stopId().isEmpty()) { + stopIdsInStopTimesandLocationGroupStops.add(stopTime.stopId()); + } + if (!stopTime.locationGroupId().isEmpty()) { + locationGroupIdsInStopTimes.add(stopTime.locationGroupId()); + } } for (String locationGroupId : locationGroupIdsInStopTimes) {