Skip to content

Commit 59c52ab

Browse files
author
Composite PHP
committed
- Add DoctrineTypes usage
- Bump doctrine/dbal version - Prevent empty transaction in CombinedTransaction - Add ability to define Doctrine Configuration in config file
1 parent 20a1fc5 commit 59c52ab

File tree

9 files changed

+140
-46
lines changed

9 files changed

+140
-46
lines changed

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"ext-pdo": "*",
1717
"psr/simple-cache": "1 - 3",
1818
"compositephp/entity": "^v0.1.11",
19-
"doctrine/dbal": "^3.5"
19+
"doctrine/dbal": "^4.2"
2020
},
2121
"require-dev": {
2222
"kodus/file-cache": "^2.0",

‎src/AbstractTable.php‎

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
use Composite\DB\MultiQuery\MultiInsert;
77
use Composite\DB\MultiQuery\MultiSelect;
88
use Composite\Entity\AbstractEntity;
9+
use Composite\Entity\Columns;
910
use Composite\Entity\Helpers\DateTimeHelper;
1011
use Doctrine\DBAL\Connection;
12+
use Doctrine\DBAL\ParameterType;
1113
use Ramsey\Uuid\UuidInterface;
1214

1315
abstract class AbstractTable
@@ -52,8 +54,13 @@ public function save(AbstractEntity $entity): void
5254
$connection = $this->getConnection();
5355
$this->checkUpdatedAt($entity);
5456

55-
$insertData = $this->prepareDataForSql($entity->toArray());
56-
$this->getConnection()->insert($this->getTableName(), $insertData);
57+
$insertData = $entity->toArray();
58+
$preparedInsertData = $this->prepareDataForSql($insertData);
59+
$this->getConnection()->insert(
60+
table: $this->getTableName(),
61+
data: $preparedInsertData,
62+
types: $this->getDoctrineTypes($insertData),
63+
);
5764

5865
if ($this->config->autoIncrementKey && ($lastInsertedId = $connection->lastInsertId())) {
5966
$insertData[$this->config->autoIncrementKey] = intval($lastInsertedId);
@@ -66,7 +73,6 @@ public function save(AbstractEntity $entity): void
6673
if (!$changedColumns = $entity->getChangedColumns()) {
6774
return;
6875
}
69-
$changedColumns = $this->prepareDataForSql($changedColumns);
7076
if ($this->config->hasUpdatedAt() && property_exists($entity, 'updated_at')) {
7177
$entity->updated_at = new \DateTimeImmutable();
7278
$changedColumns['updated_at'] = DateTimeHelper::dateTimeToString($entity->updated_at);
@@ -81,10 +87,19 @@ public function save(AbstractEntity $entity): void
8187
}
8288
$updateString = implode(', ', array_map(fn ($key) => $this->escapeIdentifier($key) . "=?", array_keys($changedColumns)));
8389
$whereString = implode(' AND ', array_map(fn ($key) => $this->escapeIdentifier($key) . "=?", array_keys($whereParams)));
90+
$preparedParams = array_merge(
91+
array_values($this->prepareDataForSql($changedColumns)),
92+
array_values($this->prepareDataForSql($whereParams)),
93+
);
94+
$types = array_merge(
95+
$this->getDoctrineTypes($changedColumns),
96+
$this->getDoctrineTypes($whereParams),
97+
);
8498

8599
$entityUpdated = (bool)$this->getConnection()->executeStatement(
86100
sql: "UPDATE " . $this->escapeIdentifier($this->getTableName()) . " SET $updateString WHERE $whereString;",
87-
params: array_merge(array_values($changedColumns), array_values($whereParams)),
101+
params: $preparedParams,
102+
types: $types,
88103
);
89104
if ($this->config->hasOptimisticLock() && !$entityUpdated) {
90105
throw new Exceptions\LockException('Failed to update entity version, concurrency modification, rolling back.');
@@ -93,6 +108,23 @@ public function save(AbstractEntity $entity): void
93108
}
94109
}
95110

111+
private function getDoctrineTypes(array $data): array
112+
{
113+
$result = [];
114+
foreach ($data as $value) {
115+
if (is_bool($value)) {
116+
$result[] = ParameterType::BOOLEAN;
117+
} elseif (is_int($value)) {
118+
$result[] = ParameterType::INTEGER;
119+
} elseif (is_null($value)) {
120+
$result[] = ParameterType::NULL;
121+
} else {
122+
$result[] = ParameterType::STRING;
123+
}
124+
}
125+
return $result;
126+
}
127+
96128
/**
97129
* @param AbstractEntity[] $entities
98130
* @throws \Throwable
@@ -124,7 +156,11 @@ public function saveMany(array $entities): void
124156
rows: $chunk,
125157
);
126158
if ($multiInsert->getSql()) {
127-
$connection->executeStatement($multiInsert->getSql(), $multiInsert->getParameters());
159+
$connection->executeStatement(
160+
sql: $multiInsert->getSql(),
161+
params: $multiInsert->getParameters(),
162+
types: $this->getDoctrineTypes(array_keys($chunk[0])),
163+
);
128164
}
129165
}
130166
}
@@ -317,6 +353,9 @@ final protected function createEntities(mixed $data, ?string $keyColumnName = nu
317353
*/
318354
protected function getPkCondition(int|string|array|AbstractEntity|UuidInterface $data): array
319355
{
356+
if (empty($this->config->primaryKeys)) {
357+
throw new \Exception("Primary keys are not defined in `" . $this::class . "` table config");
358+
}
320359
$condition = [];
321360
if ($data instanceof AbstractEntity) {
322361
if ($data->isNew()) {

‎src/CombinedTransaction.php‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ class CombinedTransaction
1717
/**
1818
* @throws Exceptions\DbException
1919
*/
20-
public function save(AbstractTable $table, AbstractEntity &$entity): void
20+
public function save(AbstractTable $table, AbstractEntity $entity): void
2121
{
22-
$this->doInTransaction($table, function () use ($table, &$entity) {
23-
$table->save($entity);
24-
});
22+
if (!$entity->isNew() && !$entity->getChangedColumns()) {
23+
return;
24+
}
25+
$this->doInTransaction($table, fn() => $table->save($entity));
2526
}
2627

2728
/**
@@ -31,6 +32,9 @@ public function save(AbstractTable $table, AbstractEntity &$entity): void
3132
*/
3233
public function saveMany(AbstractTable $table, array $entities): void
3334
{
35+
if (!$entities) {
36+
return;
37+
}
3438
$this->doInTransaction($table, fn () => $table->saveMany($entities));
3539
}
3640

@@ -41,17 +45,18 @@ public function saveMany(AbstractTable $table, array $entities): void
4145
*/
4246
public function deleteMany(AbstractTable $table, array $entities): void
4347
{
48+
if (!$entities) {
49+
return;
50+
}
4451
$this->doInTransaction($table, fn () => $table->deleteMany($entities));
4552
}
4653

4754
/**
4855
* @throws Exceptions\DbException
4956
*/
50-
public function delete(AbstractTable $table, AbstractEntity &$entity): void
57+
public function delete(AbstractTable $table, AbstractEntity $entity): void
5158
{
52-
$this->doInTransaction($table, function () use ($table, &$entity) {
53-
$table->delete($entity);
54-
});
59+
$this->doInTransaction($table, fn () => $table->delete($entity));
5560
}
5661

5762
/**
@@ -82,9 +87,7 @@ public function commit(): void
8287
{
8388
foreach ($this->transactions as $connectionName => $connection) {
8489
try {
85-
if (!$connection->commit()) {
86-
throw new Exceptions\DbException("Could not commit transaction for database `$connectionName`");
87-
}
90+
$connection->commit();
8891
// I have no idea how to simulate failed commit
8992
// @codeCoverageIgnoreStart
9093
} catch (\Throwable $e) {
@@ -119,7 +122,11 @@ public function releaseLock(): void
119122
if (!$this->cache || !$this->lockKey) {
120123
return;
121124
}
122-
$this->cache->delete($this->lockKey);
125+
if (!$this->cache->delete($this->lockKey)) {
126+
// @codeCoverageIgnoreStart
127+
throw new DbException("Failed to release lock `{$this->lockKey}`");
128+
// @codeCoverageIgnoreEnd
129+
}
123130
}
124131

125132
private function doInTransaction(AbstractTable $table, callable $callback): void

‎src/ConnectionManager.php‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,10 @@
33
namespace Composite\DB;
44

55
use Composite\DB\Exceptions\DbException;
6-
use Doctrine\Common\EventManager;
76
use Doctrine\DBAL\Configuration;
87
use Doctrine\DBAL\Connection;
98
use Doctrine\DBAL\DriverManager;
109
use Doctrine\DBAL\Exception;
11-
use Doctrine\DBAL\Schema\DefaultSchemaManagerFactory;
1210

1311
class ConnectionManager
1412
{
@@ -21,20 +19,20 @@ class ConnectionManager
2119
/**
2220
* @throws DbException
2321
*/
24-
public static function getConnection(string $name, ?Configuration $config = null, ?EventManager $eventManager = null): Connection
22+
public static function getConnection(string $name, ?Configuration $config = null): Connection
2523
{
24+
if (self::$configs === null) {
25+
self::$configs = self::loadConfigs();
26+
}
2627
if (!isset(self::$connections[$name])) {
2728
try {
28-
if (!$config) {
29-
$config = new Configuration();
30-
}
31-
if (!$config->getSchemaManagerFactory()) {
32-
$config->setSchemaManagerFactory(new DefaultSchemaManagerFactory());
29+
$connectionParams = self::$configs[$name] ?? throw new DbException("Connection config `$name` not found");
30+
if (!$config && isset($connectionParams['configuration']) && $connectionParams['configuration'] instanceof Configuration) {
31+
$config = $connectionParams['configuration'];
3332
}
3433
self::$connections[$name] = DriverManager::getConnection(
35-
params: self::getConnectionParams($name),
34+
params: $connectionParams,
3635
config: $config,
37-
eventManager: $eventManager,
3836
);
3937
} catch (Exception $e) {
4038
throw new DbException($e->getMessage(), $e->getCode(), $e);
@@ -43,18 +41,6 @@ public static function getConnection(string $name, ?Configuration $config = null
4341
return self::$connections[$name];
4442
}
4543

46-
/**
47-
* @return array<string, mixed>
48-
* @throws DbException
49-
*/
50-
private static function getConnectionParams(string $name): array
51-
{
52-
if (self::$configs === null) {
53-
self::$configs = self::loadConfigs();
54-
}
55-
return self::$configs[$name] ?? throw new DbException("Connection config `$name` not found");
56-
}
57-
5844
/**
5945
* @return array<string, array<string, mixed>>
6046
* @throws DbException

‎src/Helpers/DatabaseSpecificTrait.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@ private function identifyPlatform(): void
3939
*/
4040
private function prepareDataForSql(array $data): array
4141
{
42-
$this->identifyPlatform();
4342
foreach ($data as $columnName => $value) {
44-
if (is_bool($value) && !$this->isPostgreSQL) {
43+
if (is_bool($value)) {
4544
$data[$columnName] = $value ? 1 : 0;
4645
}
4746
}

‎tests/Table/AbstractTableTest.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Composite\DB\Tests\Table;
44

55
use Composite\DB\AbstractTable;
6+
use Composite\DB\TableConfig;
67
use Composite\DB\Tests\TestStand\Entities;
78
use Composite\DB\Tests\TestStand\Tables;
89
use Composite\Entity\AbstractEntity;
@@ -197,4 +198,25 @@ public function test_databaseSpecific(): void
197198
$postgresTable = new Tables\TestPostgresTable();
198199
$this->assertEquals('"column"', $postgresTable->escapeIdentifierPub('column'));
199200
}
200-
}
201+
202+
public function test_getPkCondition_throwsExceptionWhenNoPrimaryKeys(): void
203+
{
204+
$mockTable = new class extends AbstractTable {
205+
protected function getConfig(): TableConfig
206+
{
207+
return new TableConfig(
208+
connectionName: 'default',
209+
tableName: 'test_table',
210+
entityClass: AbstractEntity::class,
211+
primaryKeys: [], // Empty primary keys
212+
);
213+
}
214+
};
215+
216+
$this->expectException(\Exception::class);
217+
$this->expectExceptionMessage("Primary keys are not defined in `" . get_class($mockTable) . "` table config");
218+
219+
$reflectionMethod = new \ReflectionMethod($mockTable, 'getPkCondition');
220+
$reflectionMethod->invoke($mockTable, 1);
221+
}
222+
}

‎tests/Table/CombinedTransactionTest.php‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,42 @@ public static function buildLockKey_dataProvider()
204204
'more than max length' => [[str_repeat('a', 55)], sha1('composite.lock.' . str_repeat('a', 55))],
205205
];
206206
}
207-
}
207+
208+
public function test_saveWithNoChanges(): void
209+
{
210+
$autoIncrementTable = new Tables\TestAutoincrementTable();
211+
212+
$entity = new Entities\TestAutoincrementEntity(name: 'TestNoChanges');
213+
$autoIncrementTable->save($entity);
214+
215+
$transaction = new CombinedTransaction();
216+
$transaction->save($autoIncrementTable, $entity);
217+
$transaction->commit();
218+
219+
$this->assertNotNull($autoIncrementTable->findByPk($entity->id));
220+
221+
$autoIncrementTable->delete($entity);
222+
}
223+
224+
public function test_saveManyWithEmptyEntities(): void
225+
{
226+
$autoIncrementTable = new Tables\TestAutoincrementTable();
227+
228+
$transaction = new CombinedTransaction();
229+
$transaction->saveMany($autoIncrementTable, []);
230+
$transaction->commit();
231+
232+
$this->assertTrue(true);
233+
}
234+
235+
public function test_deleteManyWithEmptyEntities(): void
236+
{
237+
$autoIncrementTable = new Tables\TestAutoincrementTable();
238+
239+
$transaction = new CombinedTransaction();
240+
$transaction->deleteMany($autoIncrementTable, []);
241+
$transaction->commit();
242+
243+
$this->assertTrue(true);
244+
}
245+
}

‎tests/Traits/OptimisticLockTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function test_trait(): void
2929
$aiEntity2->name = 'John2';
3030
$aiTable2->save($aiEntity2);
3131

32-
$this->assertTrue($db->commit());
32+
$db->commit();
3333

3434
$aiEntity3 = $aiTable1->findByPk($aiEntity1->id);
3535
$this->assertEquals('John2', $aiEntity3->name);
@@ -57,7 +57,7 @@ public function test_trait(): void
5757
}
5858
$this->assertTrue($exceptionCaught);
5959

60-
$this->assertTrue($db->rollBack());
60+
$db->rollBack();
6161

6262
$olEntity3 = $olTable1->findByPk($olEntity1->id);
6363
$this->assertEquals(1, $olEntity3->getVersion());

‎tests/config.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php declare(strict_types=1);
22

3+
use Doctrine\DBAL\Configuration;
4+
35
return [
46
'sqlite' => [
57
'driver' => 'pdo_sqlite',
@@ -11,6 +13,7 @@
1113
'user' => 'test',
1214
'password' => 'test',
1315
'host' => '127.0.0.1',
16+
'configuration' => new Configuration(),
1417
],
1518
'postgres' => [
1619
'driver' => 'pdo_pgsql',

0 commit comments

Comments
 (0)