Allow composite identifiers in loggable

Composite identifiers with foreign entities Co-authored-by: Javier Spagnoletti <[email protected]>
doctrine-extensions · Jul 8, 2023 · 5ee410d · 5ee410d
1 parent 9c46ada
commit 5ee410d
Show file tree

Hide file tree

Showing 36 changed files with 1,074 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ a release.
 ### Added
 - Tree: `setSibling()` and `getSibling()` methods in the `Node` interface through the BC `@method` annotation
 - Tree: Support array of fields and directions in the `$sortByField` and `$direction` parameters at `AbstractTreeRepository::recover()`
+- Loggable: Support for composite identifiers.
 
 ### Changed
 - Named arguments have precedence over the values passed in the `$data` array in annotation classes at `Gedmo\Mapping\Annotation\`
@@ -34,6 +35,8 @@ a release.
 
 ### Deprecated
 - Tree: Not implementing `Node` interface in classes that are used as nodes
+- Implementing the `Gedmo\Tool\WrapperInterface::getIdentifier()` method without the second argument (`$flatten`) is deprecated and will
+  be required in version 4.0.
 
 ## [3.11.1] - 2023-02-20
 ### Fixed

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -65,6 +65,11 @@ parameters:
 			count: 4
 			path: src/Loggable/LoggableListener.php
 
+		-
+			message: "#^Method Gedmo\\\\Tool\\\\WrapperInterface\\<Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\>\\:\\:getIdentifier\\(\\) invoked with 2 parameters, 0-1 required\\.#"
+			count: 2
+			path: src/Loggable/LoggableListener.php
+
 		-
 			message: "#^Access to an undefined property Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\:\\:\\$associationMappings\\.$#"
 			count: 1
@@ -355,11 +360,26 @@ parameters:
 			count: 1
 			path: src/Timestampable/Mapping/Driver/Yaml.php
 
+		-
+			message: "#^PHPDoc tag \\@param references unknown parameter\\: \\$flatten$#"
+			count: 1
+			path: src/Tool/WrapperInterface.php
+
 		-
 			message: "#^Call to an undefined method Doctrine\\\\Persistence\\\\ObjectManager\\:\\:getUnitOfWork\\(\\)\\.$#"
 			count: 1
 			path: src/Tool/Wrapper/EntityWrapper.php
 
+		-
+			message: "#^Method Gedmo\\\\Tool\\\\Wrapper\\\\EntityWrapper\\:\\:getIdentifier\\(\\) has parameter \\$flatten with no type specified\\.$#"
+			count: 1
+			path: src/Tool/Wrapper/EntityWrapper.php
+
+		-
+			message: "#^Parameter \\#2 \\$em of class Gedmo\\\\Tool\\\\Wrapper\\\\EntityWrapper constructor expects Doctrine\\\\ORM\\\\EntityManagerInterface, Doctrine\\\\Persistence\\\\ObjectManager given\\.$#"
+			count: 1
+			path: src/Tool/Wrapper/EntityWrapper.php
+
 		-
 			message: "#^Access to an undefined property ProxyManager\\\\Proxy\\\\GhostObjectInterface\\:\\:\\$identifier\\.$#"
 			count: 1
@@ -370,6 +390,11 @@ parameters:
 			count: 2
 			path: src/Tool/Wrapper/MongoDocumentWrapper.php
 
+		-
+			message: "#^Method Gedmo\\\\Tool\\\\Wrapper\\\\MongoDocumentWrapper\\:\\:getIdentifier\\(\\) has parameter \\$flatten with no type specified\\.$#"
+			count: 1
+			path: src/Tool/Wrapper/MongoDocumentWrapper.php
+
 		-
 			message: "#^Access to offset 'association' on an unknown class Doctrine\\\\ODM\\\\MongoDB\\\\Mapping\\\\FieldMapping\\.$#"
 			count: 2

diff --git a/src/Loggable/Document/Repository/LogEntryRepository.php b/src/Loggable/Document/Repository/LogEntryRepository.php
@@ -106,10 +106,11 @@ public function revert($document, $version = 1)
             throw new UnexpectedValueException('Count not find any log entries under version: '.$version);
         }
 
-        $data = [];
+        $data = [[]];
         while ($log = array_shift($logs)) {
-            $data = array_merge($data, $log->getData());
+            $data[] = $log->getData();
         }
+        $data = array_merge(...$data);
         $this->fillDocument($document, $data);
     }
 

diff --git a/src/Loggable/Entity/Repository/LogEntryRepository.php b/src/Loggable/Entity/Repository/LogEntryRepository.php
@@ -77,7 +77,7 @@ public function getLogEntriesQuery($entity)
         $dql .= ' AND log.objectClass = :objectClass';
         $dql .= ' ORDER BY log.version DESC';
 
-        $objectId = (string) $wrapped->getIdentifier();
+        $objectId = (string) $wrapped->getIdentifier(false, true);
         $q = $this->_em->createQuery($dql);
         $q->setParameters(compact('objectId', 'objectClass'));
 
@@ -111,7 +111,7 @@ public function revert($entity, $version = 1)
         $dql .= ' AND log.version <= :version';
         $dql .= ' ORDER BY log.version ASC';
 
-        $objectId = (string) $wrapped->getIdentifier();
+        $objectId = (string) $wrapped->getIdentifier(false, true);
         $q = $this->_em->createQuery($dql);
         $q->setParameters(compact('objectId', 'objectClass', 'version'));
         $logs = $q->getResult();

diff --git a/src/Loggable/LoggableListener.php b/src/Loggable/LoggableListener.php
@@ -10,9 +10,11 @@
 namespace Gedmo\Loggable;
 
 use Doctrine\Common\EventArgs;
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\Persistence\Event\LoadClassMetadataEventArgs;
 use Doctrine\Persistence\ObjectManager;
 use Gedmo\Exception\InvalidArgumentException;
+use Gedmo\Loggable\Entity\LogEntry;
 use Gedmo\Loggable\Mapping\Event\LoggableAdapter;
 use Gedmo\Mapping\MappedEventSubscriber;
 use Gedmo\Tool\Wrapper\AbstractWrapper;
@@ -150,7 +152,7 @@ public function postPersist(EventArgs $args)
             $logEntry = $this->pendingLogEntryInserts[$oid];
             $logEntryMeta = $om->getClassMetadata(get_class($logEntry));
 
-            $id = $wrapped->getIdentifier();
+            $id = $wrapped->getIdentifier(false, true);
             $logEntryMeta->getReflectionProperty('objectId')->setValue($logEntry, $id);
             $uow->scheduleExtraUpdate($logEntry, [
                 'objectId' => [null, $id],
@@ -320,10 +322,10 @@ protected function createLogEntry($action, $object, LoggableAdapter $ea)
 
             // check for the availability of the primary key
             $uow = $om->getUnitOfWork();
-            if (LogEntryInterface::ACTION_CREATE === $action && $ea->isPostInsertGenerator($meta)) {
+            if (LogEntryInterface::ACTION_CREATE === $action && ($ea->isPostInsertGenerator($meta) || ($meta instanceof ClassMetadata && $meta->isIdentifierComposite))) {
                 $this->pendingLogEntryInserts[spl_object_id($object)] = $logEntry;
             } else {
-                $logEntry->setObjectId($wrapped->getIdentifier());
+                $logEntry->setObjectId($wrapped->getIdentifier(false, true));
             }
             $newValues = [];
             if (LogEntryInterface::ACTION_REMOVE !== $action && isset($config['versioned'])) {

diff --git a/src/Loggable/Mapping/Driver/Annotation.php b/src/Loggable/Mapping/Driver/Annotation.php
@@ -9,6 +9,7 @@
 
 namespace Gedmo\Loggable\Mapping\Driver;
 
+use Doctrine\ODM\MongoDB\Mapping\ClassMetadata as ClassMetadataODM;
 use Doctrine\Persistence\Mapping\ClassMetadata;
 use Gedmo\Exception\InvalidMappingException;
 use Gedmo\Mapping\Annotation\Loggable;
@@ -40,7 +41,7 @@ class Annotation extends AbstractAnnotationDriver
 
     public function validateFullMetadata(ClassMetadata $meta, array $config)
     {
-        if ($config && is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
+        if ($config && $meta instanceof ClassMetadataODM && is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
             throw new InvalidMappingException("Loggable does not support composite identifiers in class - {$meta->getName()}");
         }
         if (isset($config['versioned']) && !isset($config['loggable'])) {
@@ -87,7 +88,7 @@ public function readExtendedMetadata($meta, array &$config)
         }
 
         if (!$meta->isMappedSuperclass && $config) {
-            if (is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
+            if ($meta instanceof ClassMetadataODM && is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
                 throw new InvalidMappingException("Loggable does not support composite identifiers in class - {$meta->getName()}");
             }
             if ($this->isClassAnnotationInValid($meta, $config)) {

diff --git a/src/Loggable/Mapping/Driver/Xml.php b/src/Loggable/Mapping/Driver/Xml.php
@@ -9,6 +9,7 @@
 
 namespace Gedmo\Loggable\Mapping\Driver;
 
+use Doctrine\ODM\MongoDB\Mapping\ClassMetadata as ClassMetadataODM;
 use Doctrine\Persistence\Mapping\ClassMetadata;
 use Gedmo\Exception\InvalidMappingException;
 use Gedmo\Mapping\Driver\Xml as BaseXml;
@@ -37,7 +38,7 @@ public function readExtendedMetadata($meta, array &$config)
 
         $xml = $xml->children(self::GEDMO_NAMESPACE_URI);
 
-        if ('entity' === $xmlDoctrine->getName() || 'document' === $xmlDoctrine->getName() || 'mapped-superclass' === $xmlDoctrine->getName()) {
+        if (in_array($xmlDoctrine->getName(), ['mapped-superclass', 'entity', 'document'], true)) {
             if (isset($xml->loggable)) {
                 /**
                  * @var \SimpleXMLElement;
@@ -74,7 +75,7 @@ public function readExtendedMetadata($meta, array &$config)
         }
 
         if (!$meta->isMappedSuperclass && $config) {
-            if (is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
+            if ($meta instanceof ClassMetadataODM && is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
                 throw new InvalidMappingException("Loggable does not support composite identifiers in class - {$meta->getName()}");
             }
             if (isset($config['versioned']) && !isset($config['loggable'])) {

diff --git a/src/Loggable/Mapping/Driver/Yaml.php b/src/Loggable/Mapping/Driver/Yaml.php
@@ -9,6 +9,7 @@
 
 namespace Gedmo\Loggable\Mapping\Driver;
 
+use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
 use Gedmo\Exception\InvalidMappingException;
 use Gedmo\Mapping\Driver;
 use Gedmo\Mapping\Driver\File;
@@ -26,7 +27,7 @@
  *
  * @internal
  */
-class Yaml extends File implements Driver
+class Yaml extends File
 {
     /**
      * File extension
@@ -124,11 +125,11 @@ public function readExtendedMetadata($meta, array &$config)
         }
 
         if (!$meta->isMappedSuperclass && $config) {
-            if (is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
+            if ($meta instanceof ClassMetadata && is_array($meta->getIdentifier()) && count($meta->getIdentifier()) > 1) {
                 throw new InvalidMappingException("Loggable does not support composite identifiers in class - {$meta->getName()}");
             }
             if (isset($config['versioned']) && !isset($config['loggable'])) {
-                throw new InvalidMappingException("Class must be annoted with Loggable annotation in order to track versioned fields in class - {$meta->getName()}");
+                throw new InvalidMappingException("Class must be annotated with Loggable annotation in order to track versioned fields in class - {$meta->getName()}");
             }
         }
     }

diff --git a/src/Loggable/Mapping/Event/Adapter/ORM.php b/src/Loggable/Mapping/Event/Adapter/ORM.php
@@ -13,6 +13,7 @@
 use Gedmo\Loggable\Entity\LogEntry;
 use Gedmo\Loggable\Mapping\Event\LoggableAdapter;
 use Gedmo\Mapping\Event\Adapter\ORM as BaseAdapterORM;
+use Gedmo\Tool\Wrapper\EntityWrapper;
 
 /**
  * Doctrine event adapter for ORM adapted
@@ -39,8 +40,8 @@ public function getNewVersion($meta, $object)
     {
         $em = $this->getObjectManager();
         $objectMeta = $em->getClassMetadata(get_class($object));
-        $identifierField = $this->getSingleIdentifierFieldName($objectMeta);
-        $objectId = (string) $objectMeta->getReflectionProperty($identifierField)->getValue($object);
+        $wrapper = new EntityWrapper($object, $em);
+        $objectId = $wrapper->getIdentifier(false, true);
 
         $dql = "SELECT MAX(log.version) FROM {$meta->getName()} log";
         $dql .= ' WHERE log.objectId = :objectId';

diff --git a/src/References/Mapping/Driver/Xml.php b/src/References/Mapping/Driver/Xml.php
@@ -51,7 +51,7 @@ public function readExtendedMetadata($meta, array &$config)
 
         $xml = $xml->children(self::GEDMO_NAMESPACE_URI);
 
-        if ('entity' === $xmlDoctrine->getName() || 'document' === $xmlDoctrine->getName() || 'mapped-superclass' === $xmlDoctrine->getName()) {
+        if (in_array($xmlDoctrine->getName(), ['mapped-superclass', 'entity', 'document'], true)) {
             if (isset($xml->reference)) {
                 /**
                  * @var \SimpleXMLElement

diff --git a/src/Tool/Wrapper/EntityWrapper.php b/src/Tool/Wrapper/EntityWrapper.php
@@ -9,6 +9,7 @@
 
 namespace Gedmo\Tool\Wrapper;
 
+use Doctrine\Common\Util\ClassUtils;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Proxy\Proxy;
@@ -77,34 +78,33 @@ public function getRootObjectName()
         return $this->meta->rootEntityName;
     }
 
-    public function getIdentifier($single = true)
+    public function getIdentifier($single = true, $flatten = false)
     {
+        $flatten = 1 < \func_num_args() && true === func_get_arg(1);
         if (null === $this->identifier) {
-            if ($this->object instanceof Proxy) {
-                $uow = $this->om->getUnitOfWork();
-                if ($uow->isInIdentityMap($this->object)) {
-                    $this->identifier = $uow->getEntityIdentifier($this->object);
-                } else {
-                    $this->initialize();
-                }
+            $uow = $this->om->getUnitOfWork();
+            $this->identifier = $uow->isInIdentityMap($this->object)
+                ? $uow->getEntityIdentifier($this->object)
+                : $this->meta->getIdentifierValues($this->object);
+            if (is_array($this->identifier) && empty($this->identifier)) {
+                $this->identifier = null;
+            }
+        }
+        if (is_array($this->identifier)) {
+            if ($single) {
+                return reset($this->identifier);
             }
-            if (null === $this->identifier) {
-                $this->identifier = [];
-                $incomplete = false;
-                foreach ($this->meta->identifier as $name) {
-                    $this->identifier[$name] = $this->getPropertyValue($name);
-                    if (null === $this->identifier[$name]) {
-                        $incomplete = true;
+            if ($flatten) {
+                $id = $this->identifier;
+                foreach ($id as $i => $value) {
+                    if (is_object($value) && $this->om->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($value))) {
+                        $id[$i] = (new self($value, $this->om))->getIdentifier(false, true);
                     }
                 }
-                if ($incomplete) {
-                    $this->identifier = null;
-                }
+
+                return implode(' ', $id);
             }
         }
-        if ($single && is_array($this->identifier)) {
-            return reset($this->identifier);
-        }
 
         return $this->identifier;
     }

diff --git a/src/Tool/Wrapper/MongoDocumentWrapper.php b/src/Tool/Wrapper/MongoDocumentWrapper.php
@@ -76,7 +76,7 @@ public function hasValidIdentifier()
         return (bool) $this->getIdentifier();
     }
 
-    public function getIdentifier($single = true)
+    public function getIdentifier($single = true, $flatten = false)
     {
         if (!$this->identifier) {
             if ($this->object instanceof GhostObjectInterface) {

diff --git a/src/Tool/WrapperInterface.php b/src/Tool/WrapperInterface.php
@@ -75,10 +75,13 @@ public function getMetadata();
      * Get the object identifier, single or composite.
      *
      * @param bool $single
+     * @param bool $flatten
      *
      * @return array|mixed Array if a composite value, otherwise a single scalar
+     *
+     * @todo Uncomment the second parameter for 4.0
      */
-    public function getIdentifier($single = true);
+    public function getIdentifier($single = true/* , $flatten = false */);
 
     /**
      * Get the root object class name.

diff --git a/src/Uploadable/Mapping/Driver/Xml.php b/src/Uploadable/Mapping/Driver/Xml.php
@@ -35,7 +35,7 @@ public function readExtendedMetadata($meta, array &$config)
         $xmlDoctrine = $xml;
         $xml = $xml->children(self::GEDMO_NAMESPACE_URI);
 
-        if ('entity' === $xmlDoctrine->getName() || 'mapped-superclass' === $xmlDoctrine->getName()) {
+        if (in_array($xmlDoctrine->getName(), ['mapped-superclass', 'entity'], true)) {
             if (isset($xml->uploadable)) {
                 $xmlUploadable = $xml->uploadable;
                 $config['uploadable'] = true;