Skip to content
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

Socket.io events - advanced pub/sub - listen to specific services and not others #44

Closed
Glavin001 opened this issue Mar 26, 2014 · 10 comments · Fixed by #50
Closed

Socket.io events - advanced pub/sub - listen to specific services and not others #44

Glavin001 opened this issue Mar 26, 2014 · 10 comments · Fixed by #50
Labels
Milestone

Comments

@Glavin001
Copy link
Contributor

See https://github.com/feathersjs/feathers/blob/master/lib/providers/socketio.js#L29

Currently when there is an event it's broadcasted to all connected sockets. However, I would lie to utilize Socketio rooms to improve performance such that the client can subscribe and unsubscribe to rooms that are watch particular services or even updates to particular nodes with a given id.

I may in some cases want to join the room watching for changes to only the TODO service but not Users service (too many users? Or UI does not depend on such info).
Furthermore, I may want to watch for updates to a particular id of a given service.

I wanted to check for interest for making this a built in standard feature of feathers, or I will make a separate Socketio provider plugin.

@daffl
Copy link
Member

daffl commented Mar 26, 2014

This is something I've been thinking about for a while, too. SocketIO rooms might be a solution but I think ideally we'd want authorization on the service level so that it works for REST and SocketIO (and other possible providers). What we want is a mechanism to

  • Tell if we want to dispatch an event and to who
  • Before or after calling a service method, check if this is something we want to actually return

This is what I've been kicking around so far:

var TodoService = {
    dispatch: {
        removed: function(data, params, callback) {
            // e.g. restrict by data company id
            if(params.user.companyId !== data.companyId) {
                return callback(null, false);
            }

            callback(null, data);
        },

        updated: function(data, params, callback) {
            // restrict by data company id
            if(params.user.companyId !== data.companyId) {
                return callback(null, false);
            }

            // This will also allow to convert the data sent
            callback(null, { converted: data });
        },

        created: function(data, params, callback) {
            // TODO implement
        }
    },

    before: {
        find: function(params, callback) {
            if(!params.user) {
                return callback(new Error('You are not logged in'));
            }
        },

        get: function(id, params, callback) {
            // Pre-process the params passed to the actual service
            params.something = 'test';
            callback();
        }
    },

    after: {
        find: function(data, params, callback) {
            // Manually filter the find results
            var filtered = _.filter(data, function(current) {
                return current.companyId === params.user.companyId;
            });

            callback(null, filtered);
        },

        get: function(data, id, params, callback) {
            if(data.companyId !== params.user.companyId) {
                return callback(new Error('You are not authorized to access this information'));
            }
        }
    }
}

dispatch

Will be called when dispatching an event, e.g. via SocketIO by iterating through all open sockets. params will probably the handshake data in this case (where you'd store things like the authorized user). You can either call back with false (or null or undefined) which won't dispatch the event to that socket, with the original data, or even with something else.

before and after

Will be mixed into a service before and after the original service method(s) run. before hooks can e.g. already return an error when something requires a user object in the params. after hooks can check the data returned by the service against the params (for example restricting the things by the logged in users company).

I feel as if I wrote something like that down before but I can't find it. Either way, is that going in the right direction at all?

@Glavin001
Copy link
Contributor Author

is that going in the right direction at all?

Absolutely! That all sounds excellent. I really like how it is all in the service level.

I am using Feathersjs for a contest app and the deadline has just been announced to be this upcoming Monday. So I will be building the app over the weekend and knew I wanted to have this feature so I could show off the power of Featherjs and wow the judges with an event-based application.

Looks like the dispatch feature will solve the problem I have of wanting to emit events to only socket connections of whom are actively listening to that specific event. Although, to be more specific to my case, how would you suggest I emulate a Socket.io room listening to update event of service class with a specific id? Potentially I could change the params to include an array of subscribed "room" criteria for each connection?

The before and after service middleware will be be very useful for keeping the find and other such service functions to focus solely on retrieval and their respective goals, and separating the authentication and permission logic into the before and filtering into after. Great!
Will have to update feathers-mongoose-service with access to these awesome features later.


@daffl, do you have a plan / ETA for implementation of the above proposed features? I would be interested in developing out a Pull Request if you have not already started :). Thanks!

@daffl
Copy link
Member

daffl commented Mar 26, 2014

The dispatching mechanism should actually be fairly easy to implement by changing the section on https://github.com/feathersjs/feathers/blob/master/lib/providers/socketio.js#L26 to

