Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Security] Lazy load guard authenticators and authentication providers#21450

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 2 commits intosymfony:masterfromchalasr:guard/iterator
Feb 16, 2017

Conversation

@chalasr
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a

Authentication stops on the first authenticator that fails or succeeds, let's instantiate them only if actually needed.

ogizanagi reacted with thumbs up emoji
@ogizanagi
Copy link
Contributor

ogizanagi commentedJan 29, 2017
edited
Loading

In case a logger is provided, the number of authenticators is provided in the context array (GuardAuthenticationListener.php#L69).
Usingcount on the generator won't provide the right value. I don't think this information is really useful though...

Do you think it'll be worth it to makeRewindableGenerator implement\Countable? (It'll still require an extra check anyway for such cases).

@nicolas-grekas
Copy link
Member

I think it's a good idea to makeRewindableGenerator implement Countable, since we always have the info, isn't it?

@ogizanagi
Copy link
Contributor

@chalasr ,@nicolas-grekas : See#21455

@chalasrchalasrforce-pushed theguard/iterator branch 2 times, most recently from062580c to6e4e1baCompareJanuary 30, 2017 13:20
useSymfony\Component\DependencyInjection\ChildDefinition;
useSymfony\Component\DependencyInjection\ContainerBuilder;
useSymfony\Component\DependencyInjection\Reference;
useSymfony\Component\DependencyInjection\Argument\IteratorArgument;

Choose a reason for hiding this comment

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

alpha order, should be first I guess

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

👍 with minor comment

@chalasr
Copy link
MemberAuthor

Uses order fixed (must be the last time, moved to a real IDE everywhere)

ogizanagi reacted with laugh emoji

@chalasr
Copy link
MemberAuthor

chalasr commentedJan 31, 2017
edited
Loading

Same done for authentication providers in last commitcd6422a. This service is injected into all listeners. WDYT? I can do it in another PR if you prefer.

@chalasrchalasr changed the title[Security][Guard] Lazy load authenticators[Security] Lazy load guard authenticators and authentication providersFeb 1, 2017
nicolas-grekas added a commit that referenced this pull requestFeb 2, 2017
…anagi)This PR was merged into the 3.3-dev branch.Discussion----------[DI] Allow to count on lazy collection arguments| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#21450 (comment)| License       | MIT| Doc PR        | todo (withsymfony/symfony-docs#7336)When using the new iterator feature of the DI component to lazy load collection, we always know the number of arguments in the collection (only the invalidBehavior set to `IGNORE_ON_INVALID_REFERENCE` may change this number). So we are able to generate and use a `RewindableGenerator` implementing `\Countable` by computing this value ahead.So, in a service accepting `array|iterable`, like in the `GuardAuthenticationListener` (see#21450):```phpclass GuardAuthenticationListener implements ListenerInterface{    private $guardAuthenticators;    /**       *@param iterable|GuardAuthenticatorInterface[]  $guardAuthenticators   The authenticators, with keys that match what's passed to GuardAuthenticationProvider       *@param LoggerInterface                         $logger                A LoggerInterface instance    */    public function __construct($guardAuthenticators, LoggerInterface $logger = null)    {          // ...    }    public function handle(GetResponseEvent $event)    {        if (null !== $this->logger) {            $context = array()            if (is_array($this->guardAuthenticators) || $this->guardAuthenticators instanceof \Countable) {                $context['authenticators'] = count($this->guardAuthenticators);            }            $this->logger->debug('Checking for guard authentication credentials.', $context);        }        // ...    }}```we still keep the ability to call count without loosing the lazy load benefits.Commits-------f23e460 [DI] Allow to count on lazy collection arguments
symfony-splitter pushed a commit to symfony/dependency-injection that referenced this pull requestFeb 2, 2017
…anagi)This PR was merged into the 3.3-dev branch.Discussion----------[DI] Allow to count on lazy collection arguments| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |symfony/symfony#21450 (comment)| License       | MIT| Doc PR        | todo (withsymfony/symfony-docs#7336)When using the new iterator feature of the DI component to lazy load collection, we always know the number of arguments in the collection (only the invalidBehavior set to `IGNORE_ON_INVALID_REFERENCE` may change this number). So we are able to generate and use a `RewindableGenerator` implementing `\Countable` by computing this value ahead.So, in a service accepting `array|iterable`, like in the `GuardAuthenticationListener` (see #21450):```phpclass GuardAuthenticationListener implements ListenerInterface{    private $guardAuthenticators;    /**       *@param iterable|GuardAuthenticatorInterface[]  $guardAuthenticators   The authenticators, with keys that match what's passed to GuardAuthenticationProvider       *@param LoggerInterface                         $logger                A LoggerInterface instance    */    public function __construct($guardAuthenticators, LoggerInterface $logger = null)    {          // ...    }    public function handle(GetResponseEvent $event)    {        if (null !== $this->logger) {            $context = array()            if (is_array($this->guardAuthenticators) || $this->guardAuthenticators instanceof \Countable) {                $context['authenticators'] = count($this->guardAuthenticators);            }            $this->logger->debug('Checking for guard authentication credentials.', $context);        }        // ...    }}```we still keep the ability to call count without loosing the lazy load benefits.Commits-------f23e460 [DI] Allow to count on lazy collection arguments
@chalasr
Copy link
MemberAuthor

@nicolas-grekas still ok for you?

@nicolas-grekas
Copy link
Member

Still OK

@fabpot
Copy link
Member

Thank you@chalasr.

@fabpotfabpot merged commitcd6422a intosymfony:masterFeb 16, 2017
fabpot added a commit that referenced this pull requestFeb 16, 2017
…cation providers (chalasr)This PR was squashed before being merged into the 3.3-dev branch (closes#21450).Discussion----------[Security] Lazy load guard authenticators and authentication providers| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aAuthentication stops on the first authenticator that fails or succeeds, let's instantiate them only if actually needed.Commits-------cd6422a [SecurityBundle] Lazy load authentication providersb8a23de [Security][Guard] Lazy load authenticators
@chalasrchalasr deleted the guard/iterator branchFebruary 16, 2017 14:38
@fabpotfabpot mentioned this pull requestMay 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

5 participants

@chalasr@ogizanagi@nicolas-grekas@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp