Skip to content

Helper method to access controller context in middleware #1918

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

Merged
merged 1 commit into from
Oct 22, 2019
Merged

Helper method to access controller context in middleware #1918

merged 1 commit into from
Oct 22, 2019

Conversation

NikolayRys
Copy link
Contributor

@NikolayRys NikolayRys commented Oct 16, 2019

This is a feature suggestion.

Recently, working with Grape and writing some rescue_from handlers, I've encountered a need to access the request params, headers, and some other stuff how it was in the original controller method.

It was possible, but there was no documented and elegant way to do it. And I needed to go through the Grape source code to figure out a way to reach it.

With this change, we can save effort and time for other clients by preventing the need to rediscover this. It is useful not only for the error handling, but also for every piece of the middleware, including formatters, and any custom middleware inherited from Grape::Middleware::Base.

This PR adds the actual helper method for accessing the context, tests, and documentation for it.

@NikolayRys NikolayRys changed the title Feature suggestion: a helper method to access controller context in middleware Helper method to access controller context in middleware Oct 16, 2019
@dblock
Copy link
Member

dblock commented Oct 16, 2019

This introduces yet another way to access the same thing, so I think I am against it. I think you want to be able to do params[:x] or request.params[:x] directly like inside any API. What do you think?

@NikolayRys
Copy link
Contributor Author

NikolayRys commented Oct 16, 2019

It is another way indeed, but:

I'm essentially suggesting to just make consistent what already exists. Now I'm thinking, it would better to put them in a some common module, what do you think?

@dblock
Copy link
Member

dblock commented Oct 16, 2019

Ok, I buy this. A module would be helpful, I'd merge that.

@NikolayRys
Copy link
Contributor Author

Awesome, I'll adjust the PR

@NikolayRys
Copy link
Contributor Author

As requested, moved it out to a common module.

A documented helper method to access the context of the original controller from any Grape middleware.
Copy link
Member

@myxoh myxoh left a comment

Choose a reason for hiding this comment

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

Thanks for the hard work

@dblock
Copy link
Member

dblock commented Oct 22, 2019

Perfect.

@dblock dblock merged commit cd6d7fd into ruby-grape:master Oct 22, 2019
@NikolayRys NikolayRys deleted the convenient_env_access branch October 22, 2019 20:26
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.

3 participants