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

[SecurityBundle] Cache contexts per request in FirewallMap #20196

Merged

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Oct 10, 2016

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #19819 (comment)
License MIT
Doc PR n/a

From @HeahDude in #19819 (comment), I propose to store and retrieve Context objects per Request using SplObjectStorage.

At the moment, contexts are consumed by the Symfony\Components\Security\Http\Firewall class only, but they could be indirectly by end users if #19490 and/or #19819 come to be merged.

@@ -26,18 +26,24 @@ class FirewallMap implements FirewallMapInterface
{
protected $container;
protected $map;
protected $contexts;
Copy link
Member

Choose a reason for hiding this comment

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

Should be private

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@chalasr chalasr force-pushed the securitybundle/cache_contexts_per_request branch from 0593495 to ffacec1 Compare October 10, 2016 21:02
@fabpot
Copy link
Member

fabpot commented Oct 12, 2016

Thank you @chalasr.

@fabpot fabpot merged commit ffacec1 into symfony:master Oct 12, 2016
fabpot added a commit that referenced this pull request Oct 12, 2016
…ap (chalasr)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[SecurityBundle] Cache contexts per request in FirewallMap

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19819 (comment)
| License       | MIT
| Doc PR        | n/a

From @HeahDude in #19819 (comment), I propose to store and retrieve `Context` objects per `Request` using `SplObjectStorage`.

At the moment, contexts are consumed by the `Symfony\Components\Security\Http\Firewall` class only, but they could be indirectly by end users if #19490 and/or #19819 come to be merged.

Commits
-------

ffacec1 [SecurityBundle] Cache contexts per request in FirewallMap
@chalasr chalasr deleted the securitybundle/cache_contexts_per_request branch October 12, 2016 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants