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 1 commit
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
11 changes: 11 additions & 0 deletions src/main/java/com/graphhopper/converter/api/GisgraphySearchEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public class GisgraphySearchEntry {
@JsonProperty("adm1_name")
private String adm1Name;

@JsonProperty("adm3_name")
Copy link
Member Author

@boldtrn boldtrn Jun 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder that the @JsonProperty also for the other properties in this class work. For example see this request. The fields in json are named: adm3Name or zipCode. But as I mentioned, I wasn't able to test Gisgraphy (see #48). I just followed the existing scheme here.

private String adm3Name;

@JsonProperty("zip_code")
private List<String> zipCodes;

Expand Down Expand Up @@ -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<String> getZipCodes() {
return zipCodes;
}
Expand Down
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(), null, gisgraphyEntry.getStreetName(),
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(), gisgraphyEntry.getAdm3Name(), gisgraphyEntry.getName(),
gisgraphyEntry.getHouseNumber(), gisgraphyEntry.getZipCode(), null);
return rsp;
}
Expand Down
177 changes: 89 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,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;
Expand All @@ -21,93 +22,93 @@
* @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);

}

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

}
4 changes: 4 additions & 0 deletions src/test/resources/converter.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
nominatimURL: https://nominatim.openstreetmap.org/search/

openCageDataURL: https://api.opencagedata.com/geocode/v1/json
openCageDataKey: XXXXXXXX
Copy link
Member Author

Choose a reason for hiding this comment

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

I could in theory commit my personal ocd key here, it's a free key and is pretty limited anyway, but that way we could actually test ocd in travis. Like this the test will always fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

WDYT @karussell?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's try. Or we can also try to hide it and then it will work on travis only?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok sure, one downside of this is that we cannot run tests for any PRs if the env variable is encrypted, because travis does not share encrypted variables with forked repos. So maybe every contributor needs to set up their own keys or it's not possible at all (not sure yet).

Copy link
Member

Choose a reason for hiding this comment

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

This should not be a problem for now.

openCageData: true