ACP2E-3361: Some of the relations records are saved to DB when order record is saved

- improved solution
- added unit test

Author: aplapana

lib/internal/Magento/Framework/Model/ResourceModel/Db/VersionControl/Snapshot.php

@@ -6,13 +6,20 @@
 namespace Magento\Framework\Model\ResourceModel\Db\VersionControl;
 
 use Magento\Framework\DataObject;
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
+use Magento\Framework\Serialize\SerializerInterface;
 
 /**
  * Class Snapshot register snapshot of entity data, for tracking changes
  */
 class Snapshot implements ResetAfterRequestInterface
 {
+    /**
+     * @var SerializerInterface
+     */
+    private SerializerInterface $serializer;
+
     /**
      * Array of snapshots of entities data
      *
@@ -29,19 +36,22 @@ class Snapshot implements ResetAfterRequestInterface
      * Initialization
      *
      * @param Metadata $metadata
+     * @param SerializerInterface $serializer
      */
     public function __construct(
-        Metadata $metadata
+        Metadata $metadata,
+        SerializerInterface $serializer
     ) {
         $this->metadata = $metadata;
+        $this->serializer = $serializer;
     }
 
     /**
      * Register snapshot of entity data, for tracking changes
      *
      * @param DataObject $entity
      * @return void
-     * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+     * @throws LocalizedException
      */
     public function registerSnapshot(DataObject $entity)
     {
@@ -88,9 +98,10 @@ public function isModified(DataObject $entity)
         foreach ($this->snapshotData[$entityClass][$entity->getId()] as $field => $value) {
             $entityValue = $entity->getDataByKey($field);
             if (is_array($entityValue) && is_string($value)) {
-                $decodedValue = json_decode($value, true);
-                if (json_last_error() === JSON_ERROR_NONE) {
-                    $value = $decodedValue;
+                try {
+                    $value = $this->serializer->unserialize($value);
+                } catch (\InvalidArgumentException) {
+                    return true;
                 }
             }
             if ($entityValue != $value) {

lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/Db/VersionControl/SnapshotTest.php

@@ -7,10 +7,12 @@
 
 namespace Magento\Framework\Model\Test\Unit\ResourceModel\Db\VersionControl;
 
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Model\AbstractModel;
 use Magento\Framework\Model\ResourceModel\Db\VersionControl\Metadata;
 use Magento\Framework\Model\ResourceModel\Db\VersionControl\Snapshot;
-use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use Magento\Framework\Serialize\SerializerInterface;
+use PHPUnit\Framework\MockObject\Exception;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
@@ -19,38 +21,47 @@ class SnapshotTest extends TestCase
     /**
      * @var Snapshot
      */
-    protected $entitySnapshot;
+    protected Snapshot $entitySnapshot;
 
     /**
      * @var MockObject|Metadata
      */
-    protected $entityMetadata;
+    private Metadata $entityMetadata;
 
     /**
      * @var MockObject|AbstractModel
      */
-    protected $model;
+    private AbstractModel $model;
 
     /**
-     * Initialization
+     * @var SerializerInterface|MockObject
+     */
+    private SerializerInterface $serializer;
+
+    /**
+     * @return void
+     * @throws Exception
      */
     protected function setUp(): void
     {
-        $objectManager = new ObjectManager($this);
         $this->model = $this->createPartialMock(AbstractModel::class, ['getId']);
 
         $this->entityMetadata = $this->createPartialMock(
             Metadata::class,
             ['getFields']
         );
+        $this->serializer = $this->createMock(SerializerInterface::class);
 
-        $this->entitySnapshot = $objectManager->getObject(
-            Snapshot::class,
-            ['metadata' => $this->entityMetadata]
-        );
+        $this->entitySnapshot = new Snapshot($this->entityMetadata, $this->serializer);
+
+        parent::setUp();
     }
 
-    public function testRegisterSnapshot()
+    /**
+     * @return void
+     * @throws LocalizedException
+     */
+    public function testRegisterSnapshot(): void
     {
         $entityId = 1;
         $data = [
@@ -72,19 +83,26 @@ public function testRegisterSnapshot()
         $this->assertFalse($this->entitySnapshot->isModified($this->model));
     }
 
-    public function testIsModified()
+    /**
+     * @return void
+     * @throws LocalizedException
+     */
+    public function testIsModified(): void
     {
         $entityId = 1;
+        $options = ['option1' => 'value1', 'option2' => 'value2'];
         $data = [
             'id' => $entityId,
             'name' => 'test',
             'description' => '',
-            'custom_not_present_attribute' => ''
+            'custom_not_present_attribute' => '',
+            'options' => json_encode($options)
         ];
         $fields = [
             'id' => [],
             'name' => [],
-            'description' => []
+            'description' => [],
+            'options' => []
         ];
         $modifiedData = array_merge($data, ['name' => 'newName']);
         $this->model->expects($this->any())->method('getId')->willReturn($entityId);
@@ -93,11 +111,19 @@ public function testIsModified()
         $this->entitySnapshot->registerSnapshot($this->model);
         $this->model->setData($modifiedData);
         $this->assertTrue($this->entitySnapshot->isModified($this->model));
+
+        $this->model->setData('options', $options);
+        $this->assertTrue($this->entitySnapshot->isModified($this->model));
+
         $this->entitySnapshot->registerSnapshot($this->model);
         $this->assertFalse($this->entitySnapshot->isModified($this->model));
     }
 
-    public function testClear()
+    /**
+     * @return void
+     * @throws LocalizedException
+     */
+    public function testClear(): void
     {
         $entityId = 1;
         $data = [