search

koopjs / koop

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

Cache not storing Date field Type #358

Closed dstajan closed 4 years ago

dstajan commented 4 years ago

Hi, I 'm noticing a behavior that i think is related to caching or ttl.

When I hit the koop service and it needs to query the api I get the desired field type description { name: "timeToFetch", type: "esriFieldTypeDate", alias: "timeToFetch", sqlType: "sqlTypeOther", domain: null, defaultValue: null, length: 36 }

however every subsequent request (where the koop service is responding with cached data) it returns this field type { "name": "timeToFetch", "type": "esriFieldTypeInteger", "alias": "timeToFetch", "sqlType": "sqlTypeOther", "domain": null, "defaultValue": null }

Is this a bug? Any workarounds or suggestions?

rgwozdz commented 4 years ago

Not sure, this may be a bug. Are you using a provider that you have authored, or one of the published providers? If the former, are you setting metadata.fields? Are you able to share the contents of your getData method?

dstajan commented 4 years ago

I'm writing my own custom provider, but I am able to replicate this behavior as well with the craigslist provider. You could use it to test

when the provider runs the query it returns { name: "postDate", type: "esriFieldTypeDate", alias: "postDate", sqlType: "sqlTypeOther", domain: null, defaultValue: null, length: 36 },

but when it runs from cache

{ name: "postDate", type: "esriFieldTypeInteger", alias: "postDate", sqlType: "sqlTypeOther", domain: null, defaultValue: null },

rgwozdz commented 4 years ago

Confirmed bug. Fix coming shortly.

rgwozdz commented 4 years ago

@dstajan - I spoke a little too soon about a fix coming shortly. Need to consider how to address this. Essentially, this is what happens:

  1. Koop stores provider data in an in-memory cache
  2. If metadata.fields is not defined, Koop tries to determine field types by inspecting first feature
  3. All ISO date strings are modified to epoch time integers. Since this modification occurs on the same object that is stored in memory, follow up pulls from the cache find the modified geojson (integer time instead of ISO string). As a result the field is interpreted to be an integer instead of a Date.

This is definitely a sub-optimal. The quick fix would be to clone the data coming out of the cache so the date modification doesn't occur on the cached object. But this could have a performance impact. So I'd rather reconsider the conversion of ISO strings to integers al together - but that might take a little bit of time.

In the short term, you should be able to avoid this by defining metadata.fields in your getData function. See the doc here.

dstajan commented 4 years ago

Thanks Rich, Adding fields metadata did suppress the issue and allowed me to move forward