From 946da1640c1a8081c00aaf91008ce248e1ea5d19 Mon Sep 17 00:00:00 2001 From: Composite PHP Date: Sat, 1 Jul 2023 22:15:53 +0100 Subject: [PATCH 1/5] Move multiselect to separate class MultiSelecy --- src/AbstractTable.php | 44 ++------------------- src/MultiQuery/MultiSelect.php | 64 +++++++++++++++++++++++++++++++ tests/Table/AbstractTableTest.php | 44 +++++++++++++++++++++ 3 files changed, 112 insertions(+), 40 deletions(-) create mode 100644 src/MultiQuery/MultiSelect.php diff --git a/src/AbstractTable.php b/src/AbstractTable.php index 22b2720..cb9b850 100644 --- a/src/AbstractTable.php +++ b/src/AbstractTable.php @@ -3,6 +3,7 @@ namespace Composite\DB; use Composite\DB\MultiQuery\MultiInsert; +use Composite\DB\MultiQuery\MultiSelect; use Composite\Entity\Helpers\DateTimeHelper; use Composite\Entity\AbstractEntity; use Composite\DB\Exceptions\DbException; @@ -225,7 +226,6 @@ protected function findOneInternal(array $where): ?array * @param array> $pkList * @return array> * @throws DbException - * @throws EntityException * @throws \Doctrine\DBAL\Exception */ protected function findMultiInternal(array $pkList): array @@ -233,44 +233,8 @@ protected function findMultiInternal(array $pkList): array if (!$pkList) { return []; } - /** @var class-string $class */ - $class = $this->config->entityClass; - - $pkColumns = []; - foreach ($this->config->primaryKeys as $primaryKeyName) { - $pkColumns[$primaryKeyName] = $class::schema()->getColumn($primaryKeyName); - } - if (count($pkColumns) === 1) { - if (!array_is_list($pkList)) { - throw new DbException('Input argument $pkList must be list'); - } - /** @var \Composite\Entity\Columns\AbstractColumn $pkColumn */ - $pkColumn = reset($pkColumns); - $preparedPkValues = array_map(fn ($pk) => $pkColumn->uncast($pk), $pkList); - $query = $this->select(); - $this->buildWhere($query, [$pkColumn->name => $preparedPkValues]); - } else { - $query = $this->select(); - $expressions = []; - foreach ($pkList as $i => $pkArray) { - if (!is_array($pkArray)) { - throw new DbException('For tables with composite keys, input array must consist associative arrays'); - } - $pkOrExpr = []; - foreach ($pkArray as $pkName => $pkValue) { - if (is_string($pkName) && isset($pkColumns[$pkName])) { - $preparedPkValue = $pkColumns[$pkName]->cast($pkValue); - $pkOrExpr[] = $query->expr()->eq($pkName, ':' . $pkName . $i); - $query->setParameter($pkName . $i, $preparedPkValue); - } - } - if ($pkOrExpr) { - $expressions[] = $query->expr()->and(...$pkOrExpr); - } - } - $query->where($query->expr()->or(...$expressions)); - } - return $query->executeQuery()->fetchAllAssociative(); + $multiSelect = new MultiSelect($this->getConnection(), $this->config, $pkList); + return $multiSelect->getQueryBuilder()->executeQuery()->fetchAllAssociative(); } /** @@ -395,7 +359,7 @@ protected function select(string $select = '*'): QueryBuilder /** * @param array $where */ - private function buildWhere(QueryBuilder $query, array $where): void + private function buildWhere(\Doctrine\DBAL\Query\QueryBuilder $query, array $where): void { foreach ($where as $column => $value) { if ($value === null) { diff --git a/src/MultiQuery/MultiSelect.php b/src/MultiQuery/MultiSelect.php new file mode 100644 index 0000000..59459eb --- /dev/null +++ b/src/MultiQuery/MultiSelect.php @@ -0,0 +1,64 @@ +createQueryBuilder()->select('*')->from($tableConfig->tableName); + /** @var class-string $class */ + $class = $tableConfig->entityClass; + + $pkColumns = []; + foreach ($tableConfig->primaryKeys as $primaryKeyName) { + $pkColumns[$primaryKeyName] = $class::schema()->getColumn($primaryKeyName); + } + + if (count($pkColumns) === 1) { + if (!array_is_list($condition)) { + throw new DbException('Input argument $pkList must be list'); + } + /** @var \Composite\Entity\Columns\AbstractColumn $pkColumn */ + $pkColumn = reset($pkColumns); + $preparedPkValues = array_map(fn ($pk) => $pkColumn->uncast($pk), $condition); + $query->andWhere($query->expr()->in($pkColumn->name, $preparedPkValues)); + } else { + $expressions = []; + foreach ($condition as $i => $pkArray) { + if (!is_array($pkArray)) { + throw new DbException('For tables with composite keys, input array must consist associative arrays'); + } + $pkOrExpr = []; + foreach ($pkArray as $pkName => $pkValue) { + if (is_string($pkName) && isset($pkColumns[$pkName])) { + $preparedPkValue = $pkColumns[$pkName]->cast($pkValue); + $pkOrExpr[] = $query->expr()->eq($pkName, ':' . $pkName . $i); + $query->setParameter($pkName . $i, $preparedPkValue); + } + } + if ($pkOrExpr) { + $expressions[] = $query->expr()->and(...$pkOrExpr); + } + } + $query->where($query->expr()->or(...$expressions)); + } + $this->queryBuilder = $query; + } + + public function getQueryBuilder(): QueryBuilder + { + return $this->queryBuilder; + } +} \ No newline at end of file diff --git a/tests/Table/AbstractTableTest.php b/tests/Table/AbstractTableTest.php index 6923fdb..c426c56 100644 --- a/tests/Table/AbstractTableTest.php +++ b/tests/Table/AbstractTableTest.php @@ -101,4 +101,48 @@ public function test_illegalCreateEntity(): void $empty = $table->buildEntities(['abc']); $this->assertEmpty($empty); } + + /** + * @dataProvider buildWhere_dataProvider + */ + public function test_buildWhere($where, $expectedSQL, $expectedParams) + { + $table = new Tables\TestStrictTable(); + + $selectReflection = new \ReflectionMethod($table, 'select'); + $selectReflection->setAccessible(true); + + $queryBuilder = $selectReflection->invoke($table); + + $buildWhereReflection = new \ReflectionMethod($table, 'buildWhere'); + $buildWhereReflection->setAccessible(true); + + $buildWhereReflection->invokeArgs($table, [$queryBuilder, $where]); + + $this->assertEquals($expectedSQL, $queryBuilder->getSQL()); + } + + public static function buildWhere_dataProvider(): array + { + return [ + // Test when value is null + [ + ['column1' => null], + 'SELECT * FROM Strict WHERE column1 IS NULL', + [] + ], + // Test when value is an array + [ + ['column1' => [1, 2, 3]], + 'SELECT * FROM Strict WHERE column1 IN (1, 2, 3)', + [1, 2, 3] + ], + // Test when value is a single value + [ + ['column1' => 'value1'], + 'SELECT * FROM Strict WHERE column1 = :column1', + ['value1'] + ], + ]; + } } \ No newline at end of file From 06cd5570eb60cc424c5230e8d214e5d146f9fb7d Mon Sep 17 00:00:00 2001 From: Composite PHP Date: Sat, 14 Oct 2023 12:40:16 +0100 Subject: [PATCH 2/5] Add .github folder to export-ignore --- .gitattributes | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitattributes b/.gitattributes index af43e6b..1989899 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,5 +1,6 @@ /.gitattributes export-ignore /.gitignore export-ignore +/.github export-ignore /doc export-ignore /phpunit.xml export-ignore /tests export-ignore From 6e5824dd119593ce64db31032b6475a93bb12ecc Mon Sep 17 00:00:00 2001 From: Composite PHP Date: Sat, 14 Oct 2023 14:03:02 +0100 Subject: [PATCH 3/5] Add UUID support --- composer.json | 2 +- src/AbstractCachedTable.php | 4 +-- src/AbstractTable.php | 3 +- tests/Table/AbstractCachedTableTest.php | 28 ++++++++++--------- tests/Table/AbstractTableTest.php | 17 ++++++----- tests/Table/UniqueTableTest.php | 11 ++++---- tests/TestStand/Entities/TestUniqueEntity.php | 3 +- tests/TestStand/Interfaces/IUniqueTable.php | 3 +- .../Tables/TestUniqueCachedTable.php | 3 +- .../Tables/TestUniqueSdCachedTable.php | 3 +- tests/TestStand/Tables/TestUniqueSdTable.php | 5 ++-- tests/TestStand/Tables/TestUniqueTable.php | 3 +- 12 files changed, 49 insertions(+), 36 deletions(-) diff --git a/composer.json b/composer.json index 1cea962..5bfced2 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ "php": "^8.1", "ext-pdo": "*", "psr/simple-cache": "1 - 3", - "compositephp/entity": "^0.1.8", + "compositephp/entity": "^0.1.9", "doctrine/dbal": "^3.5" }, "require-dev": { diff --git a/src/AbstractCachedTable.php b/src/AbstractCachedTable.php index 44e0936..857f9b9 100644 --- a/src/AbstractCachedTable.php +++ b/src/AbstractCachedTable.php @@ -5,6 +5,7 @@ use Composite\DB\Exceptions\DbException; use Composite\Entity\AbstractEntity; use Psr\SimpleCache\CacheInterface; +use Ramsey\Uuid\UuidInterface; abstract class AbstractCachedTable extends AbstractTable { @@ -196,9 +197,8 @@ protected function findMultiCachedInternal(array $ids, null|int|\DateInterval $t /** * @param string|int|array|AbstractEntity $keyOrEntity - * @throws \Composite\Entity\Exceptions\EntityException */ - protected function getOneCacheKey(string|int|array|AbstractEntity $keyOrEntity): string + protected function getOneCacheKey(string|int|array|AbstractEntity|UuidInterface $keyOrEntity): string { if (!is_array($keyOrEntity)) { $condition = $this->getPkCondition($keyOrEntity); diff --git a/src/AbstractTable.php b/src/AbstractTable.php index cb9b850..822249b 100644 --- a/src/AbstractTable.php +++ b/src/AbstractTable.php @@ -11,6 +11,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Query\QueryBuilder; +use Ramsey\Uuid\UuidInterface; abstract class AbstractTable { @@ -330,7 +331,7 @@ final protected function createEntities(mixed $data, ?string $keyColumnName = nu * @return array * @throws EntityException */ - protected function getPkCondition(int|string|array|AbstractEntity $data): array + protected function getPkCondition(int|string|array|AbstractEntity|UuidInterface $data): array { $condition = []; if ($data instanceof AbstractEntity) { diff --git a/tests/Table/AbstractCachedTableTest.php b/tests/Table/AbstractCachedTableTest.php index 576d663..6c65846 100644 --- a/tests/Table/AbstractCachedTableTest.php +++ b/tests/Table/AbstractCachedTableTest.php @@ -9,12 +9,15 @@ use Composite\DB\Tests\TestStand\Tables; use Composite\Entity\AbstractEntity; use Composite\DB\Tests\Helpers; +use Ramsey\Uuid\Uuid; final class AbstractCachedTableTest extends \PHPUnit\Framework\TestCase { public static function getOneCacheKey_dataProvider(): array { $cache = Helpers\CacheHelper::getCache(); + $uuid = Uuid::uuid4(); + $uuidCacheKey = str_replace('-', '_', (string)$uuid); return [ [ new Tables\TestAutoincrementCachedTable($cache), @@ -28,16 +31,13 @@ public static function getOneCacheKey_dataProvider(): array ], [ new Tables\TestUniqueCachedTable($cache), - new Entities\TestUniqueEntity(id: '123abc', name: 'John'), - 'sqlite.TestUnique.v1.o.id_123abc', + new Entities\TestUniqueEntity(id: $uuid, name: 'John'), + 'sqlite.TestUnique.v1.o.id_' . $uuidCacheKey, ], [ - new Tables\TestUniqueCachedTable($cache), - new Entities\TestUniqueEntity( - id: implode('', array_fill(0, 100, 'a')), - name: 'John', - ), - 'ed66f06444d851a981a9ddcecbbf4d5860cd3131', + new Tables\TestCompositeCachedTable($cache), + new Entities\TestCompositeEntity(user_id: PHP_INT_MAX, post_id: PHP_INT_MAX, message: 'Text'), + '69b5bbf599d78f0274feb5cb0e6424f35cca0b57', ], ]; } @@ -45,7 +45,7 @@ public static function getOneCacheKey_dataProvider(): array /** * @dataProvider getOneCacheKey_dataProvider */ - public function test_getOneCacheKey(AbstractTable $table, AbstractEntity $object, string $expected): void + public function test_getOneCacheKey(AbstractCachedTable $table, AbstractEntity $object, string $expected): void { $reflectionMethod = new \ReflectionMethod($table, 'getOneCacheKey'); $actual = $reflectionMethod->invoke($table, $object); @@ -194,6 +194,8 @@ public function test_getCustomCacheKey(array $parts, string $expected): void public static function collectCacheKeysByEntity_dataProvider(): array { + $uuid = Uuid::uuid4(); + $uuidCacheKey = str_replace('-', '_', (string)$uuid); return [ [ new Entities\TestAutoincrementEntity(name: 'foo'), @@ -215,21 +217,21 @@ public static function collectCacheKeysByEntity_dataProvider(): array ], ], [ - new Entities\TestUniqueEntity(id: '123abc', name: 'foo'), + new Entities\TestUniqueEntity(id: $uuid, name: 'foo'), new Tables\TestUniqueCachedTable(Helpers\CacheHelper::getCache()), [ 'sqlite.TestUnique.v1.l.name_eq_foo', 'sqlite.TestUnique.v1.c.name_eq_foo', - 'sqlite.TestUnique.v1.o.id_123abc', + 'sqlite.TestUnique.v1.o.id_' . $uuidCacheKey, ], ], [ - Entities\TestUniqueEntity::fromArray(['id' => '456def', 'name' => 'bar']), + Entities\TestUniqueEntity::fromArray(['id' => $uuid, 'name' => 'bar']), new Tables\TestUniqueCachedTable(Helpers\CacheHelper::getCache()), [ 'sqlite.TestUnique.v1.l.name_eq_bar', 'sqlite.TestUnique.v1.c.name_eq_bar', - 'sqlite.TestUnique.v1.o.id_456def', + 'sqlite.TestUnique.v1.o.id_' . $uuidCacheKey, ], ], ]; diff --git a/tests/Table/AbstractTableTest.php b/tests/Table/AbstractTableTest.php index c426c56..5c45eca 100644 --- a/tests/Table/AbstractTableTest.php +++ b/tests/Table/AbstractTableTest.php @@ -7,11 +7,14 @@ use Composite\DB\Tests\TestStand\Tables; use Composite\Entity\AbstractEntity; use Composite\Entity\Exceptions\EntityException; +use Ramsey\Uuid\Uuid; +use Ramsey\Uuid\UuidInterface; final class AbstractTableTest extends \PHPUnit\Framework\TestCase { public static function getPkCondition_dataProvider(): array { + $uuid = Uuid::uuid4(); return [ [ new Tables\TestAutoincrementTable(), @@ -35,13 +38,13 @@ public static function getPkCondition_dataProvider(): array ], [ new Tables\TestUniqueTable(), - new Entities\TestUniqueEntity(id: '123abc', name: 'John'), - ['id' => '123abc'], + new Entities\TestUniqueEntity(id: $uuid, name: 'John'), + ['id' => $uuid->toString()], ], [ new Tables\TestUniqueTable(), - '123abc', - ['id' => '123abc'], + $uuid, + ['id' => $uuid->toString()], ], [ new Tables\TestAutoincrementSdTable(), @@ -55,8 +58,8 @@ public static function getPkCondition_dataProvider(): array ], [ new Tables\TestUniqueSdTable(), - new Entities\TestUniqueSdEntity(id: '123abc', name: 'John'), - ['id' => '123abc'], + new Entities\TestUniqueSdEntity(id: $uuid, name: 'John'), + ['id' => $uuid->toString()], ], ]; } @@ -64,7 +67,7 @@ public static function getPkCondition_dataProvider(): array /** * @dataProvider getPkCondition_dataProvider */ - public function test_getPkCondition(AbstractTable $table, int|string|array|AbstractEntity $object, array $expected): void + public function test_getPkCondition(AbstractTable $table, int|string|array|AbstractEntity|UuidInterface $object, array $expected): void { $reflectionMethod = new \ReflectionMethod($table, 'getPkCondition'); $actual = $reflectionMethod->invoke($table, $object); diff --git a/tests/Table/UniqueTableTest.php b/tests/Table/UniqueTableTest.php index 87b954f..e9cd434 100644 --- a/tests/Table/UniqueTableTest.php +++ b/tests/Table/UniqueTableTest.php @@ -8,6 +8,7 @@ use Composite\DB\Tests\TestStand\Entities; use Composite\DB\Tests\TestStand\Tables; use Composite\DB\Tests\TestStand\Interfaces\IUniqueTable; +use Ramsey\Uuid\Uuid; final class UniqueTableTest extends \PHPUnit\Framework\TestCase { @@ -43,7 +44,7 @@ public function test_crud(AbstractTable&IUniqueTable $table, string $class): voi $tableConfig = TableConfig::fromEntitySchema($class::schema()); $entity = new $class( - id: uniqid(), + id: Uuid::uuid4(), name: Helpers\StringHelper::getUniqueName(), ); $this->assertEntityNotExists($table, $entity); @@ -68,19 +69,19 @@ public function test_crud(AbstractTable&IUniqueTable $table, string $class): voi public function test_multiSave(): void { $e1 = new Entities\TestUniqueEntity( - id: uniqid(), + id: Uuid::uuid4(), name: Helpers\StringHelper::getUniqueName(), ); $e2 = new Entities\TestUniqueEntity( - id: uniqid(), + id: Uuid::uuid4(), name: Helpers\StringHelper::getUniqueName(), ); $e3 = new Entities\TestUniqueEntity( - id: uniqid(), + id: Uuid::uuid4(), name: Helpers\StringHelper::getUniqueName(), ); $e4 = new Entities\TestUniqueEntity( - id: uniqid(), + id: Uuid::uuid4(), name: Helpers\StringHelper::getUniqueName(), ); $table = new Tables\TestUniqueTable(); diff --git a/tests/TestStand/Entities/TestUniqueEntity.php b/tests/TestStand/Entities/TestUniqueEntity.php index 7b676bd..2a6138b 100644 --- a/tests/TestStand/Entities/TestUniqueEntity.php +++ b/tests/TestStand/Entities/TestUniqueEntity.php @@ -5,13 +5,14 @@ use Composite\DB\Attributes\{PrimaryKey}; use Composite\DB\Attributes\Table; use Composite\Entity\AbstractEntity; +use Ramsey\Uuid\UuidInterface; #[Table(connection: 'sqlite', name: 'TestUnique')] class TestUniqueEntity extends AbstractEntity { public function __construct( #[PrimaryKey] - public readonly string $id, + public readonly UuidInterface $id, public string $name, public readonly \DateTimeImmutable $created_at = new \DateTimeImmutable(), ) {} diff --git a/tests/TestStand/Interfaces/IUniqueTable.php b/tests/TestStand/Interfaces/IUniqueTable.php index b0e09dd..197beaf 100644 --- a/tests/TestStand/Interfaces/IUniqueTable.php +++ b/tests/TestStand/Interfaces/IUniqueTable.php @@ -4,10 +4,11 @@ use Composite\DB\Tests\TestStand\Entities\TestCompositeEntity; use Composite\DB\Tests\TestStand\Entities\TestUniqueEntity; +use Ramsey\Uuid\UuidInterface; interface IUniqueTable { - public function findByPk(string $id): ?TestUniqueEntity; + public function findByPk(UuidInterface $id): ?TestUniqueEntity; /** * @return TestCompositeEntity[] */ diff --git a/tests/TestStand/Tables/TestUniqueCachedTable.php b/tests/TestStand/Tables/TestUniqueCachedTable.php index 46c5e55..7cb7fa3 100644 --- a/tests/TestStand/Tables/TestUniqueCachedTable.php +++ b/tests/TestStand/Tables/TestUniqueCachedTable.php @@ -7,6 +7,7 @@ use Composite\DB\Tests\TestStand\Entities\TestUniqueEntity; use Composite\DB\Tests\TestStand\Interfaces\IUniqueTable; use Composite\Entity\AbstractEntity; +use Ramsey\Uuid\UuidInterface; class TestUniqueCachedTable extends AbstractCachedTable implements IUniqueTable { @@ -29,7 +30,7 @@ protected function getFlushCacheKeys(TestUniqueEntity|AbstractEntity $entity): a ]; } - public function findByPk(string $id): ?TestUniqueEntity + public function findByPk(UuidInterface $id): ?TestUniqueEntity { return $this->createEntity($this->findByPkInternal($id)); } diff --git a/tests/TestStand/Tables/TestUniqueSdCachedTable.php b/tests/TestStand/Tables/TestUniqueSdCachedTable.php index 8c60a0c..016133d 100644 --- a/tests/TestStand/Tables/TestUniqueSdCachedTable.php +++ b/tests/TestStand/Tables/TestUniqueSdCachedTable.php @@ -7,6 +7,7 @@ use Composite\DB\Tests\TestStand\Entities\TestUniqueSdEntity; use Composite\DB\Tests\TestStand\Interfaces\IUniqueTable; use Composite\Entity\AbstractEntity; +use Ramsey\Uuid\UuidInterface; class TestUniqueSdCachedTable extends AbstractCachedTable implements IUniqueTable { @@ -29,7 +30,7 @@ protected function getFlushCacheKeys(TestUniqueSdEntity|AbstractEntity $entity): ]; } - public function findByPk(string $id): ?TestUniqueSdEntity + public function findByPk(UuidInterface $id): ?TestUniqueSdEntity { return $this->createEntity($this->findByPkInternal($id)); } diff --git a/tests/TestStand/Tables/TestUniqueSdTable.php b/tests/TestStand/Tables/TestUniqueSdTable.php index 33cbf62..df078b9 100644 --- a/tests/TestStand/Tables/TestUniqueSdTable.php +++ b/tests/TestStand/Tables/TestUniqueSdTable.php @@ -4,6 +4,7 @@ use Composite\DB\TableConfig; use Composite\DB\Tests\TestStand\Entities\TestUniqueSdEntity; +use Ramsey\Uuid\UuidInterface; class TestUniqueSdTable extends TestUniqueTable { @@ -18,7 +19,7 @@ protected function getConfig(): TableConfig return TableConfig::fromEntitySchema(TestUniqueSdEntity::schema()); } - public function findByPk(string $id): ?TestUniqueSdEntity + public function findByPk(UuidInterface $id): ?TestUniqueSdEntity { return $this->createEntity($this->findByPkInternal($id)); } @@ -40,7 +41,7 @@ public function init(): bool " CREATE TABLE IF NOT EXISTS {$this->getTableName()} ( - `id` VARCHAR(255) NOT NULL, + `id` VARCHAR(32) NOT NULL, `name` VARCHAR(255) NOT NULL, `created_at` TIMESTAMP NOT NULL, `deleted_at` TIMESTAMP NULL DEFAULT NULL, diff --git a/tests/TestStand/Tables/TestUniqueTable.php b/tests/TestStand/Tables/TestUniqueTable.php index 7d26fca..7e223d9 100644 --- a/tests/TestStand/Tables/TestUniqueTable.php +++ b/tests/TestStand/Tables/TestUniqueTable.php @@ -7,6 +7,7 @@ use Composite\DB\Tests\TestStand\Entities\TestUniqueEntity; use Composite\DB\Tests\TestStand\Interfaces\IUniqueTable; use Composite\Entity\AbstractEntity; +use Ramsey\Uuid\UuidInterface; class TestUniqueTable extends AbstractTable implements IUniqueTable { @@ -29,7 +30,7 @@ protected function getConfig(): TableConfig return TableConfig::fromEntitySchema(TestUniqueEntity::schema()); } - public function findByPk(string $id): ?TestUniqueEntity + public function findByPk(UuidInterface $id): ?TestUniqueEntity { return $this->createEntity($this->findByPkInternal($id)); } From 0667a416e4eb7c87f1978fec7f04ff51e30b5f0b Mon Sep 17 00:00:00 2001 From: Composite PHP Date: Sat, 21 Oct 2023 14:07:41 +0100 Subject: [PATCH 4/5] - Improve OptimisticLock to work with 1 query - Add orderBy param to findOneInternal method --- src/AbstractTable.php | 64 +++++++++++-------- src/Exceptions/LockException.php | 7 ++ src/Traits/OptimisticLock.php | 12 +++- .../Tables/TestOptimisticLockTable.php | 2 +- tests/Traits/OptimisticLockTest.php | 2 +- 5 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 src/Exceptions/LockException.php diff --git a/src/AbstractTable.php b/src/AbstractTable.php index 822249b..5901139 100644 --- a/src/AbstractTable.php +++ b/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 $where + * @param array|string $orderBy * @return array|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 $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); + } + } + } + } } diff --git a/src/Exceptions/LockException.php b/src/Exceptions/LockException.php new file mode 100644 index 0000000..650194d --- /dev/null +++ b/src/Exceptions/LockException.php @@ -0,0 +1,7 @@ +lock_version; + } + + public function incrementVersion(): void + { + $this->lock_version++; + } } diff --git a/tests/TestStand/Tables/TestOptimisticLockTable.php b/tests/TestStand/Tables/TestOptimisticLockTable.php index cf318fd..8303514 100644 --- a/tests/TestStand/Tables/TestOptimisticLockTable.php +++ b/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 ); " diff --git a/tests/Traits/OptimisticLockTest.php b/tests/Traits/OptimisticLockTest.php index b64e973..9856e25 100644 --- a/tests/Traits/OptimisticLockTest.php +++ b/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); } } \ No newline at end of file From 63416c6b48d07478efb916a6b9b57e9f603e92c4 Mon Sep 17 00:00:00 2001 From: Composite PHP Date: Sat, 21 Oct 2023 23:07:23 +0100 Subject: [PATCH 5/5] Optimize save method --- src/AbstractTable.php | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/AbstractTable.php b/src/AbstractTable.php index 5901139..63da7fe 100644 --- a/src/AbstractTable.php +++ b/src/AbstractTable.php @@ -72,36 +72,20 @@ public function save(AbstractEntity &$entity): void $entity->updated_at = new \DateTimeImmutable(); $changedColumns['updated_at'] = DateTimeHelper::dateTimeToString($entity->updated_at); } - - 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(); - $versionUpdated = $connection->update( - $this->getTableName(), - $changedColumns, - $where - ); - if (!$versionUpdated) { - throw new Exceptions\LockException('Failed to update entity version, concurrency modification, rolling back.'); - } - $connection->commit(); - } catch (\Throwable $e) { - $connection->rollBack(); - throw $e; - } - } else { - $connection->update( - $this->getTableName(), - $changedColumns, - $where - ); + } + $entityUpdated = $connection->update( + table: $this->getTableName(), + data: $changedColumns, + criteria: $where, + ); + if ($this->config->hasOptimisticLock() && !$entityUpdated) { + throw new Exceptions\LockException('Failed to update entity version, concurrency modification, rolling back.'); } $entity->resetChangedColumns(); }