diff --git a/src/main/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolver.java b/src/main/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolver.java index dca2d09..b9a1d1a 100644 --- a/src/main/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolver.java +++ b/src/main/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolver.java @@ -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 response = httpClient.send( - request, - HttpResponse.BodyHandlers.ofString(StandardCharsets.UTF_8) - ); + URI uri = reverseUri(latitude, longitude, config, false); + HttpResponse 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 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 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 diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index fa59610..d38a27e 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -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 diff --git a/src/test/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolverTest.java b/src/test/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolverTest.java index c693bc1..64a2d7f 100644 --- a/src/test/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolverTest.java +++ b/src/test/java/at/procon/eventhub/geocoding/service/NominatimGeoCountryResolverTest.java @@ -81,6 +81,75 @@ class NominatimGeoCountryResolverTest { .contains("lon=16.373800"); } + @Test + void retriesWithoutContentNegotiationHeadersWhenEndpointReturns406() throws Exception { + AtomicInteger requestCount = new AtomicInteger(); + AtomicReference firstAccept = new AtomicReference<>(); + AtomicReference secondAccept = new AtomicReference<>(); + AtomicReference secondAcceptLanguage = new AtomicReference<>(); + AtomicReference secondUserAgent = new AtomicReference<>(); + AtomicReference 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();