From 90d735e6b454846ed1bd089f93a3599b503e026c Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 19 Jun 2018 15:45:57 +0200 Subject: [PATCH 1/6] Added parsing of county --- .../converter/api/AbstractAddress.java | 2 + .../graphhopper/converter/api/GHEntry.java | 16 +- .../converter/api/GisgraphySearchEntry.java | 11 ++ .../converter/api/OpenCageDataEntry.java | 2 - .../graphhopper/converter/core/Converter.java | 4 +- .../ConverterResourceGisgraphyTest.java | 177 +++++++++--------- .../ConverterResourceNominatimTest.java | 19 ++ .../ConverterResourceOpenCageDataTest.java | 45 +++++ src/test/resources/converter.yml | 4 + 9 files changed, 186 insertions(+), 94 deletions(-) create mode 100644 src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java diff --git a/src/main/java/com/graphhopper/converter/api/AbstractAddress.java b/src/main/java/com/graphhopper/converter/api/AbstractAddress.java index 6199c72..271c8fb 100644 --- a/src/main/java/com/graphhopper/converter/api/AbstractAddress.java +++ b/src/main/java/com/graphhopper/converter/api/AbstractAddress.java @@ -18,6 +18,8 @@ public abstract class AbstractAddress { @JsonProperty public String state; @JsonProperty + public String county; + @JsonProperty public String town; @JsonProperty public String village; diff --git a/src/main/java/com/graphhopper/converter/api/GHEntry.java b/src/main/java/com/graphhopper/converter/api/GHEntry.java index 7c3d03b..18cd238 100644 --- a/src/main/java/com/graphhopper/converter/api/GHEntry.java +++ b/src/main/java/com/graphhopper/converter/api/GHEntry.java @@ -22,12 +22,13 @@ public class GHEntry { private String country; private String city; private String state; + private String county; private String street; private String houseNumber; private String postcode; private String osmValue; - public GHEntry(Long osmId, String type, double lat, double lng, String name, String osmValue, String country, String city, String state, String street, String houseNumber, String postcode, Extent extent) { + public GHEntry(Long osmId, String type, double lat, double lng, String name, String osmValue, String country, String city, String state, String county, String street, String houseNumber, String postcode, Extent extent) { this.osmId = osmId; this.osmType = type; this.point = new Point(lat, lng); @@ -35,6 +36,7 @@ public GHEntry(Long osmId, String type, double lat, double lng, String name, Str this.country = country; this.city = city; this.state = state; + this.county = county; this.street = street; this.houseNumber = houseNumber; this.postcode = postcode; @@ -43,7 +45,7 @@ public GHEntry(Long osmId, String type, double lat, double lng, String name, Str } public GHEntry(Long osmId, String type, double lat, double lng, String name, String osmValue, AbstractAddress address, Extent extent) { - this(osmId, type, lat, lng, name, osmValue, address.country, address.getGHCity(), address.state, address.getStreetName(), address.houseNumber, address.postcode, extent); + this(osmId, type, lat, lng, name, osmValue, address.country, address.getGHCity(), address.state, address.county, address.getStreetName(), address.houseNumber, address.postcode, extent); } public GHEntry(){} @@ -78,6 +80,16 @@ public String getState() { return state; } + @JsonProperty + public void setCounty(String county) { + this.county = county; + } + + @JsonProperty + public String getCounty() { + return county; + } + @JsonProperty public String getCity() { return city; diff --git a/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java b/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java index ae77155..8b1301c 100644 --- a/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java +++ b/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java @@ -35,6 +35,9 @@ public class GisgraphySearchEntry { @JsonProperty("adm1_name") private String adm1Name; + @JsonProperty("adm3_name") + private String adm3Name; + @JsonProperty("zip_code") private List zipCodes; @@ -132,6 +135,14 @@ public void setAdm1Name(String adm1Name) { this.adm1Name = adm1Name; } + public String getAdm3Name() { + return adm3Name; + } + + public void setAdm3Name(String adm3Name) { + this.adm3Name = adm3Name; + } + public List getZipCodes() { return zipCodes; } diff --git a/src/main/java/com/graphhopper/converter/api/OpenCageDataEntry.java b/src/main/java/com/graphhopper/converter/api/OpenCageDataEntry.java index ab70ec8..2221d5c 100644 --- a/src/main/java/com/graphhopper/converter/api/OpenCageDataEntry.java +++ b/src/main/java/com/graphhopper/converter/api/OpenCageDataEntry.java @@ -70,8 +70,6 @@ public OCDComponents() { public String type; @JsonProperty("country_code") public String countryCode; - @JsonProperty("county") - public String county; } public OpenCageDataEntry() { diff --git a/src/main/java/com/graphhopper/converter/core/Converter.java b/src/main/java/com/graphhopper/converter/core/Converter.java index 4e804b6..bbe9940 100644 --- a/src/main/java/com/graphhopper/converter/core/Converter.java +++ b/src/main/java/com/graphhopper/converter/core/Converter.java @@ -16,7 +16,7 @@ public static GHEntry convertFromGisgraphyAddress(GisgraphyAddressEntry gisgraph GHEntry rsp = new GHEntry(null, null, gisgraphyEntry.getLat(), gisgraphyEntry.getLng(), gisgraphyEntry.getDisplayName(), null, gisgraphyEntry.getCountry(), gisgraphyEntry.getCity(), - gisgraphyEntry.getState(), gisgraphyEntry.getStreetName(), + gisgraphyEntry.getState(), null, gisgraphyEntry.getStreetName(), gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null); return rsp; } @@ -25,7 +25,7 @@ public static GHEntry convertFromGisgraphySearch(GisgraphySearchEntry gisgraphyE GHEntry rsp = new GHEntry(null, null, gisgraphyEntry.getLat(), gisgraphyEntry.getLng(), gisgraphyEntry.getLabel(), null, gisgraphyEntry.getCountry(), gisgraphyEntry.getIsIn(), - gisgraphyEntry.getAdm1Name(), gisgraphyEntry.getName(), + gisgraphyEntry.getAdm1Name(), gisgraphyEntry.getAdm3Name(), gisgraphyEntry.getName(), gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null); return rsp; } diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java index cebcff1..8ed53b2 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java @@ -2,6 +2,7 @@ import static junit.framework.TestCase.assertTrue; import static org.assertj.core.api.Assertions.assertThat; + import io.dropwizard.client.JerseyClientBuilder; import io.dropwizard.testing.ResourceHelpers; import io.dropwizard.testing.junit.DropwizardAppRule; @@ -21,93 +22,93 @@ * @author Robin Boldt */ public class ConverterResourceGisgraphyTest { - @ClassRule - public static final DropwizardAppRule RULE = - new DropwizardAppRule<>(ConverterApplication.class, ResourceHelpers.resourceFilePath("converter.yml")); - - @Test - public void testHandleForward() { - Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test forward client"); - - client.property(ClientProperties.CONNECT_TIMEOUT, 100000); - client.property(ClientProperties.READ_TIMEOUT, 100000); - - Response response = client.target( - String.format("http://localhost:%d/gisgraphy?q=berlin", RULE.getLocalPort())) - .request() - .get(); - - assertThat(response.getStatus()).isEqualTo(200); - GHResponse entry = response.readEntity(GHResponse.class); - assertTrue(entry.getHits().size()>0); - - //now try with an Address - response = client.target( - String.format("http://localhost:%d/gisgraphy?q=103+avenue+des+champs+elysees,paris", RULE.getLocalPort())) - .request() - .get(); - - assertThat(response.getStatus()).isEqualTo(200); - entry = response.readEntity(GHResponse.class); - assertTrue(entry.getHits().size()>0); - } - - @Test - public void testHandleReverse() { - Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test reverse client"); - - client.property(ClientProperties.CONNECT_TIMEOUT, 100000); - client.property(ClientProperties.READ_TIMEOUT, 100000); - - Response response = client.target( - String.format("http://localhost:%d/gisgraphy/?point=52.5487429714954,-1.81602098644987&reverse=true", RULE.getLocalPort())) - .request() - .get(); - - assertThat(response.getStatus()).isEqualTo(200); - GHResponse entry = response.readEntity(GHResponse.class); - assertTrue(entry.getHits().size()>0); - - } - - @Test - public void testHandleAutocomplete() { - Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete client"); - - client.property(ClientProperties.CONNECT_TIMEOUT, 100000); - client.property(ClientProperties.READ_TIMEOUT, 100000); - - Response response = client.target( - String.format("http://localhost:%d/gisgraphy?q=pari&autocomplete=true", RULE.getLocalPort())) - .request() - .get(); - - assertThat(response.getStatus()).isEqualTo(200); - GHResponse entry = response.readEntity(GHResponse.class); - assertTrue(entry.getHits().size()>0); - - } - - @Test - public void testHandleAutocompleteWithReverseShouldThrows() { - Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete-reverse client"); - - client.property(ClientProperties.CONNECT_TIMEOUT, 100000); - client.property(ClientProperties.READ_TIMEOUT, 100000); - - Response response = null; - try { - response = client.target( - String.format("http://localhost:%d/gisgraphy?q=pari&point=52.5487429714954,-1.81602098644987&autocomplete=true&reverse=true", RULE.getLocalPort())) - .request() - .get(); - } catch (Exception e) { - // TODO Auto-generated catch block - e.printStackTrace(); - } - - assertThat(response.getStatus()).isEqualTo(400); - - } + @ClassRule + public static final DropwizardAppRule RULE = + new DropwizardAppRule<>(ConverterApplication.class, ResourceHelpers.resourceFilePath("converter.yml")); + + @Test + public void testHandleForward() { + Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test forward client"); + + client.property(ClientProperties.CONNECT_TIMEOUT, 100000); + client.property(ClientProperties.READ_TIMEOUT, 100000); + + Response response = client.target( + String.format("http://localhost:%d/gisgraphy?q=berlin", RULE.getLocalPort())) + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + GHResponse entry = response.readEntity(GHResponse.class); + assertTrue(entry.getHits().size() > 0); + + //now try with an Address + response = client.target( + String.format("http://localhost:%d/gisgraphy?q=103+avenue+des+champs+elysees,paris", RULE.getLocalPort())) + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + entry = response.readEntity(GHResponse.class); + assertTrue(entry.getHits().size() > 0); + } + + @Test + public void testHandleReverse() { + Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test reverse client"); + + client.property(ClientProperties.CONNECT_TIMEOUT, 100000); + client.property(ClientProperties.READ_TIMEOUT, 100000); + + Response response = client.target( + String.format("http://localhost:%d/gisgraphy/?point=52.5487429714954,-1.81602098644987&reverse=true", RULE.getLocalPort())) + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + GHResponse entry = response.readEntity(GHResponse.class); + assertTrue(entry.getHits().size() > 0); + + } + + @Test + public void testHandleAutocomplete() { + Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete client"); + + client.property(ClientProperties.CONNECT_TIMEOUT, 100000); + client.property(ClientProperties.READ_TIMEOUT, 100000); + + Response response = client.target( + String.format("http://localhost:%d/gisgraphy?q=pari&autocomplete=true", RULE.getLocalPort())) + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + GHResponse entry = response.readEntity(GHResponse.class); + assertTrue(entry.getHits().size() > 0); + + } + + @Test + public void testHandleAutocompleteWithReverseShouldThrows() { + Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete-reverse client"); + + client.property(ClientProperties.CONNECT_TIMEOUT, 100000); + client.property(ClientProperties.READ_TIMEOUT, 100000); + + Response response = null; + try { + response = client.target( + String.format("http://localhost:%d/gisgraphy?q=pari&point=52.5487429714954,-1.81602098644987&autocomplete=true&reverse=true", RULE.getLocalPort())) + .request() + .get(); + } catch (Exception e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + + assertThat(response.getStatus()).isEqualTo(400); + + } } diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceNominatimTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceNominatimTest.java index 5549f91..2a2036b 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceNominatimTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceNominatimTest.java @@ -166,4 +166,23 @@ public void testIncorrectFormattedPoint() { assertThat(response.getStatus()).isEqualTo(400); } + + @Test + public void testIssue50() { + Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test issue 50"); + + client.property(ClientProperties.CONNECT_TIMEOUT, 100000); + client.property(ClientProperties.READ_TIMEOUT, 100000); + + Response response = client.target( + String.format("http://localhost:%d/nominatim?point=48.4882,2.6996&reverse=true", RULE.getLocalPort())) + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + GHResponse entry = response.readEntity(GHResponse.class); + + // OCD responds with "Seine-et-Marne", both seem to be interlinked: https://en.wikipedia.org/wiki/Fontainebleau + assertEquals("Fontainebleau", entry.getHits().get(0).getCounty()); + } } diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java new file mode 100644 index 0000000..0ffc39a --- /dev/null +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java @@ -0,0 +1,45 @@ +package com.graphhopper.converter.resource; + +import com.graphhopper.converter.ConverterApplication; +import com.graphhopper.converter.ConverterConfiguration; +import com.graphhopper.converter.api.GHResponse; +import io.dropwizard.client.JerseyClientBuilder; +import io.dropwizard.testing.ResourceHelpers; +import io.dropwizard.testing.junit.DropwizardAppRule; +import org.glassfish.jersey.client.ClientProperties; +import org.junit.ClassRule; +import org.junit.Test; + +import javax.ws.rs.client.Client; +import javax.ws.rs.core.Response; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; + +/** + * @author Robin Boldt + */ +public class ConverterResourceOpenCageDataTest { + @ClassRule + public static final DropwizardAppRule RULE = + new DropwizardAppRule<>(ConverterApplication.class, ResourceHelpers.resourceFilePath("converter.yml")); + + @Test + public void testIssue50() { + Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test issue 50"); + + client.property(ClientProperties.CONNECT_TIMEOUT, 100000); + client.property(ClientProperties.READ_TIMEOUT, 100000); + + Response response = client.target( + String.format("http://localhost:%d/opencagedata?point=48.4882,2.6996&reverse=true", RULE.getLocalPort())) + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + GHResponse entry = response.readEntity(GHResponse.class); + + assertEquals("Seine-et-Marne", entry.getHits().get(0).getCounty()); + } + +} diff --git a/src/test/resources/converter.yml b/src/test/resources/converter.yml index 9df7343..13a9e44 100644 --- a/src/test/resources/converter.yml +++ b/src/test/resources/converter.yml @@ -1 +1,5 @@ nominatimURL: https://nominatim.openstreetmap.org/search/ + +openCageDataURL: https://api.opencagedata.com/geocode/v1/json +openCageDataKey: XXXXXXXX +openCageData: true \ No newline at end of file From be13b65505f18d2d868073bd5331efd403b51304 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 20 Jun 2018 11:17:58 +0200 Subject: [PATCH 2/6] Fix gisgraphy --- .../converter/api/GisgraphyAddressEntry.java | 16 +++++++++++++--- .../converter/api/GisgraphySearchEntry.java | 11 ----------- .../graphhopper/converter/core/Converter.java | 4 ++-- .../ConverterResourceGisgraphyTest.java | 19 +++++++++++++++++++ 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java b/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java index 65b40e3..0e2f9e2 100644 --- a/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java +++ b/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java @@ -17,7 +17,7 @@ public class GisgraphyAddressEntry { private long sourceId; private String countryCode; - + private String country; private String city; @@ -36,6 +36,8 @@ public class GisgraphyAddressEntry { private String geocodingLevel; + private String adm3Name; + public long getId() { return id; } @@ -146,8 +148,8 @@ public String getDisplayName() { public String getCountry() { return country; } - - public void setCountry(String country){ + + public void setCountry(String country) { this.country = country; } @@ -159,4 +161,12 @@ public void setGeocodingLevel(String geocodingLevel) { this.geocodingLevel = geocodingLevel; } + public String getAdm3Name() { + return adm3Name; + } + + public void setAdm3Name(String adm3Name) { + this.adm3Name = adm3Name; + } + } diff --git a/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java b/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java index 8b1301c..ae77155 100644 --- a/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java +++ b/src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java @@ -35,9 +35,6 @@ public class GisgraphySearchEntry { @JsonProperty("adm1_name") private String adm1Name; - @JsonProperty("adm3_name") - private String adm3Name; - @JsonProperty("zip_code") private List zipCodes; @@ -135,14 +132,6 @@ public void setAdm1Name(String adm1Name) { this.adm1Name = adm1Name; } - public String getAdm3Name() { - return adm3Name; - } - - public void setAdm3Name(String adm3Name) { - this.adm3Name = adm3Name; - } - public List getZipCodes() { return zipCodes; } diff --git a/src/main/java/com/graphhopper/converter/core/Converter.java b/src/main/java/com/graphhopper/converter/core/Converter.java index bbe9940..e01497e 100644 --- a/src/main/java/com/graphhopper/converter/core/Converter.java +++ b/src/main/java/com/graphhopper/converter/core/Converter.java @@ -16,7 +16,7 @@ public static GHEntry convertFromGisgraphyAddress(GisgraphyAddressEntry gisgraph GHEntry rsp = new GHEntry(null, null, gisgraphyEntry.getLat(), gisgraphyEntry.getLng(), gisgraphyEntry.getDisplayName(), null, gisgraphyEntry.getCountry(), gisgraphyEntry.getCity(), - gisgraphyEntry.getState(), null, gisgraphyEntry.getStreetName(), + gisgraphyEntry.getState(), gisgraphyEntry.getAdm3Name(), gisgraphyEntry.getStreetName(), gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null); return rsp; } @@ -25,7 +25,7 @@ public static GHEntry convertFromGisgraphySearch(GisgraphySearchEntry gisgraphyE GHEntry rsp = new GHEntry(null, null, gisgraphyEntry.getLat(), gisgraphyEntry.getLng(), gisgraphyEntry.getLabel(), null, gisgraphyEntry.getCountry(), gisgraphyEntry.getIsIn(), - gisgraphyEntry.getAdm1Name(), gisgraphyEntry.getAdm3Name(), gisgraphyEntry.getName(), + gisgraphyEntry.getAdm1Name(), null, gisgraphyEntry.getName(), gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null); return rsp; } diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java index 8ed53b2..0a9ca85 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java @@ -2,6 +2,7 @@ import static junit.framework.TestCase.assertTrue; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; import io.dropwizard.client.JerseyClientBuilder; import io.dropwizard.testing.ResourceHelpers; @@ -111,4 +112,22 @@ public void testHandleAutocompleteWithReverseShouldThrows() { } + @Test + public void testIssue50() { + Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test issue 50"); + + client.property(ClientProperties.CONNECT_TIMEOUT, 100000); + client.property(ClientProperties.READ_TIMEOUT, 100000); + + Response response = client.target( + String.format("http://localhost:%d/gisgraphy?point=48.4882,2.6996&reverse=true", RULE.getLocalPort())) + .request() + .get(); + + assertThat(response.getStatus()).isEqualTo(200); + GHResponse entry = response.readEntity(GHResponse.class); + + assertEquals("Seine-et-Marne", entry.getHits().get(0).getCounty()); + } + } From 574d6ee8cae1a5b5e47a1e3fb7497701ddd867db Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 26 Jun 2018 11:34:10 +0200 Subject: [PATCH 3/6] Removed ADM3 parsing for Gisgraphy --- .../converter/api/GisgraphyAddressEntry.java | 10 ---------- .../graphhopper/converter/core/Converter.java | 2 +- .../ConverterResourceGisgraphyTest.java | 18 ------------------ 3 files changed, 1 insertion(+), 29 deletions(-) diff --git a/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java b/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java index 0e2f9e2..2c22285 100644 --- a/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java +++ b/src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java @@ -36,8 +36,6 @@ public class GisgraphyAddressEntry { private String geocodingLevel; - private String adm3Name; - public long getId() { return id; } @@ -161,12 +159,4 @@ public void setGeocodingLevel(String geocodingLevel) { this.geocodingLevel = geocodingLevel; } - public String getAdm3Name() { - return adm3Name; - } - - public void setAdm3Name(String adm3Name) { - this.adm3Name = adm3Name; - } - } diff --git a/src/main/java/com/graphhopper/converter/core/Converter.java b/src/main/java/com/graphhopper/converter/core/Converter.java index e01497e..11e7e30 100644 --- a/src/main/java/com/graphhopper/converter/core/Converter.java +++ b/src/main/java/com/graphhopper/converter/core/Converter.java @@ -16,7 +16,7 @@ public static GHEntry convertFromGisgraphyAddress(GisgraphyAddressEntry gisgraph GHEntry rsp = new GHEntry(null, null, gisgraphyEntry.getLat(), gisgraphyEntry.getLng(), gisgraphyEntry.getDisplayName(), null, gisgraphyEntry.getCountry(), gisgraphyEntry.getCity(), - gisgraphyEntry.getState(), gisgraphyEntry.getAdm3Name(), gisgraphyEntry.getStreetName(), + gisgraphyEntry.getState(), null, gisgraphyEntry.getStreetName(), gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null); return rsp; } diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java index 0a9ca85..eab9b7b 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java @@ -112,22 +112,4 @@ public void testHandleAutocompleteWithReverseShouldThrows() { } - @Test - public void testIssue50() { - Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test issue 50"); - - client.property(ClientProperties.CONNECT_TIMEOUT, 100000); - client.property(ClientProperties.READ_TIMEOUT, 100000); - - Response response = client.target( - String.format("http://localhost:%d/gisgraphy?point=48.4882,2.6996&reverse=true", RULE.getLocalPort())) - .request() - .get(); - - assertThat(response.getStatus()).isEqualTo(200); - GHResponse entry = response.readEntity(GHResponse.class); - - assertEquals("Seine-et-Marne", entry.getHits().get(0).getCounty()); - } - } From ee551696382f7e219eeb15a50c3d9a2c76f5a964 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 27 Jun 2018 09:23:36 +0200 Subject: [PATCH 4/6] Add Environment Variable support to OCD tests --- .../ConverterResourceOpenCageData.java | 6 +++-- .../ConverterResourceGisgraphyTest.java | 27 +++++++++---------- .../ConverterResourceOpenCageDataTest.java | 16 ++++++++++- src/test/resources/converter.yml | 2 +- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java b/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java index 4cd6122..d16ba5d 100644 --- a/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java +++ b/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java @@ -41,7 +41,8 @@ public Response handle(@QueryParam("q") @DefaultValue("") String query, @QueryParam("nominatim") @DefaultValue("false") boolean nominatim, @QueryParam("find_osm_id") @DefaultValue("true") boolean find_osm_id, @QueryParam("reverse") @DefaultValue("false") boolean reverse, - @QueryParam("point") @DefaultValue("false") String point + @QueryParam("point") @DefaultValue("false") String point, + @QueryParam("ocd_key") @DefaultValue("") String ocdKey ) { limit = fixLimit(limit); checkInvalidParameter(reverse, query, point); @@ -49,9 +50,10 @@ public Response handle(@QueryParam("q") @DefaultValue("") String query, WebTarget target = jerseyClient. target(url). queryParam("q", reverse ? point : query). - queryParam("key", key). queryParam("limit", limit); + target = target.queryParam("key", ocdKey.isEmpty() ? key : ocdKey); + if (!find_osm_id) { target = target.queryParam("no_annotations", "1"); } diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java index eab9b7b..50a0e2e 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java @@ -1,23 +1,20 @@ package com.graphhopper.converter.resource; -import static junit.framework.TestCase.assertTrue; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.assertEquals; - +import com.graphhopper.converter.ConverterApplication; +import com.graphhopper.converter.ConverterConfiguration; +import com.graphhopper.converter.api.GHResponse; import io.dropwizard.client.JerseyClientBuilder; import io.dropwizard.testing.ResourceHelpers; import io.dropwizard.testing.junit.DropwizardAppRule; +import org.glassfish.jersey.client.ClientProperties; +import org.junit.ClassRule; +import org.junit.Ignore; import javax.ws.rs.client.Client; import javax.ws.rs.core.Response; -import org.glassfish.jersey.client.ClientProperties; -import org.junit.ClassRule; -import org.junit.Test; - -import com.graphhopper.converter.ConverterApplication; -import com.graphhopper.converter.ConverterConfiguration; -import com.graphhopper.converter.api.GHResponse; +import static junit.framework.TestCase.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; /** * @author Robin Boldt @@ -27,7 +24,7 @@ public class ConverterResourceGisgraphyTest { public static final DropwizardAppRule RULE = new DropwizardAppRule<>(ConverterApplication.class, ResourceHelpers.resourceFilePath("converter.yml")); - @Test + @Ignore public void testHandleForward() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test forward client"); @@ -54,7 +51,7 @@ public void testHandleForward() { assertTrue(entry.getHits().size() > 0); } - @Test + @Ignore public void testHandleReverse() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test reverse client"); @@ -72,7 +69,7 @@ public void testHandleReverse() { } - @Test + @Ignore public void testHandleAutocomplete() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete client"); @@ -90,7 +87,7 @@ public void testHandleAutocomplete() { } - @Test + @Ignore public void testHandleAutocompleteWithReverseShouldThrows() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete-reverse client"); diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java index 0ffc39a..da261e0 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java @@ -7,6 +7,7 @@ import io.dropwizard.testing.ResourceHelpers; import io.dropwizard.testing.junit.DropwizardAppRule; import org.glassfish.jersey.client.ClientProperties; +import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -17,6 +18,9 @@ import static org.junit.Assert.assertEquals; /** + * In order to successfully run this class, you need to specify the ocdKey as environment variable, for example run: + * export OCD_KEY="My_Key" + * * @author Robin Boldt */ public class ConverterResourceOpenCageDataTest { @@ -24,6 +28,16 @@ public class ConverterResourceOpenCageDataTest { public static final DropwizardAppRule RULE = new DropwizardAppRule<>(ConverterApplication.class, ResourceHelpers.resourceFilePath("converter.yml")); + private static String ocdKey = "SPECIFY_AS_ENVIRONMENT_VARIABLE"; + + @BeforeClass + public static void injectKeyFromEnvironment() { + String ocdKey = System.getenv("OCD_KEY"); + if (ocdKey != null) { + ConverterResourceOpenCageDataTest.ocdKey = ocdKey; + } + } + @Test public void testIssue50() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test issue 50"); @@ -32,7 +46,7 @@ public void testIssue50() { client.property(ClientProperties.READ_TIMEOUT, 100000); Response response = client.target( - String.format("http://localhost:%d/opencagedata?point=48.4882,2.6996&reverse=true", RULE.getLocalPort())) + String.format("http://localhost:%d/opencagedata?point=48.4882,2.6996&reverse=true&ocd_key=" + ocdKey, RULE.getLocalPort())) .request() .get(); diff --git a/src/test/resources/converter.yml b/src/test/resources/converter.yml index 13a9e44..96df9e7 100644 --- a/src/test/resources/converter.yml +++ b/src/test/resources/converter.yml @@ -1,5 +1,5 @@ nominatimURL: https://nominatim.openstreetmap.org/search/ openCageDataURL: https://api.opencagedata.com/geocode/v1/json -openCageDataKey: XXXXXXXX +openCageDataKey: XXX openCageData: true \ No newline at end of file From 251dd94401ef1562eb59ed38e48c536e88f2faf2 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 2 Jul 2018 16:36:53 +0200 Subject: [PATCH 5/6] Use environment variable in config --- .../graphhopper/converter/ConverterApplication.java | 12 ++++++++++-- .../resources/ConverterResourceOpenCageData.java | 5 ++--- .../resource/ConverterResourceOpenCageDataTest.java | 12 +----------- src/test/resources/converter.yml | 2 +- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/graphhopper/converter/ConverterApplication.java b/src/main/java/com/graphhopper/converter/ConverterApplication.java index 69e305e..b4a44a7 100644 --- a/src/main/java/com/graphhopper/converter/ConverterApplication.java +++ b/src/main/java/com/graphhopper/converter/ConverterApplication.java @@ -9,6 +9,8 @@ import io.dropwizard.Application; import io.dropwizard.client.JerseyClientBuilder; import io.dropwizard.client.JerseyClientConfiguration; +import io.dropwizard.configuration.EnvironmentVariableSubstitutor; +import io.dropwizard.configuration.SubstitutingSourceProvider; import io.dropwizard.setup.Bootstrap; import io.dropwizard.setup.Environment; import io.dropwizard.util.Duration; @@ -34,7 +36,13 @@ public String getName() { @Override public void initialize(Bootstrap bootstrap) { - // nothing to do yet + // Enable variable substitution with environment variables + bootstrap.setConfigurationSourceProvider( + new SubstitutingSourceProvider(bootstrap.getConfigurationSourceProvider(), + new EnvironmentVariableSubstitutor(false) + ) + ); + } @Override @@ -65,7 +73,7 @@ public void run(ConverterConfiguration converterConfiguration, Environment envir if (converterConfiguration.isGisgraphy()) { final ConverterResourceGisgraphy resource = new ConverterResourceGisgraphy( - converterConfiguration.getGisgraphyGeocodingURL(), converterConfiguration.getGisgraphyReverseGeocodingURL(),converterConfiguration.getGisgraphySearchURL(),converterConfiguration.getGisgraphyAPIKey(), client); + converterConfiguration.getGisgraphyGeocodingURL(), converterConfiguration.getGisgraphyReverseGeocodingURL(), converterConfiguration.getGisgraphySearchURL(), converterConfiguration.getGisgraphyAPIKey(), client); environment.jersey().register(resource); } diff --git a/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java b/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java index d16ba5d..26eeec6 100644 --- a/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java +++ b/src/main/java/com/graphhopper/converter/resources/ConverterResourceOpenCageData.java @@ -41,8 +41,7 @@ public Response handle(@QueryParam("q") @DefaultValue("") String query, @QueryParam("nominatim") @DefaultValue("false") boolean nominatim, @QueryParam("find_osm_id") @DefaultValue("true") boolean find_osm_id, @QueryParam("reverse") @DefaultValue("false") boolean reverse, - @QueryParam("point") @DefaultValue("false") String point, - @QueryParam("ocd_key") @DefaultValue("") String ocdKey + @QueryParam("point") @DefaultValue("false") String point ) { limit = fixLimit(limit); checkInvalidParameter(reverse, query, point); @@ -52,7 +51,7 @@ public Response handle(@QueryParam("q") @DefaultValue("") String query, queryParam("q", reverse ? point : query). queryParam("limit", limit); - target = target.queryParam("key", ocdKey.isEmpty() ? key : ocdKey); + target = target.queryParam("key", key); if (!find_osm_id) { target = target.queryParam("no_annotations", "1"); diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java index da261e0..8158ef4 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceOpenCageDataTest.java @@ -28,16 +28,6 @@ public class ConverterResourceOpenCageDataTest { public static final DropwizardAppRule RULE = new DropwizardAppRule<>(ConverterApplication.class, ResourceHelpers.resourceFilePath("converter.yml")); - private static String ocdKey = "SPECIFY_AS_ENVIRONMENT_VARIABLE"; - - @BeforeClass - public static void injectKeyFromEnvironment() { - String ocdKey = System.getenv("OCD_KEY"); - if (ocdKey != null) { - ConverterResourceOpenCageDataTest.ocdKey = ocdKey; - } - } - @Test public void testIssue50() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test issue 50"); @@ -46,7 +36,7 @@ public void testIssue50() { client.property(ClientProperties.READ_TIMEOUT, 100000); Response response = client.target( - String.format("http://localhost:%d/opencagedata?point=48.4882,2.6996&reverse=true&ocd_key=" + ocdKey, RULE.getLocalPort())) + String.format("http://localhost:%d/opencagedata?point=48.4882,2.6996&reverse=true", RULE.getLocalPort())) .request() .get(); diff --git a/src/test/resources/converter.yml b/src/test/resources/converter.yml index 96df9e7..822ad2b 100644 --- a/src/test/resources/converter.yml +++ b/src/test/resources/converter.yml @@ -1,5 +1,5 @@ nominatimURL: https://nominatim.openstreetmap.org/search/ openCageDataURL: https://api.opencagedata.com/geocode/v1/json -openCageDataKey: XXX +openCageDataKey: ${OCD_KEY} openCageData: true \ No newline at end of file From 8a4af4b256c6d5759c4a8d61ccabdeb06676b804 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 3 Jul 2018 14:54:03 +0200 Subject: [PATCH 6/6] Use environment variable for gisgraphy as well --- converter.yml | 12 ++++++------ .../resource/ConverterResourceGisgraphyTest.java | 10 +++++----- src/test/resources/converter.yml | 7 ++++++- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/converter.yml b/converter.yml index 4bd01c1..84b20e9 100644 --- a/converter.yml +++ b/converter.yml @@ -4,13 +4,13 @@ nominatim: true nominatimEmail: openCageDataURL: https://api.opencagedata.com/geocode/v1/json -openCageDataKey: -openCageData: false +openCageDataKey: ${OCD_KEY} +openCageData: true -gisgraphyGeocodingURL: https://premium2.gisgraphy.com/geocoding/ -gisgraphyReverseGeocodingURL: https://premium2.gisgraphy.com/reversegeocoding/ -gisgraphySearchURL: https://premium2.gisgraphy.com/fulltext/ -gisgraphyAPIKey: +gisgraphyGeocodingURL: ${GIS_GEO_URL} +gisgraphyReverseGeocodingURL: ${GIS_REV_GEO_URL} +gisgraphySearchURL: ${GIS_SEARCH_URL} +gisgraphyAPIKey: ${GIS_KEY} # e.g. to restrict for local access # ipWhiteList:"localhost,127.0.0.1" diff --git a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java index 50a0e2e..2b90b6a 100644 --- a/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java +++ b/src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java @@ -8,7 +8,7 @@ import io.dropwizard.testing.junit.DropwizardAppRule; import org.glassfish.jersey.client.ClientProperties; import org.junit.ClassRule; -import org.junit.Ignore; +import org.junit.Test; import javax.ws.rs.client.Client; import javax.ws.rs.core.Response; @@ -24,7 +24,7 @@ public class ConverterResourceGisgraphyTest { public static final DropwizardAppRule RULE = new DropwizardAppRule<>(ConverterApplication.class, ResourceHelpers.resourceFilePath("converter.yml")); - @Ignore + @Test public void testHandleForward() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test forward client"); @@ -51,7 +51,7 @@ public void testHandleForward() { assertTrue(entry.getHits().size() > 0); } - @Ignore + @Test public void testHandleReverse() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test reverse client"); @@ -69,7 +69,7 @@ public void testHandleReverse() { } - @Ignore + @Test public void testHandleAutocomplete() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete client"); @@ -87,7 +87,7 @@ public void testHandleAutocomplete() { } - @Ignore + @Test public void testHandleAutocompleteWithReverseShouldThrows() { Client client = new JerseyClientBuilder(RULE.getEnvironment()).build("test autocomplete-reverse client"); diff --git a/src/test/resources/converter.yml b/src/test/resources/converter.yml index 822ad2b..21bcd09 100644 --- a/src/test/resources/converter.yml +++ b/src/test/resources/converter.yml @@ -2,4 +2,9 @@ nominatimURL: https://nominatim.openstreetmap.org/search/ openCageDataURL: https://api.opencagedata.com/geocode/v1/json openCageDataKey: ${OCD_KEY} -openCageData: true \ No newline at end of file +openCageData: true + +gisgraphyGeocodingURL: ${GIS_GEO_URL} +gisgraphyReverseGeocodingURL: ${GIS_REV_GEO_URL} +gisgraphySearchURL: ${GIS_SEARCH_URL} +gisgraphyAPIKey: ${GIS_KEY} \ No newline at end of file