Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for polygon/linestring in results #823

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/es_embedded/es/mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
"coordinate": {
"type": "geo_point"
},
"geometry": {
"type": "geo_shape"
},
"country": {
"properties": {
"default": {
Expand Down
18 changes: 14 additions & 4 deletions app/es_embedded/src/main/java/de/komoot/photon/Server.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class Server {
private static final String FIELD_VERSION = "database_version";
private static final String FIELD_LANGUAGES = "indexed_languages";
private static final String FIELD_IMPORT_DATE = "import_date";
private static final String FIELD_SUPPORT_POLYGONS = "support_polygons";

private Node esNode;

Expand Down Expand Up @@ -177,14 +178,18 @@ private void setupDirectories(URL directoryName) throws IOException, URISyntaxEx

}

public DatabaseProperties recreateIndex(String[] languages, Date importDate, boolean supportStructuredQueries) throws IOException {
public DatabaseProperties recreateIndex(String[] languages, Date importDate, boolean supportStructuredQueries, boolean supportPolygons) throws IOException {
deleteIndex();

loadIndexSettings().createIndex(esClient, PhotonIndex.NAME);

createAndPutIndexMapping(languages, supportStructuredQueries);

DatabaseProperties dbProperties = new DatabaseProperties(languages, importDate, false);
DatabaseProperties dbProperties = new DatabaseProperties()
.setLanguages(languages)
.setImportDate(importDate)
.setSupportPolygons(supportPolygons);

saveToDatabase(dbProperties);

return dbProperties;
Expand Down Expand Up @@ -239,6 +244,7 @@ public void saveToDatabase(DatabaseProperties dbProperties) throws IOException
.field(FIELD_VERSION, DATABASE_VERSION)
.field(FIELD_LANGUAGES, String.join(",", dbProperties.getLanguages()))
.field(FIELD_IMPORT_DATE, dbProperties.getImportDate() instanceof Date ? dbProperties.getImportDate().toInstant() : null)
.field(FIELD_SUPPORT_POLYGONS, Boolean.toString(dbProperties.getSupportPolygons()))
.endObject().endObject();

esClient.prepareIndex(PhotonIndex.NAME, PhotonIndex.TYPE).
Expand Down Expand Up @@ -276,11 +282,15 @@ public DatabaseProperties loadFromDatabase() {
}

String langString = properties.get(FIELD_LANGUAGES);

String importDateString = properties.get(FIELD_IMPORT_DATE);

String supportPolygons = properties.get(FIELD_SUPPORT_POLYGONS);

return new DatabaseProperties(langString == null ? null : langString.split(","),
importDateString == null ? null : Date.from(Instant.parse(importDateString)),
false);
importDateString == null ? null : Date.from(Instant.parse(importDateString)),
false,
supportPolygons == null ? null : Boolean.parseBoolean(supportPolygons));
red-fenix marked this conversation as resolved.
Show resolved Hide resolved
}

public Importer createImporter(String[] languages, String[] extraTags) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ public double[] getCoordinates() {
return new double[]{coordinate.get(Constants.LON), coordinate.get(Constants.LAT)};
}

@Override
public double[][] getGeometry() {
return null;
}

@Override
public double[] getExtent() {
final Map<String, Object> extent = (Map<String, Object>) result.getSource().get("extent");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
import de.komoot.photon.Constants;
import de.komoot.photon.PhotonDoc;
import de.komoot.photon.nominatim.model.AddressType;

import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.io.geojson.GeoJsonWriter;

import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -38,6 +44,17 @@ public static XContentBuilder convert(PhotonDoc doc, String[] languages, String[
.endObject();
}

if (doc.getGeometry() != null) {
GeoJsonWriter g = new GeoJsonWriter();

XContentParser parser = JsonXContent
.jsonXContent
.createParser(NamedXContentRegistry.EMPTY, g.write(doc.getGeometry()));

builder.field("geometry");
builder.copyCurrentStructure(parser);
}

if (doc.getHouseNumber() != null) {
builder.field("housenumber", doc.getHouseNumber());
}
Expand Down
16 changes: 11 additions & 5 deletions app/es_embedded/src/test/java/de/komoot/photon/ESBaseTester.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import de.komoot.photon.searcher.PhotonResult;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.io.TempDir;
import org.locationtech.jts.io.ParseException;
import org.locationtech.jts.io.WKTReader;

import java.io.IOException;
import java.nio.file.Path;
Expand All @@ -25,9 +27,9 @@ public class ESBaseTester {

private ElasticTestServer server;

protected PhotonDoc createDoc(double lon, double lat, int id, int osmId, String key, String value) {
protected PhotonDoc createDoc(double lon, double lat, int id, int osmId, String key, String value) throws ParseException {
Point location = FACTORY.createPoint(new Coordinate(lon, lat));
return new PhotonDoc(id, "W", osmId, key, value).names(Collections.singletonMap("name", "berlin")).centroid(location);
return new PhotonDoc(id, "W", osmId, key, value).names(Collections.singletonMap("name", "berlin")).centroid(location).geometry(new WKTReader().read("POLYGON ((6.4440619 52.1969454, 6.4441094 52.1969158, 6.4441408 52.1969347, 6.4441138 52.1969516, 6.4440933 52.1969643, 6.4440619 52.1969454))"));
}

protected PhotonResult getById(int id) {
Expand All @@ -45,17 +47,21 @@ public void tearDown() throws IOException {
}

public void setUpES() throws IOException {
setUpES(dataDirectory, "en");
setUpES(dataDirectory, false,"en");
red-fenix marked this conversation as resolved.
Show resolved Hide resolved
}

public void setUpESWithPolygons() throws IOException {
setUpES(dataDirectory, true,"en");
}
/**
* Setup the ES server
*
* @throws IOException
*/
public void setUpES(Path testDirectory, String... languages) throws IOException {
public void setUpES(Path testDirectory, boolean supportPolygons, String... languages) throws IOException {
server = new ElasticTestServer(testDirectory.toString());
server.start(TEST_CLUSTER_NAME, new String[]{});
server.recreateIndex(languages, new Date(), false);
server.recreateIndex(languages, new Date(), false, supportPolygons);
refresh();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public double[] getCoordinates() {
throw new NotImplementedException();
}

public double[][] getGeometry() {
throw new NotImplementedException();
}

@Override
public double[] getExtent() {
throw new NotImplementedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ protected PhotonDoc createDoc(double lon, double lat, int id, int osmId, String

@BeforeAll
void setUp() throws Exception {
setUpES(instanceTestDirectory, "en", "de", "fr", "it");
setUpES(instanceTestDirectory, false, "en", "de", "fr", "it");
Importer instance = getServer().createImporter(new String[]{"en", "de", "fr", "it"},
new String[]{"population", "capital"});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ void testSaveAndLoadFromDatabase() throws IOException {
setUpES();

Date now = new Date();
DatabaseProperties prop = new DatabaseProperties(new String[]{"en", "de", "fr"}, now, false);
DatabaseProperties prop = new DatabaseProperties(new String[]{"en", "de", "fr"}, now, false, false);
getServer().saveToDatabase(prop);

prop = getServer().loadFromDatabase();
Expand Down
2 changes: 1 addition & 1 deletion app/opensearch/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ dependencies {
implementation 'org.apache.httpcomponents.client5:httpclient5:5.4.1'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.18.2'

implementation('org.codelibs.opensearch:opensearch-runner:2.18.0.0') {
implementation('org.codelibs.opensearch:opensearch-runner:2.18.0.1') {
exclude(module: 'repository-url')
exclude(module: 'reindex-client')
exclude(module: 'rank-eval-client')
Expand Down
12 changes: 8 additions & 4 deletions app/opensearch/src/main/java/de/komoot/photon/Server.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ public class Server {

public static final String OPENSEARCH_MODULES =
"org.opensearch.transport.Netty4Plugin,"
+ "org.opensearch.analysis.common.CommonAnalysisPlugin";
+ "org.opensearch.analysis.common.CommonAnalysisPlugin,"
+ "org.opensearch.geo.GeoModulePlugin,"
+ "org.opensearch.geospatial.plugin.GeospatialPlugin";

protected OpenSearchClient client;
private OpenSearchRunner runner = null;
Expand Down Expand Up @@ -87,6 +89,7 @@ private HttpHost[] startInternal(String clusterName) {
.clusterName(clusterName)
.numOfNode(1)
.moduleTypes(OPENSEARCH_MODULES)
.pluginTypes(OPENSEARCH_MODULES)
red-fenix marked this conversation as resolved.
Show resolved Hide resolved
);

runner.ensureYellow();
Expand Down Expand Up @@ -119,7 +122,7 @@ public void shutdown() {
}
}

public DatabaseProperties recreateIndex(String[] languages, Date importDate, boolean supportStructuredQueries) throws IOException {
public DatabaseProperties recreateIndex(String[] languages, Date importDate, boolean supportStructuredQueries, boolean supportPolygons) throws IOException {
// delete any existing data
if (client.indices().exists(e -> e.index(PhotonIndex.NAME)).value()) {
client.indices().delete(d -> d.index(PhotonIndex.NAME));
Expand All @@ -129,7 +132,7 @@ public DatabaseProperties recreateIndex(String[] languages, Date importDate, boo

(new IndexMapping(supportStructuredQueries)).addLanguages(languages).putMapping(client, PhotonIndex.NAME);

var dbProperties = new DatabaseProperties(languages, importDate, supportStructuredQueries);
var dbProperties = new DatabaseProperties(languages, importDate, supportStructuredQueries, supportPolygons);
saveToDatabase(dbProperties);

return dbProperties;
Expand Down Expand Up @@ -180,7 +183,8 @@ public DatabaseProperties loadFromDatabase() throws IOException {

return new DatabaseProperties(dbEntry.source().languages,
dbEntry.source().importDate,
dbEntry.source().supportStructuredQueries);
dbEntry.source().supportStructuredQueries,
dbEntry.source().supportPolygons);
}

public Importer createImporter(String[] languages, String[] extraTags) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public class DBPropertyEntry {
public Date importDate;
public String[] languages;
public boolean supportStructuredQueries;
public boolean supportPolygons;

public DBPropertyEntry() {}

Expand All @@ -17,5 +18,6 @@ public DBPropertyEntry(DatabaseProperties props, String databaseVersion) {
importDate = props.getImportDate();
languages = props.getLanguages();
supportStructuredQueries = props.getSupportStructuredQueries();
supportPolygons = props.getSupportPolygons();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ private void setupBaseMappings() {
}

mappings.properties("coordinate", b -> b.geoPoint(p -> p));
mappings.properties("geometry", b -> b.geoShape(p -> p));
mappings.properties("countrycode", b -> b.keyword(p -> p.index(true)));
mappings.properties("importance", b -> b.float_(p -> p.index(false)));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ public class OpenSearchResult implements PhotonResult {
private double score = 0.0;
private final double[] extent;
private final double[] coordinates;
private final double[][] geometry;
private final Map<String, Object> infos;
private final Map<String, Map<String, String>> localeTags;

OpenSearchResult(double[] extent, double[] coordinates, Map<String, Object> infos, Map<String, Map<String, String>> localeTags) {
OpenSearchResult(double[] extent, double[] coordinates, Map<String, Object> infos, Map<String, Map<String, String>> localeTags, double[][] geometry) {
this.extent = extent;
this.coordinates = coordinates;
this.infos = infos;
this.localeTags = localeTags;
this.geometry = geometry;
}

public OpenSearchResult setScore(double score) {
Expand Down Expand Up @@ -61,6 +63,10 @@ public double[] getCoordinates() {
return coordinates;
}

public double[][] getGeometry() {
return geometry;
}

@Override
public double[] getExtent() {
return extent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
Expand All @@ -28,6 +29,7 @@ public OpenSearchResult deserialize(JsonParser p, DeserializationContext ctxt) t

final Map<String, Object> tags = new HashMap<>();
final Map<String, Map<String, String>> localeTags = new HashMap<>();
double[][] geometry = extractGeometry((ObjectNode) node.get("geometry"));

var fields = node.fields();
while (fields.hasNext()) {
Expand Down Expand Up @@ -55,7 +57,7 @@ public OpenSearchResult deserialize(JsonParser p, DeserializationContext ctxt) t
}
}

return new OpenSearchResult(extent, coordinates, tags, localeTags);
return new OpenSearchResult(extent, coordinates, tags, localeTags, geometry);
}

private double[] extractExtent(ObjectNode node) {
Expand All @@ -79,4 +81,17 @@ private double[] extractCoordinate(ObjectNode node) {
return new double[]{node.get(Constants.LON).doubleValue(), node.get(Constants.LAT).doubleValue()};
}

private double[][] extractGeometry(ObjectNode node) {
if (node == null) {
return PhotonResult.INVALID_GEOMETRY;
}

double[][] coordinates = new double[node.get("coordinates").get(0).size()][];
for(int i=0; i<node.get("coordinates").get(0).size(); i++) {
double[] coordinate = new double[] {node.get("coordinates").get(0).get(i).get(0).doubleValue(), node.get("coordinates").get(0).get(i).get(1).doubleValue()};
coordinates[i] = coordinate;
}

return coordinates;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
import de.komoot.photon.Constants;
import de.komoot.photon.PhotonDoc;
import de.komoot.photon.Utils;
import org.locationtech.jts.geom.Coordinate;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.io.geojson.GeoJsonWriter;

import java.io.IOException;
import java.io.StringWriter;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -48,6 +51,40 @@ public void serialize(PhotonDoc value, JsonGenerator gen, SerializerProvider pro
gen.writeEndObject();
}

if (value.getGeometry() != null) {
// gen.writeStringField("geometry", g.write(value.getGeometry()));

gen.writeObjectFieldStart("geometry");
gen.writeStringField("type", "Polygon");

gen.writeArrayFieldStart("coordinates");

gen.writeStartArray();

for (Coordinate c: value.getGeometry().getCoordinates()) {
gen.writeStartArray();
gen.writeNumber(c.x);
gen.writeNumber(c.y);
gen.writeEndArray();
}
gen.writeEndArray();

// gen.writeStartArray();
// gen.writeNumber(bbox.getMaxX());
// gen.writeNumber(bbox.getMinY());
// gen.writeEndArray();

gen.writeEndArray();
gen.writeEndObject();

// gen.writeObjectFieldStart("crs");
// gen.writeStringField("type", "name");
// gen.writeObjectFieldStart("properties");
// gen.writeStringField("name", "EPSG:0");
// gen.writeEndObject();
// gen.writeEndObject();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't just assume that the geometry is of type polygon. There may be pretty much anything in there: point, linestring, multilinestring, polygon, multipolygon.

Looks like we need tests for all the different geometry types to make sure this works.

Copy link
Author

@red-fenix red-fenix Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added support for LineString and Polygons for now. I have to examine the Nominatim database if they use other formats.

}

if (value.getHouseNumber() != null) {
gen.writeStringField("housenumber", value.getHouseNumber());
}
Expand Down
Loading
Loading