[FrameworkBundle][Workflow] better errors when security deps are missing #23638
Conversation
xabbuh
changed the title
| /** | ||
| * @author Christian Flothmann <christian.flothmann@sensiolabs.de> | ||
| */ | ||
| class GuardListenerPass implements CompilerPassInterface |
There was a problem hiding this comment.
The name should probably contain Workflow as this is really just the for workflow configuration.
There was a problem hiding this comment.
renamed to WorkflowGuardListenerPass
xabbuh
force-pushed
the
guard-listener-security-deps
branch
from
9bb2c28
to
f54c374
Compare
|
|
||
| public function testListenersAreNotRemovedIfAllDependenciesArePresent() | ||
| { | ||
| $this->container->setParameter('workflow.guard_listeners', ['foo.listener.guard', 'bar.listener.guard']); |
xabbuh
force-pushed
the
guard-listener-security-deps
branch
from
f54c374
to
8ec0db2
Compare
| */ | ||
| public function process(ContainerBuilder $container) | ||
| { | ||
| if (!$container->hasParameter('workflow.guard_listeners')) { |
There was a problem hiding this comment.
I don't see the place setting this parameter in FrameworkBundle. Are you sure your code works fine ?
xabbuh
force-pushed
the
guard-listener-security-deps
branch
from
8ec0db2
to
3b884b0
Compare
| return; | ||
| } | ||
|
|
||
| if (!$container->has('security.token_storage')) { |
There was a problem hiding this comment.
Could you remove the parameter workflow.has_guard_listeners? It's not need at runtime.
(I added the comment here because I guess you will do it a this line ;) )
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler; |
There was a problem hiding this comment.
I'm not sure it's need but now we add all Compiler pass in the associated component.
It's a forgetfulness or it's done on purpose? (I dont this it's necessary here but ...)
There was a problem hiding this comment.
IMO this is completely specifc to the FrameworkBundle. If you want to register the listener in the container in your own application, you will probably have better ways to check if all needed dependencies are present.
xabbuh
force-pushed
the
guard-listener-security-deps
branch
from
3b884b0
to
56ee4aa
Compare
|
👍 |
|
Thank you @xabbuh. |
…ps are missing (xabbuh) This PR was merged into the 3.3 branch. Discussion ---------- [FrameworkBundle][Workflow] better errors when security deps are missing | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23499 (comment) | License | MIT | Doc PR | All the referenced services must either be explicitly configured or the SecurityBundle must be installed with some security related config being enabled. Otherwise, you will end up with a not so useful error message. Commits ------- 56ee4aa better errors when security deps are missing
All the referenced services must either be explicitly configured or the SecurityBundle must be installed with some security related config being enabled. Otherwise, you will end up with a not so useful error message.