GIScience / ohsome-api

API for analysing OpenStreetMap history data
https://api.ohsome.org
GNU Affero General Public License v3.0
47 stars 8 forks source link

invalid geojson returned for deletions on contributions endpoint #129

Closed tyrasd closed 3 years ago

tyrasd commented 3 years ago

Deleted elements are represented by null geometries in normal contributions/geometry requests. However, when using the contributions/centroid endpoint (and perhaps also …/bbox), one gets the following invalid GeoJSON result instead:

  …
  {
    "type" : "Feature",
    "geometry" : {
      "type" : "Point",
      "coordinates" : [ ]
    },
    "properties" : {
      "@changesetId" : 84648597,
      "@deletion" : "true",
      "@osmId" : "way/800143465",
      "@osmType" : "WAY",
      "@timestamp" : "2020-05-04T21:23:50Z",
      "@version" : 2
    }
  }
  …

image

//edit: this is how I would have expected as a result:

  {
    "type" : "Feature",
    "geometry" : null,
    "properties" : {…}
  }
FabiKo117 commented 3 years ago

I've checked it with the following request that I took from our docs page and used different geometry types then. https://api.ohsome.org/v1/contributions/latest/centroid?bboxes=8.6644%2C49.4010%2C8.6663%2C49.4027&filter=landuse=construction%20and%20type:way&time=2018-07-01%2C2020-06-29&properties=metadata%2Ctags&clipGeometry=false

The issue persists when the deleted geometry is of type Point or LineString. Polygons can have an empty coordinates field, that's why I guess this issue never got our attention. I remember facing some obstacles when trying to set null for the geometry for deletions, hence the workaround with the empty coordinates. But yeah as it apparently causes to be invalid GeoJSON, we have to try again with the null geometry.

tyrasd commented 3 years ago

so you are saying it is not only a problem with the centroid endpoints, but all contribution geometries? :unamused:

tyrasd commented 3 years ago

Polygons can have an empty coordinates field

not sure I get what you mean. in what situation would polygons have empty coordinates? If I recall correctly, there are even some checks in the OSHDB and JTS that prevent polygons being created with fewer than 3 vertices.

FabiKo117 commented 3 years ago

so you are saying it is not only a problem with the centroid endpoints, but all contribution geometries? 😒

An empty coordinates array for a deletion is the default behavior.

I mean empty coordinates for polygons are conform with a valid GeoJSON.

tyrasd commented 3 years ago

I mean empty coordinates for polygons are conform with a valid GeoJSON.

ok, I see. While this is probably technically correct, I would still argue that it is not a good idea to use this loophole for a representation of "inexistent" geometries. In effect you are relying on a definition which is by definition a gray area in the standard, see section 3.1:

GeoJSON processors MAY interpret Geometry objects with empty "coordinates" arrays as null objects.

While the standard defines perfectly how unlocated features should be represented in section 3.2:

The value of the geometry member SHALL be […] in the case that the Feature is unlocated, a JSON null value.

Can you elaborate what issues you encountered when implementing the null geometries output?

FabiKo117 commented 3 years ago

I already found a solution I think. Will just have to further check if that would collide with something else.

Basically, all null objects (so either key or value) are always filtered out when creating the response GeoJSON. If I deactivate all of these filters, the response would look like this:

{
    "attribution": {
        "url": "https://ohsome.org/copyrights",
        "text": "© OpenStreetMap contributors"
    },
    "apiVersion": "1.4.0-SNAPSHOT",
    "metadata": null,
    "type": "FeatureCollection",
    "features": [
        {
            "type": "Feature",
            "geometry": null,
            "properties": {
                "@changesetId": 63955212,
                "@deletion": "true",
                "@osmId": "way/155474272",
                "@osmType": "WAY",
                "@timestamp": "2018-10-28T16:53:32Z",
                "@version": 10
            }
        }
    ]
}

But I found out that it can also be just deactivated for the JSON object mapper that is just used within the respective feature response creation. Then it looks like this:

{
    "attribution": {
        "url": "https://ohsome.org/copyrights",
        "text": "© OpenStreetMap contributors"
    },
    "apiVersion": "1.4.0-SNAPSHOT",
    "type": "FeatureCollection",
    "features": [
        {
            "type": "Feature",
            "geometry": null,
            "properties": {
                "@changesetId": 63955212,
                "@deletion": "true",
                "@osmId": "way/155474272",
                "@osmType": "WAY",
                "@timestamp": "2018-10-28T16:53:32Z",
                "@version": 10
            }
        }
    ]
}

So I think that's what you expected, right? I will push then these changes in a branch so you can have a look at it and will continue with testing if that interferes with something else.

tyrasd commented 3 years ago

yes, the second example looks fine :+1: thx