feat: 1883 flex modify stop without stop time to exclude stops associ…

…ated with a used location group (#1887)

* modified the validate method to create an exception for the rule

* changed from and to or

* should be and

* updated class comments and added tests

* optimized code

* added null checks

main/src/main/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidator.java

@@ -18,27 +18,25 @@
 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.
  *
  * <ul>
  *   <li>every stop (or platform) should have stop times;
- *   <li>every non-stop location (station, entrance etc) may not have stop times.
+ *   <li>every non-stop location (station, entrance etc) may not have stop times;
+ *   <li>if a stop is part of a location group referenced in stop_times.txt, it should not trigger a
+ *       warning.
  * </ul>
  */
 @GtfsValidator
@@ -48,19 +46,45 @@ 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<String> stopIdsInStopTimesandLocationGroupStops = new HashSet<>();
+    Set<String> locationGroupIdsInStopTimes = new HashSet<>();
+
+    for (GtfsStopTime stopTime : stopTimeTable.getEntities()) {
+      if (!stopTime.stopId().isEmpty()) {
+        stopIdsInStopTimesandLocationGroupStops.add(stopTime.stopId());
+      }
+      if (!stopTime.locationGroupId().isEmpty()) {
+        locationGroupIdsInStopTimes.add(stopTime.locationGroupId());
+      }
+    }
+
+    for (String locationGroupId : locationGroupIdsInStopTimes) {
+      List<GtfsLocationGroupStops> locationGroupStops =
+          locationGroupStopTable.byLocationGroupId(locationGroupId);
+      for (var locationGroupStop : locationGroupStops) {
+        stopIdsInStopTimesandLocationGroupStops.add(locationGroupStop.stopId());
+      }
+    }
+
     for (GtfsStop stop : stopTable.getEntities()) {
       List<GtfsStopTime> 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()) {

main/src/test/java/org/mobilitydata/gtfsvalidator/validator/LocationHasStopTimesValidatorTest.java

@@ -28,79 +28,112 @@
 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;
 
 @RunWith(JUnit4.class)
 public final class LocationHasStopTimesValidatorTest {
 
   private static List<ValidationNotice> generateNotices(
-      List<GtfsStop> stops, List<GtfsStopTime> stopTimes) {
+      List<GtfsStop> stops,
+      List<GtfsStopTime> stopTimes,
+      List<GtfsLocationGroupStops> 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();
   }
 
-  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();
   }
 
+  private static GtfsLocationGroupStops createLocationGroupStops() {
+    return new GtfsLocationGroupStops.Builder()
+        .setCsvRowNumber(2)
+        .setLocationGroupId("locationGroupId")
+        .setStopId("stopId")
+        .build();
+  }
+
   @Test
   public void stopWithStopTime_yieldsNoNotice() {
     GtfsStop stop = createLocation(STOP);
-    assertThat(generateNotices(ImmutableList.of(stop), ImmutableList.of(createStopTimeFor(stop))))
+    assertThat(
+            generateNotices(
+                ImmutableList.of(stop),
+                ImmutableList.of(createStopTimeFor(stop)),
+                ImmutableList.of(createLocationGroupStops())))
         .isEmpty();
   }
 
   @Test
-  public void unusedStop_yieldsNotice() {
-    GtfsStop stop = createLocation(STOP);
-    assertThat(generateNotices(ImmutableList.of(stop), ImmutableList.of()))
-        .containsExactly(new StopWithoutStopTimeNotice(stop));
+  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);
     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();
   }
 }