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

Shorthand for disabling service methods #106

Closed
ekryski opened this issue Feb 4, 2015 · 12 comments · Fixed by feathersjs-ecosystem/feathers-hooks#33
Closed

Shorthand for disabling service methods #106

ekryski opened this issue Feb 4, 2015 · 12 comments · Fixed by feathersjs-ecosystem/feathers-hooks#33
Labels
Milestone

Comments

@ekryski
Copy link
Contributor

ekryski commented Feb 4, 2015

I think it would be nice to be able to disable or specify that you only want certain default service methods to be enabled. Especially when using an existing service plugin (ie. mongoose, mongo, etc).

This can be obtained by extending the original service and then overriding the service methods and returning an error, like so:

var service = Service.extend({
    create: function(data, params, callback){
      if (_.isFunction(params)) {
        callback = params;
        params = {};
      }

      callback(new app.errors.NotImplemented());
    }
});

However, I thought it would be really awesome to only enable certain service methods like this:

var app = feathers();
var service = mongodb({ collection: 'users' });
app.use('/users', service.only(['find', 'get']));
@ekryski ekryski added the Feature label Feb 4, 2015
@daffl
Copy link
Member

daffl commented Feb 4, 2015

I would think this is something the pre-build services or separate hooks should do. Probably be fairly simple, too:

var onlyMixin = {
  only: function(...names) {
    var mixin = {};
    for(let name of names) {
      mixin[name] = function() {
        var callback = arguments[arguments.length - 1];
        callback(new errors.NotImplemented('Not allowed'));
      }
    }
    return this.extend(mixin);
  }
}

@ekryski
Copy link
Contributor Author

ekryski commented Feb 4, 2015

Ya it's definitely the individual service's concern but I'd like to enforce that as a required method (ie. we'll need to document it for writing service plugins). I can go through and add the mixin to the services (since it is my feature request).

You're right that it doesn't belong in feathers core. I just wanted to bring it up in this repo so that we could discuss it in a more central place.

@marshallswain
Copy link
Member

I think this needs to be in 1.1.0. It goes along great with what we were talking about in #108. I don't see why it shouldn't be in Feathers core.

@ekryski
Copy link
Contributor Author

ekryski commented Feb 12, 2015

👍

@daffl daffl added this to the 1.1.0 milestone Feb 12, 2015
@daffl
Copy link
Member

daffl commented Feb 12, 2015

I'm still a little hesitant about this but I can see the point and adding it as a plugin would be total overkill and somewhat pointless. I added it to 1.1.0 but suggest to go the other way around and disable methods (instead of telling it which ones to allow):

var service = mongodb({ collection: 'users' });
app.use('/users', service);

app.service('users').disable('find', 'update');

In fact, we could even add the external flag mentioned in the FAQ

app.use(function(req, res, next) {
  req.feathers.external = 'rest';
  next();
});

app.configure(feathers.socketio(function(io) {
  io.use(function(socket, next) {
    // For websockets the feathers object does not exist by default
    if(!socket.feathers) {
      socket.feathers = {};
    }

    socket.feathers.external = 'socketio';
    next();
  });
}));

By default and then use .disable to also conditionally disable methods:

app.service('users').disable('find', function(params, callback) {
   if(params.external === 'rest') {
    return callback(null, false);
  }
  return callback();
});

@marshallswain
Copy link
Member

Sounds good. Adding the external flag by default will be quite convenient. 👍

@ekryski
Copy link
Contributor Author

ekryski commented Feb 13, 2015

I agree. Both sound great!

@daffl daffl modified the milestones: 1.2.0, 1.1.0 Jun 22, 2015
@daffl
Copy link
Member

daffl commented Jun 22, 2015

I'm going to bump this to 1.2 (or 2.0 whichever one comes first) unless it is a must have feature for you guys for 1.1. If not I think we're feature complete and can focus on the website and the guides.

@ekryski
Copy link
Contributor Author

ekryski commented Jan 26, 2016

We ended up having a private discussion in Slack and it looks like the interface will actually just utilize hooks. So it would look something like this:

import { common as hooks } from 'feathers-hooks';

// disable socketio provider on all todo service methods
app.service('todos').before(hooks.disable('socketio'));
// or
app.service('todos').before({
  all: hooks.disable('socketio')
});

// disable for other random use cases
app.service('todos').before(hooks.disable(function(hook) {
  return true/false/promise
}));

// disable specific methods for specific transports
app.service('todos').before({
  patch: hooks.disable('socketio'),
  update: hooks.disable('socketio'),
  remove: hooks.disable('rest')
}));


// disable any external transport calls (ie. REST, socket, or any future one)
app.service('todos').before({
  remove: hooks.disable()
}));
// or
app.service('todos').before({
  remove: hooks.disable('external')
}));

// disable any call including internal calls in code for remove
app.service('todos').before({
  remove: hooks.disable('all')
}));

@mindaugasnakrosis
Copy link

mindaugasnakrosis commented Sep 10, 2018

How can hooks be disabled now?

import { common as hooks } from '@feathersjs/feathers';
......
get: [hooks.disable()],

does not seem to work.

@daffl
Copy link
Member

daffl commented Sep 10, 2018

To disable methods use the disallow hook from feathers-hooks-common.

@lock
Copy link

lock bot commented Feb 7, 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 7, 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.

4 participants