koopjs / koop

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

Objectid only query from pro not returning results in callback #320

Closed jptheripper closed 6 years ago

jptheripper commented 6 years ago

I tried to narrow the problem down as specific as I can. This is related to a koop provider (custom) rendering in Pro but not showing a popup.

The provider generates and stores a custom OBJECTID, that allows for POST that returns a feature collection with a single feature in the feature array from a third party provider

The Pro request looks like

https://www.mymanatee.org/giskoop/mapillary/detections/FeatureServer/0/query?f=json&outFields=label%2cOBJECTID&where=1%20%3d%201&objectIds=26&returnGeometry=false

note is is where 1 = 1 not 1=1 (which is really annoying) outfields are label, OBJECTID (label is a valid field)

inspecting this request in koop generates a url request that return a response which is parsed and formatted to (values simplified)

{ type: 'FeatureCollection', features: [ { type: 'Feature', properties: [Object], geometry: [Object] } ] }

however when this is sent to the callback, unlike other requests, koop returns an empty featureset.

jptheripper commented 6 years ago

any ideas on this?

rgwozdz commented 6 years ago

@jptheripper - When I click on your link I get an empty feature array. I see the parameter objectIds=26. Does a feature with that OBJECTID exist?

jptheripper commented 6 years ago

yes that is the exact problem. The request generates an api call to a 3rd api. that returns a feature collection. i pass that to the callback, like always, but for some reason, on these requests from pro, the koop results is an empty feature array.

rgwozdz commented 6 years ago

@jptheripper - what does your provider metadata look like?

jptheripper commented 6 years ago

"metadata":{ "name":"Mapillary Detections", "idField":"OBJECTID", "maxRecordCount":2000, "extent":[[-82.58,27.49],[-82.57,27.51]], "geometryType":"Point", "fields": [ { "name": "OBJECTID", "type": "esriFieldTypeOID", "alias": "OBJECTID"}, { "name": "area", "type": "Double", "alias": "Area" }, { "name": "score", "type": "Double", "alias": "Score" }, { "name": "image_key", "type": "String", "alias": "Image Link" }, { "name": "thumbnail", "type": "String", "alias": "Thumbnail" }, { "name": "key", "type": "String", "alias": "key" }, { "name": "mapillary_key", "type": "String", "alias": "Mapillary Link" }, { "name": "package", "type": "String", "alias": "package" }, { "name": "layer", "type": "String", "alias": "Layer" }, { "name": "value", "type": "String", "alias": "Value" }, { "name": "label", "type": "String", "alias": "Label" } ] }

jptheripper commented 6 years ago

note, the API does not have objectid, i have to generate it on the fly and store it locally

rgwozdz commented 6 years ago

Ok, likely has something to do with that. I'm running into the same thing right now and trying to diagnose.

rgwozdz commented 6 years ago

@jptheripper - how are you generating your objectid?

jptheripper commented 6 years ago

using a sequential number and then storing the remote (non-esri compliant key) in a json object

So i 1, do a call that returns 10 features

  1. store keys 1-10 in a json object with the corresponding values
  2. store values 1-10 in same json object with corresponding keys (for reverse lookup)

when an objectid query comes in, (without geometry and req.query.objectids is not null) to the provider, i redriect the request url to an api lookup based on the retrieved objectid values. This returns a single feature per objectid requested. The resulting FeatureCollection is sent to the callback (and i verify that there are features in it).

However, the koop endpoint returns and empty featureset. So somepoint between calling the callback with a FeatureCollection, and the end (query post processing?) the feature is being lost.

rgwozdz commented 6 years ago

I would see if you are losing it here: https://github.com/koopjs/FeatureServer/blob/a7cb3ed6f113cc45ab3bd064e1bbe8b724b3a6c3/src/query.js#L51

jptheripper commented 6 years ago

Ok yes i am. And here is why. that is expecting an array. for some reason, i have a JSON object, that you cant do a indexOf on. Should i not be sending a full JSON object to this callback?

image

jptheripper commented 6 years ago

Changing Line #51 to

  return oids.indexOf(f.attributes[oidField]) || f.attributes.hasOwnProperty(oidField) > -1

to check for a JSON key instead of an index fixes popups in Pro

rgwozdz commented 6 years ago

Won't f.attributes.hasOwnProperty(oidField) return a boolean, which will always be > -1 regardless of true or false?

jptheripper commented 6 years ago

oh shoot you are right

  return oids.indexOf(f.attributes[oidField])> -1 ||  f.attributes.hasOwnProperty(oidField) 
jptheripper commented 6 years ago

im still not sure why its expecting an array and getting a JSON object

rgwozdz commented 6 years ago

Yup, that's odd. New minor versions of FeatureServer and Winnow were just released a few moments ago. What's new is the use of farmhash for creating OBJECTIDs from the stringified feature if one isn't specified by the provider's idField. farmhash creates out-of-range 32 bit unsigned integers, but we get around that by rescaling those integers to a max value of 2,147,483,648. Granted, there is still a chance of collisions, but it greatly reduced. User testing with Pro thus far has shown fully functional behavior for rendering, table loading, table select, pop-up, zoom-to, and click-select. If you upgrade and still have problems, let me know. Happy to help trouble shoot.

jptheripper commented 6 years ago

of course, as soon as i have a workaround, functionality is included out of the box. time to back out the code :)

rgwozdz commented 6 years ago

@jptheripper - can you upgrade FeatureServer to 2.11.1 and winnow to 1.14.0 and confirm this is still a problem for you?

jptheripper commented 6 years ago

i can but likely not for a few days

jptheripper commented 6 years ago

ok i upgraded. i get the "provider has no oid" warning. However my popups dont work. Should i remove the OID i added to the attributes and the metadata ?>

rgwozdz commented 6 years ago

Yes, if you could remove what you have done, then we'll be able to see if the new out of the box OBJECTID generation in winnow solves your issue.

jptheripper commented 6 years ago

I tried with FS 2.11.1 and winnow 1.14.0. I get the "provider has no id " warning. In pro rendering works. Attribute table works and shows OBJECTID (looks like a hash). Zoom to/flash/pan do not work popups do not work

in fact now line 51 (the postprocessing) does not even get into the if, as if no features are being passed

rgwozdz commented 6 years ago

I'd like to try to reproduce, as I'm not having these issues with the providers I have tested. Which provider are you using? And you noted line 51 - which repository?

rgwozdz commented 6 years ago

Also, if you could provide the query params for the problematic request from Pro, that would be helpful.

rgwozdz commented 6 years ago

@jptheripper - just circling back on this. Anymore details you can provide?

jptheripper commented 6 years ago

unfortunately no did not work.

I updated with yarn. Confirmed winnow and featureserver versions. Removed the objectid from my metadata fields removed my objectid code from the attribute manipulation code removed the query override for objectid calls

Draws in pro, fails to identify.

rgwozdz commented 6 years ago

@jptheripper - I can't reproduce right now, but I would like to help. Is this a custom provider you are using? Also, you noted above

in fact now line 51 (the postprocessing) does not even get into the if, as if no features are being passed

Can you give some context for that?

rgwozdz commented 6 years ago

Might be helpful if you provide the snippet from your provider that defines model metadata.

jptheripper commented 6 years ago

detectionsindex.zip

jptheripper commented 6 years ago

That is with the object id reinserted

rgwozdz commented 6 years ago

@jptheripper thanks, that gives me more to go on. I'll poke around on the rest.

jptheripper commented 6 years ago

i really appreciate it

rgwozdz commented 6 years ago

@jptheripper - first a question. I notice you are assigning idField: 'OBJECTID in metadata. Koop interprets this to mean that your features already have an integer property name OBJECTID. Can you confirm that your data already has an OBJECTID property when it comes out of your provider's getData function and before its modified by FeatureServer and Winnow?

Second, I may have just reproduced the behavior you noted. If the fields array assigned in metadata is not an exact match for the properties actually being returned from the getData Pro's identify appears to fail. For example, if I add an object to metadata.fields that has name: 'someProperty' but the data delivered doesn't have features with a property named someProperty, Pro's identify fails.

To further debug, I would suggest first removing the fields array from metadata altogether as well as the idField assignment (Koop will set it by defaul) Also strip out all feature.properties in when constructing the feature in your provider's getData function. The result should be a feature service with only the OBJECTID property. Restart Koop, and see if popup works. If it does, start adding the other fields/properties back in one by one until you find which of them breaks the identify tool. It's possible its just a name mismatch. Also possible that it is a string-length issue on one of your fields. Right now koop defaults to string field lengths of 128. I believe another user has report this as a problem in Pro.

Also, beware of layer caching in Pro. I'd clear the project cache after every koop restart to ensure it doesn't cache anything.

