Allow multiple filters in AuditReader::filterBy() (#127)

* Allow multiple filters in `AuditReader::filterBy()` - [#123] * Refactoring and code cleanup
DamienHarper · Nov 21, 2019 · 0d4beaf · 0d4beaf
1 parent c1af7f6
commit 0d4beaf
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 83 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -17,5 +17,4 @@ parameters:
         - '~Parameter \#1 \$name of method Symfony\\Component\\Console\\Command\\Command\:\:setName\(\) expects string, string\|null given~'
         - '~Parameter \#1 \$tableName of method Doctrine\\DBAL\\Schema\\Schema\:\:(has|get)Table\(\) expects string, string\|null given~'
         - '~Cannot call method fetchColumn\(\) on Doctrine\\DBAL\\Driver\\Statement\|int~'
-        - '~Call to an undefined method Doctrine\\Common\\Persistence\\Mapping\\ClassMetadata\:\:get(Schema|Table)Name\(\)~'
         - '~Possibly invalid array key type object\|string~'
diff --git a/src/DoctrineAuditBundle/Manager/AuditManager.php b/src/DoctrineAuditBundle/Manager/AuditManager.php
@@ -83,7 +83,7 @@ public function insert(EntityManagerInterface $em, $entity, array $ch, string $t
     {
         /** @var ClassMetadata $meta */
         $meta = $em->getClassMetadata(\get_class($entity));
-        $this->audit($em, [
+        $this->audit([
             'action' => 'insert',
             'blame' => $this->helper->blame(),
             'diff' => $this->helper->diff($em, $entity, $ch),
@@ -114,7 +114,7 @@ public function update(EntityManagerInterface $em, $entity, array $ch, string $t
         }
         /** @var ClassMetadata $meta */
         $meta = $em->getClassMetadata(\get_class($entity));
-        $this->audit($em, [
+        $this->audit([
             'action' => 'update',
             'blame' => $this->helper->blame(),
             'diff' => $diff,
@@ -141,7 +141,7 @@ public function remove(EntityManagerInterface $em, $entity, $id, string $transac
     {
         /** @var ClassMetadata $meta */
         $meta = $em->getClassMetadata(\get_class($entity));
-        $this->audit($em, [
+        $this->audit([
             'action' => 'remove',
             'blame' => $this->helper->blame(),
             'diff' => $this->helper->summarize($em, $entity, $id),
@@ -328,18 +328,17 @@ private function associateOrDissociate(string $type, EntityManagerInterface $em,
             $data['diff']['table'] = $mapping['joinTable']['name'];
         }
 
-        $this->audit($em, $data);
+        $this->audit($data);
     }
 
     /**
      * Adds an entry to the audit table.
      *
-     * @param EntityManagerInterface $em
-     * @param array                  $data
+     * @param array $data
      *
      * @throws Exception
      */
-    private function audit(EntityManagerInterface $em, array $data): void
+    private function audit(array $data): void
     {
         $schema = $data['schema'] ? $data['schema'].'.' : '';
         $auditTable = $schema.$this->configuration->getTablePrefix().$data['table'].$this->configuration->getTableSuffix();

diff --git a/src/DoctrineAuditBundle/Reader/AuditReader.php b/src/DoctrineAuditBundle/Reader/AuditReader.php
@@ -7,7 +7,7 @@
 use DH\DoctrineAuditBundle\Exception\AccessDeniedException;
 use DH\DoctrineAuditBundle\Exception\InvalidArgumentException;
 use DH\DoctrineAuditBundle\User\UserInterface;
-use Doctrine\Common\Persistence\Mapping\ClassMetadata;
+use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Query\QueryBuilder;
 use Doctrine\DBAL\Statement;
 use Doctrine\ORM\EntityManagerInterface;
@@ -38,9 +38,9 @@ class AuditReader
     private $entityManager;
 
     /**
-     * @var ?string
+     * @var array
      */
-    private $filter;
+    private $filters = [];
 
     /**
      * AuditReader constructor.
@@ -65,31 +65,31 @@ public function getConfiguration(): AuditConfiguration
     }
 
     /**
-     * Set the filter for AuditEntry retrieving.
+     * Set the filter(s) for AuditEntry retrieving.
      *
-     * @param string $filter
+     * @param array|string $filter
      *
      * @return AuditReader
      */
-    public function filterBy(string $filter): self
+    public function filterBy($filter): self
     {
-        if (!\in_array($filter, [self::UPDATE, self::ASSOCIATE, self::DISSOCIATE, self::INSERT, self::REMOVE], true)) {
-            $this->filter = null;
-        } else {
-            $this->filter = $filter;
-        }
+        $filters = \is_array($filter) ? $filter : [$filter];
+
+        $this->filters = array_filter($filters, static function ($f) {
+            return \in_array($f, [self::UPDATE, self::ASSOCIATE, self::DISSOCIATE, self::INSERT, self::REMOVE], true);
+        });
 
         return $this;
     }
 
     /**
      * Returns current filter.
      *
-     * @return null|string
+     * @return array
      */
-    public function getFilter(): ?string
+    public function getFilters(): array
     {
-        return $this->filter;
+        return $this->filters;
     }
 
     /**
@@ -239,7 +239,7 @@ public function getAuditsCount($entity, $id = null): int
      *
      * @return mixed[]
      */
-    public function getAudit($entity, $id)
+    public function getAudit($entity, $id): array
     {
         $this->checkAuditable($entity);
         $this->checkRoles($entity, Security::VIEW_SCOPE);
@@ -257,12 +257,7 @@ public function getAudit($entity, $id)
             ->setParameter('id', $id)
         ;
 
-        if (null !== $this->filter) {
-            $queryBuilder
-                ->andWhere('type = :filter')
-                ->setParameter('filter', $this->filter)
-            ;
-        }
+        $this->filterByType($queryBuilder, $this->filters);
 
         /** @var Statement $statement */
         $statement = $queryBuilder->execute();
@@ -280,7 +275,9 @@ public function getAudit($entity, $id)
      */
     public function getEntityTableName($entity): string
     {
-        return $this->getClassMetadata($entity)->getTableName();
+        $entityName = \is_string($entity) ? $entity : \get_class($entity);
+
+        return $this->entityManager->getClassMetadata($entityName)->getTableName();
     }
 
     /**
@@ -293,7 +290,11 @@ public function getEntityTableName($entity): string
     public function getEntityAuditTableName($entity): string
     {
         $entityName = \is_string($entity) ? $entity : \get_class($entity);
-        $schema = $this->getClassMetadata($entityName)->getSchemaName() ? $this->getClassMetadata($entityName)->getSchemaName().'.' : '';
+
+        $schema = '';
+        if ($this->entityManager->getClassMetadata($entityName)->getSchemaName()) {
+            $schema = $this->entityManager->getClassMetadata($entityName)->getSchemaName().'.';
+        }
 
         return sprintf('%s%s%s%s', $schema, $this->configuration->getTablePrefix(), $this->getEntityTableName($entityName), $this->configuration->getTableSuffix());
     }
@@ -306,6 +307,48 @@ public function getEntityManager(): EntityManagerInterface
         return $this->entityManager;
     }
 
+    private function filterByType(QueryBuilder $queryBuilder, array $filters): QueryBuilder
+    {
+        if (!empty($filters)) {
+            $queryBuilder
+                ->andWhere('type IN (:filters)')
+                ->setParameter('filters', $filters, Connection::PARAM_STR_ARRAY)
+            ;
+        }
+
+        return $queryBuilder;
+    }
+
+    private function filterByTransaction(QueryBuilder $queryBuilder, ?string $transactionHash): QueryBuilder
+    {
+        if (null !== $transactionHash) {
+            $queryBuilder
+                ->andWhere('transaction_hash = :transaction_hash')
+                ->setParameter('transaction_hash', $transactionHash)
+            ;
+        }
+
+        return $queryBuilder;
+    }
+
+    /**
+     * @param QueryBuilder    $queryBuilder
+     * @param null|int|string $id
+     *
+     * @return QueryBuilder
+     */
+    private function filterByObjectId(QueryBuilder $queryBuilder, $id): QueryBuilder
+    {
+        if (null !== $id) {
+            $queryBuilder
+                ->andWhere('object_id = :object_id')
+                ->setParameter('object_id', $id)
+            ;
+        }
+
+        return $queryBuilder;
+    }
+
     /**
      * Returns an array of audited entries/operations.
      *
@@ -323,6 +366,8 @@ public function getEntityManager(): EntityManagerInterface
      */
     private function getAuditsQueryBuilder($entity, $id = null, ?int $page = null, ?int $pageSize = null, ?string $transactionHash = null, bool $strict = true): QueryBuilder
     {
+        $entityName = \is_string($entity) ? $entity : \get_class($entity);
+
         $this->checkAuditable($entity);
         $this->checkRoles($entity, Security::VIEW_SCOPE);
 
@@ -334,7 +379,7 @@ private function getAuditsQueryBuilder($entity, $id = null, ?int $page = null, ?
             throw new \InvalidArgumentException('$pageSize must be greater or equal than 1.');
         }
 
-        $storage = $this->selectStorage();
+        $storage = $this->configuration->getEntityManager() ?? $this->entityManager;
         $connection = $storage->getConnection();
 
         $queryBuilder = $connection->createQueryBuilder();
@@ -345,63 +390,28 @@ private function getAuditsQueryBuilder($entity, $id = null, ?int $page = null, ?
             ->addOrderBy('id', 'DESC')
         ;
 
-        $metadata = $this->getClassMetadata($entity);
+        $metadata = $this->entityManager->getClassMetadata($entityName);
         if ($strict && $metadata instanceof ORMMetadata && ORMMetadata::INHERITANCE_TYPE_SINGLE_TABLE === $metadata->inheritanceType) {
             $queryBuilder
                 ->andWhere('discriminator = :discriminator')
                 ->setParameter('discriminator', \is_object($entity) ? \get_class($entity) : $entity)
             ;
         }
 
+        $this->filterByObjectId($queryBuilder, $id);
+        $this->filterByType($queryBuilder, $this->filters);
+        $this->filterByTransaction($queryBuilder, $transactionHash);
+
         if (null !== $pageSize) {
             $queryBuilder
                 ->setFirstResult(($page - 1) * $pageSize)
                 ->setMaxResults($pageSize)
             ;
         }
 
-        if (null !== $id) {
-            $queryBuilder
-                ->andWhere('object_id = :object_id')
-                ->setParameter('object_id', $id)
-            ;
-        }
-
-        if (null !== $this->filter) {
-            $queryBuilder
-                ->andWhere('type = :filter')
-                ->setParameter('filter', $this->filter)
-            ;
-        }
-
-        if (null !== $transactionHash) {
-            $queryBuilder
-                ->andWhere('transaction_hash = :transaction_hash')
-                ->setParameter('transaction_hash', $transactionHash)
-            ;
-        }
-
         return $queryBuilder;
     }
 
-    /**
-     * @param object|string $entity
-     *
-     * @return ClassMetadata
-     */
-    private function getClassMetadata($entity): ClassMetadata
-    {
-        return $this->entityManager->getClassMetadata($entity);
-    }
-
-    /**
-     * @return EntityManagerInterface
-     */
-    private function selectStorage(): EntityManagerInterface
-    {
-        return $this->configuration->getEntityManager() ?? $this->entityManager;
-    }
-
     /**
      * Throws an InvalidArgumentException if given entity is not auditable.
      *

diff --git a/tests/DoctrineAuditBundle/Reader/AuditReaderTest.php b/tests/DoctrineAuditBundle/Reader/AuditReaderTest.php
@@ -28,34 +28,50 @@ public function testGetAuditConfiguration(): void
         self::assertInstanceOf(AuditConfiguration::class, $reader->getConfiguration(), 'configuration instanceof AuditConfiguration::class');
     }
 
-    public function testFilterIsNullByDefault(): void
+    public function testFilterIsEmptyByDefault(): void
     {
         $reader = $this->getReader();
 
-        self::assertNull($reader->getFilter(), 'filter is null by default.');
+        self::assertSame([], $reader->getFilters(), 'filters is empty by default.');
     }
 
-    public function testFilterCanOnlyBePartOfAllowedValues(): void
+    public function testFilterIsEmptyIfNotPartOfAllowedValues(): void
     {
         $reader = $this->getReader();
 
         $reader->filterBy('UNKNOWN');
-        self::assertNull($reader->getFilter(), 'filter is null when AuditReader::filterBy() parameter is not an allowed value.');
+        self::assertSame([], $reader->getFilters(), 'filters is empty when AuditReader::filterBy() parameter is not an allowed value.');
+
+        $reader->filterBy(['UNKNOWN1', 'UNKNOWN2']);
+        self::assertSame([], $reader->getFilters(), 'filters is empty when AuditReader::filterBy() parameter is not an allowed value.');
+    }
+
+    public function testFilterSingleValue(): void
+    {
+        $reader = $this->getReader();
 
         $reader->filterBy(AuditReader::ASSOCIATE);
-        self::assertSame(AuditReader::ASSOCIATE, $reader->getFilter(), 'filter is not null when AuditReader::filterBy() parameter is an allowed value.');
+        self::assertSame([AuditReader::ASSOCIATE], $reader->getFilters(), 'filter is not empty when AuditReader::filterBy() parameter is an allowed value.');
 
         $reader->filterBy(AuditReader::DISSOCIATE);
-        self::assertSame(AuditReader::DISSOCIATE, $reader->getFilter(), 'filter is not null when AuditReader::filterBy() parameter is an allowed value.');
+        self::assertSame([AuditReader::DISSOCIATE], $reader->getFilters(), 'filter is not empty when AuditReader::filterBy() parameter is an allowed value.');
 
         $reader->filterBy(AuditReader::INSERT);
-        self::assertSame(AuditReader::INSERT, $reader->getFilter(), 'filter is not null when AuditReader::filterBy() parameter is an allowed value.');
+        self::assertSame([AuditReader::INSERT], $reader->getFilters(), 'filter is not empty when AuditReader::filterBy() parameter is an allowed value.');
 
         $reader->filterBy(AuditReader::REMOVE);
-        self::assertSame(AuditReader::REMOVE, $reader->getFilter(), 'filter is not null when AuditReader::filterBy() parameter is an allowed value.');
+        self::assertSame([AuditReader::REMOVE], $reader->getFilters(), 'filter is not empty when AuditReader::filterBy() parameter is an allowed value.');
 
         $reader->filterBy(AuditReader::UPDATE);
-        self::assertSame(AuditReader::UPDATE, $reader->getFilter(), 'filter is not null when AuditReader::filterBy() parameter is an allowed value.');
+        self::assertSame([AuditReader::UPDATE], $reader->getFilters(), 'filter is not empty when AuditReader::filterBy() parameter is an allowed value.');
+    }
+
+    public function testFilterMultipleValues(): void
+    {
+        $reader = $this->getReader();
+
+        $reader->filterBy([AuditReader::ASSOCIATE, AuditReader::DISSOCIATE]);
+        self::assertSame([AuditReader::ASSOCIATE, AuditReader::DISSOCIATE], $reader->getFilters(), 'filter is not null when AuditReader::filterBy() parameter is composed of allowed value.');
     }
 
     public function testGetEntityTableName(): void
@@ -168,9 +184,7 @@ public function testGetAudits(): void
         $audits = $reader->getAudits(Tag::class, null, 1, 50);
 
         $i = 0;
-//        $this->assertCount(14, $audits, 'result count is ok.');
         self::assertCount(15, $audits, 'result count is ok.');
-//        $this->assertCount(12, $audits, 'result count is ok.');
         self::assertSame(AuditReader::DISSOCIATE, $audits[$i++]->getType(), 'entry'.$i.' is a dissociate operation.');
         self::assertSame(AuditReader::DISSOCIATE, $audits[$i++]->getType(), 'entry'.$i.' is a dissociate operation.');
         self::assertSame(AuditReader::ASSOCIATE, $audits[$i++]->getType(), 'entry'.$i.' is an associate operation.');