From fe210328ff6e7ed7adba40389f9619758064b3b0 Mon Sep 17 00:00:00 2001 From: Evgeniy Sokolov Date: Thu, 10 Sep 2015 21:17:31 +0200 Subject: [PATCH 1/6] drop reference loop between EntryIntrrface and Acl --- Dbal/MutableAclProvider.php | 9 +++---- Domain/Acl.php | 8 +++---- Domain/AclEntry.php | 42 +++++++++++++++++++++++++++++++++ Domain/DoctrineAclCache.php | 30 ----------------------- Domain/Entry.php | 13 +--------- Domain/FieldEntry.php | 5 ++-- Model/EntryInterface.php | 7 ------ Tests/Domain/AclTest.php | 2 +- Tests/Domain/EntryTest.php | 15 ++---------- Tests/Domain/FieldEntryTest.php | 12 +--------- 10 files changed, 58 insertions(+), 85 deletions(-) create mode 100644 Domain/AclEntry.php diff --git a/Dbal/MutableAclProvider.php b/Dbal/MutableAclProvider.php index 9129e8b..51fa9c7 100644 --- a/Dbal/MutableAclProvider.php +++ b/Dbal/MutableAclProvider.php @@ -13,6 +13,7 @@ use Doctrine\Common\PropertyChangedListener; use Doctrine\DBAL\Connection; +use Symfony\Component\Security\Acl\Domain\AclEntry; use Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity; use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity; use Symfony\Component\Security\Acl\Exception\AclAlreadyExistsException; @@ -170,13 +171,13 @@ public function propertyChanged($sender, $propertyName, $oldValue, $newValue) throw new \InvalidArgumentException('$sender must be an instance of MutableAclInterface, or EntryInterface.'); } - if ($sender instanceof EntryInterface) { - if (null === $sender->getId()) { + if ($sender instanceof AclEntry) { + if (null === $sender->getEntry()->getId()) { return; } - $ace = $sender; - $sender = $ace->getAcl(); + $ace = $sender->getEntry(); + $sender = $sender->getAcl(); } else { $ace = null; } diff --git a/Domain/Acl.php b/Domain/Acl.php index f417c8f..a2e8e2b 100644 --- a/Domain/Acl.php +++ b/Domain/Acl.php @@ -485,7 +485,7 @@ private function insertAce($property, $index, $mask, SecurityIdentityInterface $ } } - $aces[$index] = new Entry(null, $this, $sid, $strategy, $mask, $granting, false, false); + $aces[$index] = new Entry(null, $sid, $strategy, $mask, $granting, false, false); $this->onPropertyChanged($property, $oldValue, $this->$property); } @@ -543,7 +543,7 @@ private function insertFieldAce($property, $index, $field, $mask, SecurityIdenti } } - $aces[$field][$index] = new FieldEntry(null, $this, $field, $sid, $strategy, $mask, $granting, false, false); + $aces[$field][$index] = new FieldEntry(null, $field, $sid, $strategy, $mask, $granting, false, false); $this->onPropertyChanged($property, $oldValue, $this->$property); } @@ -620,7 +620,7 @@ private function updateFieldAce($property, $index, $field, $mask, $strategy = nu throw new \InvalidArgumentException('$field cannot be empty.'); } - $aces = &$this->$property; + $aces = $this->$property; if (!isset($aces[$field][$index])) { throw new \OutOfBoundsException(sprintf('The index "%d" does not exist.', $index)); } @@ -661,7 +661,7 @@ private function onPropertyChanged($name, $oldValue, $newValue) private function onEntryPropertyChanged(EntryInterface $entry, $name, $oldValue, $newValue) { foreach ($this->listeners as $listener) { - $listener->propertyChanged($entry, $name, $oldValue, $newValue); + $listener->propertyChanged(new AclEntry($this, $entry), $name, $oldValue, $newValue); } } } diff --git a/Domain/AclEntry.php b/Domain/AclEntry.php new file mode 100644 index 0000000..4586570 --- /dev/null +++ b/Domain/AclEntry.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Acl\Domain; + +use Symfony\Component\Security\Acl\Model\AclInterface; +use Symfony\Component\Security\Acl\Model\EntryInterface; + +/** + * AclEntry + * + * @author Evgeniy Sokolov + */ +class AclEntry +{ + private $acl; + private $entry; + + public function __construct(AclInterface $acl, EntryInterface $entry) + { + $this->acl = $acl; + $this->entry = $entry; + } + + public function getAcl() + { + return $this->acl; + } + + public function getEntry() + { + return $this->entry; + } +} diff --git a/Domain/DoctrineAclCache.php b/Domain/DoctrineAclCache.php index 667a19e..3742d5b 100644 --- a/Domain/DoctrineAclCache.php +++ b/Domain/DoctrineAclCache.php @@ -169,36 +169,6 @@ private function unserializeAcl($serialized) $reflectionProperty->setValue($acl, $this->permissionGrantingStrategy); $reflectionProperty->setAccessible(false); - $aceAclProperty = new \ReflectionProperty('Symfony\Component\Security\Acl\Domain\Entry', 'acl'); - $aceAclProperty->setAccessible(true); - - foreach ($acl->getObjectAces() as $ace) { - $aceAclProperty->setValue($ace, $acl); - } - foreach ($acl->getClassAces() as $ace) { - $aceAclProperty->setValue($ace, $acl); - } - - $aceClassFieldProperty = new \ReflectionProperty($acl, 'classFieldAces'); - $aceClassFieldProperty->setAccessible(true); - foreach ($aceClassFieldProperty->getValue($acl) as $aces) { - foreach ($aces as $ace) { - $aceAclProperty->setValue($ace, $acl); - } - } - $aceClassFieldProperty->setAccessible(false); - - $aceObjectFieldProperty = new \ReflectionProperty($acl, 'objectFieldAces'); - $aceObjectFieldProperty->setAccessible(true); - foreach ($aceObjectFieldProperty->getValue($acl) as $aces) { - foreach ($aces as $ace) { - $aceAclProperty->setValue($ace, $acl); - } - } - $aceObjectFieldProperty->setAccessible(false); - - $aceAclProperty->setAccessible(false); - return $acl; } diff --git a/Domain/Entry.php b/Domain/Entry.php index 55c4b37..0707cfd 100644 --- a/Domain/Entry.php +++ b/Domain/Entry.php @@ -22,7 +22,6 @@ */ class Entry implements AuditableEntryInterface { - private $acl; private $mask; private $id; private $securityIdentity; @@ -35,7 +34,6 @@ class Entry implements AuditableEntryInterface * Constructor. * * @param int $id - * @param AclInterface $acl * @param SecurityIdentityInterface $sid * @param string $strategy * @param int $mask @@ -43,10 +41,9 @@ class Entry implements AuditableEntryInterface * @param bool $auditFailure * @param bool $auditSuccess */ - public function __construct($id, AclInterface $acl, SecurityIdentityInterface $sid, $strategy, $mask, $granting, $auditFailure, $auditSuccess) + public function __construct($id, SecurityIdentityInterface $sid, $strategy, $mask, $granting, $auditFailure, $auditSuccess) { $this->id = $id; - $this->acl = $acl; $this->securityIdentity = $sid; $this->strategy = $strategy; $this->mask = $mask; @@ -55,14 +52,6 @@ public function __construct($id, AclInterface $acl, SecurityIdentityInterface $s $this->auditSuccess = $auditSuccess; } - /** - * {@inheritdoc} - */ - public function getAcl() - { - return $this->acl; - } - /** * {@inheritdoc} */ diff --git a/Domain/FieldEntry.php b/Domain/FieldEntry.php index 86b1e5b..8049cb1 100644 --- a/Domain/FieldEntry.php +++ b/Domain/FieldEntry.php @@ -28,7 +28,6 @@ class FieldEntry extends Entry implements FieldEntryInterface * Constructor. * * @param int $id - * @param AclInterface $acl * @param string $field * @param SecurityIdentityInterface $sid * @param string $strategy @@ -37,9 +36,9 @@ class FieldEntry extends Entry implements FieldEntryInterface * @param bool $auditFailure * @param bool $auditSuccess */ - public function __construct($id, AclInterface $acl, $field, SecurityIdentityInterface $sid, $strategy, $mask, $granting, $auditFailure, $auditSuccess) + public function __construct($id, $field, SecurityIdentityInterface $sid, $strategy, $mask, $granting, $auditFailure, $auditSuccess) { - parent::__construct($id, $acl, $sid, $strategy, $mask, $granting, $auditFailure, $auditSuccess); + parent::__construct($id, $sid, $strategy, $mask, $granting, $auditFailure, $auditSuccess); $this->field = $field; } diff --git a/Model/EntryInterface.php b/Model/EntryInterface.php index 0b244b7..026a073 100644 --- a/Model/EntryInterface.php +++ b/Model/EntryInterface.php @@ -21,13 +21,6 @@ */ interface EntryInterface extends \Serializable { - /** - * The ACL this ACE is associated with. - * - * @return AclInterface - */ - public function getAcl(); - /** * The primary key of this ACE. * diff --git a/Tests/Domain/AclTest.php b/Tests/Domain/AclTest.php index 84b9ba9..e6acf22 100644 --- a/Tests/Domain/AclTest.php +++ b/Tests/Domain/AclTest.php @@ -491,7 +491,7 @@ protected function getListener($expectedChanges) $listener = $this->getMock('Doctrine\Common\PropertyChangedListener'); foreach ($expectedChanges as $index => $property) { if (in_array($property, $aceProperties)) { - $class = 'Symfony\Component\Security\Acl\Domain\Entry'; + $class = 'Symfony\Component\Security\Acl\Domain\AclEntry'; } else { $class = 'Symfony\Component\Security\Acl\Domain\Acl'; } diff --git a/Tests/Domain/EntryTest.php b/Tests/Domain/EntryTest.php index ab8e481..9f3e45e 100644 --- a/Tests/Domain/EntryTest.php +++ b/Tests/Domain/EntryTest.php @@ -17,10 +17,9 @@ class EntryTest extends \PHPUnit_Framework_TestCase { public function testConstructor() { - $ace = $this->getAce($acl = $this->getAcl(), $sid = $this->getSid()); + $ace = $this->getAce($sid = $this->getSid()); $this->assertEquals(123, $ace->getId()); - $this->assertSame($acl, $ace->getAcl()); $this->assertSame($sid, $ace->getSecurityIdentity()); $this->assertEquals('foostrat', $ace->getStrategy()); $this->assertEquals(123456, $ace->getMask()); @@ -76,7 +75,6 @@ public function testSerializeUnserialize() $serialized = serialize($ace); $uAce = unserialize($serialized); - $this->assertNull($uAce->getAcl()); $this->assertInstanceOf('Symfony\Component\Security\Acl\Model\SecurityIdentityInterface', $uAce->getSecurityIdentity()); $this->assertEquals($ace->getId(), $uAce->getId()); $this->assertEquals($ace->getMask(), $uAce->getMask()); @@ -86,18 +84,14 @@ public function testSerializeUnserialize() $this->assertEquals($ace->isAuditFailure(), $uAce->isAuditFailure()); } - protected function getAce($acl = null, $sid = null) + protected function getAce($sid = null) { - if (null === $acl) { - $acl = $this->getAcl(); - } if (null === $sid) { $sid = $this->getSid(); } return new Entry( 123, - $acl, $sid, 'foostrat', 123456, @@ -107,11 +101,6 @@ protected function getAce($acl = null, $sid = null) ); } - protected function getAcl() - { - return $this->getMock('Symfony\Component\Security\Acl\Model\AclInterface'); - } - protected function getSid() { return $this->getMock('Symfony\Component\Security\Acl\Model\SecurityIdentityInterface'); diff --git a/Tests/Domain/FieldEntryTest.php b/Tests/Domain/FieldEntryTest.php index 735e2e8..f005a71 100644 --- a/Tests/Domain/FieldEntryTest.php +++ b/Tests/Domain/FieldEntryTest.php @@ -29,7 +29,6 @@ public function testSerializeUnserialize() $serialized = serialize($ace); $uAce = unserialize($serialized); - $this->assertNull($uAce->getAcl()); $this->assertInstanceOf('Symfony\Component\Security\Acl\Model\SecurityIdentityInterface', $uAce->getSecurityIdentity()); $this->assertEquals($ace->getId(), $uAce->getId()); $this->assertEquals($ace->getField(), $uAce->getField()); @@ -40,18 +39,14 @@ public function testSerializeUnserialize() $this->assertEquals($ace->isAuditFailure(), $uAce->isAuditFailure()); } - protected function getAce($acl = null, $sid = null) + protected function getAce($sid = null) { - if (null === $acl) { - $acl = $this->getAcl(); - } if (null === $sid) { $sid = $this->getSid(); } return new FieldEntry( 123, - $acl, 'foo', $sid, 'foostrat', @@ -62,11 +57,6 @@ protected function getAce($acl = null, $sid = null) ); } - protected function getAcl() - { - return $this->getMock('Symfony\Component\Security\Acl\Model\AclInterface'); - } - protected function getSid() { return $this->getMock('Symfony\Component\Security\Acl\Model\SecurityIdentityInterface'); From 5d30b6244dbbefda0c581aa86e7da6c2957b89c0 Mon Sep 17 00:00:00 2001 From: Evgeniy Sokolov Date: Thu, 10 Sep 2015 21:32:24 +0200 Subject: [PATCH 2/6] release memory for AclProvider --- Dbal/AclProvider.php | 9 +++++++++ Dbal/MutableAclProvider.php | 9 +++++++++ Model/AclProviderInterface.php | 7 +++++++ 3 files changed, 25 insertions(+) diff --git a/Dbal/AclProvider.php b/Dbal/AclProvider.php index 989e40b..14e3902 100644 --- a/Dbal/AclProvider.php +++ b/Dbal/AclProvider.php @@ -72,6 +72,15 @@ public function __construct(Connection $connection, PermissionGrantingStrategyIn $this->permissionGrantingStrategy = $permissionGrantingStrategy; } + /** + * {@inheritdoc} + */ + public function releaseMemory() + { + $this->loadedAces = array(); + $this->loadedAcls = array(); + } + /** * {@inheritdoc} */ diff --git a/Dbal/MutableAclProvider.php b/Dbal/MutableAclProvider.php index 51fa9c7..c4b53d2 100644 --- a/Dbal/MutableAclProvider.php +++ b/Dbal/MutableAclProvider.php @@ -46,6 +46,15 @@ public function __construct(Connection $connection, PermissionGrantingStrategyIn $this->propertyChanges = new \SplObjectStorage(); } + /** + * {@inheritdoc} + */ + public function releaseMemory() + { + parent::releaseMemory(); + $this->propertyChanges = new \SplObjectStorage(); + } + /** * {@inheritdoc} */ diff --git a/Model/AclProviderInterface.php b/Model/AclProviderInterface.php index f9b41cb..2e2046a 100644 --- a/Model/AclProviderInterface.php +++ b/Model/AclProviderInterface.php @@ -53,4 +53,11 @@ public function findAcl(ObjectIdentityInterface $oid, array $sids = array()); * @throws AclNotFoundException when we cannot find an ACL for all identities */ public function findAcls(array $oids, array $sids = array()); + + /** + * AclProvider for better performance can save some information in memory. + * You can ask provider to free this memory if you perform many calls + * @return void + */ + public function releaseMemory(); } From e986b09dc749016f2b68e1d14900d3f517c4851d Mon Sep 17 00:00:00 2001 From: Evgeniy Sokolov Date: Thu, 10 Sep 2015 21:46:41 +0200 Subject: [PATCH 3/6] test for memory leak in Acl --- Tests/Domain/AclTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Tests/Domain/AclTest.php b/Tests/Domain/AclTest.php index e6acf22..b64abc5 100644 --- a/Tests/Domain/AclTest.php +++ b/Tests/Domain/AclTest.php @@ -16,6 +16,7 @@ use Symfony\Component\Security\Acl\Domain\PermissionGrantingStrategy; use Symfony\Component\Security\Acl\Domain\ObjectIdentity; use Symfony\Component\Security\Acl\Domain\Acl; +use Symfony\Component\Security\Acl\Permission\MaskBuilder; class AclTest extends \PHPUnit_Framework_TestCase { @@ -476,6 +477,22 @@ public function testUpdateFieldAuditing($type) $this->assertTrue($ace->isAuditFailure()); } + public function testMemoryLeak() + { + $memoryBase = null; + + for ($i = 0; $i <= 1000; $i++) { + $acl = new Acl($i, new ObjectIdentity($i, 1), new PermissionGrantingStrategy(), array(), 1); + $acl->insertObjectAce(new RoleSecurityIdentity("ROLE_ADMIN"), MaskBuilder::MASK_VIEW); + + if (null === $memoryBase) { + $memoryBase = memory_get_usage(); + } + } + + $this->assertLessThan(500, memory_get_usage() - $memoryBase); + } + public function getUpdateFieldAuditingTests() { return array( From b9b7b22ec39701dd1ab205de1f9bea597fb50412 Mon Sep 17 00:00:00 2001 From: Evgeniy Sokolov Date: Mon, 14 Sep 2015 20:59:58 +0200 Subject: [PATCH 4/6] add release memory tests and fix getAcl calls --- Dbal/AclProvider.php | 4 +-- Dbal/MutableAclProvider.php | 31 ++++++++++--------- Domain/AclEntry.php | 3 +- Domain/ObjectIdentity.php | 2 +- Domain/UserSecurityIdentity.php | 2 +- Model/AclEntryInterface.php | 37 +++++++++++++++++++++++ Tests/Dbal/AclProviderTest.php | 40 +++++++++++++++++++++++++ Tests/Dbal/MutableAclProviderTest.php | 43 ++++++++++++++++++++------- 8 files changed, 131 insertions(+), 31 deletions(-) create mode 100644 Model/AclEntryInterface.php diff --git a/Dbal/AclProvider.php b/Dbal/AclProvider.php index 14e3902..5568229 100644 --- a/Dbal/AclProvider.php +++ b/Dbal/AclProvider.php @@ -624,9 +624,9 @@ private function hydrateObjectIdentities(Statement $stmt, array $oidLookup, arra } if (null === $fieldName) { - $loadedAces[$aceId] = new Entry((int) $aceId, $acl, $sids[$key], $grantingStrategy, (int) $mask, !!$granting, !!$auditFailure, !!$auditSuccess); + $loadedAces[$aceId] = new Entry((int) $aceId, $sids[$key], $grantingStrategy, (int) $mask, !!$granting, !!$auditFailure, !!$auditSuccess); } else { - $loadedAces[$aceId] = new FieldEntry((int) $aceId, $acl, $fieldName, $sids[$key], $grantingStrategy, (int) $mask, !!$granting, !!$auditFailure, !!$auditSuccess); + $loadedAces[$aceId] = new FieldEntry((int) $aceId, $fieldName, $sids[$key], $grantingStrategy, (int) $mask, !!$granting, !!$auditFailure, !!$auditSuccess); } } $ace = $loadedAces[$aceId]; diff --git a/Dbal/MutableAclProvider.php b/Dbal/MutableAclProvider.php index c4b53d2..dc69e15 100644 --- a/Dbal/MutableAclProvider.php +++ b/Dbal/MutableAclProvider.php @@ -13,14 +13,13 @@ use Doctrine\Common\PropertyChangedListener; use Doctrine\DBAL\Connection; -use Symfony\Component\Security\Acl\Domain\AclEntry; use Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity; use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity; use Symfony\Component\Security\Acl\Exception\AclAlreadyExistsException; use Symfony\Component\Security\Acl\Exception\ConcurrentModificationException; use Symfony\Component\Security\Acl\Model\AclCacheInterface; +use Symfony\Component\Security\Acl\Model\AclEntryInterface; use Symfony\Component\Security\Acl\Model\AclInterface; -use Symfony\Component\Security\Acl\Model\EntryInterface; use Symfony\Component\Security\Acl\Model\MutableAclInterface; use Symfony\Component\Security\Acl\Model\MutableAclProviderInterface; use Symfony\Component\Security\Acl\Model\ObjectIdentityInterface; @@ -176,11 +175,11 @@ public function findAcls(array $oids, array $sids = array()) */ public function propertyChanged($sender, $propertyName, $oldValue, $newValue) { - if (!$sender instanceof MutableAclInterface && !$sender instanceof EntryInterface) { - throw new \InvalidArgumentException('$sender must be an instance of MutableAclInterface, or EntryInterface.'); + if (!$sender instanceof MutableAclInterface && !$sender instanceof AclEntryInterface) { + throw new \InvalidArgumentException('$sender must be an instance of MutableAclInterface, or AclEntryInterface.'); } - if ($sender instanceof AclEntry) { + if ($sender instanceof AclEntryInterface) { if (null === $sender->getEntry()->getId()) { return; } @@ -301,18 +300,18 @@ public function updateAcl(MutableAclInterface $acl) // check properties for deleted, and created ACEs, and perform creations if (isset($propertyChanges['classAces'])) { - $this->updateNewAceProperty('classAces', $propertyChanges['classAces']); + $this->updateNewAceProperty('classAces', $propertyChanges['classAces'], $acl); $sharedPropertyChanges['classAces'] = $propertyChanges['classAces']; } if (isset($propertyChanges['classFieldAces'])) { - $this->updateNewFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces']); + $this->updateNewFieldAceProperty('classFieldAces', $propertyChanges['classFieldAces'], $acl); $sharedPropertyChanges['classFieldAces'] = $propertyChanges['classFieldAces']; } if (isset($propertyChanges['objectAces'])) { - $this->updateNewAceProperty('objectAces', $propertyChanges['objectAces']); + $this->updateNewAceProperty('objectAces', $propertyChanges['objectAces'], $acl); } if (isset($propertyChanges['objectFieldAces'])) { - $this->updateNewFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces']); + $this->updateNewFieldAceProperty('objectFieldAces', $propertyChanges['objectFieldAces'], $acl); } // if there have been changes to shared properties, we need to synchronize other @@ -839,7 +838,7 @@ private function deleteObjectIdentityRelations($pk) * * @param AclInterface $acl */ - private function regenerateAncestorRelations(AclInterface $acl) + private function regenerateAncestorRelations(MutableAclInterface $acl) { $pk = $acl->getId(); $this->connection->executeQuery($this->getDeleteObjectIdentityRelationsSql($pk)); @@ -859,7 +858,7 @@ private function regenerateAncestorRelations(AclInterface $acl) * @param string $name * @param array $changes */ - private function updateNewFieldAceProperty($name, array $changes) + private function updateNewFieldAceProperty($name, array $changes, MutableAclInterface $acl) { $sids = new \SplObjectStorage(); $classIds = new \SplObjectStorage(); @@ -874,14 +873,14 @@ private function updateNewFieldAceProperty($name, array $changes) $sid = $this->createOrRetrieveSecurityIdentityId($ace->getSecurityIdentity()); } - $oid = $ace->getAcl()->getObjectIdentity(); + $oid = $acl->getObjectIdentity(); if ($classIds->contains($oid)) { $classId = $classIds->offsetGet($oid); } else { $classId = $this->createOrRetrieveClassId($oid->getType()); } - $objectIdentityId = $name === 'classFieldAces' ? null : $ace->getAcl()->getId(); + $objectIdentityId = $name === 'classFieldAces' ? null : $acl->getId(); $this->connection->executeQuery($this->getInsertAccessControlEntrySql($classId, $objectIdentityId, $field, $i, $sid, $ace->getStrategy(), $ace->getMask(), $ace->isGranting(), $ace->isAuditSuccess(), $ace->isAuditFailure())); $aceId = $this->connection->executeQuery($this->getSelectAccessControlEntryIdSql($classId, $objectIdentityId, $field, $i))->fetchColumn(); @@ -932,7 +931,7 @@ private function updateOldFieldAceProperty($name, array $changes) * @param string $name * @param array $changes */ - private function updateNewAceProperty($name, array $changes) + private function updateNewAceProperty($name, array $changes, MutableAclInterface $acl) { list($old, $new) = $changes; @@ -948,14 +947,14 @@ private function updateNewAceProperty($name, array $changes) $sid = $this->createOrRetrieveSecurityIdentityId($ace->getSecurityIdentity()); } - $oid = $ace->getAcl()->getObjectIdentity(); + $oid = $acl->getObjectIdentity(); if ($classIds->contains($oid)) { $classId = $classIds->offsetGet($oid); } else { $classId = $this->createOrRetrieveClassId($oid->getType()); } - $objectIdentityId = $name === 'classAces' ? null : $ace->getAcl()->getId(); + $objectIdentityId = $name === 'classAces' ? null : $acl->getId(); $this->connection->executeQuery($this->getInsertAccessControlEntrySql($classId, $objectIdentityId, null, $i, $sid, $ace->getStrategy(), $ace->getMask(), $ace->isGranting(), $ace->isAuditSuccess(), $ace->isAuditFailure())); $aceId = $this->connection->executeQuery($this->getSelectAccessControlEntryIdSql($classId, $objectIdentityId, null, $i))->fetchColumn(); diff --git a/Domain/AclEntry.php b/Domain/AclEntry.php index 4586570..de2a44e 100644 --- a/Domain/AclEntry.php +++ b/Domain/AclEntry.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Acl\Domain; +use Symfony\Component\Security\Acl\Model\AclEntryInterface; use Symfony\Component\Security\Acl\Model\AclInterface; use Symfony\Component\Security\Acl\Model\EntryInterface; @@ -19,7 +20,7 @@ * * @author Evgeniy Sokolov */ -class AclEntry +class AclEntry implements AclEntryInterface { private $acl; private $entry; diff --git a/Domain/ObjectIdentity.php b/Domain/ObjectIdentity.php index 871bda7..5e813ff 100644 --- a/Domain/ObjectIdentity.php +++ b/Domain/ObjectIdentity.php @@ -11,7 +11,7 @@ namespace Symfony\Component\Security\Acl\Domain; -use Symfony\Component\Security\Core\Util\ClassUtils; +use Doctrine\Common\Util\ClassUtils; use Symfony\Component\Security\Acl\Exception\InvalidDomainObjectException; use Symfony\Component\Security\Acl\Model\DomainObjectInterface; use Symfony\Component\Security\Acl\Model\ObjectIdentityInterface; diff --git a/Domain/UserSecurityIdentity.php b/Domain/UserSecurityIdentity.php index ea17c63..0adea5d 100644 --- a/Domain/UserSecurityIdentity.php +++ b/Domain/UserSecurityIdentity.php @@ -11,9 +11,9 @@ namespace Symfony\Component\Security\Acl\Domain; +use Doctrine\Common\Util\ClassUtils; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\User\UserInterface; -use Symfony\Component\Security\Core\Util\ClassUtils; use Symfony\Component\Security\Acl\Model\SecurityIdentityInterface; /** diff --git a/Model/AclEntryInterface.php b/Model/AclEntryInterface.php new file mode 100644 index 0000000..e881632 --- /dev/null +++ b/Model/AclEntryInterface.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Acl\Model; + +/** + * This class represents an entry and acl. + * + * Instances MUST be immutable, as they are returned by the ACL and should not + * allow client modification. + * + * @author Evgeniy Sokolov + */ +interface AclEntryInterface +{ + /** + * Acl + * + * @return AclInterface + */ + public function getAcl(); + + /** + * Entry + * + * @return EntryInterface + */ + public function getEntry(); +} diff --git a/Tests/Dbal/AclProviderTest.php b/Tests/Dbal/AclProviderTest.php index becfb51..f374104 100644 --- a/Tests/Dbal/AclProviderTest.php +++ b/Tests/Dbal/AclProviderTest.php @@ -16,6 +16,7 @@ use Symfony\Component\Security\Acl\Domain\ObjectIdentity; use Symfony\Component\Security\Acl\Dbal\Schema; use Doctrine\DBAL\DriverManager; +use Symfony\Component\Security\Acl\Exception\AclNotFoundException; class AclProviderTest extends \PHPUnit_Framework_TestCase { @@ -139,6 +140,45 @@ public function testFindAcl() $this->assertEquals('SomeClass', $sid->getClass()); } + public function testReleaseMemory() + { + $oidCount = 300; + + for($i = 6; $i < $oidCount; $i++) { + $this->insertOidStmt->execute( + array($i, 3, (string)$i, 2, 1) + ); + + $this->insertOidAncestorStmt->execute(array($i, 2)); + $this->insertOidAncestorStmt->execute(array($i, 1)); + $this->insertOidAncestorStmt->execute(array($i, $i)); + } + + $provider = $this->getProvider(); + + $provider->findAcl(new ObjectIdentity('6', 'foo')); + + $provider->releaseMemory(); + gc_collect_cycles(); + + $memoryBase = memory_get_usage(); + $oids = array(); + + for($i = 6; $i < $oidCount; $i++) { + $oids[] = new ObjectIdentity((string)$i, 'foo'); + } + + $provider->findAcls($oids); + + $this->assertGreaterThan(100000, memory_get_usage() - $memoryBase); + + unset($oids); + $provider->releaseMemory(); + gc_collect_cycles(); + + $this->assertLessThan(1000, memory_get_usage() - $memoryBase); + } + protected function setUp() { if (!class_exists('PDO') || !in_array('sqlite', \PDO::getAvailableDrivers())) { diff --git a/Tests/Dbal/MutableAclProviderTest.php b/Tests/Dbal/MutableAclProviderTest.php index 37e1d77..15762eb 100644 --- a/Tests/Dbal/MutableAclProviderTest.php +++ b/Tests/Dbal/MutableAclProviderTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Acl\Tests\Dbal; +use Symfony\Component\Security\Acl\Domain\AclEntry; use Symfony\Component\Security\Acl\Domain\RoleSecurityIdentity; use Symfony\Component\Security\Acl\Model\FieldEntryInterface; use Symfony\Component\Security\Acl\Model\AuditableEntryInterface; @@ -40,7 +41,6 @@ public static function assertAceEquals(EntryInterface $a, EntryInterface $b) } self::assertTrue($a->getSecurityIdentity()->equals($b->getSecurityIdentity())); - self::assertSame($a->getAcl()->getId(), $b->getAcl()->getId()); if ($a instanceof AuditableEntryInterface) { self::assertSame($a->isAuditSuccess(), $b->isAuditSuccess()); @@ -196,11 +196,11 @@ public function testPropertyChangedTracksChangesToAceProperties() { $provider = $this->getProvider(); $acl = $provider->createAcl(new ObjectIdentity(1, 'Foo')); - $ace = new Entry(1, $acl, new UserSecurityIdentity('foo', 'FooClass'), 'all', 1, true, true, true); - $ace2 = new Entry(2, $acl, new UserSecurityIdentity('foo', 'FooClass'), 'all', 1, true, true, true); + $ace = new Entry(1, new UserSecurityIdentity('foo', 'FooClass'), 'all', 1, true, true, true); + $ace2 = new Entry(2, new UserSecurityIdentity('foo', 'FooClass'), 'all', 1, true, true, true); $propertyChanges = $this->getField($provider, 'propertyChanges'); - $provider->propertyChanged($ace, 'mask', 1, 3); + $provider->propertyChanged(new AclEntry($acl, $ace), 'mask', 1, 3); $changes = $propertyChanges->offsetGet($acl); $this->assertTrue(isset($changes['aces'])); $this->assertInstanceOf('\SplObjectStorage', $changes['aces']); @@ -210,7 +210,7 @@ public function testPropertyChangedTracksChangesToAceProperties() $this->assertEquals(1, $aceChanges['mask'][0]); $this->assertEquals(3, $aceChanges['mask'][1]); - $provider->propertyChanged($ace, 'strategy', 'all', 'any'); + $provider->propertyChanged(new AclEntry($acl, $ace), 'strategy', 'all', 'any'); $changes = $propertyChanges->offsetGet($acl); $this->assertTrue(isset($changes['aces'])); $this->assertInstanceOf('\SplObjectStorage', $changes['aces']); @@ -221,25 +221,48 @@ public function testPropertyChangedTracksChangesToAceProperties() $this->assertEquals('all', $aceChanges['strategy'][0]); $this->assertEquals('any', $aceChanges['strategy'][1]); - $provider->propertyChanged($ace, 'mask', 3, 1); + $provider->propertyChanged(new AclEntry($acl, $ace), 'mask', 3, 1); $changes = $propertyChanges->offsetGet($acl); $aceChanges = $changes['aces']->offsetGet($ace); $this->assertFalse(isset($aceChanges['mask'])); $this->assertTrue(isset($aceChanges['strategy'])); - $provider->propertyChanged($ace2, 'mask', 1, 3); - $provider->propertyChanged($ace, 'strategy', 'any', 'all'); + $provider->propertyChanged(new AclEntry($acl, $ace2), 'mask', 1, 3); + $provider->propertyChanged(new AclEntry($acl, $ace), 'strategy', 'any', 'all'); $changes = $propertyChanges->offsetGet($acl); $this->assertTrue(isset($changes['aces'])); $this->assertFalse($changes['aces']->contains($ace)); $this->assertTrue($changes['aces']->contains($ace2)); - $provider->propertyChanged($ace2, 'mask', 3, 4); - $provider->propertyChanged($ace2, 'mask', 4, 1); + $provider->propertyChanged(new AclEntry($acl, $ace2), 'mask', 3, 4); + $provider->propertyChanged(new AclEntry($acl, $ace2), 'mask', 4, 1); $changes = $propertyChanges->offsetGet($acl); $this->assertFalse(isset($changes['aces'])); } + public function testReleaseMemory() + { + $provider = $this->getProvider(); + + $provider->createAcl(new ObjectIdentity(0, 'Foo0')); + + $provider->releaseMemory(); + gc_collect_cycles(); + + $memoryBase = memory_get_usage(); + + for ($i = 1; $i < 200; $i++) { + $provider->createAcl(new ObjectIdentity($i, 'Foo' . $i)); + } + + $this->assertGreaterThan(100000, memory_get_usage() - $memoryBase); + + $provider->releaseMemory(); + gc_collect_cycles(); + + $this->assertLessThan(500, memory_get_usage() - $memoryBase); + } + /** * @expectedException \InvalidArgumentException */ From 9f0842e0e1c07572a7f00aa2f451475e81ce923d Mon Sep 17 00:00:00 2001 From: Evgeniy Sokolov Date: Mon, 14 Sep 2015 21:25:21 +0200 Subject: [PATCH 5/6] upgrade instructions --- UPGRADE-3.0.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 UPGRADE-3.0.md diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md new file mode 100644 index 0000000..96d0a28 --- /dev/null +++ b/UPGRADE-3.0.md @@ -0,0 +1,23 @@ +Security Component - ACL (Access Control List) +============================================== + +Security provides an infrastructure for sophisticated authorization systems, +which makes it possible to easily separate the actual authorization logic from +so called user providers that hold the users credentials. It is inspired by +the Java Spring framework. + +Resources +--------- + +Documentation: + +https://symfony.com/doc/3.0/book/security.html + +Tests +----- + +You can run the unit tests with the following command: + + $ cd path/to/Symfony/Component/Security/Acl/ + $ composer.phar install --dev + $ phpunit From 2421fb093fc1c00645a443b2521e33cb19691ab1 Mon Sep 17 00:00:00 2001 From: Evgeniy Sokolov Date: Mon, 14 Sep 2015 21:26:39 +0200 Subject: [PATCH 6/6] revert ClassUtils usage --- Domain/AclEntry.php | 6 ++++++ Domain/ObjectIdentity.php | 2 +- Domain/UserSecurityIdentity.php | 2 +- Tests/Dbal/AclProviderTest.php | 1 - UPGRADE-3.0.md | 29 +++++++++-------------------- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/Domain/AclEntry.php b/Domain/AclEntry.php index de2a44e..f34ea25 100644 --- a/Domain/AclEntry.php +++ b/Domain/AclEntry.php @@ -31,11 +31,17 @@ public function __construct(AclInterface $acl, EntryInterface $entry) $this->entry = $entry; } + /** + * @return AclInterface + */ public function getAcl() { return $this->acl; } + /** + * @return EntryInterface + */ public function getEntry() { return $this->entry; diff --git a/Domain/ObjectIdentity.php b/Domain/ObjectIdentity.php index 5e813ff..871bda7 100644 --- a/Domain/ObjectIdentity.php +++ b/Domain/ObjectIdentity.php @@ -11,7 +11,7 @@ namespace Symfony\Component\Security\Acl\Domain; -use Doctrine\Common\Util\ClassUtils; +use Symfony\Component\Security\Core\Util\ClassUtils; use Symfony\Component\Security\Acl\Exception\InvalidDomainObjectException; use Symfony\Component\Security\Acl\Model\DomainObjectInterface; use Symfony\Component\Security\Acl\Model\ObjectIdentityInterface; diff --git a/Domain/UserSecurityIdentity.php b/Domain/UserSecurityIdentity.php index 0adea5d..ea17c63 100644 --- a/Domain/UserSecurityIdentity.php +++ b/Domain/UserSecurityIdentity.php @@ -11,9 +11,9 @@ namespace Symfony\Component\Security\Acl\Domain; -use Doctrine\Common\Util\ClassUtils; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Core\Util\ClassUtils; use Symfony\Component\Security\Acl\Model\SecurityIdentityInterface; /** diff --git a/Tests/Dbal/AclProviderTest.php b/Tests/Dbal/AclProviderTest.php index f374104..8a10d2c 100644 --- a/Tests/Dbal/AclProviderTest.php +++ b/Tests/Dbal/AclProviderTest.php @@ -16,7 +16,6 @@ use Symfony\Component\Security\Acl\Domain\ObjectIdentity; use Symfony\Component\Security\Acl\Dbal\Schema; use Doctrine\DBAL\DriverManager; -use Symfony\Component\Security\Acl\Exception\AclNotFoundException; class AclProviderTest extends \PHPUnit_Framework_TestCase { diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index 96d0a28..3000a94 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -1,23 +1,12 @@ -Security Component - ACL (Access Control List) -============================================== +UPGRADE FROM 2.x to 3.0 +======================= -Security provides an infrastructure for sophisticated authorization systems, -which makes it possible to easily separate the actual authorization logic from -so called user providers that hold the users credentials. It is inspired by -the Java Spring framework. +### Entry, FieldEntry and EntryInterface -Resources ---------- + * The `acl` constructor argument and related getter getAcl() has been removed due to problems with release memory (https://github.com/symfony/symfony/issues/2376). + Classes that before work with Entry objects and call Entry::getAcl now must: + a) expect instance of AclEntryInterface and call AclEntry::getAcl and AclEntry::getEntry. + or b) additionally to EntryInterface $entry argument require AclInterface $acl argument -Documentation: - -https://symfony.com/doc/3.0/book/security.html - -Tests ------ - -You can run the unit tests with the following command: - - $ cd path/to/Symfony/Component/Security/Acl/ - $ composer.phar install --dev - $ phpunit +### MutableAclProvider + * propertyChanged method now expect as $sender argument instance of MutableAclInterface or AclEntryInterface (was MutableAclInterface or EntryInterface) \ No newline at end of file