Add a matcher API for filters in the transit service used for regular…

…Stop lookup (#6234) * Implements a RegularStop matcher that can be used for filtering RegularStops. Also moves the envelope filtering that used to happen on the client side of the findRegularStops call into the StopModelIndex. * Updates the RegularStopRequest to latest concepts. * Adds @nullable to feedId method on the RegularStopRequest. * Changes feedId to be called agency. * Update application/src/test/java/org/opentripplanner/transit/model/filter/transit/RegularStopMatcherFactoryTest.java Co-authored-by: Leonard Ehrenfried <mail@leonard.io> * Fixes test failure. * Addresses comments in code review. * Update application/src/main/java/org/opentripplanner/transit/api/request/FindRegularStopsByBoundingBoxRequest.java Co-authored-by: Thomas Gran <t2gran@gmail.com> * Update application/src/main/java/org/opentripplanner/transit/api/request/FindRegularStopsByBoundingBoxRequestBuilder.java Co-authored-by: Thomas Gran <t2gran@gmail.com> * Update application/src/main/java/org/opentripplanner/transit/model/filter/transit/RegularStopMatcherFactory.java Co-authored-by: Thomas Gran <t2gran@gmail.com> * Update application/src/main/java/org/opentripplanner/transit/model/filter/transit/RegularStopMatcherFactory.java Co-authored-by: Thomas Gran <t2gran@gmail.com> * Update application/src/main/java/org/opentripplanner/transit/service/TransitService.java Co-authored-by: Thomas Gran <t2gran@gmail.com> * Update application/src/main/java/org/opentripplanner/transit/service/TransitService.java Co-authored-by: Thomas Gran <t2gran@gmail.com> * Addresses comments in code review. --------- Co-authored-by: Leonard Ehrenfried <mail@leonard.io> Co-authored-by: Thomas Gran <t2gran@gmail.com>
opentripplanner · Jan 7, 2025 · 72019c0 · 72019c0
1 parent d61504a
commit 72019c0
Show file tree

Hide file tree

Showing 28 changed files with 313 additions and 61 deletions.
diff --git a/application/src/ext/java/org/opentripplanner/ext/parkAndRideApi/ParkAndRideResource.java b/application/src/ext/java/org/opentripplanner/ext/parkAndRideApi/ParkAndRideResource.java
@@ -42,7 +42,7 @@ public ParkAndRideResource(
     //           - serverContext.graphFinder(). This needs at least a comment!
     //           - This can be replaced with a search done with the SiteRepository
     //           - if we have a radius search there.
-    this.graphFinder = new DirectGraphFinder(serverContext.transitService()::findRegularStops);
+    this.graphFinder = new DirectGraphFinder(serverContext.transitService()::findRegularStopsByBoundingBox);
   }
 
   /** Envelopes are in latitude, longitude format */

diff --git a/application/src/ext/java/org/opentripplanner/ext/restapi/resources/IndexAPI.java b/application/src/ext/java/org/opentripplanner/ext/restapi/resources/IndexAPI.java
@@ -200,7 +200,7 @@ public List<ApiStopShort> getStopsInRadius(
 
       radius = Math.min(radius, MAX_STOP_SEARCH_RADIUS);
 
-      return new DirectGraphFinder(serverContext.transitService()::findRegularStops)
+      return new DirectGraphFinder(serverContext.transitService()::findRegularStopsByBoundingBox)
         .findClosestStops(new Coordinate(lon, lat), radius)
         .stream()
         .map(it -> StopMapper.mapToApiShort(it.stop, it.distance))
@@ -221,10 +221,9 @@ public List<ApiStopShort> getStopsInRadius(
         new Coordinate(maxLon, maxLat)
       );
 
-      var stops = transitService().findRegularStops(envelope);
+      var stops = transitService().findRegularStopsByBoundingBox(envelope);
       return stops
         .stream()
-        .filter(stop -> envelope.contains(stop.getCoordinate().asJtsCoordinate()))
         .map(StopMapper::mapToApiShort)
         .toList();
     }

diff --git a/...tion/src/ext/java/org/opentripplanner/ext/vectortiles/layers/stops/StopsLayerBuilder.java b/...tion/src/ext/java/org/opentripplanner/ext/vectortiles/layers/stops/StopsLayerBuilder.java
@@ -44,7 +44,7 @@ public StopsLayerBuilder(
 
   protected List<Geometry> getGeometries(Envelope query) {
     return transitService
-      .findRegularStops(query)
+      .findRegularStopsByBoundingBox(query)
       .stream()
       .filter(filter)
       .map(stop -> {

diff --git a/application/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java b/application/src/main/java/org/opentripplanner/apis/gtfs/datafetchers/QueryTypeImpl.java
@@ -752,9 +752,8 @@ public DataFetcher<Iterable<Object>> stopsByBbox() {
       );
 
       Stream<RegularStop> stopStream = getTransitService(environment)
-        .findRegularStops(envelope)
-        .stream()
-        .filter(stop -> envelope.contains(stop.getCoordinate().asJtsCoordinate()));
+        .findRegularStopsByBoundingBox(envelope)
+        .stream();
 
       if (args.getGraphQLFeeds() != null) {
         List<String> feedIds = args.getGraphQLFeeds();

diff --git a/application/src/main/java/org/opentripplanner/apis/transmodel/TransmodelGraphQLSchema.java b/application/src/main/java/org/opentripplanner/apis/transmodel/TransmodelGraphQLSchema.java
@@ -3,6 +3,7 @@
 import static java.lang.Boolean.TRUE;
 import static java.util.Collections.emptyList;
 import static org.opentripplanner.apis.transmodel.mapping.SeverityMapper.getTransmodelSeverity;
+import static org.opentripplanner.apis.transmodel.mapping.TransitIdMapper.mapIDToDomain;
 import static org.opentripplanner.apis.transmodel.mapping.TransitIdMapper.mapIDsToDomainNullSafe;
 import static org.opentripplanner.apis.transmodel.model.EnumTypes.FILTER_PLACE_TYPE_ENUM;
 import static org.opentripplanner.apis.transmodel.model.EnumTypes.MULTI_MODAL_MODE;
@@ -115,6 +116,7 @@
 import org.opentripplanner.routing.graphfinder.PlaceType;
 import org.opentripplanner.service.vehiclerental.model.VehicleRentalPlace;
 import org.opentripplanner.transit.api.model.FilterValues;
+import org.opentripplanner.transit.api.request.FindRegularStopsByBoundingBoxRequest;
 import org.opentripplanner.transit.api.request.TripRequest;
 import org.opentripplanner.transit.model.basic.TransitMode;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
@@ -439,10 +441,7 @@ private GraphQLSchema create() {
               .build()
           )
           .dataFetcher(env ->
-            StopPlaceType.fetchStopPlaceById(
-              TransitIdMapper.mapIDToDomain(env.getArgument("id")),
-              env
-            )
+            StopPlaceType.fetchStopPlaceById(mapIDToDomain(env.getArgument("id")), env)
           )
           .build()
       )
@@ -576,7 +575,7 @@ private GraphQLSchema create() {
           .dataFetcher(environment ->
             GqlUtil
               .getTransitService(environment)
-              .getStopLocation(TransitIdMapper.mapIDToDomain(environment.getArgument("id")))
+              .getStopLocation(mapIDToDomain(environment.getArgument("id")))
           )
           .build()
       )
@@ -610,7 +609,7 @@ private GraphQLSchema create() {
               }
               TransitService transitService = GqlUtil.getTransitService(environment);
               return ((List<String>) environment.getArgument("ids")).stream()
-                .map(id -> transitService.getStopLocation(TransitIdMapper.mapIDToDomain(id)))
+                .map(id -> transitService.getStopLocation(mapIDToDomain(id)))
                 .collect(Collectors.toList());
             }
             if (environment.getArgument("name") == null) {
@@ -661,15 +660,22 @@ private GraphQLSchema create() {
               .build()
           )
           .argument(
-            GraphQLArgument.newArgument().name("authority").type(Scalars.GraphQLString).build()
+            GraphQLArgument
+              .newArgument()
+              .name("authority")
+              .deprecate(
+                "This is the Transmodel namespace or the GTFS feedID - avoid using this. Request a new field if necessary."
+              )
+              .type(Scalars.GraphQLString)
+              .build()
           )
           .argument(
             GraphQLArgument
               .newArgument()
               .name("filterByInUse")
               .description("If true only quays with at least one visiting line are included.")
               .type(Scalars.GraphQLBoolean)
-              .defaultValue(Boolean.FALSE)
+              .defaultValueProgrammatic(Boolean.FALSE)
               .build()
           )
           .dataFetcher(environment -> {
@@ -683,24 +689,19 @@ private GraphQLSchema create() {
                 environment.getArgument("maximumLatitude")
               )
             );
+
+            var authority = environment.<String>getArgument("authority");
+            var filterInUse = environment.<Boolean>getArgument("filterByInUse");
+
+            FindRegularStopsByBoundingBoxRequest findRegularStopsByBoundingBoxRequest = FindRegularStopsByBoundingBoxRequest
+              .of(envelope)
+              .withFeedId(authority)
+              .filterByInUse(filterInUse)
+              .build();
+
             return GqlUtil
               .getTransitService(environment)
-              .findRegularStops(envelope)
-              .stream()
-              .filter(stop -> envelope.contains(stop.getCoordinate().asJtsCoordinate()))
-              .filter(stop ->
-                environment.getArgument("authority") == null ||
-                stop.getId().getFeedId().equalsIgnoreCase(environment.getArgument("authority"))
-              )
-              .filter(stop -> {
-                boolean filterByInUse = TRUE.equals(environment.getArgument("filterByInUse"));
-                boolean inUse = !GqlUtil
-                  .getTransitService(environment)
-                  .findPatterns(stop, true)
-                  .isEmpty();
-                return !filterByInUse || inUse;
-              })
-              .collect(Collectors.toList());
+              .findRegularStopsByBoundingBox(findRegularStopsByBoundingBoxRequest);
           })
           .build()
       )
@@ -1438,7 +1439,7 @@ private GraphQLSchema create() {
               .build()
           )
           .dataFetcher(environment -> {
-            var bikeParkId = TransitIdMapper.mapIDToDomain(environment.getArgument("id"));
+            var bikeParkId = mapIDToDomain(environment.getArgument("id"));
             return GqlUtil
               .getVehicleParkingService(environment)
               .listBikeParks()
@@ -1573,7 +1574,7 @@ private GraphQLSchema create() {
             return GqlUtil
               .getTransitService(environment)
               .getTransitAlertService()
-              .getAlertById(TransitIdMapper.mapIDToDomain(situationNumber));
+              .getAlertById(mapIDToDomain(situationNumber));
           })
           .build()
       )

diff --git a/application/src/main/java/org/opentripplanner/apis/transmodel/model/stop/StopPlaceType.java b/application/src/main/java/org/opentripplanner/apis/transmodel/model/stop/StopPlaceType.java
@@ -546,9 +546,8 @@ public static Collection<MonoOrMultiModalStation> fetchStopPlaces(
     );
 
     Stream<Station> stations = transitService
-      .findRegularStops(envelope)
+      .findRegularStopsByBoundingBox(envelope)
       .stream()
-      .filter(stop -> envelope.contains(stop.getCoordinate().asJtsCoordinate()))
       .map(StopLocation::getParentStation)
       .filter(Objects::nonNull)
       .distinct();

diff --git a/.../src/main/java/org/opentripplanner/apis/vectortiles/GraphInspectorVectorTileResource.java b/.../src/main/java/org/opentripplanner/apis/vectortiles/GraphInspectorVectorTileResource.java
@@ -183,7 +183,7 @@ private static LayerBuilder<?> createLayerBuilder(
       case RegularStop -> new StopLayerBuilder<>(
         layerParameters,
         locale,
-        e -> context.transitService().findRegularStops(e)
+        e -> context.transitService().findRegularStopsByBoundingBox(e)
       );
       case AreaStop -> new StopLayerBuilder<>(
         layerParameters,

diff --git a/...va/org/opentripplanner/graph_builder/module/nearbystops/StraightLineNearbyStopFinder.java b/...va/org/opentripplanner/graph_builder/module/nearbystops/StraightLineNearbyStopFinder.java
@@ -22,7 +22,7 @@ public StraightLineNearbyStopFinder(TransitService transitService, Duration dura
     // We need to accommodate straight line distance (in meters) but when streets are present we
     // use an earliest arrival search, which optimizes on time. Ideally we'd specify in meters,
     // but we don't have much of a choice here. Use the default walking speed to convert.
-    this.directGraphFinder = new DirectGraphFinder(transitService::findRegularStops);
+    this.directGraphFinder = new DirectGraphFinder(transitService::findRegularStopsByBoundingBox);
   }
 
   /**

diff --git a/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java b/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java
@@ -121,7 +121,7 @@ public interface OtpServerRequestContext {
   TraverseVisitor<State, Edge> traverseVisitor();
 
   default GraphFinder graphFinder() {
-    return GraphFinder.getInstance(graph(), transitService()::findRegularStops);
+    return GraphFinder.getInstance(graph(), transitService()::findRegularStopsByBoundingBox);
   }
 
   FlexParameters flexParameters();

diff --git a/application/src/main/java/org/opentripplanner/transit/api/model/FilterValues.java b/application/src/main/java/org/opentripplanner/transit/api/model/FilterValues.java
@@ -1,8 +1,8 @@
 package org.opentripplanner.transit.api.model;
 
-import com.beust.jcommander.internal.Nullable;
 import java.util.Collection;
 import java.util.NoSuchElementException;
+import javax.annotation.Nullable;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.service.TransitService;
 

diff --git a/...in/java/org/opentripplanner/transit/api/request/FindRegularStopsByBoundingBoxRequest.java b/...in/java/org/opentripplanner/transit/api/request/FindRegularStopsByBoundingBoxRequest.java
@@ -0,0 +1,48 @@
+package org.opentripplanner.transit.api.request;
+
+import javax.annotation.Nullable;
+import org.locationtech.jts.geom.Envelope;
+import org.opentripplanner.transit.model.site.RegularStop;
+
+/**
+ * A request for {@link RegularStop}s within a bounding box.
+ * <p/>
+ * This request is used to retrieve {@link RegularStop}s that are within a provided bounding box and
+ * match the other criteria.
+ */
+public class FindRegularStopsByBoundingBoxRequest {
+
+  private final Envelope envelope;
+
+  @Nullable
+  private final String feedId;
+
+  private final boolean filterByInUse;
+
+  FindRegularStopsByBoundingBoxRequest(
+    Envelope envelope,
+    @Nullable String feedId,
+    boolean filterByInUse
+  ) {
+    this.envelope = envelope;
+    this.feedId = feedId;
+    this.filterByInUse = filterByInUse;
+  }
+
+  public static FindRegularStopsByBoundingBoxRequestBuilder of(Envelope envelope) {
+    return new FindRegularStopsByBoundingBoxRequestBuilder(envelope);
+  }
+
+  public Envelope envelope() {
+    return envelope;
+  }
+
+  @Nullable
+  public String feedId() {
+    return feedId;
+  }
+
+  public boolean filterByInUse() {
+    return filterByInUse;
+  }
+}
diff --git a/.../org/opentripplanner/transit/api/request/FindRegularStopsByBoundingBoxRequestBuilder.java b/.../org/opentripplanner/transit/api/request/FindRegularStopsByBoundingBoxRequestBuilder.java
@@ -0,0 +1,32 @@
+package org.opentripplanner.transit.api.request;
+
+import javax.annotation.Nullable;
+import org.locationtech.jts.geom.Envelope;
+
+public class FindRegularStopsByBoundingBoxRequestBuilder {
+
+  private final Envelope envelope;
+
+  @Nullable
+  private String feedId;
+
+  private boolean filterByInUse = false;
+
+  FindRegularStopsByBoundingBoxRequestBuilder(Envelope envelope) {
+    this.envelope = envelope;
+  }
+
+  public FindRegularStopsByBoundingBoxRequestBuilder withFeedId(@Nullable String feedId) {
+    this.feedId = feedId;
+    return this;
+  }
+
+  public FindRegularStopsByBoundingBoxRequestBuilder filterByInUse(boolean filterByInUse) {
+    this.filterByInUse = filterByInUse;
+    return this;
+  }
+
+  public FindRegularStopsByBoundingBoxRequest build() {
+    return new FindRegularStopsByBoundingBoxRequest(envelope, feedId, filterByInUse);
+  }
+}
diff --git a/...ation/src/main/java/org/opentripplanner/transit/api/request/TripOnServiceDateRequest.java b/...ation/src/main/java/org/opentripplanner/transit/api/request/TripOnServiceDateRequest.java
@@ -23,7 +23,7 @@ public class TripOnServiceDateRequest {
   private final FilterValues<String> netexInternalPlanningCodes;
   private final FilterValues<TripAlteration> alterations;
 
-  protected TripOnServiceDateRequest(
+  TripOnServiceDateRequest(
     RequiredFilterValues<LocalDate> serviceDates,
     FilterValues<FeedScopedId> agencies,
     FilterValues<FeedScopedId> routes,
@@ -41,7 +41,7 @@ protected TripOnServiceDateRequest(
     this.alterations = alterations;
   }
 
-  public static TripOnServiceDateRequestBuilder of(RequiredFilterValues serviceDates) {
+  public static TripOnServiceDateRequestBuilder of(RequiredFilterValues<LocalDate> serviceDates) {
     return new TripOnServiceDateRequestBuilder(serviceDates);
   }
 

diff --git a/...rc/main/java/org/opentripplanner/transit/api/request/TripOnServiceDateRequestBuilder.java b/...rc/main/java/org/opentripplanner/transit/api/request/TripOnServiceDateRequestBuilder.java
@@ -30,9 +30,9 @@ public class TripOnServiceDateRequestBuilder {
     "alterations",
     List.of()
   );
-  private RequiredFilterValues<LocalDate> serviceDates;
+  private final RequiredFilterValues<LocalDate> serviceDates;
 
-  protected TripOnServiceDateRequestBuilder(RequiredFilterValues serviceDates) {
+  TripOnServiceDateRequestBuilder(RequiredFilterValues<LocalDate> serviceDates) {
     this.serviceDates = serviceDates;
   }
 

diff --git a/application/src/main/java/org/opentripplanner/transit/api/request/TripRequest.java b/application/src/main/java/org/opentripplanner/transit/api/request/TripRequest.java
@@ -17,7 +17,7 @@ public class TripRequest {
   private final FilterValues<String> netexInternalPlanningCodes;
   private final FilterValues<LocalDate> serviceDates;
 
-  protected TripRequest(
+  TripRequest(
     FilterValues<FeedScopedId> agencies,
     FilterValues<FeedScopedId> routes,
     FilterValues<String> netexInternalPlanningCodes,

diff --git a/application/src/main/java/org/opentripplanner/transit/api/request/TripRequestBuilder.java b/application/src/main/java/org/opentripplanner/transit/api/request/TripRequestBuilder.java
@@ -21,7 +21,7 @@ public class TripRequestBuilder {
     List.of()
   );
 
-  protected TripRequestBuilder() {}
+  TripRequestBuilder() {}
 
   public TripRequestBuilder withAgencies(FilterValues<FeedScopedId> agencies) {
     this.agencies = agencies;

diff --git a/...cation/src/main/java/org/opentripplanner/transit/model/filter/expr/ExpressionBuilder.java b/...cation/src/main/java/org/opentripplanner/transit/model/filter/expr/ExpressionBuilder.java
@@ -22,6 +22,11 @@ public static <T> ExpressionBuilder<T> of() {
     return new ExpressionBuilder<>();
   }
 
+  public ExpressionBuilder<T> matches(Matcher<T> matcher) {
+    matchers.add(matcher);
+    return this;
+  }
+
   public <V> ExpressionBuilder<T> atLeastOneMatch(
     FilterValues<V> filterValues,
     Function<V, Matcher<T>> matcherProvider

diff --git a/...tion/src/main/java/org/opentripplanner/transit/model/filter/expr/GenericUnaryMatcher.java b/...tion/src/main/java/org/opentripplanner/transit/model/filter/expr/GenericUnaryMatcher.java
@@ -0,0 +1,33 @@
+package org.opentripplanner.transit.model.filter.expr;
+
+import java.util.function.Predicate;
+
+/**
+ * A generic matcher that takes a predicate function that returns a boolean given the matched type.
+ * <p/>
+ * @param <T> The type of the entity being matched.
+ */
+public class GenericUnaryMatcher<T> implements Matcher<T> {
+
+  private final String typeName;
+  private final Predicate<T> matchPredicate;
+
+  /**
+   * @param typeName The typeName appears in the toString for easier debugging.
+   * @param matchPredicate The predicate that will be used to test the entity being matched.
+   */
+  public GenericUnaryMatcher(String typeName, Predicate<T> matchPredicate) {
+    this.typeName = typeName;
+    this.matchPredicate = matchPredicate;
+  }
+
+  @Override
+  public boolean match(T entity) {
+    return matchPredicate.test(entity);
+  }
+
+  @Override
+  public String toString() {
+    return "GenericUnaryMatcher: " + typeName;
+  }
+}