hyper opinions primary key data port

[cawilson1]

The listDocument params contain startkey, endkey, and descending. This implies that the primary keys are sortable.

Does hyper have an opinion about what a primary key should look like? The docs usually use incrementing ints. Semantic strings could also be sortable. But is it acceptable in general within hyper to define a primary key as a random string or int? If so, the listDocuments may not use all the params regardless of adapter because startkey, endkey, and descending don't have any valuable meaning.

Assuming the "keys" param on the port is an array of keys, documents can be listed that way. Right now the type specified for "keys" is string. I wonder if this is supposed to be an array of strings, or if the adapter should split at a special character.

[twilson63]

https://docs.hyper.io/create-a-document

value requirements

• must begin with lowercase letter (a-z)
• must be lowercase letters (a-z)
• digits (0-9)
• any of these characters _ $ + -

document ids should meet these standards.

[twilson63]

A common pattern for uri key value pars with multiple values is key=value1,value2,value3 I think that is the intention of the list documents via a query parameter. I can improve the docs here.

[TillaTheHun0]

To Casey's point, passing an array of keys to the adapter seems more intuitive and conventional, than a delimited string. The uri on the request to hyper will be in the delimited format, but I think it would be in the purview of the hyper app to parse those query params into an array, then pass to an adapter (opine middleware may already do this sort of thing for query params in that format. Would need to check to make sure. A common pattern i've also seen is key=value1,key=value2. Perhaps we should find middleware that supports parsing both to array if we make this change?)

[cawilson1]

Thanks for taking the time to respond.

I believe that purely from the adapter contributor's perspective, an array is a better choice for several reasons.

The contributor does not have to read through docs to find the string format.
The core team does not have to maintain the docs to describe the string format.
The contributor does not need to implement any extra string parsing logic on the adapter.

I realize the string may be there for historical reasons and make a challenging update to the port for apps currently using hyper. Tyler proposed the idea of it working for both in a conversation we had.

[twilson63]

I highly recommend we review standards, the one I lean to is URLSearchParams you can find info about it at https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams - When you pass an array in via the set

params.set('y', [1,2,3])
params.toString() // y=1,2,3

Here are more discussions:

https://stackoverflow.com/questions/6243051/how-to-pass-an-array-within-a-query-string

I know couchdb prefers x=1,2,3 - but regardless, we will need to choose what we want to support and make sure the app, implements it correctly in its specification, I think the port schema should enforce that keys is an array, so if app does not properly parse it will error.

[TillaTheHun0]

Agree. 👍

We can see the hyper opine app passes req.query directly into the data service listDocuments here

This ultimately get's spread into the object passed to the adapter by hyper core here

I think we can make the change to app-opine to parse the req.query.keys into an array, which would just be a patch version bump. Then core wouldn't need to change. Finally the Data port definition would need to be updated, which would be a breaking change. Then each data port adapter would need to be updated as a breaking change.

TL;DR:

• update app-opine (surprised it's not already performing this parsing 🤔 ) PATCH BUMP
• update port-data listDocuments to accept keys as an array of strings MAJOR BUMP
• update implementations of each data adapter MAJOR BUMP

Thoughts?

[twilson63]

Is it a breaking change? I thought additions would be minor changes, because it does not currently break functionality and actually your could argue it is a patch, because it is specified to support the property keys, it is just ports and/or adapters don’t validate which is not a new feature but a bug Those are my thoughts

…

Sent from my iPhone

On Jul 6, 2021, at 11:35 PM, Tyler Hall ***@***.***> wrote:  Agree. 👍 We can see the hyper opine app passes req.query directly into the data service listDocuments here This ultimately get's spread into the object passed to the adapter by hyper core here I think we can make the change to app-opine to parse the req.query.keys into an array, which would just be a patch version bump. Then core wouldn't need to change. Finally the Data port definition would need to be updated, which would be a breaking change. Then each data port adapter would need to be updated as a breaking change. TL;DR: update app-opine (surprised it's not already performing this parsing 🤔 ) PATCH BUMP update port-data listDocuments to accept keys as an array of strings MAJOR BUMP update implementations of each data adapter MAJOR BUMP Thoughts? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[TillaTheHun0]

Right now, keys is defined on the listDocuments schema as a string. Wouldn't this be changing keys to a string[] on the schema? Or do we want the port schema to support string | string[]?

Fine with adapters being patch bumps.

[twilson63]

That is fine with me, I think most implementations will be a comma-separated string 1,2,3 so we should support that, but if you see other implementations of other formats then lets support them too!