Skip to content

Commit b846c9a

Browse files
innocenziinnocenzi
committed
refactor(database): handle more primary key cases
1 parent 7472778 commit b846c9a

File tree

8 files changed

+85
-35
lines changed

8 files changed

+85
-35
lines changed

‎packages/database/src/Builder/QueryBuilders/InsertQueryBuilder.php‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,14 @@ public function __construct(
4949
/**
5050
* Executes the insert query and returns the primary key of the inserted record.
5151
*/
52-
public function execute(mixed ...$bindings): PrimaryKey
52+
public function execute(mixed ...$bindings): ?PrimaryKey
5353
{
5454
$id = $this->build()->execute(...$bindings);
5555

56+
if ($id === null) {
57+
return null;
58+
}
59+
5660
foreach ($this->after as $after) {
5761
$query = $after($id);
5862

@@ -98,7 +102,11 @@ public function build(mixed ...$bindings): Query
98102
$this->insert->addEntry($data);
99103
}
100104

101-
return new Query($this->insert, [...$this->bindings, ...$bindings])->onDatabase($this->onDatabase);
105+
return new Query(
106+
sql: $this->insert,
107+
bindings: [...$this->bindings, ...$bindings],
108+
primaryKeyColumn: $this->model->getPrimaryKey(),
109+
)->onDatabase($this->onDatabase);
102110
}
103111

104112
/**
@@ -135,9 +143,7 @@ private function resolveData(): array
135143

136144
// The rest are model objects
137145
$definition = inspect($model);
138-
139146
$modelClass = new ClassReflector($model);
140-
141147
$entry = [];
142148

143149
// Including all public properties
@@ -152,9 +158,13 @@ private function resolveData(): array
152158
}
153159

154160
$column = $property->getName();
155-
156161
$value = $property->getValue($model);
157162

163+
// Skip null primary key values to allow database auto-generation
164+
if ($property->getType()->getName() === PrimaryKey::class && $value === null) {
165+
continue;
166+
}
167+
158168
// BelongsTo and reverse HasMany relations are included
159169
if ($definition->isRelation($property)) {
160170
$relationModel = inspect($property->getType()->asClass());

‎packages/database/src/GenericDatabase.php‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,25 @@ public function getLastInsertId(): ?PrimaryKey
5959
{
6060
$sql = $this->lastQuery->toSql();
6161

62-
// TODO: properly determine whether a query is an insert or not
6362
if (! $sql->trim()->startsWith('INSERT')) {
6463
return null;
6564
}
6665

6766
if ($this->dialect === DatabaseDialect::POSTGRESQL) {
6867
$data = $this->lastStatement->fetch(PDO::FETCH_ASSOC);
69-
$lastInsertId = $data['id'] ?? null;
70-
} else {
71-
$lastInsertId = $this->connection->lastInsertId();
68+
69+
if (! $data) {
70+
return null;
71+
}
72+
73+
if ($this->lastQuery->primaryKeyColumn && isset($data[$this->lastQuery->primaryKeyColumn])) {
74+
return PrimaryKey::tryFrom($data[$this->lastQuery->primaryKeyColumn]);
75+
}
76+
77+
return null;
7278
}
7379

74-
return PrimaryKey::tryFrom($lastInsertId);
80+
return PrimaryKey::tryFrom($this->connection->lastInsertId());
7581
}
7682

7783
public function fetch(BuildsQuery|Query $query): array

‎packages/database/src/Query.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public function __construct(
2626
public array $bindings = [],
2727
/** @var \Closure[] $executeAfter */
2828
public array $executeAfter = [],
29+
public ?string $primaryKeyColumn = null,
2930
) {}
3031

3132
public function execute(mixed ...$bindings): ?PrimaryKey
@@ -40,8 +41,12 @@ public function execute(mixed ...$bindings): ?PrimaryKey
4041

4142
// TODO: add support for "after" queries to attach hasMany relations
4243

