From 2b1328fc3047168cf6e44d48d7a239cae39d1202 Mon Sep 17 00:00:00 2001 From: Lyn Long Date: Thu, 17 Sep 2026 14:40:56 +1000 Subject: [PATCH] fix get map feature res handler --- .../server/core/model/ogc/wms/Feature.java | 23 ++++ .../core/model/ogc/wms/FeatureInfo.java | 3 + .../model/ogc/wms/FeatureInfoResponse.java | 5 + .../core/service/geoserver/wms/WmsServer.java | 130 ++++++++++++++---- .../ogcapi/server/features/RestServices.java | 13 +- .../service/geoserver/wms/WmsServerTest.java | 43 ++++++ 6 files changed, 192 insertions(+), 25 deletions(-) create mode 100644 server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/Feature.java diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/Feature.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/Feature.java new file mode 100644 index 00000000..0502ba59 --- /dev/null +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/Feature.java @@ -0,0 +1,23 @@ +package au.org.aodn.ogcapi.server.core.model.ogc.wms; + +import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper; +import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty; +import java.util.List; +import lombok.*; + +/** + * ncWMS wrap the FeatureInfo inside a Feature element, one per layer. + */ +@Getter +@Setter +@Builder +@AllArgsConstructor +@NoArgsConstructor +public class Feature { + @JacksonXmlProperty(localName = "layer") + protected String layer; + + @JacksonXmlProperty(localName = "FeatureInfo") + @JacksonXmlElementWrapper(useWrapping = false) + protected List featureInfo; +} diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfo.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfo.java index 23c80d41..b24e0933 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfo.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfo.java @@ -10,6 +10,9 @@ @Getter @Setter public class FeatureInfo { + @JacksonXmlProperty(localName = "id") + protected String id; + @JacksonXmlProperty(localName = "time") protected ZonedDateTime time; diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfoResponse.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfoResponse.java index e9ecb26c..637b995f 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfoResponse.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/ogc/wms/FeatureInfoResponse.java @@ -37,4 +37,9 @@ public class FeatureInfoResponse { @JacksonXmlProperty(localName = "FeatureInfo") @JacksonXmlElementWrapper(useWrapping = false) protected List featureInfo; + + // ncWMS put the FeatureInfo one level down, inside a Feature element + @JacksonXmlProperty(localName = "Feature") + @JacksonXmlElementWrapper(useWrapping = false) + protected List feature; } diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServer.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServer.java index 38b628bc..84ffef86 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServer.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServer.java @@ -42,6 +42,9 @@ @Slf4j public class WmsServer { + // Max chars of a geoserver response body we put in the log + protected static final int LOG_BODY_LIMIT = 2000; + protected final XmlMapper xmlMapper; @Autowired @@ -430,36 +433,117 @@ public FeatureInfoResponse getMapFeatures(String collectionId, FeatureRequest re Optional mapServerUrl = getMapServerUrl(collectionId, request); - if (mapServerUrl.isPresent()) { - List urls = createMapFeatureQueryUrl(mapServerUrl.get(), collectionId, request); - // Try one by one, we exit when any works - for (String url : urls) { - ResponseEntity response = restTemplateUtils.handleRedirect(url, restTemplate.exchange(url, HttpMethod.GET, pretendUserEntity, String.class), String.class, pretendUserEntity); - if (response.getStatusCode().is2xxSuccessful()) { - // Now try to unify the return - if (MediaType.TEXT_HTML.isCompatibleWith(response.getHeaders().getContentType())) { - String html = response.getBody(); - // This is a simple trick to check if the html is in fact empty body, if empty - // try another url - if (html != null && (html.contains("class=\"feature\"") || html.contains("class=\"featureInfo\""))) { - // Some source strangely encode the html tags - return FeatureInfoResponse.builder() - .html(HtmlUtils.htmlUnescape(html)) - .build(); - } - } else if (MediaType.APPLICATION_XML.isCompatibleWith(response.getHeaders().getContentType())) { - FeatureInfoResponse r = xmlMapper.readValue(response.getBody(), FeatureInfoResponse.class); - // give another url a chance - if (!r.getFeatureInfo().isEmpty()) { - return r; - } + if (mapServerUrl.isEmpty()) { + log.warn("GetFeatureInfo no wms server url for uuid {} layer {}", collectionId, request.getLayerName()); + return null; + } + + List urls = createMapFeatureQueryUrl(mapServerUrl.get(), collectionId, request); + + if (urls == null || urls.isEmpty()) { + log.warn("GetFeatureInfo cannot build query url from {} for uuid {} layer {}", mapServerUrl.get(), collectionId, request.getLayerName()); + return null; + } + + log.debug("GetFeatureInfo request uuid {} layer {} x {} y {} width {} height {} bbox {}", + collectionId, request.getLayerName(), request.getX(), request.getY(), + request.getWidth(), request.getHeight(), request.getBbox()); + + // Try one by one, we exit when any works + for (String url : urls) { + log.debug("GetFeatureInfo call geoserver {}", url); + ResponseEntity response = restTemplateUtils.handleRedirect(url, restTemplate.exchange(url, HttpMethod.GET, pretendUserEntity, String.class), String.class, pretendUserEntity); + String body = response.getBody(); + + log.debug("GetFeatureInfo response status {} content-type {} body length {}", + response.getStatusCode(), response.getHeaders().getContentType(), + body == null ? 0 : body.length()); + log.debug("GetFeatureInfo response body {}", truncateForLog(body)); + + if (response.getStatusCode().is2xxSuccessful()) { + // Now try to unify the return + if (MediaType.TEXT_HTML.isCompatibleWith(response.getHeaders().getContentType())) { + // This is a simple trick to check if the html is in fact empty body, if empty + // try another url + if (body != null && (body.contains("class=\"feature\"") || body.contains("class=\"featureInfo\""))) { + // Some source strangely encode the html tags + log.debug("GetFeatureInfo html has feature marker, return it to caller"); + return FeatureInfoResponse.builder() + .html(HtmlUtils.htmlUnescape(body)) + .build(); + } + log.debug("GetFeatureInfo html has no feature marker, skip url {}", url); + } else if (isXml(response.getHeaders().getContentType())) { + FeatureInfoResponse r = xmlMapper.readValue(body, FeatureInfoResponse.class); + flattenFeatures(r); + // give another url a chance + if (r.getFeatureInfo() != null && !r.getFeatureInfo().isEmpty()) { + log.debug("GetFeatureInfo xml parsed {} feature(s), return it to caller", r.getFeatureInfo().size()); + return r; } + log.debug("GetFeatureInfo xml has no FeatureInfo element, skip url {}", url); + } else { + log.warn("GetFeatureInfo unexpected content-type {} from {}", response.getHeaders().getContentType(), url); } + } else { + log.warn("GetFeatureInfo failed with status {} from {}", response.getStatusCode(), url); } } + + log.warn("GetFeatureInfo no usable response for uuid {} layer {}, caller gets empty body", collectionId, request.getLayerName()); return null; } + /** + * Check if the content type is some flavour of xml. Geoserver wms answer text/html, but ncwms answer + * text/xml, and some server answer application/xml, so we need to accept all of them. + * + * @param contentType - The content type from the response header, can be null + * @return - True if we can parse the body as xml + */ + protected static boolean isXml(MediaType contentType) { + if (contentType == null) { + return false; + } + return MediaType.APPLICATION_XML.isCompatibleWith(contentType) + || MediaType.TEXT_XML.isCompatibleWith(contentType) + || contentType.getSubtype().endsWith("+xml"); + } + + /** + * ncWMS nest the FeatureInfo inside a Feature element, one Feature per layer. Move them up to the top + * level list so the response looks the same no matter which server answered. + * + * @param response - The parsed response, changed in place + */ + protected static void flattenFeatures(FeatureInfoResponse response) { + if (response.getFeatureInfo() != null && !response.getFeatureInfo().isEmpty()) { + return; + } + if (response.getFeature() == null) { + return; + } + response.setFeatureInfo( + response.getFeature() + .stream() + .filter(f -> f.getFeatureInfo() != null) + .flatMap(f -> f.getFeatureInfo().stream()) + .toList()); + } + + /** + * Cut a response body down to a size that is safe to put in the log. + * + * @param body - The raw response body, can be null + * @return - The body, cut at 2000 chars + */ + protected static String truncateForLog(String body) { + if (body == null) { + return null; + } + return body.length() <= LOG_BODY_LIMIT ? body : body.substring(0, LOG_BODY_LIMIT) + "... [truncated]"; + } + /** * Get the wms image/png tile * diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java b/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java index bbbcc700..f7e7b2fb 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java @@ -93,8 +93,17 @@ public ResponseEntity getDatasetMetadata(String id) { public ResponseEntity getWmsMapFeature(String collectionId, FeatureRequest request) { try { - return ResponseEntity.ok() - .body(wmsServer.getMapFeatures(collectionId, request)); + FeatureInfoResponse response = wmsServer.getMapFeatures(collectionId, request); + + if (response == null) { + log.warn("GetFeatureInfo returns null for uuid {} layer {}, popup will render nothing", collectionId, request.getLayerName()); + } else { + log.debug("GetFeatureInfo returns uuid {} layer {} html length {} featureInfo count {}", + collectionId, request.getLayerName(), + response.getHtml() == null ? 0 : response.getHtml().length(), + response.getFeatureInfo() == null ? 0 : response.getFeatureInfo().size()); + } + return ResponseEntity.ok().body(response); } catch (JsonProcessingException | URISyntaxException e) { throw new RuntimeException(e); } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServerTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServerTest.java index f3deeef9..47f1b8b3 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServerTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/geoserver/wms/WmsServerTest.java @@ -9,6 +9,7 @@ import au.org.aodn.stac.model.LinkModel; import au.org.aodn.ogcapi.server.core.model.ogc.FeatureRequest; import au.org.aodn.ogcapi.server.core.model.ogc.wms.DescribeLayerResponse; +import au.org.aodn.ogcapi.server.core.model.ogc.wms.FeatureInfoResponse; import au.org.aodn.ogcapi.server.core.service.ElasticSearchBase; import au.org.aodn.ogcapi.server.core.service.Search; import au.org.aodn.ogcapi.server.core.service.geoserver.wfs.WfsDefaultParam; @@ -308,6 +309,48 @@ public void verifyParseCorrect() throws JsonProcessingException { assertEquals("imos:srs_ghrsst_l4_gamssa_url", value.getLayerDescription().getQuery().getTypeName()); } + /** + * ncWMS answer text/xml and nest the FeatureInfo inside a Feature element, make sure we still pick it up. + */ + @Test + public void verifyNcwmsFeatureInfoParseCorrect() throws JsonProcessingException { + FeatureInfoResponse value = wmsServer.xmlMapper.readValue( + """ + + 95.33578364084919 + -20.364301475359852 + + sea_surface_temperature + + sea_surface_temperature + 297.5120817180723 + + + """, FeatureInfoResponse.class); + + assertEquals(95.33578364084919, value.getLongitude()); + assertEquals(1, value.getFeature().size()); + assertEquals("sea_surface_temperature", value.getFeature().get(0).getLayer()); + + // Before flatten the top level list is empty, that is why the popup was empty + assertNull(value.getFeatureInfo()); + + WmsServer.flattenFeatures(value); + + assertEquals(1, value.getFeatureInfo().size()); + assertEquals("sea_surface_temperature", value.getFeatureInfo().get(0).getId()); + assertEquals("297.5120817180723", value.getFeatureInfo().get(0).getValue()); + } + + @Test + public void verifyXmlContentTypeAccepted() { + assertTrue(WmsServer.isXml(MediaType.parseMediaType("text/xml;charset=ISO-8859-1"))); + assertTrue(WmsServer.isXml(MediaType.parseMediaType("application/xml"))); + assertTrue(WmsServer.isXml(MediaType.parseMediaType("application/vnd.ogc.gml+xml"))); + assertFalse(WmsServer.isXml(MediaType.TEXT_HTML)); + assertFalse(WmsServer.isXml(null)); + } + /** * Test with only one dateTime field in the describe layer */