From 733efa9b1b22e0034347e018ee35c9434a03a7f2 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 22 May 2024 11:29:52 +0200 Subject: [PATCH 1/9] Introduce primary and secondary elements for stop clusters --- .../ext/geocoder/LuceneIndexTest.java | 26 ++++++++++------- .../ext/geocoder/LuceneIndex.java | 5 +++- .../ext/geocoder/StopCluster.java | 28 ++++++++++++++----- 3 files changed, 41 insertions(+), 18 deletions(-) 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..15f205ad802 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,7 +270,7 @@ 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 @@ -276,16 +278,20 @@ 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()); + assertEquals(WESTHAFEN.getName().toString(), stop.primary().name()); + assertEquals(List.of(FERRY.name(), BUS.name()), stop.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()); + assertEquals(ALEXANDERPLATZ_STATION.getName().toString(), result.primary().name()); + assertEquals(List.of(StopClusterMapper.toAgency(BVG)), result.primary().agencies()); + assertEquals("A Publisher", result.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..b43028ff4d9 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java @@ -6,6 +6,7 @@ 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; @@ -198,7 +199,7 @@ private StopCluster toStopCluster(Document document) { var feedPublisher = StopClusterMapper.toFeedPublisher( transitService.getFeedInfo(clusterId.getFeedId()) ); - return new StopCluster( + var primary = new StopCluster.Location( clusterId, code, name, @@ -207,6 +208,8 @@ private StopCluster toStopCluster(Document document) { agencies, feedPublisher ); + + return new StopCluster(primary, List.of()); } static IndexWriterConfig iwcWithSuggestField(Analyzer analyzer, final Set suggestFields) { diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java index 8ffd44511fd..20c86e18ed9 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; @@ -15,13 +16,8 @@ * - 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 + Location primary, + Collection secondaries ) { /** * Easily serializable version of a coordinate @@ -37,4 +33,22 @@ public record Agency(FeedScopedId id, String name) {} * Easily serializable version of a feed publisher */ public record FeedPublisher(String name) {} + + public record Location( + FeedScopedId id, + @Nullable String code, + String name, + Coordinate coordinate, + Collection modes, + List agencies, + @Nullable FeedPublisher feedPublisher + ){ + public Location{ + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(coordinate); + Objects.requireNonNull(modes); + Objects.requireNonNull(agencies); + } + } } From 3686f3543d9eed105aa32431b85ff8dfa3ac2a76 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 22 May 2024 12:18:25 +0200 Subject: [PATCH 2/9] Improve mapping of primaries/secondaries --- .../ext/geocoder/LuceneIndex.java | 36 ++++++++++--------- .../ext/geocoder/LuceneStopCluster.java | 15 ++++---- .../ext/geocoder/StopClusterMapper.java | 32 +++++++++-------- .../framework/collection/ListUtils.java | 13 +++++++ 4 files changed, 56 insertions(+), 40 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java index b43028ff4d9..8502bb9887a 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java @@ -11,7 +11,6 @@ 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; @@ -42,6 +41,7 @@ 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; @@ -96,8 +96,8 @@ public LuceneIndex(TransitService transitService) { directoryWriter, StopLocation.class, stopLocation.getId().toString(), - stopLocation.getName(), - stopLocation.getCode(), + ListUtils.ofNullable(stopLocation.getName()), + ListUtils.ofNullable(stopLocation.getCode()), stopLocation.getCoordinate().latitude(), stopLocation.getCoordinate().longitude(), Set.of(), @@ -112,8 +112,8 @@ public LuceneIndex(TransitService transitService) { directoryWriter, StopLocationsGroup.class, stopLocationsGroup.getId().toString(), - stopLocationsGroup.getName(), - null, + ListUtils.ofNullable(stopLocationsGroup.getName()), + List.of(), stopLocationsGroup.getCoordinate().latitude(), stopLocationsGroup.getCoordinate().longitude(), Set.of(), @@ -130,13 +130,13 @@ public LuceneIndex(TransitService transitService) { addToIndex( directoryWriter, StopCluster.class, - stopCluster.id().toString(), - I18NString.of(stopCluster.name()), - stopCluster.code(), + stopCluster.primaryId(), + stopCluster.names(), + stopCluster.codes(), stopCluster.coordinate().lat(), stopCluster.coordinate().lon(), stopCluster.modes(), - stopCluster.agencyIds() + List.of() ) ); } @@ -233,8 +233,8 @@ private static void addToIndex( IndexWriter writer, Class type, String id, - I18NString name, - @Nullable String code, + Collection names, + Collection codes, double latitude, double longitude, Collection modes, @@ -245,13 +245,15 @@ private static void addToIndex( Document document = new Document(); document.add(new StoredField(ID, id)); 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)); } @@ -259,8 +261,8 @@ private static void addToIndex( 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)); + for (var aId : agencyIds) { + document.add(new TextField(AGENCY_IDS, aId, Store.YES)); } try { diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java index f58d7aa9af9..5a52ea2a831 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java @@ -1,17 +1,16 @@ 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, + String primaryId, + Collection secondaryIds, + Collection names, Collection modes, - Collection agencyIds -) {} + Collection codes, + StopCluster.Coordinate coordinate){ +} diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java index 6f16d4a0cce..fd808d41bcf 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -3,6 +3,7 @@ import com.google.common.collect.Iterables; import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.Optional; import org.opentripplanner.framework.collection.ListUtils; import org.opentripplanner.framework.geometry.WgsCoordinate; @@ -61,33 +62,34 @@ Iterable generateStopClusters( 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(); + + var childStops = g.getChildStops(); + var ids = childStops.stream().map(s -> s.getId().toString()).toList(); + var childNames = childStops.stream().map(StopLocation::getName).filter(Objects::nonNull).toList(); + var codes = childStops.stream().map(StopLocation::getCode).filter(Objects::nonNull).toList(); + return new LuceneStopCluster( - g.getId(), - null, - g.getName().toString(), - toCoordinate(g.getCoordinate()), + g.getId().toString(), + ids, + ListUtils.combine(List.of(g.getName()), childNames), + codes, modes, - agencies + toCoordinate(g.getCoordinate()) ); } Optional map(StopLocation sl) { - var agencies = agenciesForStopLocation(sl).stream().map(a -> a.getId().toString()).toList(); 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()), + sl.getId().toString(), + List.of(), + List.of(name), modes, - agencies + ListUtils.ofNullable(sl.getCode()), + toCoordinate(sl.getCoordinate()) ); }); } diff --git a/src/main/java/org/opentripplanner/framework/collection/ListUtils.java b/src/main/java/org/opentripplanner/framework/collection/ListUtils.java index 513c0bcc0d3..302df428d12 100644 --- a/src/main/java/org/opentripplanner/framework/collection/ListUtils.java +++ b/src/main/java/org/opentripplanner/framework/collection/ListUtils.java @@ -57,4 +57,17 @@ 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); + } + } } From 60469d74ada63ae3ddba7b11b692cb3ae7a4ff44 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 22 May 2024 12:41:24 +0200 Subject: [PATCH 3/9] Rework indexing --- .../ext/geocoder/LuceneIndex.java | 73 ++++++++++++------- .../ext/geocoder/LuceneStopCluster.java | 4 +- .../ext/geocoder/StopCluster.java | 9 +-- .../ext/geocoder/StopClusterMapper.java | 10 ++- .../framework/collection/ListUtils.java | 5 +- .../transit/model/site/GroupOfStations.java | 6 ++ .../model/site/StopLocationsGroup.java | 4 + 7 files changed, 71 insertions(+), 40 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java index 8502bb9887a..c39d18d6c22 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java @@ -65,10 +65,11 @@ public class LuceneIndex implements Serializable { 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( @@ -184,34 +185,54 @@ 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) - .toList(); - var feedPublisher = StopClusterMapper.toFeedPublisher( - transitService.getFeedInfo(clusterId.getFeedId()) - ); - var primary = new StopCluster.Location( - clusterId, - code, - name, - new Coordinate(lat, lon), - modes, - agencies, - feedPublisher - ); + var primaryId = FeedScopedId.parse(document.get(ID)); + var primary = toLocation(primaryId); return new StopCluster(primary, List.of()); } + private StopCluster.Location toLocation(FeedScopedId id) { + var loc = transitService.getStopLocation(id); + if (loc != null) { + var feedPublisher = StopClusterMapper.toFeedPublisher( + transitService.getFeedInfo(id.getFeedId()) + ); + var agencies = stopClusterMapper + .agenciesForStopLocation(loc) + .stream() + .map(StopClusterMapper::toAgency) + .toList(); + return new StopCluster.Location( + loc.getId(), + loc.getCode(), + loc.getName().toString(), + new Coordinate(loc.getLat(), loc.getLon()), + List.of(), + agencies, + feedPublisher + ); + } else { + var group = transitService.getStopLocationsGroup(id); + var feedPublisher = StopClusterMapper.toFeedPublisher( + transitService.getFeedInfo(id.getFeedId()) + ); + var agencies = stopClusterMapper + .agenciesForStopLocationsGroup(group) + .stream() + .map(StopClusterMapper::toAgency) + .toList(); + return new StopCluster.Location( + group.getId(), + group.getCode(), + group.getName().toString(), + new Coordinate(group.getLat(), group.getLon()), + List.of(), + agencies, + feedPublisher + ); + } + } + static IndexWriterConfig iwcWithSuggestField(Analyzer analyzer, final Set suggestFields) { IndexWriterConfig iwc = new IndexWriterConfig(analyzer); Codec filterCodec = new Lucene99Codec() { @@ -245,7 +266,7 @@ private static void addToIndex( Document document = new Document(); document.add(new StoredField(ID, id)); document.add(new TextField(TYPE, typeName, Store.YES)); - for(var name: names) { + 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)); diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java index 5a52ea2a831..9ed1b71da76 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java @@ -12,5 +12,5 @@ record LuceneStopCluster( Collection names, Collection modes, Collection codes, - StopCluster.Coordinate coordinate){ -} + 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 20c86e18ed9..4c51c7586dd 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java @@ -15,10 +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( - Location primary, - Collection secondaries -) { +record StopCluster(Location primary, Collection secondaries) { /** * Easily serializable version of a coordinate */ @@ -42,8 +39,8 @@ public record Location( Collection modes, List agencies, @Nullable FeedPublisher feedPublisher - ){ - public Location{ + ) { + public Location { Objects.requireNonNull(id); Objects.requireNonNull(name); Objects.requireNonNull(coordinate); diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java index fd808d41bcf..2c5e4e1aa29 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -65,7 +65,11 @@ LuceneStopCluster map(StopLocationsGroup g) { var childStops = g.getChildStops(); var ids = childStops.stream().map(s -> s.getId().toString()).toList(); - var childNames = childStops.stream().map(StopLocation::getName).filter(Objects::nonNull).toList(); + var childNames = childStops + .stream() + .map(StopLocation::getName) + .filter(Objects::nonNull) + .toList(); var codes = childStops.stream().map(StopLocation::getCode).filter(Objects::nonNull).toList(); return new LuceneStopCluster( @@ -94,11 +98,11 @@ Optional map(StopLocation sl) { }); } - private List agenciesForStopLocation(StopLocation stop) { + List agenciesForStopLocation(StopLocation stop) { return transitService.getRoutesForStop(stop).stream().map(Route::getAgency).distinct().toList(); } - private List agenciesForStopLocationsGroup(StopLocationsGroup group) { + List agenciesForStopLocationsGroup(StopLocationsGroup group) { return group .getChildStops() .stream() diff --git a/src/main/java/org/opentripplanner/framework/collection/ListUtils.java b/src/main/java/org/opentripplanner/framework/collection/ListUtils.java index 302df428d12..5964a1674e3 100644 --- a/src/main/java/org/opentripplanner/framework/collection/ListUtils.java +++ b/src/main/java/org/opentripplanner/framework/collection/ListUtils.java @@ -63,10 +63,9 @@ public static List distinctByKey( * return a list with one element. */ public static List ofNullable(T input) { - if(input == null){ + if (input == null) { return List.of(); - } - else { + } else { return List.of(input); } } diff --git a/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java b/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java index c62d6099c97..66b45317718 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java +++ b/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java @@ -51,6 +51,12 @@ public WgsCoordinate getCoordinate() { return coordinate; } + @Nullable + @Override + public String getCode() { + return null; + } + @Nonnull public Collection getChildStops() { return this.childStations.stream().flatMap(s -> s.getChildStops().stream()).toList(); diff --git a/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java b/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java index 3536f59e9b6..70b2ee45a81 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java +++ b/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java @@ -1,6 +1,7 @@ package org.opentripplanner.transit.model.site; import java.util.Collection; +import javax.annotation.Nullable; import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.framework.lang.ObjectUtils; @@ -39,4 +40,7 @@ default double getLon() { default String logName() { return ObjectUtils.ifNotNull(getName(), Object::toString, null); } + + @Nullable + String getCode(); } From b4805c58b42af2c27a56ebd4b82b68056c991c2d Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 22 May 2024 12:47:29 +0200 Subject: [PATCH 4/9] Move mapping code --- .../ext/geocoder/LuceneIndex.java | 45 +------------------ .../ext/geocoder/StopClusterMapper.java | 43 ++++++++++++++++++ 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java index c39d18d6c22..689b7419122 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java @@ -40,7 +40,6 @@ 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; @@ -186,53 +185,11 @@ public Stream queryStopClusters(String query) { private StopCluster toStopCluster(Document document) { var primaryId = FeedScopedId.parse(document.get(ID)); - var primary = toLocation(primaryId); + var primary = stopClusterMapper.toLocation(primaryId); return new StopCluster(primary, List.of()); } - private StopCluster.Location toLocation(FeedScopedId id) { - var loc = transitService.getStopLocation(id); - if (loc != null) { - var feedPublisher = StopClusterMapper.toFeedPublisher( - transitService.getFeedInfo(id.getFeedId()) - ); - var agencies = stopClusterMapper - .agenciesForStopLocation(loc) - .stream() - .map(StopClusterMapper::toAgency) - .toList(); - return new StopCluster.Location( - loc.getId(), - loc.getCode(), - loc.getName().toString(), - new Coordinate(loc.getLat(), loc.getLon()), - List.of(), - agencies, - feedPublisher - ); - } else { - var group = transitService.getStopLocationsGroup(id); - var feedPublisher = StopClusterMapper.toFeedPublisher( - transitService.getFeedInfo(id.getFeedId()) - ); - var agencies = stopClusterMapper - .agenciesForStopLocationsGroup(group) - .stream() - .map(StopClusterMapper::toAgency) - .toList(); - return new StopCluster.Location( - group.getId(), - group.getCode(), - group.getName().toString(), - new Coordinate(group.getLat(), group.getLon()), - List.of(), - agencies, - feedPublisher - ); - } - } - static IndexWriterConfig iwcWithSuggestField(Analyzer analyzer, final Set suggestFields) { IndexWriterConfig iwc = new IndexWriterConfig(analyzer); Codec filterCodec = new Lucene99Codec() { diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java index 2c5e4e1aa29..543f06949ff 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -9,6 +9,7 @@ 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.StopLocation; @@ -127,5 +128,47 @@ static StopCluster.FeedPublisher toFeedPublisher(FeedInfo fi) { } } + 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(), + 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(), + group.getCode(), + group.getName().toString(), + new StopCluster.Coordinate(group.getLat(), group.getLon()), + modes, + agencies, + feedPublisher + ); + } + } + private record DeduplicationKey(I18NString name, WgsCoordinate coordinate) {} } From f78d9c96ff2e7b4d6de06f00f2154baae7c24b1a Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 22 May 2024 15:43:37 +0200 Subject: [PATCH 5/9] Also add secondaries from stop clusters --- .../ext/geocoder/LuceneIndex.java | 41 ++++---- .../ext/geocoder/LuceneStopCluster.java | 1 - .../ext/geocoder/StopClusterMapper.java | 99 ++++++++++--------- 3 files changed, 71 insertions(+), 70 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java index 689b7419122..e0ea8ba36b9 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneIndex.java @@ -52,14 +52,13 @@ 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; @@ -96,12 +95,11 @@ public LuceneIndex(TransitService transitService) { directoryWriter, StopLocation.class, stopLocation.getId().toString(), + List.of(), ListUtils.ofNullable(stopLocation.getName()), ListUtils.ofNullable(stopLocation.getCode()), stopLocation.getCoordinate().latitude(), - stopLocation.getCoordinate().longitude(), - Set.of(), - Set.of() + stopLocation.getCoordinate().longitude() ) ); @@ -112,12 +110,11 @@ public LuceneIndex(TransitService transitService) { directoryWriter, StopLocationsGroup.class, stopLocationsGroup.getId().toString(), + List.of(), ListUtils.ofNullable(stopLocationsGroup.getName()), List.of(), stopLocationsGroup.getCoordinate().latitude(), - stopLocationsGroup.getCoordinate().longitude(), - Set.of(), - Set.of() + stopLocationsGroup.getCoordinate().longitude() ) ); @@ -131,12 +128,11 @@ public LuceneIndex(TransitService transitService) { directoryWriter, StopCluster.class, stopCluster.primaryId(), + stopCluster.secondaryIds(), stopCluster.names(), stopCluster.codes(), stopCluster.coordinate().lat(), - stopCluster.coordinate().lon(), - stopCluster.modes(), - List.of() + stopCluster.coordinate().lon() ) ); } @@ -187,7 +183,13 @@ private StopCluster toStopCluster(Document document) { var primaryId = FeedScopedId.parse(document.get(ID)); var primary = stopClusterMapper.toLocation(primaryId); - return new StopCluster(primary, List.of()); + var secondaryIds = Arrays + .stream(document.getValues(SECONDARY_IDS)) + .map(FeedScopedId::parse) + .map(stopClusterMapper::toLocation) + .toList(); + + return new StopCluster(primary, secondaryIds); } static IndexWriterConfig iwcWithSuggestField(Analyzer analyzer, final Set suggestFields) { @@ -211,17 +213,19 @@ private static void addToIndex( IndexWriter writer, Class type, String id, + 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)); for (var name : names) { document.add(new TextField(NAME, Objects.toString(name), Store.YES)); @@ -236,13 +240,6 @@ private static void addToIndex( document.add(new ContextSuggestField(SUGGEST, code, 1, typeName)); } - for (var mode : modes) { - document.add(new TextField(MODE, mode, Store.YES)); - } - for (var aId : agencyIds) { - document.add(new TextField(AGENCY_IDS, aId, 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 9ed1b71da76..b4ee0f0919b 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/LuceneStopCluster.java @@ -10,7 +10,6 @@ record LuceneStopCluster( String primaryId, Collection secondaryIds, Collection names, - Collection modes, Collection codes, StopCluster.Coordinate coordinate ) {} diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java index 543f06949ff..bb8f0e72bbe 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.stream.Collectors; import org.opentripplanner.framework.collection.ListUtils; import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.framework.i18n.I18NString; @@ -39,7 +40,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) @@ -48,13 +49,17 @@ 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 -> this.map(group).orElse(null)) + .filter(Objects::nonNull) .toList(); var stations = stopLocationsGroups.stream().map(this::map).toList(); @@ -62,8 +67,6 @@ Iterable generateStopClusters( } LuceneStopCluster map(StopLocationsGroup g) { - var modes = transitService.getModesOfStopLocationsGroup(g).stream().map(Enum::name).toList(); - var childStops = g.getChildStops(); var ids = childStops.stream().map(s -> s.getId().toString()).toList(); var childNames = childStops @@ -78,32 +81,34 @@ LuceneStopCluster map(StopLocationsGroup g) { ids, ListUtils.combine(List.of(g.getName()), childNames), codes, - modes, toCoordinate(g.getCoordinate()) ); } - Optional map(StopLocation sl) { + Optional map(List stopLocations) { + var primary = stopLocations.getFirst(); + var secondaryIds = stopLocations.stream().skip(1).map(sl -> sl.getId().toString()).toList(); + var names = stopLocations.stream().map(StopLocation::getName).toList(); + var codes = stopLocations.stream().map(StopLocation::getCode).filter(Objects::nonNull).toList(); + return Optional - .ofNullable(sl.getName()) - .map(name -> { - var modes = transitService.getModesOfStopLocation(sl).stream().map(Enum::name).toList(); - return new LuceneStopCluster( - sl.getId().toString(), - List.of(), - List.of(name), - modes, - ListUtils.ofNullable(sl.getCode()), - toCoordinate(sl.getCoordinate()) - ); - }); + .ofNullable(primary.getName()) + .map(name -> + new LuceneStopCluster( + primary.getId().toString(), + secondaryIds, + names, + codes, + toCoordinate(primary.getCoordinate()) + ) + ); } - List agenciesForStopLocation(StopLocation stop) { + private List agenciesForStopLocation(StopLocation stop) { return transitService.getRoutesForStop(stop).stream().map(Route::getAgency).distinct().toList(); } - List agenciesForStopLocationsGroup(StopLocationsGroup group) { + private List agenciesForStopLocationsGroup(StopLocationsGroup group) { return group .getChildStops() .stream() @@ -112,28 +117,10 @@ List agenciesForStopLocationsGroup(StopLocationsGroup group) { .toList(); } - private static StopCluster.Coordinate toCoordinate(WgsCoordinate c) { - return new StopCluster.Coordinate(c.latitude(), c.longitude()); - } - - static StopCluster.Agency toAgency(Agency a) { - return new StopCluster.Agency(a.getId(), a.getName()); - } - - static StopCluster.FeedPublisher toFeedPublisher(FeedInfo fi) { - if (fi == null) { - return null; - } else { - return new StopCluster.FeedPublisher(fi.getPublisherName()); - } - } - StopCluster.Location toLocation(FeedScopedId id) { var loc = transitService.getStopLocation(id); if (loc != null) { - var feedPublisher = toFeedPublisher( - transitService.getFeedInfo(id.getFeedId()) - ); + var feedPublisher = toFeedPublisher(transitService.getFeedInfo(id.getFeedId())); var modes = transitService.getModesOfStopLocation(loc).stream().map(Enum::name).toList(); var agencies = agenciesForStopLocation(loc) .stream() @@ -150,10 +137,12 @@ StopCluster.Location toLocation(FeedScopedId id) { ); } else { var group = transitService.getStopLocationsGroup(id); - var feedPublisher = toFeedPublisher( - transitService.getFeedInfo(id.getFeedId()) - ); - var modes = transitService.getModesOfStopLocationsGroup(group).stream().map(Enum::name).toList(); + 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) @@ -170,5 +159,21 @@ StopCluster.Location toLocation(FeedScopedId id) { } } + private static StopCluster.Coordinate toCoordinate(WgsCoordinate c) { + return new StopCluster.Coordinate(c.latitude(), c.longitude()); + } + + static StopCluster.Agency toAgency(Agency a) { + return new StopCluster.Agency(a.getId(), a.getName()); + } + + private static StopCluster.FeedPublisher toFeedPublisher(FeedInfo fi) { + if (fi == null) { + return null; + } else { + return new StopCluster.FeedPublisher(fi.getPublisherName()); + } + } + private record DeduplicationKey(I18NString name, WgsCoordinate coordinate) {} } From c27e8ce6f729054ad120c29ac21c6b8f870f05f5 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 29 May 2024 10:33:14 +0200 Subject: [PATCH 6/9] Add type to geocoding results --- .../java/org/opentripplanner/ext/geocoder/StopCluster.java | 7 +++++++ .../opentripplanner/ext/geocoder/StopClusterMapper.java | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java index 4c51c7586dd..30647cc7b20 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopCluster.java @@ -31,9 +31,15 @@ public record Agency(FeedScopedId id, String name) {} */ 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, @@ -43,6 +49,7 @@ public record Location( 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 bb8f0e72bbe..de8d2681fd9 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -1,5 +1,8 @@ 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; @@ -129,6 +132,7 @@ StopCluster.Location toLocation(FeedScopedId id) { return new StopCluster.Location( loc.getId(), loc.getCode(), + STOP, loc.getName().toString(), new StopCluster.Coordinate(loc.getLat(), loc.getLon()), modes, @@ -150,6 +154,7 @@ StopCluster.Location toLocation(FeedScopedId id) { return new StopCluster.Location( group.getId(), group.getCode(), + STATION, group.getName().toString(), new StopCluster.Coordinate(group.getLat(), group.getLon()), modes, From 78cd79b221ad92ba051d812bd807d5f7a2fc990f Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Wed, 29 May 2024 11:07:06 +0200 Subject: [PATCH 7/9] Add test for ofNullable --- .../opentripplanner/framework/collection/ListUtilsTest.java | 6 ++++++ 1 file changed, 6 insertions(+) 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) {} } From ab5ec25ca1b86995bd567b9d2ec1ab1f1d514fc4 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 31 May 2024 10:39:18 +0200 Subject: [PATCH 8/9] Use cast to extract code --- .../ext/geocoder/StopClusterMapper.java | 13 ++++++++++++- .../transit/model/site/GroupOfStations.java | 6 ------ .../transit/model/site/StopLocationsGroup.java | 4 ---- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java index de8d2681fd9..fbb9941c59c 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -9,6 +9,7 @@ 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; @@ -16,6 +17,7 @@ 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; @@ -153,7 +155,7 @@ StopCluster.Location toLocation(FeedScopedId id) { .toList(); return new StopCluster.Location( group.getId(), - group.getCode(), + extractCode(group), STATION, group.getName().toString(), new StopCluster.Coordinate(group.getLat(), group.getLon()), @@ -164,6 +166,15 @@ StopCluster.Location toLocation(FeedScopedId id) { } } + @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()); } diff --git a/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java b/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java index 66b45317718..c62d6099c97 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java +++ b/src/main/java/org/opentripplanner/transit/model/site/GroupOfStations.java @@ -51,12 +51,6 @@ public WgsCoordinate getCoordinate() { return coordinate; } - @Nullable - @Override - public String getCode() { - return null; - } - @Nonnull public Collection getChildStops() { return this.childStations.stream().flatMap(s -> s.getChildStops().stream()).toList(); diff --git a/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java b/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java index 70b2ee45a81..3536f59e9b6 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java +++ b/src/main/java/org/opentripplanner/transit/model/site/StopLocationsGroup.java @@ -1,7 +1,6 @@ package org.opentripplanner.transit.model.site; import java.util.Collection; -import javax.annotation.Nullable; import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.framework.lang.ObjectUtils; @@ -40,7 +39,4 @@ default double getLon() { default String logName() { return ObjectUtils.ifNotNull(getName(), Object::toString, null); } - - @Nullable - String getCode(); } From 6a73f0140556c0a4c99a731ec0f2c438028062fe Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Fri, 31 May 2024 23:52:15 +0200 Subject: [PATCH 9/9] Apply review feedback --- .../ext/geocoder/LuceneIndexTest.java | 14 +++++----- .../ext/geocoder/StopClusterMapper.java | 28 +++++++++++-------- 2 files changed, 23 insertions(+), 19 deletions(-) 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 15f205ad802..cee6cf3a2d8 100644 --- a/src/ext-test/java/org/opentripplanner/ext/geocoder/LuceneIndexTest.java +++ b/src/ext-test/java/org/opentripplanner/ext/geocoder/LuceneIndexTest.java @@ -277,17 +277,17 @@ void fuzzyStopCode(String query) { void modes() { var result = index.queryStopClusters("westh").toList(); assertEquals(1, result.size()); - var stop = result.getFirst(); - assertEquals(WESTHAFEN.getName().toString(), stop.primary().name()); - assertEquals(List.of(FERRY.name(), BUS.name()), stop.primary().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.primary().name()); - assertEquals(List.of(StopClusterMapper.toAgency(BVG)), result.primary().agencies()); - assertEquals("A Publisher", result.primary().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()); } } diff --git a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java index fbb9941c59c..d9f388ea0e8 100644 --- a/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java +++ b/src/ext/java/org/opentripplanner/ext/geocoder/StopClusterMapper.java @@ -63,23 +63,19 @@ Iterable generateStopClusters( ) .values() .stream() - .map(group -> this.map(group).orElse(null)) + .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) { + private static LuceneStopCluster map(StopLocationsGroup g) { var childStops = g.getChildStops(); var ids = childStops.stream().map(s -> s.getId().toString()).toList(); - var childNames = childStops - .stream() - .map(StopLocation::getName) - .filter(Objects::nonNull) - .toList(); - var codes = childStops.stream().map(StopLocation::getCode).filter(Objects::nonNull).toList(); + var childNames = getNames(childStops); + var codes = getCodes(childStops); return new LuceneStopCluster( g.getId().toString(), @@ -90,11 +86,19 @@ LuceneStopCluster map(StopLocationsGroup g) { ); } - Optional map(List stopLocations) { + 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 = stopLocations.stream().map(StopLocation::getName).toList(); - var codes = stopLocations.stream().map(StopLocation::getCode).filter(Objects::nonNull).toList(); + var names = getNames(stopLocations); + var codes = getCodes(stopLocations); return Optional .ofNullable(primary.getName())