Repository: DamienHarper/auditor (php)

Issue:
Argument 1 passed to DH\\Auditor\\Model\\Entry::sort() must be of the type array, null given
<!--
- 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    | 4.0.3
| PHP version          | 7.3
| Database             | MariaDB

#### Summary

> Argument 1 passed to DH\\Auditor\\Model\\Entry::sort() must be of the type array, null given, called in /var/www/app/vendor/damienharper/auditor/src/Model/Entry.php on line 160

When viewing audit log for an entity that have done a insert and does not have any diff.

#### How to reproduce

Use the viewer and press "view audit" on a entity with entries that do not have a diff.

#### Expected behavior

Not an error


Pull request description:
Better handling of binary data in changeset
Should fix #18 

Patch:
diff --git a/phpstan.neon b/phpstan.neon
index a9adbb9..ccc636b 100644
--- a/phpstan.neon
+++ b/phpstan.neon
@@ -15,10 +15,8 @@ parameters:
         # false positives
         - '~Parameter \#1 \$name of method Symfony\\Component\\Console\\Command\\Command\:\:setName\(\) expects string, string\|null given~'
         - '~Parameter \#1 \$(first|max)Results? of method Doctrine\\DBAL\\Query\\QueryBuilder\:\:set(First|Max)Results?\(\) expects int, null given~'
-        - '~Parameter \#1 \$event of method Symfony\\Contracts\\EventDispatcher\\EventDispatcherInterface\:\:dispatch\(\) expects object, string given~'
-        - '~Parameter \#2 \$eventName of method Symfony\\Contracts\\EventDispatcher\\EventDispatcherInterface\:\:dispatch\(\) expects string\|null, DH\\Auditor\\Event\\LifecycleEvent given\.~'
         - '~Parameter \#1 \$entity of method DH\\Auditor\\Provider\\Doctrine\\Persistence\\Schema\\SchemaManager\:\:(create|update)AuditTable\(\) expects string, string\|null given.~'
         - '~Cannot call method setFetchMode\(\) on Doctrine\\DBAL\\Driver\\ResultStatement\|int~'
-        - '~Cannot call method fetchColumn\(\) on Doctrine\\DBAL\\Driver\\ResultStatement\|int~'
         - '~Cannot call method fetchAll\(\) on Doctrine\\DBAL\\Driver\\ResultStatement\|int~'
         - '~Parameter \#1 \$keep of method DH\\Auditor\\Provider\\Doctrine\\Persistence\\Command\\CleanAuditLogsCommand\:\:validateKeepArgument\(\) expects string, string\|null given~'
+        - '~Parameter \#1 \$eventName of method Symfony\\Component\\EventDispatcher\\EventDispatcherInterface\:\:dispatch\(\) expects string, DH\\Auditor\\Event\\LifecycleEvent given~'
diff --git a/src/Provider/Doctrine/Auditing/Event/DoctrineSubscriber.php b/src/Provider/Doctrine/Auditing/Event/DoctrineSubscriber.php
index 6df57d2..1d71d82 100644
--- a/src/Provider/Doctrine/Auditing/Event/DoctrineSubscriber.php
+++ b/src/Provider/Doctrine/Auditing/Event/DoctrineSubscriber.php
@@ -14,7 +14,7 @@ use Doctrine\ORM\Events;
 class DoctrineSubscriber implements EventSubscriber
 {
     /**
-     * @var \DH\Auditor\Provider\Doctrine\Auditing\Transaction\TransactionManager
+     * @var TransactionManager
      */
     private $transactionManager;
 
diff --git a/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php b/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php
index 03e08cc..90bca9b 100644
--- a/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php
+++ b/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php
@@ -98,18 +98,18 @@ trait AuditTrait
             $n = null;
 
             if (
-                !isset($meta->embeddedClasses[$fieldName]) &&
-                $meta->hasField($fieldName) &&
-                $this->provider->isAuditedField($entity, $fieldName)
+                !isset($meta->embeddedClasses[$fieldName])
+                && $meta->hasField($fieldName)
+                && $this->provider->isAuditedField($entity, $fieldName)
             ) {
                 $mapping = $meta->fieldMappings[$fieldName];
                 $type = Type::getType($mapping['type']);
                 $o = $this->value($entityManager, $type, $old);
                 $n = $this->value($entityManager, $type, $new);
             } elseif (
-                $meta->hasAssociation($fieldName) &&
-                $meta->isSingleValuedAssociation($fieldName) &&
-                $this->provider->isAuditedField($entity, $fieldName)
+                $meta->hasAssociation($fieldName)
+                && $meta->isSingleValuedAssociation($fieldName)
+                && $this->provider->isAuditedField($entity, $fieldName)
             ) {
                 $o = $this->summarize($entityManager, $old);
                 $n = $this->summarize($entityManager, $new);
diff --git a/src/Provider/Doctrine/Auditing/Transaction/TransactionProcessor.php b/src/Provider/Doctrine/Auditing/Transaction/TransactionProcessor.php
index 1db79dd..ed215ef 100644
--- a/src/Provider/Doctrine/Auditing/Transaction/TransactionProcessor.php
+++ b/src/Provider/Doctrine/Auditing/Transaction/TransactionProcessor.php
@@ -214,6 +214,22 @@ class TransactionProcessor implements TransactionProcessorInterface
         $auditTable = $schema.$configuration->getTablePrefix().$data['table'].$configuration->getTableSuffix();
         $dt = new DateTime('now', new DateTimeZone($this->provider->getAuditor()->getConfiguration()->getTimezone()));
 
+        $diffs = json_encode($data['diff']);
+        if (false === $diffs) {
+            // json_encode failed, probably because of an unsupported type (resource type)
+            // let's replace resources with a "simple" representation
+            foreach ($data['diff'] as $property => $changeset) {
+                if (isset($changeset['old']) && \is_resource($changeset['old'])) {
+                    $data['diff'][$property]['old'] = get_resource_type($changeset['old']).'#'.get_resource_id($changeset['old']);
+                }
+                if (isset($changeset['new']) && \is_resource($changeset['new'])) {
+                    $data['diff'][$property]['new'] = get_resource_type($changeset['new']).'#'.get_resource_id($changeset['new']);
+                }
+            }
+
+            $diffs = json_encode($data['diff']);
+        }
+
         $payload = [
             'entity' => $data['entity'],
             'table' => $auditTable,
@@ -221,7 +237,7 @@ class TransactionProcessor implements TransactionProcessorInterface
             'object_id' => (string) $data['id'],
             'discriminator' => $data['discriminator'],
             'transaction_hash' => (string) $data['transaction_hash'],
-            'diffs' => json_encode($data['diff']),
+            'diffs' => $diffs,
             'blame_id' => $data['blame']['user_id'],
             'blame_user' => $data['blame']['username'],
             'blame_user_fqdn' => $data['blame']['user_fqdn'],
diff --git a/src/Provider/Doctrine/DoctrineProvider.php b/src/Provider/Doctrine/DoctrineProvider.php
index 5f4ddad..2d20167 100644
--- a/src/Provider/Doctrine/DoctrineProvider.php
+++ b/src/Provider/Doctrine/DoctrineProvider.php
@@ -202,8 +202,8 @@ class DoctrineProvider extends AbstractProvider
         }
 
         // are columns excluded and is field part of them?
-        if (isset($entityOptions['ignored_columns']) &&
-            \in_array($field, $entityOptions['ignored_columns'], true)) {
+        if (isset($entityOptions['ignored_columns'])
+            && \in_array($field, $entityOptions['ignored_columns'], true)) {
             // yes => $field is not audited
             return false;
         }
diff --git a/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php b/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php
index b27e780..595e975 100644
--- a/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php
+++ b/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php
@@ -44,8 +44,8 @@ class CreateSchemaListener implements EventSubscriber
         if (!$this->provider->isAuditable($metadata->name)) {
             $audited = false;
             if (
-                $metadata->rootEntityName === $metadata->name &&
-                ClassMetadataInfo::INHERITANCE_TYPE_SINGLE_TABLE === $metadata->inheritanceType
+                $metadata->rootEntityName === $metadata->name
+                && ClassMetadataInfo::INHERITANCE_TYPE_SINGLE_TABLE === $metadata->inheritanceType
             ) {
                 foreach ($metadata->subClasses as $subClass) {
                     if ($this->provider->isAuditable($subClass)) {
diff --git a/src/Provider/Doctrine/Persistence/Reader/Reader.php b/src/Provider/Doctrine/Persistence/Reader/Reader.php
index 2be3784..bbbdd17 100644
--- a/src/Provider/Doctrine/Persistence/Reader/Reader.php
+++ b/src/Provider/Doctrine/Persistence/Reader/Reader.php
@@ -22,7 +22,7 @@ class Reader
     private $provider;
 
     /**
-     * AuditReader constructor.
+     * Reader constructor.
      */
     public function __construct(DoctrineProvider $provider)
     {
@@ -71,9 +71,9 @@ class Reader
 
         $metadata = $entityManager->getClassMetadata($entity);
         if (
-            $config['strict'] &&
-            $metadata instanceof ORMMetadata &&
-            ORMMetadata::INHERITANCE_TYPE_SINGLE_TABLE === $metadata->inheritanceType
+            $config['strict']
+            && $metadata instanceof ORMMetadata
+            && ORMMetadata::INHERITANCE_TYPE_SINGLE_TABLE === $metadata->inheritanceType
         ) {
             $query->addFilter(Query::DISCRIMINATOR, $entity);
         }


Test patch:
diff --git a/tests/Provider/Doctrine/Auditing/Transaction/TransactionProcessorTest.php b/tests/Provider/Doctrine/Auditing/Transaction/TransactionProcessorTest.php
index c2bece4..2485754 100644
--- a/tests/Provider/Doctrine/Auditing/Transaction/TransactionProcessorTest.php
+++ b/tests/Provider/Doctrine/Auditing/Transaction/TransactionProcessorTest.php
@@ -20,8 +20,8 @@ use PHPUnit\Framework\TestCase;
  */
 final class TransactionProcessorTest extends TestCase
 {
-    use ReflectionTrait;
     use DefaultSchemaSetupTrait;
+    use ReflectionTrait;
 
     public function testInsert(): void
     {
diff --git a/tests/Provider/Doctrine/Fixtures/Issue18/DataObject.php b/tests/Provider/Doctrine/Fixtures/Issue18/DataObject.php
new file mode 100644
index 0000000..479c27f
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue18/DataObject.php
@@ -0,0 +1,68 @@
+<?php
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue18;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(name="data_object")
+ */
+class DataObject
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer", options={"unsigned": true})
+     * @ORM\GeneratedValue(strategy="IDENTITY")
+     */
+    protected $id;
+
+    /**
+     * @ORM\Column(type="binary")
+     */
+    protected $data;
+
+    /**
+     * Get the value of id.
+     *
+     * @return mixed
+     */
+    public function getId(): int
+    {
+        return $this->id;
+    }
+
+    /**
+     * Set the value of id.
+     *
+     * @return DataObject
+     */
+    public function setId(int $id): self
+    {
+        $this->id = $id;
+
+        return $this;
+    }
+
+    /**
+     * Get the value of data.
+     */
+    public function getData()
+    {
+        return $this->data;
+    }
+
+    /**
+     * Set the value of data.
+     *
+     * @param mixed $data
+     *
+     * @return DataObject
+     */
+    public function setData($data): self
+    {
+        $this->data = $data;
+
+        return $this;
+    }
+}
diff --git a/tests/Provider/Doctrine/Fixtures/Issue18/file1.txt b/tests/Provider/Doctrine/Fixtures/Issue18/file1.txt
new file mode 100644
index 0000000..f079749
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue18/file1.txt
@@ -0,0 +1,1 @@
+test1
\ No newline at end of file
diff --git a/tests/Provider/Doctrine/Fixtures/Issue18/file2.txt b/tests/Provider/Doctrine/Fixtures/Issue18/file2.txt
new file mode 100644
index 0000000..d606037
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue18/file2.txt
@@ -0,0 +1,1 @@
+test2
\ No newline at end of file
diff --git a/tests/Provider/Doctrine/InheritanceTest.php b/tests/Provider/Doctrine/InheritanceTest.php
index 7a6fc24..25271a0 100644
--- a/tests/Provider/Doctrine/InheritanceTest.php
+++ b/tests/Provider/Doctrine/InheritanceTest.php
@@ -19,8 +19,8 @@ use PHPUnit\Framework\TestCase;
  */
 final class InheritanceTest extends TestCase
 {
-    use SchemaSetupTrait;
     use ReaderTrait;
+    use SchemaSetupTrait;
 
     public function testAuditingSubclass(): void
     {
diff --git a/tests/Provider/Doctrine/Issues/Issue18Test.php b/tests/Provider/Doctrine/Issues/Issue18Test.php
new file mode 100644
index 0000000..7d179d9
--- /dev/null
+++ b/tests/Provider/Doctrine/Issues/Issue18Test.php
@@ -0,0 +1,70 @@
+<?php
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Issues;
+
+use DH\Auditor\Model\Transaction;
+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\Issue18\DataObject;
+use DH\Auditor\Tests\Provider\Doctrine\Traits\ReaderTrait;
+use DH\Auditor\Tests\Provider\Doctrine\Traits\Schema\SchemaSetupTrait;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @internal
+ */
+final class Issue18Test extends TestCase
+{
+    use ReaderTrait;
+    use SchemaSetupTrait;
+
+    public function testIssue18(): void
+    {
+        $storageServices = [
+            DataObject::class => $this->provider->getStorageServiceForEntity(DataObject::class),
+        ];
+
+        $reader = $this->createReader();
+
+        $dataObject = new DataObject();
+        $dataObject->setId(1);
+        $dataObject->setData(fopen(__DIR__.'/../Fixtures/Issue18/file1.txt', 'r'));
+        $storageServices[DataObject::class]->getEntityManager()->persist($dataObject);
+        $this->flushAll($storageServices);
+
+        $dataObject->setData(fopen(__DIR__.'/../Fixtures/Issue18/file2.txt', 'r'));
+        $storageServices[DataObject::class]->getEntityManager()->persist($dataObject);
+        $this->flushAll($storageServices);
+
+        $audits = $reader->createQuery(DataObject::class)->execute();
+        self::assertCount(2, $audits, 'results count ok.');
+        self::assertSame(Transaction::UPDATE, $audits[0]->getType(), 'Transaction::UPDATE operation.');
+        self::assertSame(Transaction::INSERT, $audits[1]->getType(), 'Transaction::INSERT operation.');
+
+        self::assertIsArray($audits[0]->getDiffs(), 'Valid diffs');
+        self::assertIsArray($audits[1]->getDiffs(), 'Valid diffs');
+    }
+
+    private function createAndInitDoctrineProvider(): void
+    {
+        $auditor = $this->createAuditor();
+        $this->provider = new DoctrineProvider($this->createProviderConfiguration());
+
+        $entityManager = $this->createEntityManager([
+            __DIR__.'/../../../../src/Provider/Doctrine/Auditing/Annotation',
+            __DIR__.'/../Fixtures/Issue18',
+        ]);
+        $this->provider->registerStorageService(new StorageService('default', $entityManager));
+        $this->provider->registerAuditingService(new AuditingService('default', $entityManager));
+
+        $auditor->registerProvider($this->provider);
+    }
+
+    private function configureEntities(): void
+    {
+        $this->provider->getConfiguration()->setEntities([
+            DataObject::class => ['enabled' => true],
+        ]);
+    }
+}
diff --git a/tests/Provider/Doctrine/Issues/Issue37Test.php b/tests/Provider/Doctrine/Issues/Issue37Test.php
index bc5bb3f..4ddf3c5 100644
--- a/tests/Provider/Doctrine/Issues/Issue37Test.php
+++ b/tests/Provider/Doctrine/Issues/Issue37Test.php
@@ -16,8 +16,8 @@ use PHPUnit\Framework\TestCase;
  */
 final class Issue37Test extends TestCase
 {
-    use SchemaSetupTrait;
     use ReaderTrait;
+    use SchemaSetupTrait;
 
     public function testIssue37(): void
     {
@@ -46,8 +46,8 @@ final class Issue37Test extends TestCase
 
         $audits = $reader->createQuery(Locale::class)->execute();
         self::assertCount(2, $audits, 'results count ok.');
-        self::assertSame('en_US', $audits[0]->getObjectId(), 'AuditEntry::object_id is a string.');
-        self::assertSame('fr_FR', $audits[1]->getObjectId(), 'AuditEntry::object_id is a string.');
+        self::assertSame('en_US', $audits[0]->getObjectId(), 'Entry::object_id is a string.');
+        self::assertSame('fr_FR', $audits[1]->getObjectId(), 'Entry::object_id is a string.');
 
         $user1 = new User();
         $user1
@@ -80,14 +80,6 @@ final class Issue37Test extends TestCase
         ]);
         $this->provider->registerStorageService(new StorageService('default', $entityManager));
         $this->provider->registerAuditingService(new AuditingService('default', $entityManager));
-//        $this->provider->registerEntityManager(
-//            $this->createEntityManager([
-//                __DIR__.'/../../../../src/Provider/Doctrine/Auditing/Annotation',
-//                __DIR__.'/../Fixtures/Issue37',
-//            ]),
-//            DoctrineProvider::BOTH,
-//            'default'
-//        );
 
         $auditor->registerProvider($this->provider);
     }
@@ -99,30 +91,4 @@ final class Issue37Test extends TestCase
             User::class => ['enabled' => true],
         ]);
     }
-
-//    public function testIssue40(): void
-//    {
-//        $em = $this->getEntityManager();
-//        $reader = $this->getReader($this->getAuditConfiguration());
-//
-//        $coreCase = new CoreCase();
-//        $coreCase->type = 'type1';
-//        $coreCase->status = 'status1';
-//        $em->persist($coreCase);
-//        $this->flushAll($entityManagers);
-//
-//        $dieselCase = new DieselCase();
-//        $dieselCase->coreCase = $coreCase;
-//        $dieselCase->setName('yo');
-//        $em->persist($dieselCase);
-//        $this->flushAll($entityManagers);
-//
-//        $audits = $reader->getAudits(CoreCase::class);
-//        self::assertCount(1, $audits, 'results count ok.');
-//        self::assertSame(Reader::INSERT, $audits[0]->getType(), 'AuditReader::INSERT operation.');
-//
-//        $audits = $reader->getAudits(DieselCase::class);
-//        self::assertCount(1, $audits, 'results count ok.');
-//        self::assertSame(Reader::INSERT, $audits[0]->getType(), 'AuditReader::INSERT operation.');
-//    }
 }
diff --git a/tests/Provider/Doctrine/Issues/Issue40Test.php b/tests/Provider/Doctrine/Issues/Issue40Test.php
index 42b213e..b92b98c 100644
--- a/tests/Provider/Doctrine/Issues/Issue40Test.php
+++ b/tests/Provider/Doctrine/Issues/Issue40Test.php
@@ -17,8 +17,8 @@ use PHPUnit\Framework\TestCase;
  */
 final class Issue40Test extends TestCase
 {
-    use SchemaSetupTrait;
     use ReaderTrait;
+    use SchemaSetupTrait;
 
     public function testIssue40(): void
     {
@@ -43,11 +43,11 @@ final class Issue40Test extends TestCase
 
         $audits = $reader->createQuery(CoreCase::class)->execute();
         self::assertCount(1, $audits, 'results count ok.');
-        self::assertSame(Transaction::INSERT, $audits[0]->getType(), 'AuditReader::INSERT operation.');
+        self::assertSame(Transaction::INSERT, $audits[0]->getType(), 'Reader::INSERT operation.');
 
         $audits = $reader->createQuery(DieselCase::class)->execute();
         self::assertCount(1, $audits, 'results count ok.');
-        self::assertSame(Transaction::INSERT, $audits[0]->getType(), 'AuditReader::INSERT operation.');
+        self::assertSame(Transaction::INSERT, $audits[0]->getType(), 'Reader::INSERT operation.');
     }
 
     private function createAndInitDoctrineProvider(): void
@@ -61,14 +61,6 @@ final class Issue40Test extends TestCase
         ]);
         $this->provider->registerStorageService(new StorageService('default', $entityManager));
         $this->provider->registerAuditingService(new AuditingService('default', $entityManager));
-//        $this->provider->registerEntityManager(
-//            $this->createEntityManager([
-//                __DIR__.'/../../../../src/Provider/Doctrine/Auditing/Annotation',
-//                __DIR__.'/../Fixtures/Issue40',
-//            ]),
-//            DoctrineProvider::BOTH,
-//            'default'
-//        );
 
         $auditor->registerProvider($this->provider);
     }
diff --git a/tests/Provider/Doctrine/Persistence/Reader/ReaderTest.php b/tests/Provider/Doctrine/Persistence/Reader/ReaderTest.php
index 7c4a914..c35fc45 100644
--- a/tests/Provider/Doctrine/Persistence/Reader/ReaderTest.php
+++ b/tests/Provider/Doctrine/Persistence/Reader/ReaderTest.php
@@ -412,8 +412,8 @@ final class ReaderTest extends TestCase
         $reader = $this->createReader();
         $audits = $reader->getAuditsByTransactionHash($hash);
 
-        self::assertCount(2, $audits, 'AuditReader::getAllAuditsByTransactionHash() is ok.');
-        self::assertCount(1, $audits[Author::class], 'AuditReader::getAllAuditsByTransactionHash() is ok.');
-        self::assertCount(2, $audits[Post::class], 'AuditReader::getAllAuditsByTransactionHash() is ok.');
+        self::assertCount(2, $audits, 'Reader::getAllAuditsByTransactionHash() is ok.');
+        self::assertCount(1, $audits[Author::class], 'Reader::getAllAuditsByTransactionHash() is ok.');
+        self::assertCount(2, $audits[Post::class], 'Reader::getAllAuditsByTransactionHash() is ok.');
     }
 }