Remove scheduleExtraUpdate calls (#464)

Remove scheduleExtraUpdate calls

README.md

@@ -9,11 +9,10 @@ It currently handles:
 
  * [Blameable](/docs/blameable.md)
  * [Geocodable](/docs/geocodable.md)
- * joinable
  * [Loggable](/docs/loggable.md)
  * [Sluggable](/docs/sluggable.md)
  * [SoftDeletable](/docs/soft-deletable.md)
- * sortable
+ * [Sortable](/docs/sortable.md)
  * [Uuidable](/docs/uuidable.md)
  * [Timestampable](/docs/timestampable.md)
  * [Translatable](/docs/translatable.md)

composer.json

@@ -35,7 +35,7 @@
         "jakub-onderka/php-parallel-lint": "^1.0",
         "phpstan/phpstan": "^0.12.2",
         "phpunit/phpunit": "^8.5",
-        "rector/rector": "^0.6.1",
+        "rector/rector": "^0.6.2",
         "symplify/easy-coding-standard": "^7.1",
         "symplify/phpstan-extensions": "^7.1",
         "phpstan/phpstan-doctrine": "^0.12.4",
@@ -52,9 +52,9 @@
         }
     },
     "scripts": {
-        "check-cs": "ecs check src tests --ansi",
-        "fix-cs": "ecs check src tests --fix --ansi",
-        "phpstan": "phpstan analyse src tests --ansi --error-format symplify",
-        "rector": "rector process --config rector-ci.yaml"
+        "check-cs": "vendor/bin/ecs check src tests --ansi",
+        "fix-cs": "vendor/bin/ecs check src tests --fix --ansi",
+        "phpstan": "vendor/bin/phpstan analyse src tests --ansi --error-format symplify",
+        "rector": "vendor/bin/rector process --config rector-ci.yaml"
     }
 }

docs/sortable.md

@@ -0,0 +1,38 @@
+# Sortable
+
+@todo
+
+## Entity
+
+```php
+<?php
+
+declare(strict_types=1);
+
+namespace App\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+use Knp\DoctrineBehaviors\Contract\Entity\SortableInterface;
+use Knp\DoctrineBehaviors\Model\Sortable\SortableTrait;
+
+/**
+ * @ORM\Entity
+ */
+class Category implements SortableInterface
+{
+    use SortableTrait;
+}
+```
+
+## Usage
+
+@todo
+
+```php
+$category = new Category;
+
+$entityManager->persist($category);
+$entityManager->flush();
+
+$category->getSort(); // 1
+```

ecs.yaml

@@ -32,6 +32,9 @@ parameters:
             - "*Repository.php"
 
 services:
+    Symplify\CodingStandard\Sniffs\CleanCode\CognitiveComplexitySniff:
+        max_cognitive_complexity: 8
+
     PhpCsFixer\Fixer\ClassNotation\FinalClassFixer: ~
 
     # see single LICENSE.txt file in the root directory

src/ORM/Blameable/BlameableSubscriber.php

@@ -77,9 +77,6 @@ public function prePersist(LifecycleEventArgs $lifecycleEventArgs): void
                 $entity->setCreatedBy($user);
 
                 $this->unitOfWork->propertyChanged($entity, self::CREATED_BY, null, $user);
-                $this->unitOfWork->scheduleExtraUpdate($entity, [
-                    self::CREATED_BY => [null, $user],
-                ]);
             }
         }
 
@@ -89,9 +86,6 @@ public function prePersist(LifecycleEventArgs $lifecycleEventArgs): void
                 $entity->setUpdatedBy($user);
 
                 $this->unitOfWork->propertyChanged($entity, self::UPDATED_BY, null, $user);
-                $this->unitOfWork->scheduleExtraUpdate($entity, [
-                    self::UPDATED_BY => [null, $user],
-                ]);
             }
         }
     }
@@ -115,9 +109,6 @@ public function preUpdate(LifecycleEventArgs $lifecycleEventArgs): void
         $entity->setUpdatedBy($user);
 
         $this->unitOfWork->propertyChanged($entity, self::UPDATED_BY, $oldValue, $user);
-        $this->unitOfWork->scheduleExtraUpdate($entity, [
-            self::UPDATED_BY => [$oldValue, $user],
-        ]);
     }
 
     /**
@@ -136,9 +127,6 @@ public function preRemove(LifecycleEventArgs $lifecycleEventArgs): void
             $entity->setDeletedBy($user);
 
             $this->unitOfWork->propertyChanged($entity, self::DELETED_BY, $oldValue, $user);
-            $this->unitOfWork->scheduleExtraUpdate($entity, [
-                self::DELETED_BY => [$oldValue, $user],
-            ]);
         }
     }
 

src/ORM/Geocodable/GeocodableSubscriber.php

@@ -87,10 +87,6 @@ private function updateLocation(LifecycleEventArgs $lifecycleEventArgs, $overrid
             }
 
             $unitOfWork->propertyChanged($entity, 'location', $oldValue, $entity->getLocation());
-
-            $unitOfWork->scheduleExtraUpdate($entity, [
-                'location' => [$oldValue, $entity->getLocation()],
-            ]);
         }
     }
 }

tests/AbstractBehaviorTestCase.php

@@ -5,6 +5,7 @@
 namespace Knp\DoctrineBehaviors\Tests;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Logging\DebugStack;
 use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
 use Doctrine\ORM\EntityManagerInterface;
 use Knp\DoctrineBehaviors\Tests\HttpKernel\DoctrineBehaviorsKernel;
@@ -17,6 +18,11 @@ abstract class AbstractBehaviorTestCase extends AbstractKernelTestCase
      */
     protected $entityManager;
 
+    /**
+     * @var DebugStack
+     */
+    protected $debugStack;
+
     protected function setUp(): void
     {
         $customConfig = $this->provideCustomConfig();
@@ -49,4 +55,13 @@ protected function provideCustomConfig(): ?string
     {
         return null;
     }
+
+    protected function enableDebugStackLogger(): void
+    {
+        $this->debugStack = new DebugStack();
+
+        $this->entityManager->getConnection()
+            ->getConfiguration()
+            ->setSQLLogger($this->debugStack);
+    }
 }

tests/ORM/BlameableTest.php

@@ -57,10 +57,20 @@ public function testUpdate(): void
         /** @var BlameableEntity $entity */
         $entity = $this->blameableRepository->find($id);
 
+        $this->enableDebugStackLogger();
+
         $entity->setTitle('test'); // need to modify at least one column to trigger onUpdate
         $this->entityManager->flush();
         $this->entityManager->clear();
 
+        $this->assertCount(3, $this->debugStack->queries);
+        $this->assertSame('"START TRANSACTION"', $this->debugStack->queries[1]['sql']);
+        $this->assertSame(
+            'UPDATE BlameableEntity SET title = ?, updatedBy = ? WHERE id = ?',
+            $this->debugStack->queries[2]['sql']
+        );
+        $this->assertSame('"COMMIT"', $this->debugStack->queries[3]['sql']);
+
         /** @var BlameableEntity $entity */
         $entity = $this->blameableRepository->find($id);
 
@@ -94,4 +104,35 @@ public function testRemove(): void
 
         $this->assertSame('user3', $entity->getDeletedBy());
     }
+
+    public function testExtraSqlCalls(): void
+    {
+        $entity = new BlameableEntity();
+
+        $this->enableDebugStackLogger();
+
+        $this->entityManager->persist($entity);
+        $this->entityManager->flush();
+
+        $expectedCount = $this->isPostgreSql() ? 4 : 3;
+        $startKey = $this->isPostgreSql() ? 2 : 1;
+
+        $this->assertCount($expectedCount, $this->debugStack->queries);
+        $this->assertSame('"START TRANSACTION"', $this->debugStack->queries[$startKey]['sql']);
+
+        $sql2 = $this->debugStack->queries[$startKey + 1]['sql'];
+        if ($this->isPostgreSql()) {
+            $this->assertSame(
+                'INSERT INTO BlameableEntity (id, title, createdBy, updatedBy, deletedBy) VALUES (?, ?, ?, ?, ?)',
+                $sql2
+            );
+        } else {
+            $this->assertSame(
+                'INSERT INTO BlameableEntity (title, createdBy, updatedBy, deletedBy) VALUES (?, ?, ?, ?)',
+                $sql2
+            );
+        }
+
+        $this->assertSame('"COMMIT"', $this->debugStack->queries[$startKey + 2]['sql']);
+    }
 }

tests/ORM/SoftDeletableTest.php

@@ -110,4 +110,28 @@ public function testRestore(): void
 
         $this->assertFalse($entity->isDeleted());
     }
+
+    public function testExtraSqlCalls(): void
+    {
+        $entity = new SoftDeletableEntity();
+        $this->entityManager->persist($entity);
+        $this->entityManager->flush();
+
+        $id = $entity->getId();
+        $this->assertNotNull($id);
+        $this->assertFalse($entity->isDeleted());
+
+        $this->enableDebugStackLogger();
+
+        $this->entityManager->remove($entity);
+        $this->entityManager->flush();
+
+        $this->assertCount(3, $this->debugStack->queries);
+        $this->assertSame('"START TRANSACTION"', $this->debugStack->queries[1]['sql']);
+        $this->assertSame(
+            'UPDATE SoftDeletableEntity SET deletedAt = ? WHERE id = ?',
+            $this->debugStack->queries[2]['sql']
+        );
+        $this->assertSame('"COMMIT"', $this->debugStack->queries[3]['sql']);
+    }
 }