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

AuthorizationsController: Memoize strategy.authorize result #1072

Merged

Conversation

mdeutsch
Copy link

@mdeutsch mdeutsch commented Apr 4, 2018

This allows easy access to the authorize response in a subclass of Doorkeeper::AuthorizationsController. It's equivalent to #568, which made the same change to TokensController.

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/PredicateName has the wrong namespace - should be Naming
.rubocop.yml: Style/AccessorMethodName has the wrong namespace - should be Naming
Error: The `Style/TrailingCommaInLiteral` cop no longer exists. Please use `Style/TrailingCommaInArrayLiteral` and/or `Style/TrailingCommaInHashLiteral` instead.
(obsolete configuration found in .rubocop.yml, please update it)
obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

@mdeutsch mdeutsch force-pushed the memoize_authorization_response branch from c8e04b8 to fd373d2 Compare April 4, 2018 15:08
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/FileName has the wrong namespace - should be Naming
.rubocop.yml: Style/PredicateName has the wrong namespace - should be Naming
.rubocop.yml: Style/AccessorMethodName has the wrong namespace - should be Naming
Error: The `Style/TrailingCommaInLiteral` cop no longer exists. Please use `Style/TrailingCommaInArrayLiteral` and/or `Style/TrailingCommaInHashLiteral` instead.
(obsolete configuration found in .rubocop.yml, please update it)
obsolete parameter MaxLineLength (for Style/IfUnlessModifier) found in .rubocop.yml
`Style/IfUnlessModifier: MaxLineLength` has been removed. Use `Metrics/LineLength: Max` instead

@nbulaj
Copy link
Member

nbulaj commented Apr 4, 2018

Could you please add an changelog entry to the NEWS.md file? Thanks!

@nbulaj
Copy link
Member

nbulaj commented Apr 8, 2018

Could you please resolve the conflicts?

Because strategy.authorize is not idempotent, we need to memoize the
result if we need access to the response after the original call.
This comes in really handy when subclassing Doorkeeper::AuthorizationsController.

See also doorkeeper-gem#568,
which made a similar change to TokensController.
@mdeutsch mdeutsch force-pushed the memoize_authorization_response branch from 4bd7c59 to 4662c9c Compare April 9, 2018 13:15
@mdeutsch
Copy link
Author

mdeutsch commented Apr 9, 2018

I rebased onto master and resolved the conflict.

@nbulaj nbulaj merged commit cd1f02c into doorkeeper-gem:master Apr 9, 2018
@mdeutsch mdeutsch deleted the memoize_authorization_response branch April 10, 2018 13:08
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