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

Execute middlewares before executing endpoint's method #20

Closed
toksdotdev opened this issue Jul 9, 2018 · 16 comments
Closed

Execute middlewares before executing endpoint's method #20

toksdotdev opened this issue Jul 9, 2018 · 16 comments

Comments

@toksdotdev
Copy link
Contributor

I feel it would be awesome if we could execute a middleware before entering a specific endpoint method in a controller. I know there are work around this, but this should be easily available.

e.g. something of this fashion:

app.post('/user', [ validate_some_stuffs(...) ], (req, res) => {  // validation middle-ware executes first

});
@aichbauer
Copy link
Owner

That would be a awesome feature 👍

Any idea on how we could structure these middleware? I am open for ideas 😀

@toksdotdev
Copy link
Contributor Author

Having thought about this, I feel for backward compatibility, we could check:

  • If the value of 'GET /user:id' which is ('UserController.get') is of type String, take the value as the controller and method
    i.e. 'GET /user:id': 'UserController.get'

  • Else use the new method we want to adopt below:

Note each middleware would receive both the request, response object and optionally a next callback

const routes = {
  'POST /user': 'UserController.create', // this old style still supported

  'GET /user:id': {
    path: 'UserController.get',
    middlewares: [
      // An array of middle wares could be specified 
      // without the open & closing brackets i.e. ()

      // e.g. 
      // 
      // checkIfAuthenticated   ----> correct
      // checkIfAuthenticated() ----> wrong   

      // The later is wrong because the middle-ware would be triggered
      // on `const routes` initialization, instead of before the controller
      // method execution and would prevent the middleware from accessing the 
      // `request` and `response` params that could be passed.
      // 
    ]
  },
};

export default routes;

Middle-wares would be executed in descending order for simplicity.

Final example

const routes = {
  'GET /user:id': {
    path: 'UserController.get',
    middlewares: [
         checkIfAutheticated,
         verifyFacebookAuth,  // this would come in handy for passport authentication
    ],
    
  'POST /user': 'UserController.create'
  },
};

Please feel free to correct me were I might be wrong...

@toksdotdev
Copy link
Contributor Author

This middleware execution could be executed right before this code clock below is called, with some slight modifications.

if (requestMethod === 'get') {
router.route(myPath).get(contr[controllerMethod]);
} else if (requestMethod === 'post') {
router.route(myPath).post(contr[controllerMethod]);
} else if (requestMethod === 'put') {
router.route(myPath).put(contr[controllerMethod]);
} else if (requestMethod === 'delete') {
router.route(myPath).delete(contr[controllerMethod]);
}

I could send in a PR if you deem this feature fit...

@aichbauer
Copy link
Owner

Hey @tnkemdilim,

this sounds great. Send a PR and we can see if there is something we can improve...

@toksdotdev
Copy link
Contributor Author

Gotcha!!!

@toksdotdev
Copy link
Contributor Author

Sorry, should send in a PR shortly, as I've been quite busy.

@aichbauer
Copy link
Owner

@tnkemdilim is there any update on this issue? I really like the idea of executing middleware before a specific route.

@joneldiablo
Copy link

Hi! I have this code:

app.use('/public', mapRoutes(config.publicRoutes, './controllers/'));
app.use('/private', config.verifyToken, mapRoutes(config.privateRoutes, './controllers/'));

but I can't get access to req.params in config.verifyToken function, the params not exist before mapRoutes, any idea how I can evaluate path params like /users/:ID?

@toksdotdev
Copy link
Contributor Author

toksdotdev commented Oct 25, 2018

I'm currently working on a PR for this; I'll see if I can wrap up, and submit for review immediately.

Sorry for the delay @aichbauer & @joneldiablo

toksdotdev added a commit to toksdotdev-forks/express-routes-mapper that referenced this issue Oct 26, 2018
@toksdotdev
Copy link
Contributor Author

I just added the feature; it's awaiting review. @aichbauer and @joneldiablo

@aichbauer
Copy link
Owner

@tnkemdilim I will review it, during this week or next week. Thank you for your contribution 👍

@joneldiablo
Copy link

hi @tnkemdilim @aichbauer yesterday I did a pull request to the TNkemdilim fork, to use a middleware for all routes

@alkanyunus
Copy link

@joneldiablo so we can't use it yet :/ waiting...

@joneldiablo
Copy link

joneldiablo commented Nov 13, 2018

@saxahan you can use the @tnkemdilim fork XD or wait for @aichbauer accept the pr
XD

@toksdotdev
Copy link
Contributor Author

Just made necessary updates to the PR...

@aichbauer
Copy link
Owner

I'll review it tomorrow. if everything is okay i'll release a new version tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants