koopjs / koop

Transform, query, and download geospatial data on the web.
http://koopjs.github.io
Other
651 stars 125 forks source link

PBF Output errors with "Unknown geometry type" when geojson.filtersApplied.all is set to true #984

Closed jag-eagle-technology closed 2 months ago

jag-eagle-technology commented 2 months ago

As per title,

If I provide geojson to the output geoservices plugin with the filltersApplied object all field set to true and request PBF, I receive the error "Unknown geometry type". Setting geometry, where, offset, limit and projection individually does not cause this issue.

rgwozdz commented 2 months ago

Thanks for reporting this @jag-eagle-technology.

rgwozdz commented 2 months ago

@jag-eagle-technology - which Koop release are you using? I am having trouble reproducing this, and we already pushed a patch for a similar error:

https://github.com/koopjs/koop/pull/945

That would have gone out in koop-core 10.3.7; can you update to latest release if you haven't already (@koopjs/koop-core@10.4.5 and confirm you still get the error reported above?

jag-eagle-technology commented 2 months ago

I'm using the latest version. Here's a basic reproduction example:

https://github.com/jag-eagle-technology/koop-pbf-issue

New to koop so apologies if I'm missing something obvious.

rgwozdz commented 2 months ago

No, you are spot on. It's a bug you have found. I just overlooked something while I was trying to reproduce. Thanks for the details! We'll sort it out and get a patch shortly.

rgwozdz commented 2 months ago

@jag-eagle-technology

Ok, the good news is that this could maybe be viewed as not so much a bug, but maybe just a doc change.

The issue is that when you go filtersApplied: { all: true }, Koop doesn't pipe the provider data through any post-provider filtering or transforms; including the transform from GeoJSON to Esri JSON. The PBF encoder expects Esri JSON, but it ends up getting GeoJSON, and then throws the error.

This is sort of a problem with what the name filtersApplied implies and our docs. It suggests that the only thing happening in that stage is "filtering" and if you have done that already in your provider you can skip everything with all. But that isn't true - that stage is also where the GeoJSON to Esri JSON conversion occurs, which is required for FeatureServer routes. (I think this transform was originally executed in filtering stage to avoid looping through all the data a second time). When filtersApplied is instead set with individual properties like:

geojson.filtersApplied = {
        geometry: true,
        where: true,
        offset: true,
        limit: true,
        projection: true,
      }

the GeoJSON is still piped through the noted post-processing stage, and while the filtering is skipped, the conversion is still made.

In the short-term, filtersApplied should not use the all property, but instead set each individual property to true. Honestly, it's probably best to eliminate the use of all.

jag-eagle-technology commented 2 months ago

Makes sense. I'd tried querying for JSON directly - but found the transform (Esri JSON not Koop) information was dropped so the geometry wasn't valid.