Merge pull request #321 from binaryfire/fix/has-uuids-explicit-id

fix: `HasUuids` and `HasUlids` traits ignoring explicit IDs

Author: albertcht

composer.json

@@ -216,6 +216,7 @@
         "phpunit/phpunit": "10.5.45",
         "pusher/pusher-php-server": "^7.2",
         "swoole/ide-helper": "~5.1.0",
+        "symfony/uid": "^7.4",
         "symfony/yaml": "^7.3"
     },
     "config": {

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

@@ -14,7 +14,7 @@ trait HasUlids
     public function creating(): void
     {
         foreach ($this->uniqueIds() as $column) {
-            if (empty($model->{$column})) {
+            if (empty($this->{$column})) {
                 $this->{$column} = $this->newUniqueId();
             }
         }

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

@@ -14,7 +14,7 @@ trait HasUuids
     public function creating(): void
     {
         foreach ($this->uniqueIds() as $column) {
-            if (empty($model->{$column})) {
+            if (empty($this->{$column})) {
                 $this->{$column} = $this->newUniqueId();
             }
         }

tests/Core/Database/Eloquent/Concerns/HasUlidsTest.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Hypervel\Tests\Core\Database\Eloquent\Concerns;
+
+use Hyperf\Stringable\Str;
+use Hypervel\Database\Eloquent\Concerns\HasUlids;
+use Hypervel\Database\Eloquent\Model;
+use Hypervel\Foundation\Testing\RefreshDatabase;
+use Hypervel\Testbench\TestCase;
+
+/**
+ * @internal
+ * @coversNothing
+ */
+class HasUlidsTest extends TestCase
+{
+    use RefreshDatabase;
+
+    protected bool $migrateRefresh = true;
+
+    protected function migrateFreshUsing(): array
+    {
+        return [
+            '--database' => $this->getRefreshConnection(),
+            '--realpath' => true,
+            '--path' => __DIR__ . '/migrations',
+        ];
+    }
+
+    public function testAutoGeneratesUlidWhenNotProvided(): void
+    {
+        $model = HasUlidsTestModel::create(['name' => 'Test']);
+
+        $this->assertNotNull($model->id);
+        $this->assertTrue(Str::isUlid($model->id));
+    }
+
+    public function testRespectsExplicitUlidWhenProvided(): void
+    {
+        $explicitId = strtolower((string) Str::ulid());
+
+        $model = HasUlidsTestModel::create([
+            'id' => $explicitId,
+            'name' => 'Test',
+        ]);
+
+        $this->assertSame($explicitId, $model->id);
+    }
+
+    public function testRespectsExplicitUlidInFirstOrCreate(): void
+    {
+        $explicitId = strtolower((string) Str::ulid());
+
+        $model = HasUlidsTestModel::firstOrCreate(
+            ['id' => $explicitId],
+            ['name' => 'Test']
+        );
+
+        $this->assertSame($explicitId, $model->id);
+    }
+}
+
+class HasUlidsTestModel extends Model
+{
+    use HasUlids;
+
+    protected ?string $table = 'has_ulids_test_models';
+
+    protected array $fillable = ['id', 'name'];
+
+    public bool $timestamps = true;
+}

tests/Core/Database/Eloquent/Concerns/HasUuidsTest.php

@@ -0,0 +1,94 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Hypervel\Tests\Core\Database\Eloquent\Concerns;
+
+use Hyperf\Stringable\Str;
+use Hypervel\Database\Eloquent\Concerns\HasUuids;
+use Hypervel\Database\Eloquent\Model;
+use Hypervel\Foundation\Testing\RefreshDatabase;
+use Hypervel\Testbench\TestCase;
+
+/**
+ * @internal
+ * @coversNothing
+ */
+class HasUuidsTest extends TestCase
+{
+    use RefreshDatabase;
+
+    protected bool $migrateRefresh = true;
+
+    protected function migrateFreshUsing(): array
+    {
+        return [
+            '--database' => $this->getRefreshConnection(),
+            '--realpath' => true,
+            '--path' => __DIR__ . '/migrations',
+        ];
+    }
+
+    public function testAutoGeneratesUuidWhenNotProvided(): void
+    {
+        $model = HasUuidsTestModel::create(['name' => 'Test']);
+
+        $this->assertNotNull($model->id);
+        $this->assertTrue(Str::isUuid($model->id));
+    }
+
+    public function testRespectsExplicitUuidWhenProvided(): void
+    {
+        $explicitId = (string) Str::orderedUuid();
+
+        $model = HasUuidsTestModel::create([
+            'id' => $explicitId,
+            'name' => 'Test',
+        ]);
+
+        $this->assertSame($explicitId, $model->id);
+    }
+
+    public function testRespectsExplicitUuidInFirstOrCreate(): void
+    {
+        $explicitId = (string) Str::orderedUuid();
+
+        $model = HasUuidsTestModel::firstOrCreate(
+            ['id' => $explicitId],
+            ['name' => 'Test']
+        );
+
+        $this->assertSame($explicitId, $model->id);
+    }
+
+    public function testDoesNotOverwriteIdOnExistingRecord(): void
+    {
+        $explicitId = (string) Str::orderedUuid();
+
+        // Create first
+        HasUuidsTestModel::create([
+            'id' => $explicitId,
+            'name' => 'Original',
+        ]);
+
+        // firstOrCreate should find existing, not create new
+        $model = HasUuidsTestModel::firstOrCreate(
+            ['id' => $explicitId],
+            ['name' => 'Should Not Be Used']
+        );
+
+        $this->assertSame($explicitId, $model->id);
+        $this->assertSame('Original', $model->name);
+    }
+}
+
+class HasUuidsTestModel extends Model
+{
+    use HasUuids;
+
+    protected ?string $table = 'has_uuids_test_models';
+
+    protected array $fillable = ['id', 'name'];
+
+    public bool $timestamps = true;
+}

tests/Core/Database/Eloquent/Concerns/migrations/2025_01_01_000000_create_has_uuids_test_models.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+use Hyperf\Database\Migrations\Migration;
+use Hyperf\Database\Schema\Blueprint;
+use Hyperf\Database\Schema\Schema;
+
+return new class extends Migration {
+    public function up(): void
+    {
+        // Use string(36) for UUID and string(26) for ULID for SQLite compatibility
+        Schema::create('has_uuids_test_models', function (Blueprint $table) {
+            $table->string('id', 36)->primary();
+            $table->string('name');
+            $table->timestamps();
+        });
+
+        Schema::create('has_ulids_test_models', function (Blueprint $table) {
+            $table->string('id', 26)->primary();
+            $table->string('name');
+            $table->timestamps();
+        });
+    }
+};