- Improve OptimisticLock to work with 1 query

- Add orderBy param to findOneInternal method

Author: compositephp

src/AbstractTable.php

@@ -73,22 +73,23 @@ public function save(AbstractEntity &$entity): void
                 $changedColumns['updated_at'] = DateTimeHelper::dateTimeToString($entity->updated_at);
             }
 
-            if ($this->config->hasOptimisticLock() && isset($entity->version)) {
-                $currentVersion = $entity->version;
+
+            if ($this->config->hasOptimisticLock()
+                && method_exists($entity, 'getVersion')
+                && method_exists($entity, 'incrementVersion')) {
+                $where['lock_version'] = $entity->getVersion();
+                $entity->incrementVersion();
+                $changedColumns['lock_version'] = $entity->getVersion();
+
                 try {
                     $connection->beginTransaction();
-                    $connection->update(
+                    $versionUpdated = $connection->update(
                         $this->getTableName(),
                         $changedColumns,
                         $where
                     );
-                    $versionUpdated = $connection->update(
-                        $this->getTableName(),
-                        ['version' => $currentVersion + 1],
-                        $where + ['version' => $currentVersion]
-                    );
                     if (!$versionUpdated) {
-                        throw new DbException('Failed to update entity version, concurrency modification, rolling back.');
+                        throw new Exceptions\LockException('Failed to update entity version, concurrency modification, rolling back.');
                     }
                     $connection->commit();
                 } catch (\Throwable $e) {
@@ -213,13 +214,15 @@ protected function findByPkInternal(mixed $pk): ?array
 
     /**
      * @param array<string, mixed> $where
+     * @param array<string, string>|string $orderBy
      * @return array<string, mixed>|null
      * @throws \Doctrine\DBAL\Exception
      */
-    protected function findOneInternal(array $where): ?array
+    protected function findOneInternal(array $where, array|string $orderBy = []): ?array
     {
         $query = $this->select();
         $this->buildWhere($query, $where);
+        $this->applyOrderBy($query, $orderBy);
         return $query->fetchAssociative() ?: null;
     }
 
@@ -259,22 +262,7 @@ protected function findAllInternal(
                 $query->setParameter($param, $value);
             }
         }
-        if ($orderBy) {
-            if (is_array($orderBy)) {
-                foreach ($orderBy as $column => $direction) {
-                    $query->addOrderBy($column, $direction);
-                }
-            } else {
-                foreach (explode(',', $orderBy) as $orderByPart) {
-                    $orderByPart = trim($orderByPart);
-                    if (preg_match('/(.+)\s(asc|desc)$/i', $orderByPart, $orderByPartMatch)) {
-                        $query->addOrderBy($orderByPartMatch[1], $orderByPartMatch[2]);
-                    } else {
-                        $query->addOrderBy($orderByPart);
-                    }
-                }
-            }
-        }
+        $this->applyOrderBy($query, $orderBy);
         if ($limit > 0) {
             $query->setMaxResults($limit);
         }
@@ -398,4 +386,28 @@ private function formatData(array $data): array
         }
         return $data;
     }
+
+    /**
+     * @param array<string, string>|string $orderBy
+     */
+    private function applyOrderBy(QueryBuilder $query, string|array $orderBy): void
+    {
+        if (!$orderBy) {
+            return;
+        }
+        if (is_array($orderBy)) {
+            foreach ($orderBy as $column => $direction) {
+                $query->addOrderBy($column, $direction);
+            }
+        } else {
+            foreach (explode(',', $orderBy) as $orderByPart) {
+                $orderByPart = trim($orderByPart);
+                if (preg_match('/(.+)\s(asc|desc)$/i', $orderByPart, $orderByPartMatch)) {
+                    $query->addOrderBy($orderByPartMatch[1], $orderByPartMatch[2]);
+                } else {
+                    $query->addOrderBy($orderByPart);
+                }
+            }
+        }
+    }
 }

src/Exceptions/LockException.php

@@ -0,0 +1,7 @@
+<?php declare(strict_types=1);
+
+namespace Composite\DB\Exceptions;
+
+class LockException extends DbException
+{
+}

src/Traits/OptimisticLock.php

@@ -4,5 +4,15 @@
 
 trait OptimisticLock
 {
-    public int $version = 1;
+    protected int $lock_version = 1;
+
+    public function getVersion(): int
+    {
+        return $this->lock_version;
+    }
+
+    public function incrementVersion(): void
+    {
+        $this->lock_version++;
+    }
 }

tests/TestStand/Tables/TestOptimisticLockTable.php

@@ -32,7 +32,7 @@ public function init(): bool
             (
                 `id` INTEGER NOT NULL CONSTRAINT TestAutoincrement_pk PRIMARY KEY AUTOINCREMENT,
                 `name` VARCHAR(255) NOT NULL,
-                `version` INTEGER NOT NULL DEFAULT 1,
+                `lock_version` INTEGER NOT NULL DEFAULT 1,
                 `created_at` TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL
             );
             "

tests/Traits/OptimisticLockTest.php

@@ -60,7 +60,7 @@ public function test_trait(): void
         $this->assertTrue($db->rollBack());
 
         $olEntity3 = $olTable1->findByPk($olEntity1->id);
-        $this->assertEquals(1, $olEntity3->version);
+        $this->assertEquals(1, $olEntity3->getVersion());
         $this->assertEquals('John', $olEntity3->name);
     }
 }