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

Introduce enum for Photon's address type #517

Merged
merged 3 commits into from
Nov 25, 2020
Merged

Introduce enum for Photon's address type #517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5f262ae
introduce enum for Photon's address type
lonvia Nov 24, 2020
d7ad175
remove unused accessors
lonvia Nov 24, 2020
7a6b4f0
cleanup imports
lonvia Nov 24, 2020
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
103 changes: 51 additions & 52 deletions src/main/java/de/komoot/photon/PhotonDoc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import com.neovisionaries.i18n.CountryCode;
import com.vividsolutions.jts.geom.Envelope;
import com.vividsolutions.jts.geom.Point;
import de.komoot.photon.nominatim.model.AddressType;
import lombok.Getter;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;

import java.util.*;
Expand All @@ -16,7 +16,6 @@
* @author christoph
*/
@Getter
@Setter
@Slf4j
public class PhotonDoc {
final private long placeId;
Expand All @@ -35,14 +34,8 @@ public class PhotonDoc {
final private long linkedPlaceId; // 0 if unset
final private int rankAddress;

private Map<String, String> street;
private Map<String, String> locality;
private Map<String, String> district;
private Map<String, String> city;
private Map<String, String> county; // careful, this is county not count_r_y
private Set<Map<String, String>> context = new HashSet<Map<String, String>>();
private Map<String, String> country;
private Map<String, String> state;
private Map<AddressType, Map<String, String>> addressParts = new EnumMap<>(AddressType.class);
private Set<Map<String, String>> context = new HashSet<>();
private String houseNumber;
private Point centroid;

Expand Down Expand Up @@ -95,14 +88,8 @@ public PhotonDoc(PhotonDoc other) {
this.centroid = other.centroid;
this.linkedPlaceId = other.linkedPlaceId;
this.rankAddress = other.rankAddress;
this.street = other.street;
this.locality = other.locality;
this.district = other.district;
this.city = other.city;
this.county= other.county;
this.addressParts = other.addressParts;
this.context = other.context;
this.country = other.country;
this.state = other.state;
}

public String getUid() {
Expand All @@ -120,19 +107,8 @@ public static PhotonDoc create(long placeId, String osmType, long osmId, Map<Str
"", null, null, null, 0, 0, null, null, 0, 0);
}

/**
* Return the GeocodeJSON place type.
*
* @return A string representation of the type
*/
public final String getObjectType() {
if (rankAddress >= 28) return "house";
if (rankAddress >= 26) return "street";
if (rankAddress >= 13 && rankAddress <= 16) return "city";
if (rankAddress >= 5 && rankAddress <= 12) return "region";
if (rankAddress == 4) return "country";

return "locality";
public AddressType getAddressType() {
return AddressType.fromRank(rankAddress);
}

public boolean isUsefulForIndex() {
Expand All @@ -153,12 +129,12 @@ public boolean isUsefulForIndex() {
public void completeFromAddress() {
if (address == null) return;

street = extractAddress(street, "street");
city = extractAddress(city, "city");
district = extractAddress(district, "suburb");
locality = extractAddress(locality, "neighbourhood");
county = extractAddress(county, "county");
state = extractAddress(state, "state");
extractAddress(AddressType.STREET, "street");
extractAddress(AddressType.CITY, "city");
extractAddress(AddressType.DISTRICT, "suburb");
extractAddress(AddressType.LOCALITY, "neighbourhood");
extractAddress(AddressType.COUNTY, "county");
extractAddress(AddressType.STATE, "state");

String addressPostCode = address.get("postcode");
if (addressPostCode != null && !addressPostCode.equals(postcode)) {
Expand All @@ -173,31 +149,54 @@ public void completeFromAddress() {
/**
* Extract an address field from an address tag and replace the appropriate address field in the document.
*
* @param existingField The current value of the document's address field.
* @param addressType The type of address field to fill.
* @param addressFieldName The name of the address tag to use (without the 'addr:' prefix).
*
* @return 'existingField' potentially with the name field replaced. If existingField was null and
* the address field could be found, then a new map with the address as single entry is returned.
*/
private Map<String, String> extractAddress(Map<String, String> existingField, String addressFieldName) {
private void extractAddress(AddressType addressType, String addressFieldName) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you get addressFieldName also from addressType.getName? Then you could remove it from the parameter list.

BTW: thanks for introducing me to EnumMap - really interesting :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The mapping between the GeocodeJSON names and the OSM addr:* tag names is not exactly 1:1. suburb and neighbourhood are already different and we might even add more variants (like 'province' for 'state'). We could add the mapping into the new AddressType enum as an extra field though. Something for an extra PR.

String field = address.get(addressFieldName);

if (field == null) return existingField;

Map<String, String> map = (existingField == null) ? new HashMap<>() : existingField;

String existingName = map.get("name");
if (!field.equals(existingName)) {
if (log.isDebugEnabled()) {
log.debug("Replacing " + addressFieldName + " name '" + existingName + "' with '" + field + "' for osmId #" + osmId);
if (field != null) {
Map<String, String> map = addressParts.computeIfAbsent(addressType, k -> new HashMap<>());

String existingName = map.get("name");
if (!field.equals(existingName)) {
if (log.isDebugEnabled()) {
log.debug("Replacing " + addressFieldName + " name '" + existingName + "' with '" + field + "' for osmId #" + osmId);
}
// we keep the former name in the context as it might be helpful when looking up typos
if (!Objects.isNull(existingName)) {
context.add(ImmutableMap.of("formerName", existingName));
}
map.put("name", field);
}
// we keep the former name in the context as it might be helpful when looking up typos
if(!Objects.isNull(existingName)) {
context.add(ImmutableMap.of("formerName", existingName));
}
map.put("name", field);
}
}

return map;
public void setPostcode(String postcode) {
this.postcode = postcode;
}

/**
* Set names for the given address part if it is not already set.
*
* @return True, if the address was inserted.
*/
public boolean setAddressPartIfNew(AddressType addressType, Map<String, String> names) {
return addressParts.computeIfAbsent(addressType, k -> names) == names;
}

public void setCountry(Map<String, String> names) {
addressParts.put(AddressType.COUNTRY, names);
}

public void setHouseNumber(String houseNumber) {
this.houseNumber = houseNumber;
}

public void setCentroid(Point centroid) {
this.centroid = centroid;
}
}
14 changes: 6 additions & 8 deletions src/main/java/de/komoot/photon/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.google.common.collect.SetMultimap;
import com.neovisionaries.i18n.CountryCode;
import com.vividsolutions.jts.geom.Envelope;
import de.komoot.photon.nominatim.model.AddressType;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;

Expand All @@ -23,12 +24,13 @@ public class Utils {
private static final Joiner commaJoiner = Joiner.on(", ").skipNulls();

public static XContentBuilder convert(PhotonDoc doc, String[] languages) throws IOException {
final AddressType atype = doc.getAddressType();
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
.field(Constants.OSM_ID, doc.getOsmId())
.field(Constants.OSM_TYPE, doc.getOsmType())
.field(Constants.OSM_KEY, doc.getTagKey())
.field(Constants.OSM_VALUE, doc.getTagValue())
.field(Constants.OBJECT_TYPE, doc.getObjectType())
.field(Constants.OBJECT_TYPE, atype == null ? "locality" : atype.getName())
.field(Constants.IMPORTANCE, doc.getImportance());

if (doc.getCentroid() != null) {
Expand All @@ -47,16 +49,12 @@ public static XContentBuilder convert(PhotonDoc doc, String[] languages) throws
}

writeName(builder, doc.getName(), languages);
writeIntlNames(builder, doc.getCity(), "city", languages);
writeIntlNames(builder, doc.getCountry(), "country", languages);
for (Map.Entry<AddressType, Map<String, String>> entry : doc.getAddressParts().entrySet()) {
writeIntlNames(builder, entry.getValue(), entry.getKey().getName(), languages);
}
CountryCode countryCode = doc.getCountryCode();
if (countryCode != null)
builder.field(Constants.COUNTRYCODE, countryCode.getAlpha2());
writeIntlNames(builder, doc.getState(), "state", languages);
writeIntlNames(builder, doc.getStreet(), "street", languages);
writeIntlNames(builder, doc.getLocality(), "locality", languages);
writeIntlNames(builder, doc.getDistrict(), "district", languages);
writeIntlNames(builder, doc.getCounty(), "county", languages);
writeContext(builder, doc.getContext(), languages);
writeExtent(builder, doc.getBbox());

Expand Down
65 changes: 24 additions & 41 deletions src/main/java/de/komoot/photon/nominatim/NominatimConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import de.komoot.photon.Importer;
import de.komoot.photon.PhotonDoc;
import de.komoot.photon.nominatim.model.AddressRow;
import de.komoot.photon.nominatim.model.AddressType;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.dbcp2.BasicDataSource;
import org.postgis.jts.JtsWrapper;
Expand Down Expand Up @@ -289,14 +290,25 @@ public AddressRow mapRow(ResultSet rs, int rowNum) throws SQLException {
}
};

boolean isPoi = doc.getRankAddress() > 28;
long placeId = (isPoi) ? doc.getParentPlaceId() : doc.getPlaceId();
AddressType atype = doc.getAddressType();

List<AddressRow> terms = template.query("SELECT " + selectColsAddress + " FROM placex p, place_addressline pa WHERE p.place_id = pa.address_place_id and pa.place_id = ? and pa.cached_rank_address > 4 and pa.address_place_id != ? and pa.isaddress order by rank_address desc,fromarea desc,distance asc,rank_search desc", new Object[]{placeId, placeId}, rowMapper);
if (atype == null || atype == AddressType.COUNTRY) {
return Collections.emptyList();
}

long placeId = (atype == AddressType.HOUSE) ? doc.getParentPlaceId() : doc.getPlaceId();

List<AddressRow> terms = template.query("SELECT " + selectColsAddress
+ " FROM placex p, place_addressline pa"
+ " WHERE p.place_id = pa.address_place_id and pa.place_id = ?"
+ " and pa.cached_rank_address > 4 and pa.address_place_id != ? and pa.isaddress"
+ " ORDER BY rank_address desc, fromarea desc, distance asc, rank_search desc",
rowMapper, placeId, placeId);

if (isPoi) {
if (atype == AddressType.HOUSE) {
// need to add the term for the parent place ID itself
terms.addAll(0, template.query("SELECT " + selectColsAddress + " FROM placex p WHERE p.place_id = ?", new Object[]{placeId}, rowMapper));
terms.addAll(0, template.query("SELECT " + selectColsAddress + " FROM placex p WHERE p.place_id = ?",
rowMapper, placeId));
}

return terms;
Expand Down Expand Up @@ -465,44 +477,15 @@ public PhotonDoc mapRow(ResultSet resultSet, int i) throws SQLException {
*/
private void completePlace(PhotonDoc doc) {
final List<AddressRow> addresses = getAddresses(doc);
final AddressType doctype = doc.getAddressType();
for (AddressRow address : addresses) {
if (address.isCity()) {
if (doc.getCity() == null) {
doc.setCity(address.getName());
} else {
doc.getContext().add(address.getName());
}
continue;
}

if (address.isStreet() && doc.getStreet() == null) {
doc.setStreet(address.getName());
continue;
}
AddressType atype = address.getAddressType();

if (address.isLocality() && doc.getLocality() == null) {
doc.setLocality(address.getName());
continue;
}

if (address.isDistrict() && doc.getDistrict() == null) {
doc.setDistrict(address.getName());
continue;
}

if (address.isCounty() && doc.getCounty() == null) {
doc.setCounty(address.getName());
continue;
}

if (address.isState() && doc.getState() == null) {
doc.setState(address.getName());
continue;
}

// no specifically handled item, check if useful for context
if (address.isUsefulForContext()) {
doc.getContext().add(address.getName());
if (atype != null
&& (atype == doctype || !doc.setAddressPartIfNew(atype, address.getName()))
&& address.isUsefulForContext()) {
// no specifically handled item, check if useful for context
doc.getContext().add(address.getName());
}
}
// finally, overwrite gathered information with higher prio
Expand Down
83 changes: 11 additions & 72 deletions src/main/java/de/komoot/photon/nominatim/model/AddressRow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,88 +13,27 @@
*/
@Data
public class AddressRow {
final private long placeId;
final private Map<String, String> name;
final private String osmKey;
final private String osmValue;
final private int rankAddress;
private static final String[] USEFUL_CONTEXT_KEYS = new String[]{"boundary", "landuse", "place"}; // must be in alphabetic order to speed up lookup
private final long placeId;
private final Map<String, String> name;
private final String osmKey;
private final String osmValue;
private final int rankAddress;

static private final String[] USEFUL_CONTEXT_KEYS = new String[]{"boundary", "landuse", "place"}; // must be in alphabetic order to speed up lookup

/**
* @return whether nominatim thinks this place is a street
*/
public boolean isStreet() {
return 26 <= rankAddress && rankAddress < 28;
}

/**
* @return whether nominatim thinks this place is a locality (=neighbourhood)
*/
public boolean isLocality() {
return 22 <= rankAddress && rankAddress < 26;
}

/**
* @return whether nominatim thinks this place is a district (=suburb)
*/
public boolean isDistrict() {
return 17 <= rankAddress && rankAddress < 22;
}

/**
* @return whether nominatim thinks this place is a town or city
*/
public boolean isCity() {
return 13 <= rankAddress && rankAddress < 17;
}

/**
* @return whether nominatim thinks this place is a county
*/
public boolean isCounty() { return 10 <= rankAddress && rankAddress < 13; }

/**
* @return whether nominatim thinks this place is a state
*/
public boolean isState() { return 5 <= rankAddress && rankAddress < 10; }

public boolean isCountry() {
return (rankAddress == 4 && "boundary".equals(osmKey) && "administrative".equals(osmValue));
public AddressType getAddressType() {
return AddressType.fromRank(rankAddress);
}


public boolean isPostcode() {
private boolean isPostcode() {
if ("place".equals(osmKey) && "postcode".equals(osmValue)) {
return true;
}

if ("boundary".equals(osmKey) && "postal_code".equals(osmValue)) {
return true;
}

return false;
return "boundary".equals(osmKey) && "postal_code".equals(osmValue);
}

public boolean isUsefulForContext() {
if (name.isEmpty()) {
return false;
}

if (isPostcode()) {
return false;
}

if (rankAddress < 4) {
// continent, sea, ...
return false;
}

if (Arrays.binarySearch(USEFUL_CONTEXT_KEYS, osmKey) >= 0) {
return true;
}

return false;
return !name.isEmpty() && !isPostcode() && Arrays.binarySearch(USEFUL_CONTEXT_KEYS, osmKey) >= 0;
}

@Override
Expand Down
Loading