V1 prototype

[azazeal]

This PR contains an ambitious prototype that aims to serve as the foundation of the next major version of this package.

This iteration tries to ensure that the behavior of the package is consistent across the different data stores we aim to support. In order to achieve this, this implementation enforces validations against the 3 different types of tokens (buckets, keys & values).

Any literal may be considered a valid bucket if it:

• a) is non-empty (and therefore non-nil).
• b) is up to 50 bytes long.
• c) consists of valid UTF8 encoded runes.
• d) does not end in white space.

Similarly, any literal may be considered a valid key if it:

• a) is non-empty (and therefore non-nil).
• b) is up to 200 bytes long.

And, finally, any literal may be considered a valid value if it:

• a) is not nil.
• b) is up to 16 KiB (or 16,384 bytes) long.

Because of the above, and disregarding the expansion of most functions to also accept a Context, this implementation would only be backwards compatible in scenarios where the above constraints were already respected (even accidentally).

[mohammed90]

There was a discussion at caddyserver/caddy#6097 about these upcoming changes. Given our hard experience with Badger (see linked thread) and the transitive dependencies each driver/database pulls, do you think it's possible to break the drivers from the core? Something akin to what the database/sql package offers. It allows downstream users to determine their desired DB to support and trim their transitive deps. It also avoids build constraints imposed by the drivers (Badger in our case).

[azazeal]

There was a discussion at caddyserver/caddy#6097 about these upcoming changes. Given our hard experience with Badger (see linked thread) and the transitive dependencies each driver/database pulls, do you think it's possible to break the drivers from the core? Something akin to what the database/sql package offers. It allows downstream users to determine their desired DB to support and trim their transitive deps. It also avoids build constraints imposed by the drivers (Badger in our case).

With this implementation, the drivers are broken from the core. For example, unless you import _ "github.com/smallstep/nosql/driver/badger" then you won't end up building the driver for it. You may pick and choose which drivers you want to include, much like database/sql does.

This however, doesn't mean that you won't end up getting the transitive dependencies. To avoid that, we'd have to convert each driver directory in their own Go module (an option I don't believe we're willing to consider) or extract each driver into their own repo/package (e.x. github.com/smallstep/nosqlpgdrv, github.com/smallstep/nosqlmysqldrv, github.com/smallstep/nosqlbadgerdrv & github.com/smallstep/nosqlboltdrv), with the additional overhead this entails.

Your suggestion/request isn't unreasonable, and something that personally I'd like to see addressed, so I'll try to see where the team stands on and take it from there.

Hope this helps.

[mohammed90]

Thank you for the consideration! I understand the maintenance burden. I'll keep an eye on here 🙂

[hslatman]

It's a bit surprising to see the testing package being used in a package that doesn't look like a test or a test utility package (from a quick skim). I supposed this is used as a test utility?

Also:

Suggested change

	// If bucket is set, then the returned value will be utf-8 valid and not contain the zero byte.
	func New(t *testing.T, minSize, maxSize int, bucket bool) (tok []byte) {
	if minSize == maxSize {
	// If bucket is set, then the returned value will be utf-8 valid and not contain the zero byte.
	func New(t *testing.T, minSize, maxSize int, bucket bool) (tok []byte) {
	t.Helper()
	if minSize == maxSize {

[azazeal]

Nothing surprising here, it's an internal package, used by tests exclusively.

[hslatman]

Proving my point, it is surprising, because it's not clear for someone seeing this code for the first time that it's only used in tests. It would be clear(er) if this were in something like testutils

[azazeal]

I avoided a generic testutils package. I left the token package just for drivers that may want to generate a token in order to test something.

[mohammed90]

I'd hate to be demanding, but can you at least put badger behind a build tag 🙂

[azazeal]

Since this PR has been open for quite a while, I'd like to just comment that the plan for it is to be merged.

There's internal issues, so we've paused the merge for the time being.

[azazeal]

I'd hate to be demanding, but can you at least put badger behind a build tag 🙂

If you don't

import _ "github.com/smallstep/nosql/badger/v{1,2}

then you don't end up building badger libs.

You may add your own build tag in the calling program that decides whether to add said import or not to your builds. Won't that work for you, @mohammed90?

[mohammed90]

If you don't

import _ "github.com/smallstep/nosql/badger/v{1,2}

then you don't end up building badger libs.

Oh, I missed that. That is not bad, and works.

I was hoping it'd be feasible to de-bundle the storage drivers themselves so they don't end up being transitive deps downstream. The discussion in caddyserver/caddy#6613 (comment) shows another need for slimming the dep tree. I tried my hand at making them more like database/sql drivers, but the dependency on internal/each makes it unfeasible unless it's exposed. It's a complex matter. Thank you for the effort!