E5E7 Handle middleware passed after the service to app.use by dbkaplun · Pull Request #178 · feathersjs/feathers · GitHub
[go: up one dir, main page]
Skip to content

Handle middleware passed after the service to app.use #178

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.

Sign up for GitHub

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

Merged
daffl merged 3 commits into from
Dec 2, 2015
Merged

Handle middleware passed after the service to app.use #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a44dbdc
Handle middleware passed after the service to app.use
Dec 1, 2015
0abcd45
Fix post-service middleware to execute after service
Dec 1, 2015
387d62a
Adds test for post-service middleware
Dec 1, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions lib/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,22 @@ module.exports = {
return protoService;
},

use: function () {
var args = _.toArray(arguments);
var location = args.shift();
var service = args.pop();
use: function (location) {
var service, middleware = _(arguments)
.slice(1)
.reduce(function (middleware, arg) {
if (typeof arg === 'function') {
middleware[service ? 'after' : 'before'].push(arg);
} else if (!service) {
service = arg;
} else {
throw new Error('invalid arg passed to app.use');
}
return middlewar 10BC0 e;
}, {
before: [],
after: []
});
var hasMethod = function(methods) {
return _.some(methods, function(name) {
return (service && typeof service[name] === 'function');
Expand All @@ -73,7 +85,7 @@ module.exports = {

this.service(location, service, {
// Any arguments left over are other middleware that we want to pass to the providers
middleware: args
middleware: middleware
});

return this;
Expand Down
17 changes: 10 additions & 7 deletions lib/providers/rest/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,29 @@ module.exports = function (config) {
return;
}

var middleware = (options && options.middleware) || [];
var middleware = (options || {}).middleware || {};
var before = middleware.before || [];
var after = middleware.after || [];

var uri = path.indexOf('/') === 0 ? path : '/' + path;
var baseRoute = app.route(uri);
var idRoute = app.route(uri + '/:id');

debug('Adding REST provider for service `' + path + '` at base route `' + uri + '`');

// GET / -> service.find(cb, params)
baseRoute.get.apply(baseRoute, middleware.concat(app.rest.find(service), handler));
baseRoute.get.apply(baseRoute, before.concat(app.rest.find(service), after, handler));
// POST -> service.create(data, params, cb)
baseRoute.post.apply(baseRoute, middleware.concat(app.rest.create(service), handler));
baseRoute.post.apply(baseRoute, before.concat(app.rest.create(service), after, handler));

// GET /:id -> service.get(id, params, cb)
idRoute.get.apply(idRoute, middleware.concat(app.rest.get(service), handler));
idRoute.get.apply(idRoute, before.concat(app.rest.get(service), after, handler));
// PUT /:id -> service.update(id, data, params, cb)
idRoute.put.apply(idRoute, middleware.concat(app.rest.update(service), handler));
idRoute.put.apply(idRoute, before.concat(app.rest.update(service), after, handler));
// PATCH /:id -> service.patch(id, data, params, callback)
idRoute.patch.apply(idRoute, middleware.concat(app.rest.patch(service), handler));
idRoute.patch.apply(idRoute, before.concat(app.rest.patch(service), after, handler));
// DELETE /:id -> service.remove(id, params, cb)
idRoute.delete.apply(idRoute, middleware.concat(app.rest.remove(service), handler));
idRoute.delete.apply(idRoute, before.concat(app.rest.remove(service), after, handler));
});
};
};
14 changes: 9 additions & 5 deletions test/application.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,29 +120,33 @@ describe('Feathers application', function () {
callback(null, {
id: name,
description: 'You have to do ' + name + '!',
stuff: params.stuff
preService: params.preService
});
}
};

var app = feathers()
.configure(feathers.rest())
.use('/todo', function (req, res, next) {
req.feathers.stuff = 'custom middleware';
req.feathers.preService = 'pre-service middleware';
next();
}, todoService)
}, todoService, function (req, res, next) {
res.set('post-service', res.data.id);
Copy link
Member
@daffl daffl Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could set and then check for res.data.postService instead since the handler should run last.

Copy link
Author
@dbkaplun dbkaplun Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I don't understand, can you rephrase please?

Copy link
Member
@daffl daffl Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is to instead set something like:

res.data.postService = true;

and then in the test check for

assert.ok(data.postService, 'Post-service middleware updated response');

instead of assert.equal(response.headers['post-service'], 'dishes', 'Post-service middleware updated response');.

Copy link
Author
@dbkaplun dbkaplun Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I did it this way because I wanted to ensure that the post-service handler actually runs after the service, but I can change it if you like.

Copy link
Member
@daffl daffl Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Lets leave it then. I will merge shortly. Thanks again!

next();
})
.use('/otherTodo', todoService);

var server = app.listen(6995).on('listening', function () {
request('http://localhost:6995/todo/dishes', function (error, response, body) {
assert.ok(response.statusCode === 200, 'Got OK status code');
var data = JSON.parse(body);
assert.equal(data.stuff, 'custom middleware', 'Custom middleware updated params');
assert.equal(data.preService, 'pre-service middleware', 'Pre-service middleware updated response');
assert.equal(response.headers['post-service'], 'dishes', 'Post-service middleware updated response');

request('http://localhost:6995/otherTodo/dishes', function (error, response, body) {
assert.ok(response.statusCode === 200, 'Got OK status code');
var data = JSON.parse(body);
assert.ok(!data.stuff, 'Custom middleware not run for different service.');
assert.ok(!data.preService && !response.headers['post-service'], 'Custom middleware not run for different service.');
server.close(done);
});
});
Expand Down
0