_.each(services, function(service, path) {
  // If the service emits events that we want to listen to (Event mixin)
  if (typeof service.on === 'function' && service._serviceEvents) {
    _.each(service._serviceEvents, function(ev) {
      service.on(ev, function(data) {
        var dispatcher = service.dispatch && service.dispatch[ev];
        var eventName = path + ' ' + ev;

        if(dispatcher) {
            io.sockets.clients().forEach(function(socket) {
                dispatcher(data, socket.handshake, function(error, socketData) {
                    if(!error && socketData) {
                        socket.emit(eventName, socketData);
                    }
                });
            });
        } else {
            io.sockets.emit(eventName, data);
        }
      });
    });
  }
});

Since we have to change the core for this (before and after hooks will change how we initialize a service, too) it probably makes a core feature. This should probably come hand in hand with shared REST and SocketIO based authorization (which can be a plugin I think).

@Glavin001
Copy link
Contributor Author

This should probably come hand in hand with shared REST and SocketIO based authorization (which can be a plugin I think).

I see you have recently started: https://github.com/feathersjs/feathers-hooks 😃

If you make Issues, let me know with a tag and I'll try and help best I can!

The dispatching mechanism should actually be fairly easy to implement by changing the section on https://github.com/feathersjs/feathers/blob/master/lib/providers/socketio.js#L26 to

@daffl are you interested in submitting a Pull Request for that code you wrote above? I'd be interested in looking into that over the weekend as well. Don't want to step on any toes if you're already working on it.

@daffl
Copy link
Member

daffl commented Mar 29, 2014

So I think the before and after hooks can be a plugin (feathers-hooks), but the dispatching (removed, updated and created) has to be part of the core (since we'll have to change the code mentioned above).

I haven't done any work on the code above so feel free to branch of master, paste it in and give the dispatching functionality a shot if you have time for it.

@daffl
Copy link
Member

daffl commented Mar 29, 2014

I got the first working version for the before and after hooks up at feathers-hooks.

@daffl
Copy link
Member

daffl commented Mar 31, 2014

So with the hooks being a separate plugin, I'm actually wondering if the dispatching mechanism (since it has to be part of the core) should look more like this:

var TodoService = {
    remove: function(id, params, callback) {

    },

    update: function(id, data, params, callback) {

    },

    create: function(data, params, callback) {

    },

    removed: function(data, params, callback) {
        // e.g. restrict by data company id
        if(params.user.companyId !== data.companyId) {
            return callback(null, false);
        }

        callback(null, data);
    },

    updated: function(data, params, callback) {
        // restrict by data company id
        if(params.user.companyId !== data.companyId) {
            return callback(null, false);
        }

        // This will also allow to convert the data sent
        callback(null, { converted: data });
    },

    created: function(data, params, callback) {
        // TODO implement
    }
}

Not a huge difference but it might make more sense to move the removed, updated and created methods to the service top level.

@Glavin001
Copy link
Contributor Author

Not a huge difference but it might make more sense to move the removed, updated and created methods to the service top level.

I agree. As long as there are no naming conflicts (since Feathers .use is decorating Express's .use) then I think this is the way to go.

Sorry I didn't get a chance to work on this over the weekend, hopefully tonight :).

@daffl
Copy link
Member

daffl commented Apr 3, 2014

I pushed an initial implementation into the event-filtering branch.

Pull request #49 is also related (it passes the SocketIO handshake.feathers parameter as service parameters similar to what you can do with setting req.feathers in the middleware).

@daffl daffl closed this as completed in #50 Apr 8, 2014
daffl pushed a commit that referenced this issue Aug 23, 2018
Small grammatical change to line 59
daffl added a commit that referenced this issue Aug 25, 2018
* Update plugin generator to new plugin infrastructure

* Remove Node 4
daffl pushed a commit that referenced this issue Aug 28, 2018
* chore(package): update sinon to version 6.0.0

* Update Travis install script
daffl added a commit that referenced this issue Aug 28, 2018
daffl pushed a commit that referenced this issue Aug 29, 2018
daffl pushed a commit that referenced this issue Aug 29, 2018
* chore(package): update sinon to version 6.0.0

* Update Travis install script
daffl added a commit that referenced this issue Aug 29, 2018
daffl pushed a commit that referenced this issue Aug 29, 2018
@lock
Copy link

lock bot commented Feb 8, 2019

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue with a link to this issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants