Repository: DamienHarper/auditor (php)

Issue:
Ignore attribute is not recognized on child classes
<!--
- Please do not report an issue for a version of `auditor` that is no longer supported. Currently supported versions are listed here: https://github.com/DamienHarper/auditor#version-information
- Please fill in this template according to your issue.
- Please keep the table shown below at the top of your issue.
- You can retrieve `auditor` version by running `composer info | grep "auditor"`.
- You can retrieve PHP version by running `php -v`.
- Please post code as text (using proper markup). Do not post screenshots of code.
- Please remove this comment before submitting your issue.
-->

| Q                    | A
| ---------------------| -----------------------
| `auditor` version    | 2.0.1 (auditor bundle 5.0.2)
| PHP version          | 8.1.5
| Database             | MariaDB

#### Summary

The `Ignore` attribute is not respected when it's on a private property in a parent class and a field was changed in a child class.

#### Current behavior

Here is my class hierarchy:

```
BaseOrder
  |- PizzaOrder
```

I have an `updatedAt` property defined as follows:

```
// Defined on BaseOrder
#[ORM\Column(name: "updatedAt", type: "datetime_immutable", nullable: true)]
#[Audit\Ignore]
private ?\DateTimeImmutable $updatedAt = null;
```

When making changes to a `PizzaOrder` the changes to `updatedAt` appear in the audit log.

#### How to reproduce

See above, please let me know if you'd like a working code sample.

#### Expected behavior

`Ignore` annotation can be defined in parent classes on private variables. Note that it does work as expected for protected variables.


Pull request:
Loads auditor attributes/annotations from full class hierarchy.
Fixes #101 

Patch:
diff --git a/src/Provider/Doctrine/Auditing/Annotation/AnnotationLoader.php b/src/Provider/Doctrine/Auditing/Annotation/AnnotationLoader.php
index f11288a..3423c5a 100644
--- a/src/Provider/Doctrine/Auditing/Annotation/AnnotationLoader.php
+++ b/src/Provider/Doctrine/Auditing/Annotation/AnnotationLoader.php
@@ -8,6 +8,7 @@ use Doctrine\Common\Annotations\AnnotationReader;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Mapping\Entity;
 use Doctrine\Persistence\Mapping\ClassMetadata;
+use ReflectionClass;
 
 class AnnotationLoader
 {
@@ -41,7 +42,6 @@ class AnnotationLoader
         $annotation = null;
         $auditableAnnotation = null;
         $securityAnnotation = null;
-        $annotationProperty = null;
         $reflection = $metadata->getReflectionClass();
 
         // Check that we have an Entity annotation or attribute
@@ -78,13 +78,21 @@ class AnnotationLoader
 
         $roles = null === $securityAnnotation ? null : [Security::VIEW_SCOPE => $securityAnnotation->view];
 
-        $config = [
-            'ignored_columns' => [],
+        // Are there any Ignore annotation or attribute?
+        $ignoredColumns = $this->getAllProperties($reflection);
+
+        return [
+            'ignored_columns' => $ignoredColumns,
             'enabled' => $auditableAnnotation->enabled,
             'roles' => $roles,
         ];
+    }
+
+    private function getAllProperties(ReflectionClass $reflection): array
+    {
+        $annotationProperty = null;
+        $properties = [];
 
-        // Are there any Ignore annotation or attribute?
         foreach ($reflection->getProperties() as $property) {
             $attributes = \PHP_VERSION_ID >= 80000 && method_exists($property, 'getAttributes') ? $property->getAttributes(Ignore::class) : null;
             if (\is_array($attributes) && \count($attributes) > 0) {
@@ -94,11 +102,14 @@ class AnnotationLoader
             }
 
             if (null !== $annotationProperty) {
-                // TODO: $property->getName() might not be the column name
-                $config['ignored_columns'][] = $property->getName();
+                $properties[] = $property->getName();
             }
         }
 
-        return $config;
+        if (false !== $reflection->getParentClass()) {
+            $properties = array_unique(array_merge($this->getAllProperties($reflection->getParentClass()), $properties));
+        }
+
+        return $properties;
     }
 }


Test patch:
diff --git a/tests/Provider/Doctrine/Auditing/Annotation/AnnotationLoaderTest.php b/tests/Provider/Doctrine/Auditing/Annotation/AnnotationLoaderTest.php
index 66aba78..4a12ead 100644
--- a/tests/Provider/Doctrine/Auditing/Annotation/AnnotationLoaderTest.php
+++ b/tests/Provider/Doctrine/Auditing/Annotation/AnnotationLoaderTest.php
@@ -69,7 +69,7 @@ final class AnnotationLoaderTest extends TestCase
     public function testLoadEntitiesWithAttributesOnly(): void
     {
         if (\PHP_VERSION_ID < 80000) {
-            self::markTestSkipped('PHP 8.0+ is required.');
+            self::markTestSkipped('PHP > 8.0 is required.');
         }
 
         $entityManager = $this->createEntityManager(
@@ -89,7 +89,7 @@ final class AnnotationLoaderTest extends TestCase
     public function testLoadEntitiesWithAnnotationsOnlyButNoAnnotationReader(): void
     {
         if (\PHP_VERSION_ID < 80000) {
-            self::markTestSkipped('PHP 8.0+ is required.');
+            self::markTestSkipped('PHP > 8.0 is required.');
         }
 
         $entityManager = $this->createEntityManager(
diff --git a/tests/Provider/Doctrine/DoctrineProviderTest.php b/tests/Provider/Doctrine/DoctrineProviderTest.php
index 648f1f2..a54291a 100644
--- a/tests/Provider/Doctrine/DoctrineProviderTest.php
+++ b/tests/Provider/Doctrine/DoctrineProviderTest.php
@@ -5,7 +5,9 @@ declare(strict_types=1);
 namespace DH\Auditor\Tests\Provider\Doctrine;
 
 use DH\Auditor\Exception\ProviderException;
+use DH\Auditor\Provider\Doctrine\Auditing\Annotation\AnnotationLoader;
 use DH\Auditor\Provider\Doctrine\Configuration;
+use DH\Auditor\Provider\Doctrine\DoctrineProvider;
 use DH\Auditor\Provider\Doctrine\Service\AuditingService;
 use DH\Auditor\Provider\Doctrine\Service\StorageService;
 use DH\Auditor\Provider\Service\StorageServiceInterface;
@@ -13,6 +15,8 @@ use DH\Auditor\Security\RoleCheckerInterface;
 use DH\Auditor\Security\SecurityProviderInterface;
 use DH\Auditor\Tests\Fixtures\Provider\AuditNoStorageProvider;
 use DH\Auditor\Tests\Fixtures\Provider\StorageNoAuditProvider;
+use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Entity\Annotation\AuditedEntity as AuditedEntityWithAnnotation;
+use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Entity\Attribute\AuditedEntity as AuditedEntityWithAttribute;
 use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Entity\Standard\Blog\Comment;
 use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Entity\Standard\Blog\Post;
 use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Entity\Standard\Blog\Tag;
@@ -409,6 +413,66 @@ final class DoctrineProviderTest extends TestCase
         self::assertFalse($provider->isAuditedField(Comment::class, 'id'), 'Field "'.Comment::class.'::$id" is audited but "'.Comment::class.'" entity is not.');
     }
 
+    public function testLoadEntitiesWithAnnotationsOnly(): void
+    {
+        $entityManager = $this->createEntityManager(
+            [
+                __DIR__.'/../../../src/Provider/Doctrine/Auditing/Annotation',
+                __DIR__.'/Fixtures/Entity/Annotation',
+            ],
+            'default',
+            null,
+            false
+        );
+        $annotationLoader = new AnnotationLoader($entityManager);
+        $loaded = $annotationLoader->load();
+        self::assertCount(2, $loaded);
+
+        $auditor = $this->createAuditor();
+        $provider = new DoctrineProvider($this->createProviderConfiguration(['entities' => $loaded]));
+        $provider->registerStorageService(new StorageService('default', $entityManager));
+        $provider->registerAuditingService(new AuditingService('default', $entityManager));
+        $auditor->registerProvider($provider);
+
+        self::assertTrue($provider->isAudited(AuditedEntityWithAnnotation::class), '"'.AuditedEntityWithAnnotation::class.'" is audited.');
+        self::assertTrue($provider->isAuditedField(AuditedEntityWithAnnotation::class, 'auditedField'), 'Field "'.AuditedEntityWithAnnotation::class.'::$auditedField" is audited.');
+        self::assertFalse($provider->isAuditedField(AuditedEntityWithAnnotation::class, 'ignoredField'), 'Field "'.AuditedEntityWithAnnotation::class.'::$ignoredField" is ignored.');
+        self::assertFalse($provider->isAuditedField(AuditedEntityWithAnnotation::class, 'ignoredProtectedField'), 'Field "'.AuditedEntityWithAnnotation::class.'::$ignoredProtectedField" is ignored.');
+        self::assertFalse($provider->isAuditedField(AuditedEntityWithAnnotation::class, 'ignoredPrivateField'), 'Field "'.AuditedEntityWithAnnotation::class.'::$ignoredPrivateField" is ignored.');
+    }
+
+    public function testLoadEntitiesWithAttributesOnly(): void
+    {
+        if (\PHP_VERSION_ID < 80000) {
+            self::markTestSkipped('PHP > 8.0 is required.');
+        }
+
+        $entityManager = $this->createEntityManager(
+            [
+                __DIR__.'/../../../src/Provider/Doctrine/Auditing/Annotation',
+                __DIR__.'/Fixtures/Entity/Attribute',
+            ],
+            'default',
+            null,
+            true
+        );
+        $annotationLoader = new AnnotationLoader($entityManager);
+        $loaded = $annotationLoader->load();
+        self::assertCount(2, $loaded);
+
+        $auditor = $this->createAuditor();
+        $provider = new DoctrineProvider($this->createProviderConfiguration(['entities' => $loaded]));
+        $provider->registerStorageService(new StorageService('default', $entityManager));
+        $provider->registerAuditingService(new AuditingService('default', $entityManager));
+        $auditor->registerProvider($provider);
+
+        self::assertTrue($provider->isAudited(AuditedEntityWithAttribute::class), '"'.AuditedEntityWithAttribute::class.'" is audited.');
+        self::assertTrue($provider->isAuditedField(AuditedEntityWithAttribute::class, 'auditedField'), 'Field "'.AuditedEntityWithAttribute::class.'::$auditedField" is audited.');
+        self::assertFalse($provider->isAuditedField(AuditedEntityWithAttribute::class, 'ignoredField'), 'Field "'.AuditedEntityWithAttribute::class.'::$ignoredField" is ignored.');
+        self::assertFalse($provider->isAuditedField(AuditedEntityWithAttribute::class, 'ignoredProtectedField'), 'Field "'.AuditedEntityWithAttribute::class.'::$ignoredProtectedField" is ignored.');
+        self::assertFalse($provider->isAuditedField(AuditedEntityWithAttribute::class, 'ignoredPrivateField'), 'Field "'.AuditedEntityWithAttribute::class.'::$ignoredPrivateField" is ignored.');
+    }
+
     /**
      * @depends testIsAuditedHonorsEnabledFlag
      */
diff --git a/tests/Provider/Doctrine/Fixtures/Entity/Annotation/AuditedEntity.php b/tests/Provider/Doctrine/Fixtures/Entity/Annotation/AuditedEntity.php
index 428f9b0..3fd49af 100644
--- a/tests/Provider/Doctrine/Fixtures/Entity/Annotation/AuditedEntity.php
+++ b/tests/Provider/Doctrine/Fixtures/Entity/Annotation/AuditedEntity.php
@@ -22,6 +22,16 @@ class AuditedEntity
      */
     public string $ignoredField;
 
+    /**
+     * @Audit\Ignore
+     */
+    protected string $ignoredProtectedField;
+
+    /**
+     * @Audit\Ignore
+     */
+    private string $ignoredPrivateField;
+
     /**
      * @ORM\Id
      * @ORM\GeneratedValue(strategy="IDENTITY")
diff --git a/tests/Provider/Doctrine/Fixtures/Entity/Attribute/AuditedEntity.php b/tests/Provider/Doctrine/Fixtures/Entity/Attribute/AuditedEntity.php
index 8579dc3..69af1df 100644
--- a/tests/Provider/Doctrine/Fixtures/Entity/Attribute/AuditedEntity.php
+++ b/tests/Provider/Doctrine/Fixtures/Entity/Attribute/AuditedEntity.php
@@ -17,6 +17,12 @@ class AuditedEntity
     #[Audit\Ignore]
     public string $ignoredField;
 
+    #[Audit\Ignore]
+    protected string $ignoredProtectedField;
+
+    #[Audit\Ignore]
+    private string $ignoredPrivateField;
+
     #[ORM\Id, ORM\GeneratedValue(strategy: 'IDENTITY')]
     #[ORM\Column(type: 'integer')]
     private int $id;
diff --git a/tests/Provider/Doctrine/Fixtures/Issue101/ChildEntity.php b/tests/Provider/Doctrine/Fixtures/Issue101/ChildEntity.php
new file mode 100644
index 0000000..b35c739
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue101/ChildEntity.php
@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue101;
+
+use DH\Auditor\Provider\Doctrine\Auditing\Annotation as Audit;
+use Doctrine\ORM\Mapping as ORM;
+
+#[ORM\Entity]
+#[Audit\Auditable]
+class ChildEntity extends ParentEntity
+{
+}
diff --git a/tests/Provider/Doctrine/Fixtures/Issue101/ParentEntity.php b/tests/Provider/Doctrine/Fixtures/Issue101/ParentEntity.php
new file mode 100644
index 0000000..1ee6aab
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue101/ParentEntity.php
@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue101;
+
+use DH\Auditor\Provider\Doctrine\Auditing\Annotation as Audit;
+use Doctrine\ORM\Mapping as ORM;
+
+#[ORM\MappedSuperclass]
+#[ORM\Table(name: 'entity')]
+#[Audit\Auditable]
+class ParentEntity
+{
+    public string $auditedField;
+
+    #[Audit\Ignore]
+    public string $ignoredField;
+
+    #[Audit\Ignore]
+    protected string $ignoredProtectedField;
+
+    #[Audit\Ignore]
+    private string $ignoredPrivateField;
+
+    #[ORM\Id, ORM\GeneratedValue(strategy: 'IDENTITY')]
+    #[ORM\Column(type: 'integer')]
+    private int $id;
+}
diff --git a/tests/Provider/Doctrine/Issues/Issue101Test.php b/tests/Provider/Doctrine/Issues/Issue101Test.php
new file mode 100644
index 0000000..5f01391
--- /dev/null
+++ b/tests/Provider/Doctrine/Issues/Issue101Test.php
@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Issues;
+
+use DH\Auditor\Provider\Doctrine\DoctrineProvider;
+use DH\Auditor\Provider\Doctrine\Service\AuditingService;
+use DH\Auditor\Provider\Doctrine\Service\StorageService;
+use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue101\ChildEntity;
+use DH\Auditor\Tests\Provider\Doctrine\Traits\Schema\SchemaSetupTrait;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @internal
+ *
+ * @small
+ */
+final class Issue101Test extends TestCase
+{
+    use SchemaSetupTrait;
+
+    protected function setUp(): void
+    {
+        if (\PHP_VERSION_ID < 80000) {
+            self::markTestSkipped('PHP > 8.0 is required.');
+        }
+
+        parent::setUp();
+
+        // provider with 1 em for both storage and auditing
+        $this->createAndInitDoctrineProvider();
+
+        // declare audited entites
+        $this->configureEntities();
+
+        // setup entity and audit schemas
+        $this->setupEntitySchemas();
+        $this->setupAuditSchemas();
+
+        // setup (seed) entities
+        $this->setupEntities();
+    }
+
+    public function testIssue101(): void
+    {
+        self::assertTrue($this->provider->isAudited(ChildEntity::class), '"'.ChildEntity::class.'" is audited.');
+        self::assertTrue($this->provider->isAuditedField(ChildEntity::class, 'auditedField'), 'Field "'.ChildEntity::class.'::$auditedField" is audited.');
+        self::assertFalse($this->provider->isAuditedField(ChildEntity::class, 'ignoredField'), 'Field "'.ChildEntity::class.'::$ignoredField" is ignored.');
+        self::assertFalse($this->provider->isAuditedField(ChildEntity::class, 'ignoredProtectedField'), 'Field "'.ChildEntity::class.'::$ignoredProtectedField" is ignored.');
+        self::assertFalse($this->provider->isAuditedField(ChildEntity::class, 'ignoredPrivateField'), 'Field "'.ChildEntity::class.'::$ignoredPrivateField" is ignored.');
+    }
+
+    private function createAndInitDoctrineProvider(): void
+    {
+        $auditor = $this->createAuditor();
+        $this->provider = new DoctrineProvider($this->createProviderConfiguration());
+
+        $entityManager = $this->createEntityManager(
+            [
+                __DIR__.'/../../../../src/Provider/Doctrine/Auditing/Annotation',
+                __DIR__.'/../Fixtures/Issue101',
+            ],
+            'default',
+            null,
+            true
+        );
+
+        $this->provider->registerStorageService(new StorageService('default', $entityManager));
+        $this->provider->registerAuditingService(new AuditingService('default', $entityManager));
+
+        $auditor->registerProvider($this->provider);
+    }
+}