Cluster consolidated stops in geocoder

pom.xml

@@ -690,6 +690,12 @@
             <version>${junit.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.google.truth</groupId>
+            <artifactId>truth</artifactId>
+            <version>1.4.2</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>com.tngtech.archunit</groupId>
             <artifactId>archunit</artifactId>

src/ext-test/java/org/opentripplanner/ext/geocoder/LuceneIndexTest.java

@@ -18,6 +18,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
+import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationRepository;
+import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationService;
 import org.opentripplanner.model.FeedInfo;
 import org.opentripplanner.transit.model._data.TransitModelForTest;
 import org.opentripplanner.transit.model.basic.TransitMode;
@@ -160,8 +162,12 @@ public FeedInfo getFeedInfo(String feedId) {
         );
       }
     };
-    index = new LuceneIndex(transitService);
-    mapper = new StopClusterMapper(transitService);
+    var stopConsolidationService = new DefaultStopConsolidationService(
+      new DefaultStopConsolidationRepository(),
+      transitModel
+    );
+    index = new LuceneIndex(transitService, stopConsolidationService);
+    mapper = new StopClusterMapper(transitService, stopConsolidationService);
   }
 
   @Test

src/ext-test/java/org/opentripplanner/ext/geocoder/StopClusterMapperTest.java

@@ -0,0 +1,57 @@
+package org.opentripplanner.ext.geocoder;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationRepository;
+import org.opentripplanner.ext.stopconsolidation.internal.DefaultStopConsolidationService;
+import org.opentripplanner.ext.stopconsolidation.model.ConsolidatedStopGroup;
+import org.opentripplanner.transit.model._data.TransitModelForTest;
+import org.opentripplanner.transit.model.framework.Deduplicator;
+import org.opentripplanner.transit.model.site.RegularStop;
+import org.opentripplanner.transit.model.site.StopLocation;
+import org.opentripplanner.transit.service.DefaultTransitService;
+import org.opentripplanner.transit.service.StopModel;
+import org.opentripplanner.transit.service.TransitModel;
+
+class StopClusterMapperTest {
+
+  private static final TransitModelForTest TEST_MODEL = TransitModelForTest.of();
+  private static final RegularStop STOP_A = TEST_MODEL.stop("A").build();
+  private static final RegularStop STOP_B = TEST_MODEL.stop("B").build();
+  private static final RegularStop STOP_C = TEST_MODEL.stop("C").build();
+  private static final List<RegularStop> STOPS = List.of(STOP_A, STOP_B, STOP_C);
+  private static final StopModel STOP_MODEL = TEST_MODEL
+    .stopModelBuilder()
+    .withRegularStops(STOPS)
+    .build();
+  private static final TransitModel TRANSIT_MODEL = new TransitModel(
+    STOP_MODEL,
+    new Deduplicator()
+  );
+  private static final List<StopLocation> LOCATIONS = STOPS
+    .stream()
+    .map(StopLocation.class::cast)
+    .toList();
+
+  @Test
+  void clusterConsolidatedStops() {
+    var repo = new DefaultStopConsolidationRepository();
+    repo.addGroups(List.of(new ConsolidatedStopGroup(STOP_A.getId(), List.of(STOP_B.getId()))));
+
+    var service = new DefaultStopConsolidationService(repo, TRANSIT_MODEL);
+    var mapper = new StopClusterMapper(new DefaultTransitService(TRANSIT_MODEL), service);
+
+    var clusters = mapper.generateStopClusters(LOCATIONS, List.of());
+
+    var expected = new LuceneStopCluster(
+      STOP_A.getId().toString(),
+      List.of(STOP_B.getId().toString()),
+      List.of(STOP_A.getName(), STOP_B.getName()),
+      List.of(STOP_A.getCode(), STOP_B.getCode()),
+      new StopCluster.Coordinate(STOP_A.getLat(), STOP_A.getLon())
+    );
+    assertThat(clusters).contains(expected);
+  }
+}