43-
return isset($query->bindings['id'])
44-
? new PrimaryKey($query->bindings['id'])
44+
if (! $this->primaryKeyColumn) {
45+
return null;
46+
}
47+
48+
return isset($query->bindings[$this->primaryKeyColumn])
49+
? new PrimaryKey($query->bindings[$this->primaryKeyColumn])
4550
: $database->getLastInsertId();
4651
}
4752

‎tests/Integration/Database/Builder/CustomPrimaryKeyTest.php‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,28 @@ final class CustomPrimaryKeyTest extends FrameworkIntegrationTestCase
1717
{
1818
public function test_model_with_custom_primary_key_name(): void
1919
{
20-
$this->migrate(CreateMigrationsTable::class, CreateFrierenModelMigration::class);
20+
$this->migrate(CreateMigrationsTable::class, CreateCustomPrimaryKeyUserModelTable::class);
2121

22-
$frieren = model(FrierenModel::class)->create(name: 'Frieren', magic: 'Time Magic');
22+
$frieren = model(CustomPrimaryKeyUserModel::class)->create(name: 'Frieren', magic: 'Time Magic');
2323

24-
$this->assertInstanceOf(FrierenModel::class, $frieren);
24+
$this->assertInstanceOf(CustomPrimaryKeyUserModel::class, $frieren);
2525
$this->assertInstanceOf(PrimaryKey::class, $frieren->uuid);
2626
$this->assertSame('Frieren', $frieren->name);
2727
$this->assertSame('Time Magic', $frieren->magic);
2828

29-
$retrieved = model(FrierenModel::class)->get($frieren->uuid);
29+
$retrieved = model(CustomPrimaryKeyUserModel::class)->get($frieren->uuid);
3030
$this->assertNotNull($retrieved);
3131
$this->assertSame('Frieren', $retrieved->name);
3232
$this->assertTrue($frieren->uuid->equals($retrieved->uuid));
3333
}
3434

3535
public function test_update_or_create_with_custom_primary_key(): void
3636
{
37-
$this->migrate(CreateMigrationsTable::class, CreateFrierenModelMigration::class);
37+
$this->migrate(CreateMigrationsTable::class, CreateCustomPrimaryKeyUserModelTable::class);
3838

39-
$frieren = model(FrierenModel::class)->create(name: 'Frieren', magic: 'Time Magic');
39+
$frieren = model(CustomPrimaryKeyUserModel::class)->create(name: 'Frieren', magic: 'Time Magic');
4040

41-
$updated = model(FrierenModel::class)->updateOrCreate(
41+
$updated = model(CustomPrimaryKeyUserModel::class)->updateOrCreate(
4242
find: ['name' => 'Frieren'],
4343
update: ['magic' => 'Advanced Time Magic'],
4444
);
@@ -67,7 +67,7 @@ public function test_model_without_id_property_still_works(): void
6767
}
6868
}
6969

70-
final class FrierenModel
70+
final class CustomPrimaryKeyUserModel
7171
{
7272
public ?PrimaryKey $uuid = null;
7373

@@ -95,13 +95,13 @@ public function __construct(
9595
) {}
9696
}
9797

98-
final class CreateFrierenModelMigration implements DatabaseMigration
98+
final class CreateCustomPrimaryKeyUserModelTable implements DatabaseMigration
9999
{
100-
public string $name = '001_create_frieren_model';
100+
public string $name = '001_create_user_model';
101101

102102
public function up(): QueryStatement
103103
{
104-
return CreateTableStatement::forModel(FrierenModel::class)
104+
return CreateTableStatement::forModel(CustomPrimaryKeyUserModel::class)
105105
->primary(name: 'uuid')
106106
->text('name')
107107
->text('magic');

‎tests/Integration/Database/Builder/ModelQueryBuilderTest.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ public function test_insert(): void
6464
$insertedId = $builderWithId->execute();
6565
$this->assertInstanceOf(PrimaryKey::class, $insertedId);
6666

67-
$insertedIdWithoutPk = $builderWithoutId->execute();
68-
$this->assertInstanceOf(PrimaryKey::class, $insertedIdWithoutPk);
67+
$this->assertNull($builderWithoutId->execute());
6968

7069
$retrieved = model(TestUserModel::class)->get($insertedId);
7170
$this->assertNotNull($retrieved);

‎tests/Integration/Database/Builder/NestedWhereTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ public function test_deeply_nested_where_groups(): void
7070

7171
public function test_complex_nested_where_scenario(): void
7272
{
73-
// Test a realistic complex query:
7473
// WHERE status = 'published'
7574
// AND (
7675
// (category = 'fiction' AND rating > 4.0)
@@ -115,7 +114,6 @@ public function test_complex_nested_where_scenario(): void
115114

116115
public function test_where_group_without_existing_conditions(): void
117116
{
118-
// Test starting with a group
119117
$query = query('books')
120118
->select()
121119
->whereGroup(function ($group): void {

‎tests/Integration/Database/ModelsWithoutIdTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,8 @@ final class CreateCacheEntryMigration implements DatabaseMigration
365365
public function up(): QueryStatement
366366
{
367367
return CreateTableStatement::forModel(CacheEntry::class)
368-
->text('cache_key')
369-
->text('cache_value')
368+
->string('cache_key')
369+
->string('cache_value')
370370
->integer('ttl')
371371
->unique('cache_key');
372372
}

‎tests/Integration/Database/QueryStatements/BelongsToStatementTest.php‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,24 @@ final class BelongsToStatementTest extends FrameworkIntegrationTestCase
1818
{
1919
public function test_belongs_to_vs_foreign_key(): void
2020
{
21+
$customersMigration = new class() implements DatabaseMigration {
22+
private(set) string $name = '0001_create_customers';
23+
24+
public function up(): QueryStatement
25+
{
26+
return new CreateTableStatement('customers')
27+
->primary()
28+
->text('name');
29+
}
30+
31+
public function down(): ?QueryStatement
32+
{
33+
return null;
34+
}
35+
};
36+
2137
$belongsToMigration = new class() implements DatabaseMigration {
22-
private(set) string $name = '0001_test_belongs_to';
38+
private(set) string $name = '0002_test_belongs_to';
2339

2440
public function up(): QueryStatement
2541
{
@@ -36,7 +52,7 @@ public function down(): ?QueryStatement
3652
};
3753

3854
$foreignKeyMigration = new class() implements DatabaseMigration {
39-
private(set) string $name = '0002_test_foreign_key';
55+
private(set) string $name = '0003_test_foreign_key';
4056

4157
public function up(): QueryStatement
4258
{
@@ -53,15 +69,31 @@ public function down(): ?QueryStatement
5369
}
5470
};
5571

56-
$this->migrate(CreateMigrationsTable::class, $belongsToMigration, $foreignKeyMigration);
72+
$this->migrate(CreateMigrationsTable::class, $customersMigration, $belongsToMigration, $foreignKeyMigration);
5773

5874
$this->expectNotToPerformAssertions();
5975
}
6076

6177
public function test_foreign_key_allows_different_column_names(): void
6278
{
63-
$migration = new class() implements DatabaseMigration {
64-
private(set) string $name = '0003_test_different_column_names';
79+
$categoriesMigration = new class() implements DatabaseMigration {
80+
private(set) string $name = '0001_create_categories';
81+
82+
public function up(): QueryStatement
83+
{
84+
return new CreateTableStatement('categories')
85+
->primary()
86+
->text('name');
87+
}
88+
89+
public function down(): ?QueryStatement
90+
{
91+
return null;
92+
}
93+
};
94+
95+
$productsMigration = new class() implements DatabaseMigration {
96+
private(set) string $name = '0002_test_different_column_names';
6597

6698
public function up(): QueryStatement
6799
{
@@ -78,7 +110,7 @@ public function down(): ?QueryStatement
78110
}
79111
};
80112

81-
$this->migrate(CreateMigrationsTable::class, $migration);
113+
$this->migrate(CreateMigrationsTable::class, $categoriesMigration, $productsMigration);
82114

83115
$this->expectNotToPerformAssertions();
84116
}

0 commit comments

Comments
 (0)