-
Notifications
You must be signed in to change notification settings - Fork 450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Define server plugin interface, move Agent queries code into a built-in plugin #353
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good change!
var errMessage = this._checkRequest(request); | ||
if (errMessage) return callback(new ShareDBError(ERROR_CODE.ERR_MESSAGE_BADLY_FORMED, errMessage)); | ||
if (plugin) { | ||
try { | ||
plugin.checkRequest(request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this method should be optional? Or do we want to enforce that all plugins validate requests?
}; | ||
} | ||
|
||
QueryServerPlugin.prototype.close = function(callback) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also be optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From Alec:
- Consider inverting so that plugins store a map of per-agent states, instead of storing state on the agent.
I'm going to merge this and other plugin PRs into a feature branch, in case we want to publish small bugfixes while this is under development.
We want to develop support for ShareDB plugins that can add support for new actions on top of ShareDB's functionality - #337
This is one part of that work, adding the ability for the server
Agent#_handleMessage
to call message handling functions in registered plugins.As a feasibility check, this PR moves the core query code from the server Agent class into a built-in server plugin, QueryServerPlugin, with no functionality changes. There are also no changes to client code in this PR.
Things that I would like to add: