Skip to content

Commit 516463c

Browse files
barbosa89barbosa89
authored
Merge pull request #118 from phenixphp/bugfix/improve-insert-get-id
Improve insert get ID
2 parents 1f905fd + 267ce1a commit 516463c

13 files changed

Lines changed: 311 additions & 42 deletions

File tree

‎src/Database/Concerns/Query/HasDriver.php‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎src/Database/Concerns/Query/HasTransaction.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Phenix\Database\Concerns\Query;
66

77
use Amp\Sql\SqlConnection;
8+
use Amp\Sql\SqlResult;
89
use Amp\Sql\SqlTransaction;
910
use Closure;
1011
use Phenix\Database\TransactionContext;
@@ -85,7 +86,7 @@ public function setTransaction(SqlTransaction $transaction): self
8586
return $this;
8687
}
8788

88-
protected function exec(string $dml, array $params = []): mixed
89+
protected function exec(string $dml, array $params = []): SqlResult
8990
{
9091
return $this->getExecutor()->prepare($dml)->execute($params);
9192
}

‎src/Database/Dialects/Postgres/Compilers/Insert.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,27 @@ public function compile(QueryAst $ast): CompiledClause
6161

6262
$parts[] = 'ON CONFLICT DO NOTHING';
6363

64+
if (! empty($ast->returning)) {
65+
$parts[] = 'RETURNING';
66+
$parts[] = Arr::implodeDeeply($ast->returning, ', ');
67+
}
68+
6469
$sql = Arr::implodeDeeply($parts);
6570
$sql = $this->convertPlaceholders($sql);
6671

6772
return new CompiledClause($sql, $ast->params);
6873
}
6974

7075
$result = parent::compile($ast);
76+
$parts = [$result->sql];
77+
78+
if (! empty($ast->returning)) {
79+
$parts[] = 'RETURNING';
80+
$parts[] = Arr::implodeDeeply($ast->returning, ', ');
81+
}
7182

7283
return new CompiledClause(
73-
$this->convertPlaceholders($result->sql),
84+
$this->convertPlaceholders(Arr::implodeDeeply($parts)),
7485
$result->params
7586
);
7687
}

‎src/Database/Dialects/Sqlite/Compilers/Insert.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Phenix\Database\Dialects\Sqlite\Compilers;
66

7+
use Phenix\Database\Dialects\CompiledClause;
78
use Phenix\Database\Dialects\Compilers\InsertCompiler;
89
use Phenix\Database\QueryAst;
910
use Phenix\Util\Arr;
@@ -40,4 +41,20 @@ protected function compileUpsert(QueryAst $ast): string
4041
Arr::implodeDeeply($updateColumns, ', ')
4142
);
4243
}
44+
45+
public function compile(QueryAst $ast): CompiledClause
46+
{
47+
$result = parent::compile($ast);
48+
$parts = [$result->sql];
49+
50+
if (! empty($ast->returning)) {
51+
$parts[] = 'RETURNING';
52+
$parts[] = Arr::implodeDeeply($ast->returning, ', ');
53+
}
54+
55+
return new CompiledClause(
56+
Arr::implodeDeeply($parts),
57+
$result->params
58+
);
59+
}
4360
}

‎src/Database/Grammar.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44

55
namespace Phenix\Database;
66

7+
use Amp\Mysql\MysqlConnectionPool;
8+
use Amp\Postgres\PostgresConnectionPool;
9+
use Amp\Sql\SqlConnection;
710
use Phenix\Database\Constants\Driver;
11+
use Phenix\Facades\Config;
12+
use Phenix\Sqlite\SqliteConnection;
813

914
abstract class Grammar
1015
{
@@ -21,4 +26,34 @@ public function getDriver(): Driver
2126
{
2227
return $this->driver;
2328
}
29+
30+
protected function resolveDriver(SqlConnection $connection): void
31+
{
32+
$driver = $this->resolveDriverFromConnection($connection);
33+
$driver ??= $this->resolveDriverFromConfig();
34+
35+
$this->driver = $driver;
36+
}
37+
38+
protected function resolveDriverFromConfig(): Driver
39+
{
40+
$default = Config::get('database.default');
41+
42+
return Driver::tryFrom($default) ?? Driver::MYSQL;
43+
}
44+
45+
protected function resolveDriverFromConnection(SqlConnection $connection): Driver|null
46+
{
47+
$driver = null;
48+
49+
if ($connection instanceof MysqlConnectionPool) {
50+
$driver = Driver::MYSQL;
51+
} elseif ($connection instanceof PostgresConnectionPool) {
52+
$driver = Driver::POSTGRESQL;
53+
} elseif ($connection instanceof SqliteConnection) {
54+
$driver = Driver::SQLITE;
55+
}
56+
57+
return $driver;
58+
}
2459
}

‎src/Database/Models/DatabaseModel.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,13 @@ public function getModelKeyName(): string
107107
return $this->modelKey->getName();
108108
}
109109

110+
public function getModelKeyColumnName(): string
111+
{
112+
$this->modelKey ??= $this->findModelKey();
113+
114+
return $this->modelKey->getColumnName();
115+
}
116+
110117
public function setConnection(SqlConnection|string $connection): void
111118
{
112119
$this->modelConnection = $connection;
@@ -167,7 +174,8 @@ public function save(TransactionManager|null $transactionManager = null): bool
167174
->update($data);
168175
}
169176

170-
$result = $queryBuilder->insertRow($data);
177+
$result = $queryBuilder
178+
->insertGetId($data, $this->getModelKeyColumnName());
171179

172180
if ($result) {
173181
if (! $this->keyIsInitialized()) {

‎src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public function __construct()
4646
$this->relationships = [];
4747
$this->connection = App::make(Connection::default());
4848

49-
$this->resolveDriverFromConnection($this->connection);
49+
$this->resolveDriver($this->connection);
5050
}
5151

5252
public function __clone(): void
@@ -204,8 +204,7 @@ public function create(array $attributes): DatabaseModel
204204

205205
$queryBuilder = clone $this;
206206
$queryBuilder->setModel($model);
207-
208-
$result = $queryBuilder->insertRow($data);
207+
$result = $queryBuilder->insertGetId($data, $model->getModelKeyColumnName());
209208

210209
if ($result) {
211210
$modelKeyName = $model->getModelKeyName();

‎src/Database/QueryBase.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
use Closure;
88
use Phenix\Database\Concerns\Query\BuildsQuery;
9-
use Phenix\Database\Concerns\Query\HasDriver;
109
use Phenix\Database\Concerns\Query\HasJoinClause;
1110
use Phenix\Database\Concerns\Query\HasLock;
1211
use Phenix\Database\Constants\Action;
@@ -17,7 +16,6 @@
1716

1817
abstract class QueryBase extends Clause implements QueryBuilder, Builder
1918
{
20-
use HasDriver;
2119
use BuildsQuery;
2220
use HasLock;
2321
use HasJoinClause;

‎src/Database/QueryBuilder.php‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
namespace Phenix\Database;
66

7-
use Amp\Mysql\Internal\MysqlPooledResult;
87
use Amp\Sql\SqlConnection;
98
use Amp\Sql\SqlQueryError;
109
use Amp\Sql\SqlResult;
@@ -17,6 +16,7 @@
1716
use Phenix\Database\Concerns\Query\HasTransaction;
1817
use Phenix\Database\Constants\Action;
1918
use Phenix\Database\Constants\Connection;
19+
use Phenix\Database\Constants\Driver;
2020

2121
use function is_string;
2222

@@ -32,7 +32,7 @@ public function __construct()
3232

3333
$this->connection = App::make(Connection::default());
3434

35-
$this->resolveDriverFromConnection($this->connection);
35+
$this->resolveDriver($this->connection);
3636
}
3737

3838
public function __clone(): void
@@ -56,7 +56,7 @@ public function connection(SqlConnection|string $connection): self
5656

5757
$this->connection = $connection;
5858

59-
$this->resolveDriverFromConnection($this->connection);
59+
$this->resolveDriver($this->connection);
6060

6161
return $this;
6262
}
@@ -169,15 +169,22 @@ public function insertFrom(Closure $subquery, array $columns, bool $ignore = fal
169169
}
170170
}
171171

172-
public function insertRow(array $data): int|string|bool
172+
public function insertGetId(array $data, string $columns = 'id'): int|string|false|null
173173
{
174-
[$dml, $params] = parent::insert($data);
174+
$this->returning((array) $columns);
175175

176176
try {
177-
/** @var MysqlPooledResult $result */
177+
[$dml, $params] = parent::insert($data);
178178
$result = $this->exec($dml, $params);
179179

180-
return $result->getLastInsertId();
180+
if (method_exists($result, 'getLastInsertId') && $this->driver !== Driver::POSTGRESQL) {
181+
return $result->getLastInsertId();
182+
}
183+
184+
$row = $result->fetchRow();
185+
$row ??= [];
186+
187+
return array_values($row)[0] ?? null;
181188
} catch (SqlQueryError|SqlTransactionError $e) {
182189
report($e);
183190

‎tests/Feature/Database/Models/Postgres/DatabaseModelTest.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Phenix\Database\Constants\Connection;
6+
use Phenix\Database\Constants\Driver;
7+
use Phenix\Database\Models\QueryBuilders\DatabaseQueryBuilder;
8+
use Phenix\Facades\Config;
9+
use Tests\Feature\Database\Models\User;
10+
use Tests\Mocks\Database\PostgresqlConnectionPool;
11+
use Tests\Mocks\Database\Result;
12+
use Tests\Mocks\Database\Statement;
13+
14+
use function Pest\Faker\faker;
15+
16+
it('saves a new model on postgresql using its mapped key column', function (): void {
17+
Config::set('database.default', Driver::POSTGRESQL->value);
18+
19+
$capturedSql = '';
20+
$connection = $this->getMockBuilder(PostgresqlConnectionPool::class)->getMock();
21+
22+
$connection->expects($this->once())
23+
->method('prepare')
24+
->willReturnCallback(function (string $sql) use (&$capturedSql): Statement {
25+
$capturedSql = $sql;
26+
27+
$result = new Result([['user_id' => 77]]);
28+
$result->setLastInsertedId(77);
29+
30+
return new Statement($result);
31+
});
32+
33+
$this->app->swap(Connection::name(Driver::POSTGRESQL->value), $connection);
34+
35+
$model = new User();
36+
$model->setConnection(Driver::POSTGRESQL->value);
37+
$model->name = 'John Doe';
38+
$model->email = faker()->email();
39+
40+
expect($model->save())->toBeTrue();
41+
expect($model->id)->toBe(77);
42+
expect($model->isExisting())->toBeTrue();
43+
expect($capturedSql)->toContain('RETURNING id');
44+
});
45+
46+
it('creates a new model on postgresql using its mapped key column', function (): void {
47+
Config::set('database.default', Driver::POSTGRESQL->value);
48+
49+
$capturedSql = '';
50+
$connection = $this->getMockBuilder(PostgresqlConnectionPool::class)->getMock();
51+
52+
$connection->expects($this->once())
53+
->method('prepare')
54+
->willReturnCallback(function (string $sql) use (&$capturedSql): Statement {
55+
$capturedSql = $sql;
56+
57+
return new Statement(new Result([['user_id' => 88]]));
58+
});
59+
60+
$queryBuilder = new DatabaseQueryBuilder();
61+
$queryBuilder->connection($connection);
62+
$queryBuilder->setModel(new User());
63+
64+
$model = $queryBuilder->create([
65+
'name' => 'Jane Doe',
66+
'email' => faker()->email(),
67+
]);
68+
69+
expect($model->id)->toBe(88);
70+
expect($model->isExisting())->toBeTrue();
71+
expect($capturedSql)->toContain('RETURNING id');
72+
});

0 commit comments

Comments
 (0)