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.

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

OGCAPI - GeoJSON output improvements #118

Merged
merged 8 commits into from
Oct 23, 2024

Conversation

davidblasby
Copy link
Contributor

@davidblasby davidblasby commented Oct 16, 2024

This PR improves the GeoJSON output to be more compliant with the OGCAPI-Records specification.

  1. I've added all the Table 8 and Table 9 attributes, with the format the same as in the specification.
  2. I also added a gn-elastic-index-record property that has the IndexRecord (Elastic JSON Index document) in it
  3. I also added a gn-record-xml property that has the metadata record's XML text in it.

NOTE:

  1. I noticed that there was a lot of "JSON Strings" in the IndexRecord. For example;
image Its now represented as a normal json object: image
  1. The geometries property was in a "JSON Strings", but I've converted it to a normal JSON object. I also added a getGeometriesAsJsonString() method to access it as normal. I also added a geometriesAsJsonString property to the JSON with the old format.

  2. I was a little worried about changing the GeoJSON output too much (other things might depend on it). So, I created an IGeoJsonConverter interface with two implementation. GeoJsonConverter (the old one) and OgcApiGeoJsonConverter (the new one).

  3. I noticed a few bugs, which I've fixed.

  4. I've added a few more OgcApi* model objects.

  5. Here is an example of the new GeoJSON output - based on one of the "iso19139" Geonetwork sample records.

    • notice the gn-elastic-index-record (all the elastic index properties)
    • notice gn-record-xml (underlying metadata iso19139 xml)
    • notice that the defined properties (table 8 & 9, below) are in the proper format (as best I could do the conversion)
      items.geojson

Table 8 & 9 from the spec:
image
image

@fxprunayre
Copy link
Member

  1. I noticed that there was a lot of "JSON Strings" in the IndexRecord. For example;

This is for example one things improved in GN5 codebase https://github.com/GeoCat/geonetwork/blob/data-upload/src/shared/index-model/src/test/java/org/geonetwork/index/model/record/IndexRecordTest.java where the index can read/write a document for GN4 index (geonetwork/geonetwork#4). BTW for OGC API Records, you probably have all properties needed in the microservice model.

I was a little worried about changing the GeoJSON output too much (other things might depend on it).

AFAIK, only OGC API Records is used in that repository so it should be quite safe.

@davidblasby
Copy link
Contributor Author

Thanks, @fxprunayre, for your reply.

I think for searching and most things, everything needed is in the Index - the queryables work very well.

I am a bit worried about some info (from the xml) not being in the index json for displaying the record-details-page, but most things are there. I included the XML incase this happens.

@davidblasby
Copy link
Contributor Author

@josegar74 @fxprunayre - I am planning on merging this wed evening (european time).

Copy link

sonarcloud bot commented Oct 23, 2024

@davidblasby davidblasby merged commit dd235b3 into geonetwork:main Oct 23, 2024
2 checks passed
fxprunayre added a commit that referenced this pull request Nov 19, 2024
Fix NPE on DCAT item response eg.
http://localhost:9901/collections/main/items/f946d4e0-710b-11dc-83f9-000086f6a62e?f=dcat

```
2024-11-19 11:15:29.659 ERROR 547670 --- [nio-9901-exec-1] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is java.lang.RuntimeException: java.lang.NullPointerException] with root cause

java.lang.NullPointerException: null
	at org.fao.geonet.index.converter.DcatConverter.convert(DcatConverter.java:128) ~[classes/:na]
	at org.fao.geonet.ogcapi.records.controller.ItemApiController.collectionsCollectionIdItemsRecordIdGetAsJsonLd(ItemApiController.java:365) ~[classes/:na]
	at org.fao.geonet.ogcapi.records.controller.ItemApiController.collectionsCollectionIdItemsRecordIdGet(ItemApiController.java:191) ~[classes/:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:566) ~[na:na]
	at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:190) ~[spring-web-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:138) ~[spring-web-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:105) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:878) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:792) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1040) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	a
```

DCAT conversion is for now based on Elasticsearch index document which is properly returned for search but not for the items operation with now the new GeoJSON document with `doc.get("properties").get("gn-elastic-index-record")` which can be used for the conversion.

DCAT will be better supported with formatters geonetwork/geonetwork#72
but for 4.4.6, this is relevant to be fixed.

Related to
#118
fxprunayre added a commit that referenced this pull request Nov 19, 2024
Fix NPE on DCAT item response eg.
http://localhost:9901/collections/main/items/f946d4e0-710b-11dc-83f9-000086f6a62e?f=dcat

```
2024-11-19 11:15:29.659 ERROR 547670 --- [nio-9901-exec-1] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is java.lang.RuntimeException: java.lang.NullPointerException] with root cause

java.lang.NullPointerException: null
	at org.fao.geonet.index.converter.DcatConverter.convert(DcatConverter.java:128) ~[classes/:na]
	at org.fao.geonet.ogcapi.records.controller.ItemApiController.collectionsCollectionIdItemsRecordIdGetAsJsonLd(ItemApiController.java:365) ~[classes/:na]
	at org.fao.geonet.ogcapi.records.controller.ItemApiController.collectionsCollectionIdItemsRecordIdGet(ItemApiController.java:191) ~[classes/:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:566) ~[na:na]
	at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:190) ~[spring-web-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:138) ~[spring-web-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:105) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:878) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:792) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1040) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898) ~[spring-webmvc-5.2.12.RELEASE.jar:5.2.12.RELEASE]
	a
```

DCAT conversion is for now based on Elasticsearch index document which is properly returned for search but not for the items operation with now the new GeoJSON document with `doc.get("properties").get("gn-elastic-index-record")` which can be used for the conversion.

DCAT will be better supported with formatters geonetwork/geonetwork#72
but for 4.4.6, this is relevant to be fixed.

Related to
#118
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants