Skip to content

Commit 7be939c

Browse files
committed
Fix HasUuids and HasUlids traits ignoring explicit IDs
The `creating()` method in both traits incorrectly referenced an undefined `$model` variable instead of `$this`, causing the condition to always evaluate to true. This meant explicit IDs passed during model creation were always overwritten with auto-generated ones. - Fix `$model->{$column}` to `$this->{$column}` in both traits - Add symfony/uid as dev dependency for ULID testing - Add test coverage for explicit ID scenarios
1 parent df1da4c commit 7be939c

File tree

6 files changed

+189
-2
lines changed

6 files changed

+189
-2
lines changed

composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@
216216
"phpunit/phpunit": "10.5.45",
217217
"pusher/pusher-php-server": "^7.2",
218218
"swoole/ide-helper": "~5.1.0",
219+
"symfony/uid": "^7.4",
219220
"symfony/yaml": "^7.3"
220221
},
221222
"config": {

src/core/src/Database/Eloquent/Concerns/HasUlids.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ trait HasUlids
1414
public function creating(): void
1515
{
1616
foreach ($this->uniqueIds() as $column) {
17-
if (empty($model->{$column})) {
17+
if (empty($this->{$column})) {
1818
$this->{$column} = $this->newUniqueId();
1919
}
2020
}

src/core/src/Database/Eloquent/Concerns/HasUuids.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ trait HasUuids
1414
public function creating(): void
1515
{
1616
foreach ($this->uniqueIds() as $column) {
17-
if (empty($model->{$column})) {
17+
if (empty($this->{$column})) {
1818
$this->{$column} = $this->newUniqueId();
1919
}
2020
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Hypervel\Tests\Core\Database\Eloquent\Concerns;
6+
7+
use Hyperf\Stringable\Str;
8+
use Hypervel\Database\Eloquent\Concerns\HasUlids;
9+
use Hypervel\Database\Eloquent\Model;
10+
use Hypervel\Foundation\Testing\RefreshDatabase;
11+
use Hypervel\Testbench\TestCase;
12+
13+
/**
14+
* @internal
15+
* @coversNothing
16+
*/
17+
class HasUlidsTest extends TestCase
18+
{
19+
use RefreshDatabase;
20+
21+
protected function migrateFreshUsing(): array
22+
{
23+
return [
24+
'--realpath' => true,
25+
'--path' => __DIR__ . '/migrations',
26+
];
27+
}
28+
29+
public function testAutoGeneratesUlidWhenNotProvided(): void
30+
{
31+
$model = HasUlidsTestModel::create(['name' => 'Test']);
32+
33+
$this->assertNotNull($model->id);
34+
$this->assertTrue(Str::isUlid($model->id));
35+
}
36+
37+
public function testRespectsExplicitUlidWhenProvided(): void
38+
{
39+
$explicitId = strtolower((string) Str::ulid());
40+
41+
$model = HasUlidsTestModel::create([
42+
'id' => $explicitId,
43+
'name' => 'Test',
44+
]);
45+
46+
$this->assertSame($explicitId, $model->id);
47+
}
48+
49+
public function testRespectsExplicitUlidInFirstOrCreate(): void
50+
{
51+
$explicitId = strtolower((string) Str::ulid());
52+
53+
$model = HasUlidsTestModel::firstOrCreate(
54+
['id' => $explicitId],
55+
['name' => 'Test']
56+
);
57+
58+
$this->assertSame($explicitId, $model->id);
59+
}
60+
}
61+
62+
class HasUlidsTestModel extends Model
63+
{
64+
use HasUlids;
65+
66+
protected ?string $table = 'has_ulids_test_models';
67+
68+
protected array $fillable = ['id', 'name'];
69+
70+
public bool $timestamps = true;
71+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Hypervel\Tests\Core\Database\Eloquent\Concerns;
6+
7+
use Hyperf\Stringable\Str;
8+
use Hypervel\Database\Eloquent\Concerns\HasUuids;
9+
use Hypervel\Database\Eloquent\Model;
10+
use Hypervel\Foundation\Testing\RefreshDatabase;
11+
use Hypervel\Testbench\TestCase;
12+
13+
/**
14+
* @internal
15+
* @coversNothing
16+
*/
17+
class HasUuidsTest extends TestCase
18+
{
19+
use RefreshDatabase;
20+
21+
protected function migrateFreshUsing(): array
22+
{
23+
return [
24+
'--realpath' => true,
25+
'--path' => __DIR__ . '/migrations',
26+
];
27+
}
28+
29+
public function testAutoGeneratesUuidWhenNotProvided(): void
30+
{
31+
$model = HasUuidsTestModel::create(['name' => 'Test']);
32+
33+
$this->assertNotNull($model->id);
34+
$this->assertTrue(Str::isUuid($model->id));
35+
}
36+
37+
public function testRespectsExplicitUuidWhenProvided(): void
38+
{
39+
$explicitId = (string) Str::orderedUuid();
40+
41+
$model = HasUuidsTestModel::create([
42+
'id' => $explicitId,
43+
'name' => 'Test',
44+
]);
45+
46+
$this->assertSame($explicitId, $model->id);
47+
}
48+
49+
public function testRespectsExplicitUuidInFirstOrCreate(): void
50+
{
51+
$explicitId = (string) Str::orderedUuid();
52+
53+
$model = HasUuidsTestModel::firstOrCreate(
54+
['id' => $explicitId],
55+
['name' => 'Test']
56+
);
57+
58+
$this->assertSame($explicitId, $model->id);
59+
}
60+
61+
public function testDoesNotOverwriteIdOnExistingRecord(): void
62+
{
63+
$explicitId = (string) Str::orderedUuid();
64+
65+
// Create first
66+
HasUuidsTestModel::create([
67+
'id' => $explicitId,
68+
'name' => 'Original',
69+
]);
70+
71+
// firstOrCreate should find existing, not create new
72+
$model = HasUuidsTestModel::firstOrCreate(
73+
['id' => $explicitId],
74+
['name' => 'Should Not Be Used']
75+
);
76+
77+
$this->assertSame($explicitId, $model->id);
78+
$this->assertSame('Original', $model->name);
79+
}
80+
}
81+
82+
class HasUuidsTestModel extends Model
83+
{
84+
use HasUuids;
85+
86+
protected ?string $table = 'has_uuids_test_models';
87+
88+
protected array $fillable = ['id', 'name'];
89+
90+
public bool $timestamps = true;
91+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Hyperf\Database\Migrations\Migration;
6+
use Hyperf\Database\Schema\Blueprint;
7+
use Hyperf\Database\Schema\Schema;
8+
9+
return new class extends Migration {
10+
public function up(): void
11+
{
12+
Schema::create('has_uuids_test_models', function (Blueprint $table) {
13+
$table->uuid('id')->primary();
14+
$table->string('name');
15+
$table->timestamps();
16+
});
17+
18+
Schema::create('has_ulids_test_models', function (Blueprint $table) {
19+
$table->ulid('id')->primary();
20+
$table->string('name');
21+
$table->timestamps();
22+
});
23+
}
24+
};

0 commit comments

Comments
 (0)