diff --git a/src/ext-test/java/org/opentripplanner/ext/geocoder/LuceneIndexTest.java b/src/ext-test/java/org/opentripplanner/ext/geocoder/LuceneIndexTest.java index b1fb33dfdd3..cee6cf3a2d8 100644 --- a/src/ext-test/java/org/opentripplanner/ext/geocoder/LuceneIndexTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/geocoder/LuceneIndexTest.java @@ -10,7 +10,9 @@ import java.time.LocalDate; import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; +import javax.annotation.Nonnull; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -213,13 +215,13 @@ class StopClusters { ) void stopClustersWithTypos(String searchTerm) { var results = index.queryStopClusters(searchTerm).toList(); - var ids = results.stream().map(StopCluster::id).toList(); + var ids = results.stream().map(primaryId()).toList(); assertEquals(List.of(ALEXANDERPLATZ_STATION.getId()), ids); } @Test void fuzzyStopClusters() { - var result1 = index.queryStopClusters("arts").map(StopCluster::id).toList(); + var result1 = index.queryStopClusters("arts").map(primaryId()).toList(); assertEquals(List.of(ARTS_CENTER.getId()), result1); } @@ -227,7 +229,7 @@ void fuzzyStopClusters() { void deduplicatedStopClusters() { var result = index.queryStopClusters("lich").toList(); assertEquals(1, result.size()); - assertEquals(LICHTERFELDE_OST_1.getName().toString(), result.getFirst().name()); + assertEquals(LICHTERFELDE_OST_1.getName().toString(), result.getFirst().primary().name()); } @ParameterizedTest @@ -259,7 +261,7 @@ void deduplicatedStopClusters() { } ) void stopClustersWithSpace(String query) { - var result = index.queryStopClusters(query).map(StopCluster::id).toList(); + var result = index.queryStopClusters(query).map(primaryId()).toList(); assertEquals(List.of(FIVE_POINTS_STATION.getId()), result); } @@ -268,24 +270,28 @@ void stopClustersWithSpace(String query) { void fuzzyStopCode(String query) { var result = index.queryStopClusters(query).toList(); assertEquals(1, result.size()); - assertEquals(ARTS_CENTER.getName().toString(), result.getFirst().name()); + assertEquals(ARTS_CENTER.getName().toString(), result.getFirst().primary().name()); } @Test void modes() { var result = index.queryStopClusters("westh").toList(); assertEquals(1, result.size()); - var stop = result.getFirst(); - assertEquals(WESTHAFEN.getName().toString(), stop.name()); - assertEquals(List.of(FERRY.name(), BUS.name()), stop.modes()); + var cluster = result.getFirst(); + assertEquals(WESTHAFEN.getName().toString(), cluster.primary().name()); + assertEquals(List.of(FERRY.name(), BUS.name()), cluster.primary().modes()); } @Test void agenciesAndFeedPublisher() { - var result = index.queryStopClusters("alexanderplatz").toList().getFirst(); - assertEquals(ALEXANDERPLATZ_STATION.getName().toString(), result.name()); - assertEquals(List.of(StopClusterMapper.toAgency(BVG)), result.agencies()); - assertEquals("A Publisher", result.feedPublisher().name()); + var cluster = index.queryStopClusters("alexanderplatz").toList().getFirst(); + assertEquals(ALEXANDERPLATZ_STATION.getName().toString(), cluster.primary().name()); + assertEquals(List.of(StopClusterMapper.toAgency(BVG)), cluster.primary().agencies()); + assertEquals("A Publisher", cluster.primary().feedPublisher().name()); } } + + private static @Nonnull Function primaryId() { + return c -> c.primary().id(); + } } diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java index 56769db0028..e0ea8ba36b9 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java @@ -6,11 +6,11 @@ import java.io.Serializable; import java.util.Arrays; import java.util.Collection; +import java.util.List; import java.util.Map; 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,7 +40,7 @@ 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.geocoder.StopCluster.Coordinate; +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; @@ -52,22 +52,22 @@ public class LuceneIndex implements Serializable { private static final String TYPE = "type"; private static final String ID = "id"; + private static final String SECONDARY_IDS = "secondary_ids"; private static final String SUGGEST = "suggest"; private static final String NAME = "name"; private static final String NAME_NGRAM = "name_ngram"; private static final String CODE = "code"; private static final String LAT = "latitude"; private static final String LON = "longitude"; - private static final String MODE = "mode"; - private static final String AGENCY_IDS = "agency_ids"; private final TransitService transitService; private final Analyzer analyzer; private final SuggestIndexSearcher searcher; + private final StopClusterMapper stopClusterMapper; public LuceneIndex(TransitService transitService) { this.transitService = transitService; - StopClusterMapper stopClusterMapper = new StopClusterMapper(transitService); + this.stopClusterMapper = new StopClusterMapper(transitService); this.analyzer = new PerFieldAnalyzerWrapper( @@ -95,12 +95,11 @@ public LuceneIndex(TransitService transitService) { directoryWriter, StopLocation.class, stopLocation.getId().toString(), - stopLocation.getName(), - stopLocation.getCode(), + List.of(), + ListUtils.ofNullable(stopLocation.getName()), + ListUtils.ofNullable(stopLocation.getCode()), stopLocation.getCoordinate().latitude(), - stopLocation.getCoordinate().longitude(), - Set.of(), - Set.of() + stopLocation.getCoordinate().longitude() ) ); @@ -111,12 +110,11 @@ public LuceneIndex(TransitService transitService) { directoryWriter, StopLocationsGroup.class, stopLocationsGroup.getId().toString(), - stopLocationsGroup.getName(), - null, + List.of(), + ListUtils.ofNullable(stopLocationsGroup.getName()), + List.of(), stopLocationsGroup.getCoordinate().latitude(), - stopLocationsGroup.getCoordinate().longitude(), - Set.of(), - Set.of() + stopLocationsGroup.getCoordinate().longitude() ) ); @@ -129,13 +127,12 @@ public LuceneIndex(TransitService transitService) { addToIndex( directoryWriter, StopCluster.class, - stopCluster.id().toString(), - I18NString.of(stopCluster.name()), - stopCluster.code(), + stopCluster.primaryId(), + stopCluster.secondaryIds(), + stopCluster.names(), + stopCluster.codes(), stopCluster.coordinate().lat(), - stopCluster.coordinate().lon(), - stopCluster.modes(), - stopCluster.agencyIds() + stopCluster.coordinate().lon() ) ); } @@ -183,30 +180,16 @@ public Stream queryStopClusters(String query) { } private StopCluster toStopCluster(Document document) { - var clusterId = FeedScopedId.parse(document.get(ID)); - var name = document.get(NAME); - var code = document.get(CODE); - var lat = document.getField(LAT).numericValue().doubleValue(); - var lon = document.getField(LON).numericValue().doubleValue(); - var modes = Arrays.asList(document.getValues(MODE)); - var agencies = Arrays - .stream(document.getValues(AGENCY_IDS)) - .map(id -> transitService.getAgencyForId(FeedScopedId.parse(id))) - .filter(Objects::nonNull) - .map(StopClusterMapper::toAgency) + var primaryId = FeedScopedId.parse(document.get(ID)); + var primary = stopClusterMapper.toLocation(primaryId); + + var secondaryIds = Arrays + .stream(document.getValues(SECONDARY_IDS)) + .map(FeedScopedId::parse) + .map(stopClusterMapper::toLocation) .toList(); - var feedPublisher = StopClusterMapper.toFeedPublisher( - transitService.getFeedInfo(clusterId.getFeedId()) - ); - return new StopCluster( - clusterId, - code, - name, - new Coordinate(lat, lon), - modes, - agencies, - feedPublisher - ); + + return new StopCluster(primary, secondaryIds); } static IndexWriterConfig iwcWithSuggestField(Analyzer analyzer, final Set suggestFields) { @@ -230,36 +213,33 @@ private static void addToIndex( IndexWriter writer, Class type, String id, - I18NString name, - @Nullable String code, + Collection secondaryIds, + Collection names, + Collection codes, double latitude, - double longitude, - Collection modes, - Collection agencyIds + double longitude ) { String typeName = type.getSimpleName(); Document document = new Document(); document.add(new StoredField(ID, id)); + for (var secondaryId : secondaryIds) { + document.add(new StoredField(SECONDARY_IDS, secondaryId)); + } document.add(new TextField(TYPE, typeName, Store.YES)); - document.add(new TextField(NAME, Objects.toString(name), Store.YES)); - document.add(new TextField(NAME_NGRAM, Objects.toString(name), Store.YES)); - document.add(new ContextSuggestField(SUGGEST, Objects.toString(name), 1, typeName)); + for (var name : names) { + document.add(new TextField(NAME, Objects.toString(name), Store.YES)); + document.add(new TextField(NAME_NGRAM, Objects.toString(name), Store.YES)); + document.add(new ContextSuggestField(SUGGEST, Objects.toString(name), 1, typeName)); + } document.add(new StoredField(LAT, latitude)); document.add(new StoredField(LON, longitude)); - if (code != null) { + for (var code : codes) { document.add(new TextField(CODE, code, Store.YES)); document.add(new ContextSuggestField(SUGGEST, code, 1, typeName)); } - for (var mode : modes) { - document.add(new TextField(MODE, mode, Store.YES)); - } - for (var ids : agencyIds) { - document.add(new TextField(AGENCY_IDS, ids, Store.YES)); - } - try { writer.addDocument(document); } catch (IOException ex) { diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java index f58d7aa9af9..b4ee0f0919b 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java @@ -1,17 +1,15 @@ package org.opentripplanner.ext.geocoder; import java.util.Collection; -import javax.annotation.Nullable; -import org.opentripplanner.transit.model.framework.FeedScopedId; +import org.opentripplanner.framework.i18n.I18NString; /** * A package-private helper type for transporting data before serializing. */ record LuceneStopCluster( - FeedScopedId id, - @Nullable String code, - String name, - StopCluster.Coordinate coordinate, - Collection modes, - Collection agencyIds + String primaryId, + Collection secondaryIds, + Collection names, + Collection codes, + StopCluster.Coordinate coordinate ) {} diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java index 8ffd44511fd..30647cc7b20 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java @@ -2,6 +2,7 @@ import java.util.Collection; import java.util.List; +import java.util.Objects; import javax.annotation.Nullable; import org.opentripplanner.transit.model.framework.FeedScopedId; @@ -14,15 +15,7 @@ * - if a stop has a parent station only the parent is returned * - if stops are closer than 10 meters to each and have an identical name, only one is returned */ -record StopCluster( - FeedScopedId id, - @Nullable String code, - String name, - Coordinate coordinate, - Collection modes, - List agencies, - @Nullable FeedPublisher feedPublisher -) { +record StopCluster(Location primary, Collection secondaries) { /** * Easily serializable version of a coordinate */ @@ -37,4 +30,29 @@ public record Agency(FeedScopedId id, String name) {} * Easily serializable version of a feed publisher */ public record FeedPublisher(String name) {} + + public enum LocationType { + STATION, + STOP, + } + + public record Location( + FeedScopedId id, + @Nullable String code, + LocationType type, + String name, + Coordinate coordinate, + Collection modes, + List agencies, + @Nullable FeedPublisher feedPublisher + ) { + public Location { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(type); + Objects.requireNonNull(coordinate); + Objects.requireNonNull(modes); + Objects.requireNonNull(agencies); + } + } } diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java index 6f16d4a0cce..d9f388ea0e8 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -1,15 +1,23 @@ package org.opentripplanner.ext.geocoder; +import static org.opentripplanner.ext.geocoder.StopCluster.LocationType.STATION; +import static org.opentripplanner.ext.geocoder.StopCluster.LocationType.STOP; + 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.framework.collection.ListUtils; import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.model.FeedInfo; +import org.opentripplanner.transit.model.framework.FeedScopedId; import org.opentripplanner.transit.model.network.Route; import org.opentripplanner.transit.model.organization.Agency; +import org.opentripplanner.transit.model.site.Station; import org.opentripplanner.transit.model.site.StopLocation; import org.opentripplanner.transit.model.site.StopLocationsGroup; import org.opentripplanner.transit.service.TransitService; @@ -37,7 +45,7 @@ Iterable generateStopClusters( Collection stopLocations, Collection stopLocationsGroups ) { - var stops = stopLocations + List stops = stopLocations .stream() // remove stop locations without a parent station .filter(sl -> sl.getParentStation() == null) @@ -46,50 +54,63 @@ Iterable generateStopClusters( .toList(); // if they are very close to each other and have the same name, only one is chosen (at random) - var deduplicatedStops = ListUtils - .distinctByKey( - stops, - sl -> new DeduplicationKey(sl.getName(), sl.getCoordinate().roundToApproximate100m()) + var deduplicatedStops = stops + .stream() + .collect( + Collectors.groupingBy(sl -> + new DeduplicationKey(sl.getName(), sl.getCoordinate().roundToApproximate100m()) + ) ) + .values() .stream() - .flatMap(s -> this.map(s).stream()) + .map(group -> map(group).orElse(null)) + .filter(Objects::nonNull) .toList(); - var stations = stopLocationsGroups.stream().map(this::map).toList(); + var stations = stopLocationsGroups.stream().map(StopClusterMapper::map).toList(); return Iterables.concat(deduplicatedStops, stations); } - LuceneStopCluster map(StopLocationsGroup g) { - var modes = transitService.getModesOfStopLocationsGroup(g).stream().map(Enum::name).toList(); - var agencies = agenciesForStopLocationsGroup(g) - .stream() - .map(s -> s.getId().toString()) - .toList(); + private static LuceneStopCluster map(StopLocationsGroup g) { + var childStops = g.getChildStops(); + var ids = childStops.stream().map(s -> s.getId().toString()).toList(); + var childNames = getNames(childStops); + var codes = getCodes(childStops); + return new LuceneStopCluster( - g.getId(), - null, - g.getName().toString(), - toCoordinate(g.getCoordinate()), - modes, - agencies + g.getId().toString(), + ids, + ListUtils.combine(List.of(g.getName()), childNames), + codes, + toCoordinate(g.getCoordinate()) ); } - Optional map(StopLocation sl) { - var agencies = agenciesForStopLocation(sl).stream().map(a -> a.getId().toString()).toList(); + private static List getCodes(Collection childStops) { + return childStops.stream().map(StopLocation::getCode).filter(Objects::nonNull).toList(); + } + + private static List getNames(Collection childStops) { + return childStops.stream().map(StopLocation::getName).filter(Objects::nonNull).toList(); + } + + private static Optional map(List stopLocations) { + var primary = stopLocations.getFirst(); + var secondaryIds = stopLocations.stream().skip(1).map(sl -> sl.getId().toString()).toList(); + var names = getNames(stopLocations); + var codes = getCodes(stopLocations); + return Optional - .ofNullable(sl.getName()) - .map(name -> { - var modes = transitService.getModesOfStopLocation(sl).stream().map(Enum::name).toList(); - return new LuceneStopCluster( - sl.getId(), - sl.getCode(), - name.toString(), - toCoordinate(sl.getCoordinate()), - modes, - agencies - ); - }); + .ofNullable(primary.getName()) + .map(name -> + new LuceneStopCluster( + primary.getId().toString(), + secondaryIds, + names, + codes, + toCoordinate(primary.getCoordinate()) + ) + ); } private List agenciesForStopLocation(StopLocation stop) { @@ -105,6 +126,59 @@ private List agenciesForStopLocationsGroup(StopLocationsGroup group) { .toList(); } + StopCluster.Location toLocation(FeedScopedId id) { + var loc = transitService.getStopLocation(id); + if (loc != null) { + var feedPublisher = toFeedPublisher(transitService.getFeedInfo(id.getFeedId())); + var modes = transitService.getModesOfStopLocation(loc).stream().map(Enum::name).toList(); + var agencies = agenciesForStopLocation(loc) + .stream() + .map(StopClusterMapper::toAgency) + .toList(); + return new StopCluster.Location( + loc.getId(), + loc.getCode(), + STOP, + loc.getName().toString(), + new StopCluster.Coordinate(loc.getLat(), loc.getLon()), + modes, + agencies, + feedPublisher + ); + } else { + var group = transitService.getStopLocationsGroup(id); + var feedPublisher = toFeedPublisher(transitService.getFeedInfo(id.getFeedId())); + var modes = transitService + .getModesOfStopLocationsGroup(group) + .stream() + .map(Enum::name) + .toList(); + var agencies = agenciesForStopLocationsGroup(group) + .stream() + .map(StopClusterMapper::toAgency) + .toList(); + return new StopCluster.Location( + group.getId(), + extractCode(group), + STATION, + group.getName().toString(), + new StopCluster.Coordinate(group.getLat(), group.getLon()), + modes, + agencies, + feedPublisher + ); + } + } + + @Nullable + private static String extractCode(StopLocationsGroup group) { + if (group instanceof Station station) { + return station.getCode(); + } else { + return null; + } + } + private static StopCluster.Coordinate toCoordinate(WgsCoordinate c) { return new StopCluster.Coordinate(c.latitude(), c.longitude()); } @@ -113,7 +187,7 @@ static StopCluster.Agency toAgency(Agency a) { return new StopCluster.Agency(a.getId(), a.getName()); } - static StopCluster.FeedPublisher toFeedPublisher(FeedInfo fi) { + private static StopCluster.FeedPublisher toFeedPublisher(FeedInfo fi) { if (fi == null) { return null; } else { diff --git a/src/main/java/org/opentripplanner/framework/collection/ListUtils.java b/src/main/java/org/opentripplanner/framework/collection/ListUtils.java index 513c0bcc0d3..5964a1674e3 100644 --- a/src/main/java/org/opentripplanner/framework/collection/ListUtils.java +++ b/src/main/java/org/opentripplanner/framework/collection/ListUtils.java @@ -57,4 +57,16 @@ public static List distinctByKey( return ret; } + + /** + * Take a single nullable variable and return an empty list if it is null. Otherwise + * return a list with one element. + */ + public static List ofNullable(T input) { + if (input == null) { + return List.of(); + } else { + return List.of(input); + } + } } diff --git a/src/test/java/org/opentripplanner/framework/collection/ListUtilsTest.java b/src/test/java/org/opentripplanner/framework/collection/ListUtilsTest.java index 6e72d5626c5..33dce1f5574 100644 --- a/src/test/java/org/opentripplanner/framework/collection/ListUtilsTest.java +++ b/src/test/java/org/opentripplanner/framework/collection/ListUtilsTest.java @@ -56,5 +56,11 @@ void distinctByKey() { assertEquals(List.of(first, third), deduplicated); } + @Test + void ofNullable() { + assertEquals(List.of(), ListUtils.ofNullable(null)); + assertEquals(List.of("A"), ListUtils.ofNullable("A")); + } + private record Wrapper(int i, String string) {} }