Closed olivier-rey closed 4 years ago
VincentLanglet
commented
4 years ago Diff here https://github.com/sonata-project/SonataAdminBundle/compare/3.49.1...3.50.0
Commit https://github.com/sonata-project/SonataAdminBundle/commit/b0350529ef428108632ffa342f998b7243064ff8
Was it a BC-break @phansys ?
Maybe related to https://github.com/sonata-project/SonataAdminBundle/pull/5696
phansys
commented
4 years ago Was it a BC-break @phansys ?
Yes. See https://github.com/sonata-project/SonataAdminBundle/pull/5579#discussion_r291804779.
VincentLanglet
commented
4 years ago Seems like there is another.
$this->getContainer()->has($manipulatorId)was returning something, but
public function __construct(Pool $pool, array $aclObjectManipulators, RegistryInterface $registry = null)is certainly injecting an empty array in @olivier-rey situation.
This seems to be injected by the ObjectAclManipulatorCompilerPass.
How are declared your objectAclManipulators @olivier-rey ?
olivier-rey
commented
4 years ago @VincentLanglet Yes empty array on $aclObjectManipulators...
by checking objectAclManipulatorCompilerPass, i get an empty $availableManagers array as well.
the condition is_subclass_of is always false:
For $id "sonata.admin.manipulator.acl.object.orm"
$container->getDefinition($id)->getClass() returns "%sonata.admin.manipulator.acl.object.orm.class%"
The command works if i remove the !is_subclass_of part of the condition.
phansys
commented
4 years ago Could you please check with something like this?
$class = $container->getDefinition($id)->getClass();
if (!class_exists($class, false) && $container->hasParameter($class)) {
$class = $container->getParameter($class);
}
if (0 !== strpos($id, 'sonata.admin.manipulator.acl.object.') || !is_subclass_of($class, ObjectAclManipulatorInterface::class)) {
continue;
}the condition is_subclass_of is always false: For $id "sonata.admin.manipulator.acl.object.orm" $container->getDefinition($id)->getClass() returns "%sonata.admin.manipulator.acl.object.orm.class%"
That's weird because the call to is_subclass() should be never executed if the service id is "sonata.admin.manipulator.acl.object.orm", since the condition 0 !== strpos($id, 'sonata.admin.manipulator.acl.object.') should be met first.
EDIT: Nevermind, I just realized that these conditions are around an OR operator.
olivier-rey
commented
4 years ago EDIT: Nevermind, I just realized that these conditions are around an
ORoperator.
You made me confused with your last comment, and i didn't see your EDIT message immediatly ;)
Declaring the $class first does't work as there is some null getDefinition().
The problem seems to be caused by the containerBuilder returning the parameter (%sonata.admin.manipulator.acl.object.orm.class%) and not the parameter value for some services...
1 - If i modify objectAclManipulatorCompilerPass with:
foreach ($container->getDefinitions() as $definition) {
dump($definition->getClass());
}
die;bin/console ca:cl |grep manip
returns
"%sonata.admin.manipulator.acl.object.orm.class%"
"%sonata.admin.manipulator.acl.admin.class%"
"%sonata.admin.object.manipulator.acl.admin.class%"I then compared with the debug:container command (with same kind of modification), and it's ok:
bin/console debug:container |grep -i manip
returns
"Sonata\DoctrineORMAdminBundle\Util\ObjectAclManipulator"
"Sonata\AdminBundle\Util\AdminAclManipulator"
"Sonata\AdminBundle\Util\AdminObjectAclManipulator"2 - If i modify sonata-project/doctrine-orm-admin-bundle/src/Resources/config/security.xml
<service id="sonata.admin.manipulator.acl.object.orm" class="%sonata.admin.manipulator.acl.object.orm.class%" public="true"/>
for
<service id="sonata.admin.manipulator.acl.object.orm" class="Sonata\DoctrineORMAdminBundle\Util\ObjectAclManipulator" public="true"/>
The Acl command works fine.
phansys
commented
4 years ago We can't modify the definition for "sonata.admin.manipulator.acl.object.orm" since it would be a BC break (even if currently using a class parameter as extension point is discouraged). Please, check with this snippet and let me know the result.
if (0 !== strpos($id, 'sonata.admin.manipulator.acl.object.') || null === $class = $container->getDefinition($id)->getClass()) {
continue;
}
if (!class_exists($class, false) && $container->hasParameter($class) {
$class = $container->getParameter($class);
}
if (!is_subclass_of($class, ObjectAclManipulatorInterface::class)) {
continue;
}We can't modify the definition for "sonata.admin.manipulator.acl.object.orm" since it would be a BC break (even if currently using a class parameter as extension point is discouraged).
Yep sure...
Well this works:
if (0 !== strpos($id, 'sonata.admin.manipulator.acl.object.') || null === $class = $container->getDefinition($id)->getClass()) {
continue;
}
$class = str_replace("%","",$class);
if (!class_exists($class, false) && $container->hasParameter($class)) {
$class = $container->getParameter($class);
}
if (!is_subclass_of($class, ObjectAclManipulatorInterface::class)) {
continue;
}Excellent @olivier-rey.
I'd replace the call to str_replace() with trim():
if (0 !== strpos($id, 'sonata.admin.manipulator.acl.object.') || null === $class = $container->getDefinition($id)->getClass()) {
continue;
}
// We trim the possible "%" characters around the class definition since it could be using "%parameter%" syntax.
$class = trim($class, '%');
if (!class_exists($class, false) && $container->hasParameter($class)) {
$class = $container->getParameter($class);
}
if (!is_subclass_of($class, ObjectAclManipulatorInterface::class)) {
continue;
}Could you please submit a PR with the fix?
olivier-rey
commented
4 years ago This is my first PR on github... Mistake with #6075... Hope #6077 is fine :)
I can't generate acl from command anymore with new sonata-admin versions (~3.5)
with version 3.50 i get bunch of:
with version 3.57:
Works well with:
Sonata packages
Other Sonata packages
Symfony packages
PHP version
Thanks in advance for your help :)