From 6fcf3cff252041747937b6c5463238c37f81a397 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Mon, 30 Mar 2026 22:28:43 +0000 Subject: [PATCH 1/3] refactor: streamline driver resolution logic in database classes --- src/Database/Concerns/Query/HasDriver.php | 27 -------------- src/Database/Grammar.php | 35 +++++++++++++++++++ .../QueryBuilders/DatabaseQueryBuilder.php | 2 +- src/Database/QueryBase.php | 2 -- src/Database/QueryBuilder.php | 5 +-- 5 files changed, 39 insertions(+), 32 deletions(-) delete mode 100644 src/Database/Concerns/Query/HasDriver.php diff --git a/src/Database/Concerns/Query/HasDriver.php b/src/Database/Concerns/Query/HasDriver.php deleted file mode 100644 index 6334f30b..00000000 --- a/src/Database/Concerns/Query/HasDriver.php +++ /dev/null @@ -1,27 +0,0 @@ -setDriver(Driver::MYSQL); - } elseif ($pool instanceof PostgresConnectionPool) { - $this->setDriver(Driver::POSTGRESQL); - } elseif ($pool instanceof SqliteConnection) { - $this->setDriver(Driver::SQLITE); - } else { - $this->setDriver(Driver::MYSQL); - } - } -} diff --git a/src/Database/Grammar.php b/src/Database/Grammar.php index 114a6c4b..1857db3b 100644 --- a/src/Database/Grammar.php +++ b/src/Database/Grammar.php @@ -4,7 +4,12 @@ namespace Phenix\Database; +use Amp\Mysql\MysqlConnectionPool; +use Amp\Postgres\PostgresConnectionPool; +use Amp\Sql\SqlConnection; use Phenix\Database\Constants\Driver; +use Phenix\Facades\Config; +use Phenix\Sqlite\SqliteConnection; abstract class Grammar { @@ -21,4 +26,34 @@ public function getDriver(): Driver { return $this->driver; } + + protected function resolveDriver(SqlConnection $connection): void + { + $driver = $this->resolveDriverFromConnection($connection); + $driver ??= $this->resolveDriverFromConfig(); + + $this->driver = $driver; + } + + protected function resolveDriverFromConfig(): Driver + { + $default = Config::get('database.default'); + + return Driver::tryFrom($default) ?? Driver::MYSQL; + } + + protected function resolveDriverFromConnection(SqlConnection $connection): Driver|null + { + $driver = null; + + if ($connection instanceof MysqlConnectionPool) { + $driver = Driver::MYSQL; + } elseif ($connection instanceof PostgresConnectionPool) { + $driver = Driver::POSTGRESQL; + } elseif ($connection instanceof SqliteConnection) { + $driver = Driver::SQLITE; + } + + return $driver; + } } diff --git a/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php b/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php index 2203be47..fc1df915 100644 --- a/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php +++ b/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php @@ -46,7 +46,7 @@ public function __construct() $this->relationships = []; $this->connection = App::make(Connection::default()); - $this->resolveDriverFromConnection($this->connection); + $this->resolveDriver($this->connection); } public function __clone(): void diff --git a/src/Database/QueryBase.php b/src/Database/QueryBase.php index 4b69b661..253ca55b 100644 --- a/src/Database/QueryBase.php +++ b/src/Database/QueryBase.php @@ -6,7 +6,6 @@ use Closure; use Phenix\Database\Concerns\Query\BuildsQuery; -use Phenix\Database\Concerns\Query\HasDriver; use Phenix\Database\Concerns\Query\HasJoinClause; use Phenix\Database\Concerns\Query\HasLock; use Phenix\Database\Constants\Action; @@ -17,7 +16,6 @@ abstract class QueryBase extends Clause implements QueryBuilder, Builder { - use HasDriver; use BuildsQuery; use HasLock; use HasJoinClause; diff --git a/src/Database/QueryBuilder.php b/src/Database/QueryBuilder.php index 7f064a29..3fe64523 100644 --- a/src/Database/QueryBuilder.php +++ b/src/Database/QueryBuilder.php @@ -17,6 +17,7 @@ use Phenix\Database\Concerns\Query\HasTransaction; use Phenix\Database\Constants\Action; use Phenix\Database\Constants\Connection; +use Phenix\Database\Constants\Driver; use function is_string; @@ -32,7 +33,7 @@ public function __construct() $this->connection = App::make(Connection::default()); - $this->resolveDriverFromConnection($this->connection); + $this->resolveDriver($this->connection); } public function __clone(): void @@ -56,7 +57,7 @@ public function connection(SqlConnection|string $connection): self $this->connection = $connection; - $this->resolveDriverFromConnection($this->connection); + $this->resolveDriver($this->connection); return $this; } From c57cc7d1f66a5a17df849814b335adff28c44ff4 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 31 Mar 2026 16:52:30 +0000 Subject: [PATCH 2/3] feat: implement insertGetId method to return generated IDs for insert operations --- .../Concerns/Query/HasTransaction.php | 3 +- .../Dialects/Postgres/Compilers/Insert.php | 13 +++- .../Dialects/Sqlite/Compilers/Insert.php | 17 +++++ src/Database/Models/DatabaseModel.php | 10 ++- .../QueryBuilders/DatabaseQueryBuilder.php | 3 +- src/Database/QueryBuilder.php | 19 +++-- .../Models/Postgres/DatabaseModelTest.php | 72 +++++++++++++++++++ tests/Unit/Database/QueryBuilderTest.php | 64 +++++++++++++++++ .../Postgres/InsertIntoStatementTest.php | 42 +++++++++++ .../Sqlite/InsertIntoStatementTest.php | 42 +++++++++++ 10 files changed, 275 insertions(+), 10 deletions(-) create mode 100644 tests/Feature/Database/Models/Postgres/DatabaseModelTest.php diff --git a/src/Database/Concerns/Query/HasTransaction.php b/src/Database/Concerns/Query/HasTransaction.php index b8dc4dc5..58ec5484 100644 --- a/src/Database/Concerns/Query/HasTransaction.php +++ b/src/Database/Concerns/Query/HasTransaction.php @@ -5,6 +5,7 @@ namespace Phenix\Database\Concerns\Query; use Amp\Sql\SqlConnection; +use Amp\Sql\SqlResult; use Amp\Sql\SqlTransaction; use Closure; use Phenix\Database\TransactionContext; @@ -85,7 +86,7 @@ public function setTransaction(SqlTransaction $transaction): self return $this; } - protected function exec(string $dml, array $params = []): mixed + protected function exec(string $dml, array $params = []): SqlResult { return $this->getExecutor()->prepare($dml)->execute($params); } diff --git a/src/Database/Dialects/Postgres/Compilers/Insert.php b/src/Database/Dialects/Postgres/Compilers/Insert.php index c30306f1..23d44c01 100644 --- a/src/Database/Dialects/Postgres/Compilers/Insert.php +++ b/src/Database/Dialects/Postgres/Compilers/Insert.php @@ -61,6 +61,11 @@ public function compile(QueryAst $ast): CompiledClause $parts[] = 'ON CONFLICT DO NOTHING'; + if (! empty($ast->returning)) { + $parts[] = 'RETURNING'; + $parts[] = Arr::implodeDeeply($ast->returning, ', '); + } + $sql = Arr::implodeDeeply($parts); $sql = $this->convertPlaceholders($sql); @@ -68,9 +73,15 @@ public function compile(QueryAst $ast): CompiledClause } $result = parent::compile($ast); + $parts = [$result->sql]; + + if (! empty($ast->returning)) { + $parts[] = 'RETURNING'; + $parts[] = Arr::implodeDeeply($ast->returning, ', '); + } return new CompiledClause( - $this->convertPlaceholders($result->sql), + $this->convertPlaceholders(Arr::implodeDeeply($parts)), $result->params ); } diff --git a/src/Database/Dialects/Sqlite/Compilers/Insert.php b/src/Database/Dialects/Sqlite/Compilers/Insert.php index b19e563a..cdad0799 100644 --- a/src/Database/Dialects/Sqlite/Compilers/Insert.php +++ b/src/Database/Dialects/Sqlite/Compilers/Insert.php @@ -4,6 +4,7 @@ namespace Phenix\Database\Dialects\Sqlite\Compilers; +use Phenix\Database\Dialects\CompiledClause; use Phenix\Database\Dialects\Compilers\InsertCompiler; use Phenix\Database\QueryAst; use Phenix\Util\Arr; @@ -40,4 +41,20 @@ protected function compileUpsert(QueryAst $ast): string Arr::implodeDeeply($updateColumns, ', ') ); } + + public function compile(QueryAst $ast): CompiledClause + { + $result = parent::compile($ast); + $parts = [$result->sql]; + + if (! empty($ast->returning)) { + $parts[] = 'RETURNING'; + $parts[] = Arr::implodeDeeply($ast->returning, ', '); + } + + return new CompiledClause( + Arr::implodeDeeply($parts), + $result->params + ); + } } diff --git a/src/Database/Models/DatabaseModel.php b/src/Database/Models/DatabaseModel.php index 5a84fca1..83852c61 100644 --- a/src/Database/Models/DatabaseModel.php +++ b/src/Database/Models/DatabaseModel.php @@ -107,6 +107,13 @@ public function getModelKeyName(): string return $this->modelKey->getName(); } + public function getModelKeyColumnName(): string + { + $this->modelKey ??= $this->findModelKey(); + + return $this->modelKey->getColumnName(); + } + public function setConnection(SqlConnection|string $connection): void { $this->modelConnection = $connection; @@ -167,7 +174,8 @@ public function save(TransactionManager|null $transactionManager = null): bool ->update($data); } - $result = $queryBuilder->insertRow($data); + $result = $queryBuilder + ->insertGetId($data, $this->getModelKeyColumnName()); if ($result) { if (! $this->keyIsInitialized()) { diff --git a/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php b/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php index fc1df915..3a7c3ed8 100644 --- a/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php +++ b/src/Database/Models/QueryBuilders/DatabaseQueryBuilder.php @@ -204,8 +204,7 @@ public function create(array $attributes): DatabaseModel $queryBuilder = clone $this; $queryBuilder->setModel($model); - - $result = $queryBuilder->insertRow($data); + $result = $queryBuilder->insertGetId($data, $model->getModelKeyColumnName()); if ($result) { $modelKeyName = $model->getModelKeyName(); diff --git a/src/Database/QueryBuilder.php b/src/Database/QueryBuilder.php index 3fe64523..47c16433 100644 --- a/src/Database/QueryBuilder.php +++ b/src/Database/QueryBuilder.php @@ -4,7 +4,6 @@ namespace Phenix\Database; -use Amp\Mysql\Internal\MysqlPooledResult; use Amp\Sql\SqlConnection; use Amp\Sql\SqlQueryError; use Amp\Sql\SqlResult; @@ -170,15 +169,25 @@ public function insertFrom(Closure $subquery, array $columns, bool $ignore = fal } } - public function insertRow(array $data): int|string|bool + public function insertGetId(array $data, string $columns = 'id'): int|string|false|null { - [$dml, $params] = parent::insert($data); + $this->returning((array) $columns); try { - /** @var MysqlPooledResult $result */ + [$dml, $params] = parent::insert($data); $result = $this->exec($dml, $params); - return $result->getLastInsertId(); + if (method_exists($result, 'getLastInsertId') && $this->driver !== Driver::POSTGRESQL) { + /** @var int|string|null $lastInsertId */ + $lastInsertId = $result->getLastInsertId(); + + return $lastInsertId; + } + + $row = $result->fetchRow(); + $row ??= []; + + return array_values($row)[0] ?? null; } catch (SqlQueryError|SqlTransactionError $e) { report($e); diff --git a/tests/Feature/Database/Models/Postgres/DatabaseModelTest.php b/tests/Feature/Database/Models/Postgres/DatabaseModelTest.php new file mode 100644 index 00000000..542182a2 --- /dev/null +++ b/tests/Feature/Database/Models/Postgres/DatabaseModelTest.php @@ -0,0 +1,72 @@ +value); + + $capturedSql = ''; + $connection = $this->getMockBuilder(PostgresqlConnectionPool::class)->getMock(); + + $connection->expects($this->once()) + ->method('prepare') + ->willReturnCallback(function (string $sql) use (&$capturedSql): Statement { + $capturedSql = $sql; + + $result = new Result([['user_id' => 77]]); + $result->setLastInsertedId(77); + + return new Statement($result); + }); + + $this->app->swap(Connection::name(Driver::POSTGRESQL->value), $connection); + + $model = new User(); + $model->setConnection(Driver::POSTGRESQL->value); + $model->name = 'John Doe'; + $model->email = faker()->email(); + + expect($model->save())->toBeTrue(); + expect($model->id)->toBe(77); + expect($model->isExisting())->toBeTrue(); + expect($capturedSql)->toContain('RETURNING id'); +}); + +it('creates a new model on postgresql using its mapped key column', function (): void { + Config::set('database.default', Driver::POSTGRESQL->value); + + $capturedSql = ''; + $connection = $this->getMockBuilder(PostgresqlConnectionPool::class)->getMock(); + + $connection->expects($this->once()) + ->method('prepare') + ->willReturnCallback(function (string $sql) use (&$capturedSql): Statement { + $capturedSql = $sql; + + return new Statement(new Result([['user_id' => 88]])); + }); + + $queryBuilder = new DatabaseQueryBuilder(); + $queryBuilder->connection($connection); + $queryBuilder->setModel(new User()); + + $model = $queryBuilder->create([ + 'name' => 'Jane Doe', + 'email' => faker()->email(), + ]); + + expect($model->id)->toBe(88); + expect($model->isExisting())->toBeTrue(); + expect($capturedSql)->toContain('RETURNING id'); +}); diff --git a/tests/Unit/Database/QueryBuilderTest.php b/tests/Unit/Database/QueryBuilderTest.php index a9e63944..7f28c86a 100644 --- a/tests/Unit/Database/QueryBuilderTest.php +++ b/tests/Unit/Database/QueryBuilderTest.php @@ -7,6 +7,7 @@ use League\Uri\Http; use Phenix\Data\Collection; use Phenix\Database\Constants\Connection; +use Phenix\Database\Constants\Driver; use Phenix\Database\Paginator; use Phenix\Database\QueryBuilder; use Phenix\Database\TransactionManager; @@ -99,6 +100,69 @@ expect($result)->toBeTrue(); }); +it('inserts row and returns generated id on postgresql', function () { + $capturedSql = ''; + $connection = $this->getMockBuilder(PostgresqlConnectionPool::class)->getMock(); + + $connection->expects($this->once()) + ->method('prepare') + ->willReturnCallback(function (string $sql) use (&$capturedSql): Statement { + $capturedSql = $sql; + + return new Statement(new Result([['id' => 123]])); + }); + + $query = new QueryBuilder(); + $query->connection($connection); + $query->setDriver(Driver::POSTGRESQL); + + $insertedId = $query->table('users')->insertGetId(['name' => 'Tony']); + + expect($insertedId)->toBe(123); + expect($capturedSql)->toContain('RETURNING id'); +}); + +it('inserts row and returns generated id on mysql', function () { + $result = new Result(); + $result->setLastInsertedId(321); + + $connection = $this->getMockBuilder(MysqlConnectionPool::class)->getMock(); + + $connection->expects($this->once()) + ->method('prepare') + ->willReturnCallback(fn () => new Statement($result)); + + $query = new QueryBuilder(); + $query->connection($connection); + + $insertedId = $query->table('users')->insertGetId(['name' => 'Tony']); + + expect($insertedId)->toBe(321); +}); + +it('inserts row and returns generated custom id on postgresql', function (): void { + $capturedSql = ''; + $connection = $this->getMockBuilder(PostgresqlConnectionPool::class)->getMock(); + + $connection->expects($this->once()) + ->method('prepare') + ->willReturnCallback(function (string $sql) use (&$capturedSql): Statement { + $capturedSql = $sql; + + return new Statement(new Result([['user_id' => 456]])); + }); + + $query = new QueryBuilder(); + $query->connection($connection); + $query->setDriver(Driver::POSTGRESQL); + + $insertedId = $query->table('users') + ->insertGetId(['name' => 'Tony'], 'user_id'); + + expect($insertedId)->toBe(456); + expect($capturedSql)->toContain('RETURNING user_id'); +}); + it('fails on insert records', function () { $connection = $this->getMockBuilder(MysqlConnectionPool::class)->getMock(); diff --git a/tests/Unit/Database/QueryGenerator/Postgres/InsertIntoStatementTest.php b/tests/Unit/Database/QueryGenerator/Postgres/InsertIntoStatementTest.php index e491633d..4204a7fc 100644 --- a/tests/Unit/Database/QueryGenerator/Postgres/InsertIntoStatementTest.php +++ b/tests/Unit/Database/QueryGenerator/Postgres/InsertIntoStatementTest.php @@ -74,6 +74,48 @@ expect($params)->toBe([$email, $name]); }); +it('generates insert into statement with returning clause', function () { + $query = new QueryGenerator(Driver::POSTGRESQL); + + $name = faker()->name; + $email = faker()->freeEmail; + + $sql = $query->table('users') + ->returning(['id']) + ->insert([ + 'name' => $name, + 'email' => $email, + ]); + + [$dml, $params] = $sql; + + $expected = "INSERT INTO users (email, name) VALUES ($1, $2) RETURNING id"; + + expect($dml)->toBe($expected); + expect($params)->toBe([$email, $name]); +}); + +it('generates insert ignore into statement with returning clause', function () { + $query = new QueryGenerator(Driver::POSTGRESQL); + + $name = faker()->name; + $email = faker()->freeEmail; + + $sql = $query->table('users') + ->returning(['id', 'email']) + ->insertOrIgnore([ + 'name' => $name, + 'email' => $email, + ]); + + [$dml, $params] = $sql; + + $expected = "INSERT INTO users (email, name) VALUES ($1, $2) ON CONFLICT DO NOTHING RETURNING id, email"; + + expect($dml)->toBe($expected); + expect($params)->toBe([$email, $name]); +}); + it('generates upsert statement to handle duplicate keys', function () { $query = new QueryGenerator(Driver::POSTGRESQL); diff --git a/tests/Unit/Database/QueryGenerator/Sqlite/InsertIntoStatementTest.php b/tests/Unit/Database/QueryGenerator/Sqlite/InsertIntoStatementTest.php index 5de88752..e319b049 100644 --- a/tests/Unit/Database/QueryGenerator/Sqlite/InsertIntoStatementTest.php +++ b/tests/Unit/Database/QueryGenerator/Sqlite/InsertIntoStatementTest.php @@ -74,6 +74,48 @@ expect($params)->toBe([$email, $name]); }); +it('generates insert into statement with returning clause', function () { + $query = new QueryGenerator(Driver::SQLITE); + + $name = faker()->name; + $email = faker()->freeEmail; + + $sql = $query->table('users') + ->returning(['id']) + ->insert([ + 'name' => $name, + 'email' => $email, + ]); + + [$dml, $params] = $sql; + + $expected = "INSERT INTO users (email, name) VALUES (?, ?) RETURNING id"; + + expect($dml)->toBe($expected); + expect($params)->toBe([$email, $name]); +}); + +it('generates insert ignore into statement with returning clause', function () { + $query = new QueryGenerator(Driver::SQLITE); + + $name = faker()->name; + $email = faker()->freeEmail; + + $sql = $query->table('users') + ->returning(['id', 'email']) + ->insertOrIgnore([ + 'name' => $name, + 'email' => $email, + ]); + + [$dml, $params] = $sql; + + $expected = "INSERT OR IGNORE INTO users (email, name) VALUES (?, ?) RETURNING id, email"; + + expect($dml)->toBe($expected); + expect($params)->toBe([$email, $name]); +}); + it('generates upsert statement to handle duplicate keys', function () { $query = new QueryGenerator(Driver::SQLITE); From 267ce1a43756595283947b3c0b6a2b4aaf61fd41 Mon Sep 17 00:00:00 2001 From: barbosa89 Date: Tue, 31 Mar 2026 17:02:49 +0000 Subject: [PATCH 3/3] fix: simplify insertGetId method by removing unnecessary variable assignment --- src/Database/QueryBuilder.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Database/QueryBuilder.php b/src/Database/QueryBuilder.php index 47c16433..283c69fb 100644 --- a/src/Database/QueryBuilder.php +++ b/src/Database/QueryBuilder.php @@ -178,10 +178,7 @@ public function insertGetId(array $data, string $columns = 'id'): int|string|fal $result = $this->exec($dml, $params); if (method_exists($result, 'getLastInsertId') && $this->driver !== Driver::POSTGRESQL) { - /** @var int|string|null $lastInsertId */ - $lastInsertId = $result->getLastInsertId(); - - return $lastInsertId; + return $result->getLastInsertId(); } $row = $result->fetchRow();