plugin extension points: merge level-hooks / level-sublevel

[dominictarr]

I'm not suggesting we do this right away, but I am suggesting this is something worth considering.

Also, I don't want to merge this, unless there is a consensus that this is an easy and effective way to build plugins.

I'd also intend to refactor the code style, etc, to match levelup.

I think that given these base features, and maybe one or two more,
a wide array of plugins could be developed.

[dominictarr]

I realize that this could be construed as going against the modularity principle, but I think this may be worthwile, because A) we already have the core separated into leveldown B) this would provide a firm standard basis for more modularity.

Also, I don't want to suggest we merge immediately, but we should put it on the table.

[juliangruber]

those 2 plugins you suggest to go into core will be crucial for the future development of levelUp.

With hooks multilevel will work with more plugins or e.g. a middleware layer can easily be implemented.

SubLevel will come in handy when we decide to give each plugin its own meta db.

[dominictarr]

see also #97

[vweevers]

@ralphtheninja I think a new issue is in order (in community perhaps), because the hooks of level-sublevel are a powerful concept, and we don't have a replacement atm.

[ralphtheninja]

I can re-open and move to community.

[vweevers]

One part of this (sublevels) is code complete: Level/abstract-level#8. As for hooks, let's define some requirements:

1. They don't have to support range options like level-hooks did, because sublevels are builtin
2. For the same reason, we don't need post hooks. For that you can use events of a sublevel
3. They should support promises
4. They should support adding operations (to a batch). This is the main use case IMO.
5. To start simple, execute hooks sequentially (not in parallel like level-hooksdown).
6. If a db.batch(..) calls contains operations with a sublevel property, execute the hooks of that sublevel, instead of hooks of the db? TBD.

High-level, I see three ways to implement hooks:

1. As an abstract-level layer (similar to level-hooksdown).
   • Downside: hooks would receive already-encoded keys and values. Unless we override public methods - e.g. put() instead of _put() - but I'd really like to avoid that pattern.
2. Builtin to abstract-level, and executed before encoding (of keys and values) and prefixing (of sublevel keys).
   • Gotta take performance into account. If one or more hooks are present, put() and del() should internally use a batch(). Initially we can do that eagerly, later on perhaps only if one of the hooks wants to add operations.
3. Builtin to abstract-level, and executed after encoding and before prefixing.
   • I don't see a use case for this

[vweevers]

cc @bcomnes as the author of level-hooksdown: if hooks were builtin, which features are a must for you?

[bcomnes]

Let me think about ideas for a bit. The only reason I wrote hookdown was to better support hooks and indexes when working with sub level downs at the time.

Areas for simplification should definitely be welcomed (eg if only pre or post hooks can do the job of both, why have the other). Simplifying the order hooks run in (series) is also probably a better api for community plugins in order to clarify behavior when various hooks are combined in unanticipated ways. You can always combine actions to run in parallel in a single hook. I believe this option was added mainly because I wasn't sure what the best approach was at the time.

So +1 to those changes if the functionality can be built in.

[vweevers]

Further reducing the scope of this feature, to make it easier to land something:

1. The primary use case to focus on atm is atomically updating an index (that being a sublevel).
2. Hooks will be built-in to abstract-level and run before encoding and prefixing.
3. Hooks will be synchronous (removing the "should support promises" requirement above)
4. Hooks should be stateless, or to be more precise, not be affected by whether the db is open(ing)
5. Hooks are just functions, without any options.
6. Performance of hooks is not important, except for not impacting performance when 0 hooks are present.
7. We'll start with only a "prewrite" hook, that runs on put(), del() and batch(). But not clear() for now, because that will require deeper integration (especially on abstract-level implementations that clear natively meaning the deleted keys are not exposed to JS).
8. The API will be marked as experimental (meaning subject to change without notice).
9. Don't call hooks on operations that were themselves added by hooks (regardless of which hook).

[bcomnes]

Sounds good

[vweevers]

After implementing an initial version, I realized that I reduced the scope too much. It would box us into a corner by adding hooks in the wrong place - serving one use case but being unable to extend it later without breaking changes. That's vague, but long story short I'm gonna take another use case into account: modifying operations. This has to consider encodings, options, events, sublevels and validation, so as a result the hook will be more future-proof.

As for performance, specifically on db.batch([]), I may have found a way to even increase performance when no hooks are present, by internally using a class instead of a plain object for operations. That's faster to create and clone, compared to Object.assign({}, operation). Will have to find a place for userland options though (the foo in { type, key, foo: 2 }).

Such a class might also give hooks and events access to lazily encoded keys/values, roughly like so:

Click to expand

const kKey = Symbol('key')
const kEncodedKey = Symbol('encodedKey')
const kNone = Symbol('none')

// Skipping values for brevity
class Operation {
  constructor (type, key, keyEncoding) {
    this.type = type
    this.key = key
    this.keyEncoding = keyEncoding
  }

  get key () {
    return this[kKey]
  }

  set key (key) {
    if (key === null || key === undefined) {
      // throw
    }

    this[kKey] = key
    this[kEncodedKey] = kNone
  }

  get encodedKey () {
    let encodedKey = this[kEncodedKey]

    if (encodedKey === kNone) {
      encodedKey = this[kEncodedKey] = this.keyEncoding.encode(this[kKey])
    }

    return encodedKey
  }
}

Plus, in scenarios where a db is wrapped (like sublevels) we can possibly skip work if we see that op instanceof Operation, meaning we already validated and normalized the operation.

Edit: I'm not gonna do that, hurts userland options and modularity.

[vweevers]

Level/abstract-level#45