src/ext/java/org/opentripplanner/ext/geocoder/GeocoderResource.java

@@ -25,10 +25,10 @@
 @Produces(MediaType.APPLICATION_JSON)
 public class GeocoderResource {
 
-  private final OtpServerRequestContext serverContext;
+  private final LuceneIndex luceneIndex;
 
   public GeocoderResource(@Context OtpServerRequestContext requestContext) {
-    serverContext = requestContext;
+    luceneIndex = requestContext.lucenceIndex();
   }
 
   /**
@@ -71,7 +71,7 @@ public Response textSearch(
   @GET
   @Path("stopClusters")
   public Response stopClusters(@QueryParam("query") String query) {
-    var clusters = LuceneIndex.forServer(serverContext).queryStopClusters(query).toList();
+    var clusters = luceneIndex.queryStopClusters(query).toList();
 
     return Response.status(Response.Status.OK).entity(clusters).build();
   }
@@ -96,8 +96,7 @@ private List<SearchResult> query(
   }
 
   private Collection<SearchResult> queryStopLocations(String query, boolean autocomplete) {
-    return LuceneIndex
-      .forServer(serverContext)
+    return luceneIndex
       .queryStopLocations(query, autocomplete)
       .map(sl ->
         new SearchResult(
@@ -111,8 +110,7 @@ private Collection<SearchResult> queryStopLocations(String query, boolean autoco
   }
 
   private Collection<? extends SearchResult> queryStations(String query, boolean autocomplete) {
-    return LuceneIndex
-      .forServer(serverContext)
+    return luceneIndex
       .queryStopLocationGroups(query, autocomplete)
       .map(sc ->
         new SearchResult(

src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java

@@ -11,6 +11,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Stream;
+import javax.annotation.Nullable;
 import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.en.EnglishAnalyzer;
 import org.apache.lucene.analysis.miscellaneous.PerFieldAnalyzerWrapper;
@@ -40,12 +41,14 @@
 import org.apache.lucene.search.suggest.document.FuzzyCompletionQuery;
 import org.apache.lucene.search.suggest.document.SuggestIndexSearcher;
 import org.apache.lucene.store.ByteBuffersDirectory;
+import org.opentripplanner.ext.stopconsolidation.StopConsolidationService;
 import org.opentripplanner.framework.collection.ListUtils;
 import org.opentripplanner.framework.i18n.I18NString;
-import org.opentripplanner.standalone.api.OtpServerRequestContext;
 import org.opentripplanner.transit.model.framework.FeedScopedId;
 import org.opentripplanner.transit.model.site.StopLocation;
 import org.opentripplanner.transit.model.site.StopLocationsGroup;
+import org.opentripplanner.transit.service.DefaultTransitService;
+import org.opentripplanner.transit.service.TransitModel;
 import org.opentripplanner.transit.service.TransitService;
 
 public class LuceneIndex implements Serializable {
@@ -65,9 +68,24 @@ public class LuceneIndex implements Serializable {
   private final SuggestIndexSearcher searcher;
   private final StopClusterMapper stopClusterMapper;
 
-  public LuceneIndex(TransitService transitService) {
+  /**
+   * Since the {@link TransitService} is request scoped, we don't inject it into this class.
+   * However, we do need some methods in the service and that's why we instantiate it manually in this
+   * constructor.
+   */
+  public LuceneIndex(TransitModel transitModel, StopConsolidationService stopConsolidationService) {
+    this(new DefaultTransitService(transitModel), stopConsolidationService);
+  }
+
+  /**
+   * This method is only visible for testing.
+   */
+  LuceneIndex(
+    TransitService transitService,
+    @Nullable StopConsolidationService stopConsolidationService
+  ) {
     this.transitService = transitService;
-    this.stopClusterMapper = new StopClusterMapper(transitService);
+    this.stopClusterMapper = new StopClusterMapper(transitService, stopConsolidationService);
 
     this.analyzer =
       new PerFieldAnalyzerWrapper(
@@ -144,18 +162,6 @@ public LuceneIndex(TransitService transitService) {
     }
   }
 
-  public static synchronized LuceneIndex forServer(OtpServerRequestContext serverContext) {
-    var graph = serverContext.graph();
-    var existingIndex = graph.getLuceneIndex();
-    if (existingIndex != null) {
-      return existingIndex;
-    }
-
-    var newIndex = new LuceneIndex(serverContext.transitService());
-    graph.setLuceneIndex(newIndex);
-    return newIndex;
-  }
-
   public Stream<StopLocation> queryStopLocations(String query, boolean autocomplete) {
     return matchingDocuments(StopLocation.class, query, autocomplete)
       .map(document -> transitService.getStopLocation(FeedScopedId.parse(document.get(ID))));
@@ -252,6 +258,7 @@ private Stream<Document> matchingDocuments(
     String searchTerms,
     boolean autocomplete
   ) {
+    searchTerms = searchTerms.strip();
     try {
       if (autocomplete) {
         var completionQuery = new FuzzyCompletionQuery(

src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java

@@ -3,13 +3,16 @@
 import static org.opentripplanner.ext.geocoder.StopCluster.LocationType.STATION;
 import static org.opentripplanner.ext.geocoder.StopCluster.LocationType.STOP;
 
+import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.Iterables;
 import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
+import org.opentripplanner.ext.stopconsolidation.StopConsolidationService;
+import org.opentripplanner.ext.stopconsolidation.model.StopReplacement;
 import org.opentripplanner.framework.collection.ListUtils;
 import org.opentripplanner.framework.geometry.WgsCoordinate;
 import org.opentripplanner.framework.i18n.I18NString;
@@ -28,9 +31,14 @@
 class StopClusterMapper {
 
   private final TransitService transitService;
+  private final StopConsolidationService stopConsolidationService;
 
-  StopClusterMapper(TransitService transitService) {
+  StopClusterMapper(
+    TransitService transitService,
+    @Nullable StopConsolidationService stopConsolidationService
+  ) {
     this.transitService = transitService;
+    this.stopConsolidationService = stopConsolidationService;
   }
 
   /**
@@ -45,16 +53,71 @@ Iterable<LuceneStopCluster> generateStopClusters(
     Collection<StopLocation> stopLocations,
     Collection<StopLocationsGroup> stopLocationsGroups
   ) {
+    var stopClusters = buildStopClusters(stopLocations);
+    var stationClusters = buildStationClusters(stopLocationsGroups);
+    var consolidatedStopClusters = buildConsolidatedStopClusters();
+
+    return Iterables.concat(stopClusters, stationClusters, consolidatedStopClusters);
+  }
+
+  private Iterable<LuceneStopCluster> buildConsolidatedStopClusters() {
+    var multiMap = stopConsolidationService
+      .replacements()
+      .stream()
+      .collect(
+        ImmutableListMultimap.toImmutableListMultimap(
+          StopReplacement::primary,
+          StopReplacement::secondary
+        )
+      );
+    return multiMap
+      .keySet()
+      .stream()
+      .map(primary -> {
+        var secondaryIds = multiMap.get(primary);
+        var secondaries = secondaryIds
+          .stream()
+          .map(transitService::getStopLocation)
+          .filter(Objects::nonNull)
+          .toList();
+        var codes = ListUtils.combine(
+          ListUtils.ofNullable(primary.getCode()),
+          getCodes(secondaries)
+        );
+        var names = ListUtils.combine(
+          ListUtils.ofNullable(primary.getName()),
+          getNames(secondaries)
+        );
+
+        return new LuceneStopCluster(
+          primary.getId().toString(),
+          secondaryIds.stream().map(id -> id.toString()).toList(),
+          names,
+          codes,
+          toCoordinate(primary.getCoordinate())
+        );
+      })
+      .toList();
+  }
+
+  private static List<LuceneStopCluster> buildStationClusters(
+    Collection<StopLocationsGroup> stopLocationsGroups
+  ) {
+    return stopLocationsGroups.stream().map(StopClusterMapper::map).toList();
+  }
+
+  private List<LuceneStopCluster> buildStopClusters(Collection<StopLocation> stopLocations) {
     List<StopLocation> stops = stopLocations
       .stream()
       // remove stop locations without a parent station
       .filter(sl -> sl.getParentStation() == null)
+      .filter(sl -> !stopConsolidationService.isPartOfConsolidatedStop(sl))
       // stops without a name (for example flex areas) are useless for searching, so we remove them, too
       .filter(sl -> sl.getName() != null)
       .toList();
 
     // if they are very close to each other and have the same name, only one is chosen (at random)
-    var deduplicatedStops = stops
+    return stops
       .stream()
       .collect(
         Collectors.groupingBy(sl ->
@@ -66,9 +129,6 @@ Iterable<LuceneStopCluster> generateStopClusters(
       .map(group -> map(group).orElse(null))
       .filter(Objects::nonNull)
       .toList();
-    var stations = stopLocationsGroups.stream().map(StopClusterMapper::map).toList();
-
-    return Iterables.concat(deduplicatedStops, stations);
   }
 
   private static LuceneStopCluster map(StopLocationsGroup g) {

src/ext/java/org/opentripplanner/ext/geocoder/configure/GeocoderModule.java

@@ -0,0 +1,31 @@
+package org.opentripplanner.ext.geocoder.configure;
+
+import dagger.Module;
+import dagger.Provides;
+import jakarta.inject.Singleton;
+import javax.annotation.Nullable;
+import org.opentripplanner.ext.geocoder.LuceneIndex;
+import org.opentripplanner.ext.stopconsolidation.StopConsolidationService;
+import org.opentripplanner.framework.application.OTPFeature;
+import org.opentripplanner.transit.service.TransitModel;
+
+/**
+ * This module builds the Lucene geocoder based on whether the feature flag is on or off.
+ */
+@Module
+public class GeocoderModule {
+
+  @Provides
+  @Singleton
+  @Nullable
+  LuceneIndex luceneIndex(
+    TransitModel transitModel,
+    @Nullable StopConsolidationService stopConsolidationService
+  ) {
+    if (OTPFeature.SandboxAPIGeocoder.isOn()) {
+      return new LuceneIndex(transitModel, stopConsolidationService);
+    } else {
+      return null;
+    }
+  }
+}

src/ext/java/org/opentripplanner/ext/stopconsolidation/StopConsolidationService.java

@@ -43,4 +43,6 @@ public interface StopConsolidationService {
    * For a given stop id return the primary stop if it is part of a consolidated stop group.
    */
   Optional<StopLocation> primaryStop(FeedScopedId id);
+
+  boolean isPartOfConsolidatedStop(StopLocation sl);
 }

src/ext/java/org/opentripplanner/ext/stopconsolidation/internal/DefaultStopConsolidationService.java

@@ -67,6 +67,11 @@ public boolean isSecondaryStop(StopLocation stop) {
     return repo.groups().stream().anyMatch(r -> r.secondaries().contains(stop.getId()));
   }
 
+  @Override
+  public boolean isPartOfConsolidatedStop(StopLocation sl) {
+    return isSecondaryStop(sl) || isPrimaryStop(sl);
+  }
+
   @Override
   public boolean isActive() {
     return !repo.groups().isEmpty();

src/main/java/org/opentripplanner/apis/APIEndpoints.java

@@ -61,8 +61,6 @@ private APIEndpoints() {
     addIfEnabled(SandboxAPIMapboxVectorTilesApi, VectorTilesResource.class);
     addIfEnabled(SandboxAPIParkAndRideApi, ParkAndRideResource.class);
     addIfEnabled(SandboxAPIGeocoder, GeocoderResource.class);
-    // scheduled to be removed and only here for backwards compatibility
-    addIfEnabled(SandboxAPIGeocoder, GeocoderResource.GeocoderResourceOldPath.class);
 
     // scheduled to be removed
     addIfEnabled(APIBikeRental, BikeRental.class);