Harden Nominatim country resolution fallback
This commit is contained in:
parent
c91d3cc1c4
commit
7584bb8578
|
|
@ -25,6 +25,7 @@ import java.util.concurrent.ConcurrentHashMap;
|
|||
import java.util.concurrent.atomic.AtomicLong;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
/**
|
||||
|
|
@ -49,6 +50,7 @@ public class NominatimGeoCountryResolver implements GeoCountryResolver {
|
|||
private final AtomicLong nextRemoteRequestNanos = new AtomicLong(0L);
|
||||
private final Object requestGate = new Object();
|
||||
|
||||
@Autowired
|
||||
public NominatimGeoCountryResolver(
|
||||
EventHubProperties properties,
|
||||
ObjectMapper objectMapper
|
||||
|
|
@ -165,18 +167,18 @@ public class NominatimGeoCountryResolver implements GeoCountryResolver {
|
|||
try {
|
||||
synchronized (requestGate) {
|
||||
awaitRequestPermit(effectiveMinimumRequestInterval(config));
|
||||
URI uri = reverseUri(latitude, longitude, config);
|
||||
HttpRequest request = HttpRequest.newBuilder(uri)
|
||||
.timeout(config.getReadTimeout())
|
||||
.header("Accept", "application/json")
|
||||
.header("Accept-Language", config.getAcceptLanguage())
|
||||
.header("User-Agent", config.getUserAgent())
|
||||
.GET()
|
||||
.build();
|
||||
HttpResponse<String> response = httpClient.send(
|
||||
request,
|
||||
HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8)
|
||||
URI uri = reverseUri(latitude, longitude, config, false);
|
||||
HttpResponse<String> response = sendRequest(uri, config, true);
|
||||
if (response.statusCode() == 406) {
|
||||
URI compatibilityUri = reverseUri(latitude, longitude, config, true);
|
||||
LOG.info(
|
||||
"Nominatim endpoint rejected the standard request with HTTP 406 for {}. "
|
||||
+ "Retrying once with a legacy-compatible request matching JsonNominatimClient.",
|
||||
uri.getHost()
|
||||
);
|
||||
awaitRequestPermit(effectiveMinimumRequestInterval(config));
|
||||
response = sendCompatibilityRequest(compatibilityUri, config);
|
||||
}
|
||||
if (response.statusCode() == 404) {
|
||||
return result(
|
||||
GeoCountryResolutionStatus.NOT_FOUND,
|
||||
|
|
@ -191,6 +193,14 @@ public class NominatimGeoCountryResolver implements GeoCountryResolver {
|
|||
);
|
||||
}
|
||||
if (response.statusCode() / 100 != 2) {
|
||||
String responseSummary = responseBodySummary(response.body());
|
||||
LOG.warn(
|
||||
"Nominatim reverse lookup failed for {},{} with HTTP status {} and response body: {}",
|
||||
latitude,
|
||||
longitude,
|
||||
response.statusCode(),
|
||||
responseSummary
|
||||
);
|
||||
return result(
|
||||
GeoCountryResolutionStatus.ERROR,
|
||||
latitude,
|
||||
|
|
@ -200,7 +210,8 @@ public class NominatimGeoCountryResolver implements GeoCountryResolver {
|
|||
null,
|
||||
false,
|
||||
true,
|
||||
"Nominatim reverse lookup failed with HTTP status " + response.statusCode() + "."
|
||||
"Nominatim reverse lookup failed with HTTP status " + response.statusCode()
|
||||
+ (responseSummary == null ? "." : ": " + responseSummary)
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -277,25 +288,98 @@ public class NominatimGeoCountryResolver implements GeoCountryResolver {
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
private HttpResponse<String> sendRequest(
|
||||
URI uri,
|
||||
EventHubProperties.Nominatim config,
|
||||
boolean includeContentNegotiationHeaders
|
||||
) throws IOException, InterruptedException {
|
||||
HttpRequest.Builder requestBuilder = HttpRequest.newBuilder(uri)
|
||||
.version(HttpClient.Version.HTTP_1_1)
|
||||
.timeout(config.getReadTimeout())
|
||||
.GET();
|
||||
if (config.getUserAgent() != null && !config.getUserAgent().isBlank()) {
|
||||
requestBuilder.header("User-Agent", config.getUserAgent());
|
||||
}
|
||||
if (includeContentNegotiationHeaders) {
|
||||
requestBuilder.header("Accept", "application/json");
|
||||
if (config.getAcceptLanguage() != null && !config.getAcceptLanguage().isBlank()) {
|
||||
requestBuilder.header("Accept-Language", config.getAcceptLanguage());
|
||||
}
|
||||
}
|
||||
return httpClient.send(
|
||||
requestBuilder.build(),
|
||||
HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Sends the same kind of plain HTTP GET used by the previously working
|
||||
* fr.dudie JsonNominatimClient: no explicit Accept, Accept-Language or
|
||||
* User-Agent headers and only legacy-compatible reverse parameters.
|
||||
*/
|
||||
private HttpResponse<String> sendCompatibilityRequest(
|
||||
URI uri,
|
||||
EventHubProperties.Nominatim config
|
||||
) throws IOException, InterruptedException {
|
||||
HttpRequest request = HttpRequest.newBuilder(uri)
|
||||
.version(HttpClient.Version.HTTP_1_1)
|
||||
.timeout(config.getReadTimeout())
|
||||
.header("User-Agent", "")
|
||||
.GET()
|
||||
.build();
|
||||
return httpClient.send(
|
||||
request,
|
||||
HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8)
|
||||
);
|
||||
}
|
||||
|
||||
private String responseBodySummary(String body) {
|
||||
if (body == null || body.isBlank()) {
|
||||
return null;
|
||||
}
|
||||
String normalized = body.replaceAll("\\s+", " ").trim();
|
||||
return normalized.length() <= 500 ? normalized : normalized.substring(0, 500) + "...";
|
||||
}
|
||||
|
||||
private URI reverseUri(
|
||||
BigDecimal latitude,
|
||||
BigDecimal longitude,
|
||||
EventHubProperties.Nominatim config
|
||||
EventHubProperties.Nominatim config,
|
||||
boolean legacyCompatible
|
||||
) {
|
||||
StringBuilder query = new StringBuilder(config.getBaseUrl())
|
||||
.append("/reverse?format=jsonv2")
|
||||
.append("&lat=").append(url(latitude.toPlainString()))
|
||||
.append("&lon=").append(url(longitude.toPlainString()))
|
||||
.append("&zoom=3")
|
||||
.append("&addressdetails=1")
|
||||
.append("&layer=address")
|
||||
.append("&accept-language=").append(url(config.getAcceptLanguage()));
|
||||
StringBuilder query = new StringBuilder(normalizedBaseUrl(config.getBaseUrl()))
|
||||
.append("/reverse?format=jsonv2");
|
||||
|
||||
// JsonNominatimClient puts the identifying email directly into the URL.
|
||||
if (config.getEmail() != null && !config.getEmail().isBlank()) {
|
||||
query.append("&email=").append(url(config.getEmail()));
|
||||
}
|
||||
|
||||
query.append("&lat=").append(url(latitude.toPlainString()))
|
||||
.append("&lon=").append(url(longitude.toPlainString()))
|
||||
.append("&zoom=3");
|
||||
|
||||
if (!legacyCompatible) {
|
||||
query.append("&addressdetails=1")
|
||||
.append("&layer=address");
|
||||
if (config.getAcceptLanguage() != null && !config.getAcceptLanguage().isBlank()) {
|
||||
query.append("&accept-language=").append(url(config.getAcceptLanguage()));
|
||||
}
|
||||
}
|
||||
return URI.create(query.toString());
|
||||
}
|
||||
|
||||
private String normalizedBaseUrl(String baseUrl) {
|
||||
if (baseUrl == null || baseUrl.isBlank()) {
|
||||
throw new IllegalArgumentException("Nominatim base URL must not be blank.");
|
||||
}
|
||||
return baseUrl.endsWith("/")
|
||||
? baseUrl.substring(0, baseUrl.length() - 1)
|
||||
: baseUrl;
|
||||
}
|
||||
|
||||
private Duration effectiveMinimumRequestInterval(EventHubProperties.Nominatim config) {
|
||||
Duration configured = config.getMinimumRequestInterval() == null
|
||||
? Duration.ZERO
|
||||
|
|
|
|||
|
|
@ -56,19 +56,19 @@ eventhub:
|
|||
nominatim:
|
||||
# Public OSM Nominatim requires an identifying User-Agent and no more than one request/second.
|
||||
# Configure a self-hosted endpoint here when higher throughput is required.
|
||||
base-url: ${NOMINATIM_BASE_URL:https://nominatim.openstreetmap.org}
|
||||
base-url: ${NOMINATIM_BASE_URL:http://nominatim.iothings.at:7070}
|
||||
# Deliberate opt-in is required for the donated public service. Prefer a self-hosted or contracted endpoint for production/bulk processing.
|
||||
public-service-enabled: ${NOMINATIM_PUBLIC_SERVICE_ENABLED:false}
|
||||
user-agent: ${NOMINATIM_USER_AGENT:eventhub-tachograph/0.1 (Nominatim reverse geocoding)}
|
||||
email: ${NOMINATIM_EMAIL:}
|
||||
email: ${NOMINATIM_EMAIL:martin.schweitzer@procon.co.at}
|
||||
accept-language: ${NOMINATIM_ACCEPT_LANGUAGE:en}
|
||||
connect-timeout: ${NOMINATIM_CONNECT_TIMEOUT:10s}
|
||||
read-timeout: ${NOMINATIM_READ_TIMEOUT:20s}
|
||||
minimum-request-interval: ${NOMINATIM_MIN_REQUEST_INTERVAL:1s}
|
||||
connect-timeout: ${NOMINATIM_CONNECT_TIMEOUT:5s}
|
||||
read-timeout: ${NOMINATIM_READ_TIMEOUT:30s}
|
||||
minimum-request-interval: ${NOMINATIM_MIN_REQUEST_INTERVAL:0s}
|
||||
cache-ttl: ${NOMINATIM_CACHE_TTL:30d}
|
||||
cache-max-entries: ${NOMINATIM_CACHE_MAX_ENTRIES:100000}
|
||||
coordinate-decimal-places: ${NOMINATIM_COORDINATE_DECIMAL_PLACES:4}
|
||||
max-remote-lookups-per-execution: ${NOMINATIM_MAX_REMOTE_LOOKUPS_PER_EXECUTION:25}
|
||||
coordinate-decimal-places: ${NOMINATIM_COORDINATE_DECIMAL_PLACES:3}
|
||||
max-remote-lookups-per-execution: ${NOMINATIM_MAX_REMOTE_LOOKUPS_PER_EXECUTION:100000}
|
||||
|
||||
tachograph:
|
||||
default-chunk-days: 1
|
||||
|
|
|
|||
|
|
@ -81,6 +81,75 @@ class NominatimGeoCountryResolverTest {
|
|||
.contains("lon=16.373800");
|
||||
}
|
||||
|
||||
@Test
|
||||
void retriesWithoutContentNegotiationHeadersWhenEndpointReturns406() throws Exception {
|
||||
AtomicInteger requestCount = new AtomicInteger();
|
||||
AtomicReference<String> firstAccept = new AtomicReference<>();
|
||||
AtomicReference<String> secondAccept = new AtomicReference<>();
|
||||
AtomicReference<String> secondAcceptLanguage = new AtomicReference<>();
|
||||
AtomicReference<String> secondUserAgent = new AtomicReference<>();
|
||||
AtomicReference<String> secondQuery = new AtomicReference<>();
|
||||
server = HttpServer.create(new InetSocketAddress(0), 0);
|
||||
server.createContext("/reverse", exchange -> {
|
||||
int currentRequest = requestCount.incrementAndGet();
|
||||
if (currentRequest == 1) {
|
||||
firstAccept.set(exchange.getRequestHeaders().getFirst("Accept"));
|
||||
byte[] body = "Not acceptable".getBytes(StandardCharsets.UTF_8);
|
||||
exchange.sendResponseHeaders(406, body.length);
|
||||
exchange.getResponseBody().write(body);
|
||||
exchange.close();
|
||||
return;
|
||||
}
|
||||
secondAccept.set(exchange.getRequestHeaders().getFirst("Accept"));
|
||||
secondAcceptLanguage.set(exchange.getRequestHeaders().getFirst("Accept-Language"));
|
||||
secondUserAgent.set(exchange.getRequestHeaders().getFirst("User-Agent"));
|
||||
secondQuery.set(exchange.getRequestURI().getRawQuery());
|
||||
byte[] body = ("{\"place_id\":1,\"display_name\":\"Vienna, Austria\","
|
||||
+ "\"address\":{\"country\":\"Austria\",\"country_code\":\"at\"}}")
|
||||
.getBytes(StandardCharsets.UTF_8);
|
||||
exchange.getResponseHeaders().add("Content-Type", "application/json");
|
||||
exchange.sendResponseHeaders(200, body.length);
|
||||
exchange.getResponseBody().write(body);
|
||||
exchange.close();
|
||||
});
|
||||
server.start();
|
||||
|
||||
EventHubProperties properties = new EventHubProperties();
|
||||
EventHubProperties.Nominatim config = properties.getReverseGeocoding().getNominatim();
|
||||
config.setBaseUrl("http://localhost:" + server.getAddress().getPort());
|
||||
config.setUserAgent("eventhub-test/1.0");
|
||||
config.setEmail("email@address");
|
||||
config.setMinimumRequestInterval(Duration.ZERO);
|
||||
|
||||
NominatimGeoCountryResolver resolver = new NominatimGeoCountryResolver(
|
||||
properties,
|
||||
new ObjectMapper(),
|
||||
HttpClient.newHttpClient(),
|
||||
Clock.systemUTC()
|
||||
);
|
||||
|
||||
GeoCountryResolution result = resolver.resolve(
|
||||
new BigDecimal("48.2082"),
|
||||
new BigDecimal("16.3738"),
|
||||
true
|
||||
);
|
||||
|
||||
assertThat(result.resolved()).isTrue();
|
||||
assertThat(result.countryCode()).isEqualTo("AT");
|
||||
assertThat(requestCount).hasValue(2);
|
||||
assertThat(firstAccept).hasValue("application/json");
|
||||
assertThat(secondAccept.get()).isNull();
|
||||
assertThat(secondAcceptLanguage.get()).isNull();
|
||||
assertThat(secondUserAgent).hasValue("");
|
||||
assertThat(secondQuery.get())
|
||||
.contains("format=jsonv2")
|
||||
.contains("email=email%40address")
|
||||
.contains("zoom=3")
|
||||
.doesNotContain("layer=")
|
||||
.doesNotContain("accept-language=")
|
||||
.doesNotContain("addressdetails=");
|
||||
}
|
||||
|
||||
@Test
|
||||
void requiresExplicitOptInForPublicOsmEndpoint() {
|
||||
EventHubProperties properties = new EventHubProperties();
|
||||
|
|
|
|||
Loading…
Reference in New Issue