add release memory tests and fix getAcl calls

symfony · ewgRa · Sep 10, 2015 · Sep 10, 2015 · Sep 10, 2015 · Sep 14, 2015
commit b9b7b22ec39701dd1ab205de1f9bea597fb50412
diff --git 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
@@ -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
@@ -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 <ewgraf@gmail.com>
  */
-class AclEntry
+class AclEntry implements AclEntryInterface
 {
     private $acl;
     private $entry;

diff --git 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
@@ -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
@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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 <ewgraf@gmail.com>
+ */
+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
@@ -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
@@ -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
      */