Improvements for writing GeoJson for GML with nested geometries #1765
Conversation
Because geometries are not exported as part of the properties, the behavior so far when encountering one while exporting the properties was to ignore it. But this caused problems, as the property name was already written. Refactored to check for a geometry property before the property name is written.
...in case no geometry properties are found with the normal mechanism checking only the main properties.
tfr42
added
enhancement
| // geometry was exported before | ||
| // need to throw an exception here, because if we do nothing the Json is | ||
| // invalid (condition should be handled before) | ||
| throw new IOException("Geometry should not be rendered as part of GeoJson properties."); |
There was a problem hiding this comment.
Can you please explain why do you see the need to throw an exception here or is it intended to prevent misuse of the API here? Does throwing the exception stops the generation of the GFI GeoJSON output?
There was a problem hiding this comment.
This code should not be reached, the exception is thrown to raise a failure if it does happen nonetheless.
If such a case occurs and the behavior here would be as before, doing nothing, no valid Json would be created and the process would fail with the "Dangling name" exception mentioned in the PR description, because there would be a Json field w/o a value.
So the behavior would be similar, just a different exception.
Now we catch that case earlier though to prevent the field from being written at all and thus to prevent the exception.
The intention here seemed clearly to not export the geometry again, so I guess only other alternative would be creating a null value for the field or an empty object. Depends on what is preferable - I went for omitting the field in that case.
There was a problem hiding this comment.
@stempler Thanks for the explanation. Would it be possible that you provide a workspace with a example data set to verify the behaviour in an integration test?
There was a problem hiding this comment.
I think the added unit tests in GeoJsonFeatureWriterTest already cover the issue and fix quite well.
The data used in the unit tests could be used in test workspaces as well I guess. Do you have an example of a similar integration test like what you have in mind?
|
Thank you for your contribution. Your pull request has been discussed by the TMC 2024-12-18. Please see the following remarks from the TMC members inline. |
These changes are related to issues we encountered specifically with GetFeatureInfo requests for INSPIRE Governmental Services data using GeoJson format. The solution probably also covers other GML application schemas with a similar structure for defining geometries.
For these cases the WMS would yield an internal error without helpful message as result for the GFI request.
In the logs error messages similar to this would show up:
The first commit attempts to solve this problem, by not writing the incomplete Json field, which before resulted in the shown error, as no value was written for the field.
The second commit attempts to retrieve the geometry properties in such a schema (previously it would not identify them), so when the GFI is requested including geometries, they can be included instead of having a
nullvalue for the geometry. This has the same restriction as before, that if there are multiple geometries the GeoJson representation can't handle this.