kuzzleio / kuzzle

Open-source Back-end, self-hostable & ready to use - Real-time, storage, advanced search - Web, Apps, Mobile, IoT -
https://kuzzle.io
Apache License 2.0
1.43k stars 123 forks source link

plugins - pass plugins functions as 1st class citizens #834

Closed benoitvidis closed 5 years ago

benoitvidis commented 7 years ago

For the time being, hooks, pipes, controllers and authentication strategy methods must be declared as strings by plugins, which is both unnatural and ugly imo.

Proposal

1.x

Move to a transitional model where the existing declaration is supported but where plugins can also directly expose their methods, i.e.

class MyPlugin {
  constructor () {
    this.hooks = {
      'document:beforeCreate': this._onBeforeCreate.bind(this),
      'server:afterNow': [
        this._doSomethingWithNow.bind(this),
        this._doSomethingElse.bind(this)
      ]
    };

    this.controllers = {
      aController: {
        hello: request => 'Hello'
      }
    };

    this.strategies = {
      mine: {
        config: {},
        methods: {
          create: this._authCreate.bind(this),
          delete: this._authDelete.bind(this),
          // ....
        }
      }
    };

  }

  init (config, context) {

  }

  _onBeforeCreate (request) {
    // ...
  }

}

We should then document the previous method as deprecated in the documentation

v2

Remove declaration as string support.

scottinet commented 7 years ago

This may conflict with a new feature: dynamic authentication strategy additions.

To make that feature works on a Kuzzle cluster, we need to propagate the strategy properties across all nodes, in a serialized format. So strings are required there.

A way we could implement the change described in this issue is to keep the current behavior in strategy properties while calling the strategy.add plugin context accessor, allowing functions only in plugin static methods: hooks, pipes, strategies exposed at plugin initialization, and everything that is not a dynamic strategy addition.

ScreamZ commented 5 years ago

@scottinet @benoitvidis @Aschen

Any update expected about this ? :)

Except the cluster mode issue, I think this can be back-compatible with something like typeof variable === "string" ? call(variable) ? variable()

Aschen commented 5 years ago

@ScreamZ Done in https://github.com/kuzzleio/kuzzle/pull/1227