rgwozdz commented 6 years ago

@jptheripper - after some additional testing, I find that having string fields with value lengths greater than the default 128 chars causes the identify failure. I'm going to put together a PR that lets you set the field length in metadata.

jptheripper commented 6 years ago

Thats odd, because when i use my own sequence, without altering the other fields, the identify works

rgwozdz commented 6 years ago

Hmm, ok. I don't quite understand how your metadata is being set in your getData function. It looks like it's may not be being set on every request. I think this could lead to some odd behavior. Maybe you have already looked at it, but the Craigslist provider has a basic implementation of metadata on every request: https://github.com/dmfenton/koop-provider-craigslist/blob/master/model.js#L18-L22. We have tested this provider with Pro and identify works well.

The Craigslist provider doesn't define a field array in metadata, instead it leaves it up to FeatureServer to build it dynamically from the data. I'm doing some testing on field definition via metadata and finding a lot can go wrong in Pro if fields set in your metadata don't properly match what's in the payload. In addition, string length overflow appears to be a problem for identify. But maybe this is unrelated to this issue you are experiencing.

If you have a solution using a sequence from your OBJECTID in your provider, I would encourage you to proceed. There is quite a lot in your provider for me to step through, I won't be able to get to it as soon as I would like.

rgwozdz commented 6 years ago

One more thing, if your features are changing rapidly (e.g. a timestamp property that gets updated on every request) and are using the out-of-the-box method of hashing to create OBJECTIDS, you can also experience Objectid-only queries failing to return results. That's because the numeric hash is built from the entire feature, and if any property has changed, the new hash will be different. Thus the OBJECTID will have changed and won't be found.

Is there anything in your feature that changes across requests?

jptheripper commented 6 years ago

My data dont change, they have a unique key, its just a 3digit alphanumeric guid

I never really figured out the proper place for the metadata. i just have those three files. if a request comes to koop, and there is no geometry/objectid, etc.. it has a "else" that loads that reference file and returns the metadata.

I am probably doing it completely wrong. no model, no route, literally just the index, provider, and reference file with metadata

jptheripper commented 6 years ago

Ok, well, good news and bad news. Stripping out the features in the metadata did make the popup open, but it is blank

When i click on a feature in pro, all the features highlight, and then i get a blank popup image

I am doing attribute manipulation ( I have to ). the api call returns a few attributes, i have to add more on the fly. hence the field array in the metadata.

Also zoomto and panto from the attribute table do not work

rgwozdz commented 6 years ago

Ok, thanks for the addition info. I've been continuing work on this as well. I found an issue in winnow and have added some field checking in FeatureServer, as well as the ability to set string lengths in metadata. PRs are in process and hope to have a release soon. I think this will help with some of these issues. Pro seems very sensitive to mismatches between the fields array and what properties are in the feature's attributes.

jptheripper commented 6 years ago

well, if i assign my own OID it works, 99%. The popup doesnt have the OID (although the table does) and koop still thinks it has no OID

i think that is because i am doing callback (null, body) after modifying the request, but

function query (data, params) { is checking the data parameter to see if the id field is set.

any ideas?

rgwozdz commented 6 years ago

@jptheripper - not sure about that. I was finally able to reproduce the Objectid only query from pro not returning results. Patch just released for winnow (1.15.2). In addition, I've updated FeatureServer (2.12.0) to 1) allow field string lengths to be set in provider's metadata, and 2) print console warnings if fields set in provider's metadata fail to match properties found on feature properties. I'm getting good result with this in Pro.

jptheripper commented 6 years ago

well, that is yet closer.

  1. Draws in Pro
  2. Attribute table opens and populates
  3. Right click zoom /pan to to doesnt work
  4. Clicking on a feature creates an empty popup with the OID as the title, but the attributes, including OID, blank (must be a format/attribute error)

this is winnow 1.15.2 and Featureserver 2.12.1

jptheripper commented 6 years ago

Unfortunately winnow 1.15.2 and featureserver 2.12.1 broke my workaround . AM going to have to investigate more

rgwozdz commented 6 years ago

@jptheripper - can we close this issue since it refers specifically to "Objectid only query from pro not returning results"? Seems like that is resolved at lease. Perhaps you could open another issue for the remaining difficulties your are having?

rgwozdz commented 6 years ago

Closing; with upgrades to winnow and featureserver, I can no longer reproduce an instance of "objectid only query from pro not returning results".