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.

Sign up for GitHub

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

Jump to bottom

Added parsing of county #51

Merged
merged 6 commits into from
Jul 3, 2018
Merged

Added parsing of county #51

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
90d735e
Added parsing of county
boldtrn Jun 19, 2018
be13b65
Fix gisgraphy
boldtrn Jun 20, 2018
574d6ee
Removed ADM3 parsing for Gisgraphy
boldtrn Jun 26, 2018
ee55169
Add Environment Variable support to OCD tests
boldtrn Jun 27, 2018
251dd94
Use environment variable in config
boldtrn Jul 2, 2018
8a4af4b
Use environment variable for gisgraphy as well
boldtrn Jul 3, 2018
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
2 changes: 2 additions & 0 deletions src/main/java/com/graphhopper/converter/api/AbstractAddress.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public abstract class AbstractAddress {
@JsonProperty
public String state;
@JsonProperty
public String county;
@JsonProperty
public String town;
@JsonProperty
public String village;
Expand Down
16 changes: 14 additions & 2 deletions src/main/java/com/graphhopper/converter/api/GHEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,21 @@ 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);
this.name = name;
this.country = country;
this.city = city;
this.state = state;
this.county = county;
this.street = street;
this.houseNumber = houseNumber;
this.postcode = postcode;
Expand All @@ -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(){}
Expand Down Expand Up @@ -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;
Expand Down
16 changes: 13 additions & 3 deletions src/main/java/com/graphhopper/converter/api/GisgraphyAddressEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class GisgraphyAddressEntry {
private long sourceId;

private String countryCode;

private String country;

private String city;
Expand All @@ -36,6 +36,8 @@ public class GisgraphyAddressEntry {

private String geocodingLevel;

private String adm3Name;

public long getId() {
return id;
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

}
2 changes: 0 additions & 2 deletions src/main/java/com/graphhopper/converter/api/OpenCageDataEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ public OCDComponents() {
public String type;
@JsonProperty("country_code")
public String countryCode;
@JsonProperty("county")
public String county;
}

public OpenCageDataEntry() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/graphhopper/converter/core/Converter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(), gisgraphyEntry.getAdm3Name(), gisgraphyEntry.getStreetName(),
Copy link
Member Author

Choose a reason for hiding this comment

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

When thinking about this a bit more, I am not sure if we should do this. We convert Adm3Name to county, but maybe this is not true for other regions of the world? Gisgraphy does not return county, maybe we just return null then instead?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, admin3 is not always county. Do you plan to add county @gisgraphy ?

gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null);
return rsp;
}
Expand All @@ -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(), null, gisgraphyEntry.getName(),
gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null);
return rsp;
}
Expand Down
196 changes: 108 additions & 88 deletions src/test/java/com/graphhopper/converter/resource/ConverterResourceGisgraphyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

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;
import io.dropwizard.testing.junit.DropwizardAppRule;
Expand All @@ -21,93 +23,111 @@
* @author Robin Boldt
*/
public class ConverterResourceGisgraphyTest {
@ClassRule
public static final DropwizardAppRule<ConverterConfiguration> 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<ConverterConfiguration> 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);

}

@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());
}

}
19 changes: 19 additions & 0 deletions src/test/java/com/graphhopper/converter/resource/ConverterResourceNominatimTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Loading