Skip to content

Commit 334779a

Browse files
authored
Merge pull request #897 from lonvia/extended-address-parsing
Rework name filtering for place names and addresses
2 parents b904b39 + 3cedd1d commit 334779a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+994
-832
lines changed

app/es_embedded/src/main/java/de/komoot/photon/elasticsearch/PhotonDocConverter.java

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import de.komoot.photon.Constants;
44
import de.komoot.photon.DatabaseProperties;
55
import de.komoot.photon.PhotonDoc;
6-
import de.komoot.photon.ConfigExtraTags;
76
import de.komoot.photon.nominatim.model.AddressType;
87

98
import org.elasticsearch.common.xcontent.*;
@@ -12,7 +11,6 @@
1211
import org.locationtech.jts.io.geojson.GeoJsonWriter;
1312

1413
import java.io.IOException;
15-
import java.util.HashMap;
1614
import java.util.Map;
1715
import java.util.Set;
1816

@@ -60,24 +58,16 @@ public static XContentBuilder convert(PhotonDoc doc, DatabaseProperties dbProper
6058
builder.field("postcode", doc.getPostcode());
6159
}
6260

63-
writeName(builder, doc, dbProperties.getLanguages());
61+
write(builder, doc.getName(), "name");
6462

65-
for (AddressType entry : doc.getAddressParts().keySet()) {
66-
Map<String, String> fNames = new HashMap<>();
67-
68-
doc.copyAddressName(fNames, "default", entry, "name");
69-
70-
for (String language : dbProperties.getLanguages()) {
71-
doc.copyAddressName(fNames, language, entry, "name:" + language);
72-
}
73-
74-
write(builder, fNames, entry.getName());
63+
for (var entry : doc.getAddressParts().entrySet()) {
64+
write(builder, entry.getValue(), entry.getKey().getName());
7565
}
7666

7767
String countryCode = doc.getCountryCode();
7868
if (countryCode != null)
7969
builder.field(Constants.COUNTRYCODE, countryCode);
80-
writeContext(builder, doc.getContextByLanguage(dbProperties.getLanguages()));
70+
writeContext(builder, doc.getContext());
8171
writeExtraTags(builder, dbProperties.configExtraTags().filterExtraTags(doc.getExtratags()));
8272
writeExtent(builder, doc.getBbox());
8373

@@ -88,7 +78,7 @@ public static XContentBuilder convert(PhotonDoc doc, DatabaseProperties dbProper
8878
}
8979

9080
private static void writeExtraTags(XContentBuilder builder, Map<String, String> extraTags) throws IOException {
91-
if (extraTags.size() > 0) {
81+
if (!extraTags.isEmpty()) {
9282
builder.startObject("extra");
9383
for (var entry : extraTags.entrySet()) {
9484
builder.field(entry.getKey(), entry.getValue());
@@ -114,25 +104,6 @@ private static void writeExtent(XContentBuilder builder, Envelope bbox) throws I
114104
builder.endObject();
115105
}
116106

117-
private static void writeName(XContentBuilder builder, PhotonDoc doc, String[] languages) throws IOException {
118-
Map<String, String> fNames = new HashMap<>();
119-
120-
doc.copyName(fNames, "default", "name");
121-
122-
for (String language : languages) {
123-
doc.copyName(fNames, language, "name:" + language);
124-
}
125-
126-
doc.copyName(fNames, "alt", "alt_name");
127-
doc.copyName(fNames, "int", "int_name");
128-
doc.copyName(fNames, "loc", "loc_name");
129-
doc.copyName(fNames, "old", "old_name");
130-
doc.copyName(fNames, "reg", "reg_name");
131-
doc.copyName(fNames, "housename", "addr:housename");
132-
133-
write(builder, fNames, "name");
134-
}
135-
136107
private static void write(XContentBuilder builder, Map<String, String> fNames, String name) throws IOException {
137108
if (fNames.isEmpty()) return;
138109

app/es_embedded/src/test/java/de/komoot/photon/ESBaseTester.java

Lines changed: 0 additions & 83 deletions
This file was deleted.

app/es_embedded/src/test/java/de/komoot/photon/ElasticTestServer.java renamed to app/es_embedded/src/test/java/de/komoot/photon/TestServer.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
66
import org.elasticsearch.action.get.GetResponse;
77

8-
public class ElasticTestServer extends Server {
9-
public ElasticTestServer(String mainDirectory) {
8+
public class TestServer extends Server {
9+
public static final String TEST_CLUSTER_NAME = "photon-test";
10+
11+
public TestServer(String mainDirectory) {
1012
super(mainDirectory);
1113
}
1214

@@ -15,7 +17,7 @@ protected IndexSettings loadIndexSettings() {
1517
return new IndexSettings().setShards(1);
1618
}
1719

18-
public PhotonResult getById(String id) {
20+
public PhotonResult getByID(String id) {
1921
GetResponse response = esClient.prepareGet(PhotonIndex.NAME,PhotonIndex.TYPE, id).execute().actionGet();
2022

2123
return response.isExists() ? new ElasticGetIdResult(response) : null;
@@ -24,4 +26,17 @@ public PhotonResult getById(String id) {
2426
public void refresh() {
2527
esClient.admin().indices().refresh(new RefreshRequest(PhotonIndex.NAME)).actionGet();
2628
}
29+
30+
public void startTestServer(String clusterName) {
31+
start(TEST_CLUSTER_NAME, new String[]{});
32+
}
33+
34+
public void stopTestServer() {
35+
shutdown();
36+
}
37+
38+
public void refreshTestServer() {
39+
refreshIndexes();
40+
}
41+
2742
}

app/es_embedded/src/test/java/de/komoot/photon/elasticsearch/ElasticResultTest.java

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,13 @@
1212
import org.junit.jupiter.api.TestInstance;
1313
import org.junit.jupiter.api.io.TempDir;
1414

15-
import java.io.IOException;
1615
import java.nio.file.Path;
1716
import java.util.*;
1817

1918
import static org.junit.jupiter.api.Assertions.*;
2019

2120
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
2221
class ElasticResultTest extends ESBaseTester {
23-
private Map<String, String> makeMap(String... kv) {
24-
Map<String, String> result = new HashMap<>();
25-
for (int i = 0; i < kv.length; i += 2) {
26-
result.put(kv[i], kv[i + 1]);
27-
}
28-
29-
return result;
30-
}
31-
3222
protected PhotonDoc createDoc(double lon, double lat, int id, int osmId, String key, String value) {
3323
Point location = FACTORY.createPoint(new Coordinate(lon, lat));
3424
return new PhotonDoc(id, "W", osmId, key, value).centroid(location);
@@ -45,21 +35,23 @@ void setUp(@TempDir Path dataDirectory) throws Exception {
4535

4636
instance.add(List.of(
4737
createDoc(45.2, -7.45, 123, 1123, "place", "city")
48-
.names(makeMap("name", "München", "name:it", "Monacco", "name:en", "Munich"))
49-
.address(Collections.singletonMap("state", "Bavaria"))
38+
.names(makeDocNames("name", "München", "name:it", "Monacco", "name:en", "Munich"))
39+
.addAddresses(Map.of("state", "Bavaria"), getProperties().getLanguages())
5040
.countryCode("de")
51-
.extraTags(makeMap("population", "many", "capital", "yes", "maxage", "99"))));
41+
.extraTags(Map.of("population", "many", "capital", "yes", "maxage", "99"))));
5242
instance.add(List.of(
5343
createDoc(0, 0, 99, 11999, "place", "locality")
54-
.names(makeMap("name", "null island"))));
44+
.names(makeDocNames("name", "null island"))));
5545
instance.add(List.of(
5646
createDoc(-179, 1.0001, 923, 1923, "place", "house")
5747
.houseNumber("34")
58-
.bbox(FACTORY.createMultiPoint(new Coordinate[]{new Coordinate(-179.5, 1.0), new Coordinate(-178.5, 1.1)}))
59-
.address(makeMap("street", "Hauptstr", "city", "Hamburg"))));
48+
.bbox(FACTORY.createMultiPointFromCoords(
49+
new Coordinate[]{new Coordinate(-179.5, 1.0), new Coordinate(-178.5, 1.1)}))
50+
.addAddresses(Map.of("street", "Hauptstr", "city", "Hamburg"),
51+
getProperties().getLanguages())));
6052
instance.add(List.of(
6153
new PhotonDoc(42, "N", 42, "place", "hamlet")
62-
.names(makeMap("name", "nowhere"))));
54+
.names(makeDocNames("name", "nowhere"))));
6355

6456
instance.finish();
6557
refresh();
@@ -93,12 +85,12 @@ void testGet() {
9385
@Test
9486
void testGetMap() {
9587
assertAll("getMap",
96-
() -> assertEquals(makeMap("default", "München", "en", "Munich", "it", "Monacco"),
88+
() -> assertEquals(Map.of("default", "München", "en", "Munich", "it", "Monacco"),
9789
search("München").getMap("name")),
98-
() -> assertEquals(makeMap("default", "null island"),
90+
() -> assertEquals(Map.of("default", "null island"),
9991
search("null island").getMap("name")),
10092
() -> assertNull(search("Hauptstr 34").getMap("name")),
101-
() -> assertEquals(makeMap("population", "many", "capital", "yes"),
93+
() -> assertEquals(Map.of("population", "many", "capital", "yes"),
10294
search("München").getMap("extra"))
10395
);
10496
}

app/es_embedded/src/test/java/de/komoot/photon/elasticsearch/UpdaterTest.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010

1111
import java.io.IOException;
1212
import java.nio.file.Path;
13-
import java.util.*;
13+
import java.util.List;
14+
import java.util.Map;
1415

1516
import static org.junit.jupiter.api.Assertions.*;
1617

@@ -19,19 +20,23 @@ class UpdaterTest extends ESBaseTester {
1920
@TempDir
2021
private Path dataDirectory;
2122

23+
private PhotonDoc createDoc(String... names) {
24+
return new PhotonDoc()
25+
.placeId(1234).osmType("N").osmId(1000).tagKey("place").tagValue("city")
26+
.names(makeDocNames(names));
27+
}
28+
2229
@Test
2330
void addNameToDoc() throws IOException {
24-
Map<String, String> names = new HashMap<>();
25-
names.put("name", "Foo");
26-
PhotonDoc doc = new PhotonDoc(1234, "N", 1000, "place", "city").names(names);
31+
PhotonDoc doc = createDoc("name", "Foo");
2732

2833
setUpES(dataDirectory);
2934
Importer instance = makeImporter();
3035
instance.add(List.of(doc));
3136
instance.finish();
3237
refresh();
3338

34-
names.put("name:en", "Enfoo");
39+
doc.names(makeDocNames("name", "Foo", "name:en", "Enfoo"));
3540
Updater updater = makeUpdater();
3641
updater.addOrUpdate(List.of(doc));
3742
updater.finish();
@@ -47,18 +52,15 @@ void addNameToDoc() throws IOException {
4752

4853
@Test
4954
void removeNameFromDoc() throws IOException {
50-
Map<String, String> names = new HashMap<>();
51-
names.put("name", "Foo");
52-
names.put("name:en", "Enfoo");
53-
PhotonDoc doc = new PhotonDoc(1234, "N", 1000, "place", "city").names(names);
55+
PhotonDoc doc = createDoc("name", "Foo", "name:en", "Enfoo");
5456

5557
setUpES(dataDirectory);
5658
Importer instance = makeImporter();
5759
instance.add(List.of(doc));
5860
instance.finish();
5961
refresh();
6062

61-
names.remove("name");
63+
doc.names(makeDocNames("name:en", "Enfoo"));
6264
Updater updater = makeUpdater();
6365
updater.addOrUpdate(List.of(doc));
6466
updater.finish();
@@ -74,9 +76,7 @@ void removeNameFromDoc() throws IOException {
7476

7577
@Test
7678
void addExtraTagsToDoc() throws IOException {
77-
Map<String, String> names = new HashMap<>();
78-
names.put("name", "Foo");
79-
PhotonDoc doc = new PhotonDoc(1234, "N", 1000, "place", "city").names(names);
79+
PhotonDoc doc = createDoc("name", "Foo");
8080

8181
getProperties().setExtraTags(List.of("website"));
8282
setUpES(dataDirectory);
@@ -90,7 +90,7 @@ void addExtraTagsToDoc() throws IOException {
9090

9191
assertNull(response.get("extra"));
9292

93-
doc.extraTags(Collections.singletonMap("website", "http://site.foo"));
93+
doc.extraTags(Map.of("website", "http://site.foo"));
9494
Updater updater = makeUpdater();
9595
updater.addOrUpdate(List.of(doc));
9696
updater.finish();
@@ -102,7 +102,7 @@ void addExtraTagsToDoc() throws IOException {
102102
Map<String, String> extra = response.getMap("extra");
103103

104104
assertNotNull(extra);
105-
assertEquals(Collections.singletonMap("website", "http://site.foo"), extra);
105+
assertEquals(Map.of("website", "http://site.foo"), extra);
106106
}
107107

108108
@Test

0 commit comments

Comments
 (0)