Replace $container has check with hasParameter
#7769
Conversation
When extensions are being loaded one by one into tmpContainer inside `vendor/symfony/dependency-injection/Compiler/MergeExtensionConfigurationPass.php` `sonata_admin` configuration is loaded alone, and other extension services are not present. By replacing `has` check with `hasParameter` check we can make sure parameter exists instead of service. Since we can easily configure extra parameter, but to get service we would need to load most of the acl bundle extension services.
| @@ -144,7 +144,7 @@ public function load(array $configs, ContainerBuilder $container): void | |||
|
|
|||
| break; | |||
| case 'sonata.admin.security.handler.acl': | |||
| if (!$container->has('security.acl.provider')) { | |||
| if (!$container->hasParameter('security.acl.provider')) { | |||
There was a problem hiding this comment.
why don't we instead check for the presence of the bundle enabled?, something like:
| if (!$container->hasParameter('security.acl.provider')) { | |
| if (!isset($bundles['AclBundle'])) { |
There was a problem hiding this comment.
that actually might work better. I was checking bundles in compiler passes and extensions and it never came to my mind to simply replace this check with bundle check, although I wonder if that's the only bundle which can do ACL. I'll add it in front and I guess I can leave this as fallback since it should work with simple or statement.
There was a problem hiding this comment.
Keep in mind that the only integration we support is that bundle, We use some interfaces from there and dependencies also declared there. I wouldn't compplicate it any further, to me we should remove other checks and leave only the bundle one, wdyt @VincentLanglet ?
There was a problem hiding this comment.
I thought this might be the case. If support is only with symfony/acl-bundle, then bundle check and error message change should be enough - if it only checks if bundle exists, then the message should probably not mention security.acl.provider at all, just the part with required bundle.
Buratinas
force-pushed
the
replace-has-with-has-parameter-check
branch
from
c848555 to
ce87f98
Compare
We only need to fail fast if: - bundle does not exist This means if we use AclBundle we will succeed this check.
Buratinas
force-pushed
the
replace-has-with-has-parameter-check
branch
from
ce87f98 to
9c63ee9
Compare
|
Thank you @Buratinas |
When extensions are being loaded one by one into tmpContainer inside
vendor/symfony/dependency-injection/Compiler/MergeExtensionConfigurationPass.phpsonata_adminconfiguration is loaded alone, and other extension services are not present.By replacing
hascheck withhasParametercheck we can make sure parameter exists instead of service.Since we can easily configure extra parameter,
but to get service we would need to load most of the acl bundle extension services.
Subject
I am targeting this branch, because I'm working with this and it's the latest branch.
Closes #7768.
Changelog