framework icon indicating copy to clipboard operation
framework copied to clipboard

[9.x] Add UUID support to native castables

Open ollieread opened this issue 1 year ago • 9 comments

This is a small PR which adds support for the castable type uuid. Since we have a uuid option for migrations, it only makes sense that we also support adding it as a castable type.

I appreciate that it does couple models with the ramsey\uuid package, but that is included in the framework, and has been for some time.

ollieread avatar Sep 02 '22 13:09 ollieread

@ollieread does the corresponding "set" operation already just work because UUIDs are stringable? Any tests?

taylorotwell avatar Sep 02 '22 13:09 taylorotwell

@ollieread does the corresponding "set" operation already just work because UUIDs are stringable?

@taylorotwell The Uuid instance does have a magic __toString, but the interface I've used in docblock does not. Happy to change it if you think that's best?

Any tests?

Happy to add some.

ollieread avatar Sep 02 '22 13:09 ollieread

@taylorotwell Tests added as well as some better handling for converting between strings and UUIDs

ollieread avatar Sep 02 '22 14:09 ollieread

Awesome idea!

ralphjsmit avatar Sep 07 '22 11:09 ralphjsmit

Please mark as ready if you want another review.

driesvints avatar Sep 12 '22 08:09 driesvints

Can be this replicated for ULID too?

DarkGhostHunter avatar Sep 17 '22 22:09 DarkGhostHunter

Readying this for review again so Taylor can review @ollieread's answer here: https://github.com/laravel/framework/pull/43978#discussion_r967836179

driesvints avatar Sep 19 '22 10:09 driesvints

So - I think this PR looks generally fine, but I am curious what the usefulness of it is. What would I want to do with a UUID object? I understand creating domain specific ID value objects like ProductId, etc. where maybe you have some extra functionality on the object. But, what would I do with a plain UUID object? Why not just use a string? What are the actual benefits?

taylorotwell avatar Sep 21 '22 15:09 taylorotwell

Why not just use a string? What are the actual benefits?

The database may be using a binary column (recommended by MySQL) to store UUID. I need a native castable to retrieve the binary as UUID, even if is transformed to a string. And then, from a string to a binary form.

I think this PR falls relatively short because the idea of a native castable is to transform whatever the database spits as UUID, and whatever the app receives back to what the database is using. That could be fixed by allowing the UUID to be binary by using uuid:string (default) and uuid:binary:

protected $casts = [
    'user_id'    => 'uuid',        // The column type is a string.
    'article_id' => 'uuid:ulid',   // The column type is a 26-char string.
    'image_id'   => 'uuid:binary', // The column type is binary.
];

With this, all bases are covered. I think It may open this to use them as primary key types in Laravel 10:

protected $keyType = 'uuid:binary';

And open up ULID casting like ulid, ulid:uuid and ulid:binary.

DarkGhostHunter avatar Sep 21 '22 17:09 DarkGhostHunter

Going to hold off on this for now. Can easily write a custom cast if you wish.

taylorotwell avatar Sep 26 '22 14:09 taylorotwell