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

add user middleware chain function #1913

Merged
merged 7 commits into from Jun 18, 2022
Merged

add user middleware chain function #1913

merged 7 commits into from Jun 18, 2022

Conversation

magickeha
Copy link
Contributor

According to different business scenarios, the choice of middleware chain should be user-selectable, and considering performance issues, invalid middleware should be eliminated

@kevwan
Copy link
Contributor

kevwan commented Jun 1, 2022

I think it over and over, and don't think let users pass a chain is not easy to understand.

Need more easy-to-understand solution for this.

@kevwan kevwan self-requested a review June 1, 2022 16:03
@magickeha
Copy link
Contributor Author

I think it over and over, and don't think let users pass a chain is not easy to understand.

Need more easy-to-understand solution for this.

On the basis of not affecting the functions of the existing framework, it may be a good option to allow users to customize some functions in the middleware processing.
Here is my reason:

  1. Users can adjust the order of middleware based on their own business scenarios
  2. You can or add your own middleware in
  3. Remove the middleware that is not used by itself to avoid invalid judgment
  4. When the user does not need custom middleware, the default middleware can be used

@magickeha
Copy link
Contributor Author

I think it over and over, and don't think let users pass a chain is not easy to understand.

Need more easy-to-understand solution for this.

If the implementation is not elegant enough, please provide some suggestions, thanks

@kevwan
Copy link
Contributor

kevwan commented Jun 2, 2022

I'm thinking of the following method:

svr.WithMiddlewares(middlewares ...Middleware)

And default middlewares are disabled.

@kunyu
Copy link
Contributor

kunyu commented Jun 6, 2022

I'm thinking of the following method:

svr.WithMiddlewares(middlewares ...Middleware)

And default middlewares are disabled.

I don't think this way can customize middleware order

@magickeha
Copy link
Contributor Author

The calling order of the middleware before entering the route is relatively fixed. I think it is also a good choice to determine the execution order of the middleware through the order of the variable parameters passed in.

I'm thinking of the following method:

svr.WithMiddlewares(middlewares ...Middleware)

And default middlewares are disabled.

I don't think this way can customize middleware order

The calling order of the middleware before entering the route is relatively fixed. I think it is also a good choice to determine the execution order of the middleware through the order of the variable parameters passed in.

Copy link
Contributor

@kevwan kevwan left a comment

Choose a reason for hiding this comment

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

LGTM

@kevwan kevwan merged commit 6976ba7 into zeromicro:master Jun 18, 2022
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

Successfully merging this pull request may close these issues.

None yet

3 participants