laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
31.85k stars 10.79k forks source link

Passing value to class castable attribute #51950

Open BenjaminMINK opened 2 weeks ago

BenjaminMINK commented 2 weeks ago

Laravel Version

10.x

PHP Version

8.3

Database Driver & Version

No response

Description

Hello,

I would like to report that seems to me an issue when passing value to a class castable attribute.

Let's say a model has an Enum Collection attribute countries:

$casts = ['countries' => AsEnumCollection::class.':'.Country::class];

After fetching the model, we can have something like : countries: ['FRA', 'USA']

But if we want to manually use the castAttribute method with a custom value the result will always be the same as fetched before:

$value = ['FRA', 'USA', 'JPN'];
$result = $model->castAttribute('countries', $value);

// $result: ['FRA', 'USA']

Shouldn't we get our passing value as a result?

In the https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Eloquent/Casts/AsEnumCollection.php#L31, the $value argument is never used, shouldn't be used if passed to the method? Is it by design? The same thing can be observed on the other casting classes.

Thank you!

Steps To Reproduce

Pass a custom value to model's method castAttribute, result will always be the data retrieved before:

$value = ['FRA', 'USA', 'JPN'];
$result = $model->castAttribute('countries', $value);

// $result: ['FRA', 'USA']
BenjaminMINK commented 2 weeks ago

Sorry if I'm unclear, what I would expect by passing a custom value is that the castable class can return correct casting for any passed value

driesvints commented 2 weeks ago

Hi @BenjaminMINK. I don't think this works like in your example. AsEnumCollection is currently documented to work like follow: https://laravel.com/docs/11.x/eloquent-mutators#casting-arrays-of-enums

Can you try that and let me know if it works?

BenjaminMINK commented 2 weeks ago

Hello,

Thank you for your answer, but what I've showed in my example is for Laravel 10.x.

I've also tested with the new syntax in Laravel 11.x, the same "issue" happens, in fact, the casting classes haven't changed since 10.x, so the same thing happens.

BenjaminMINK commented 2 weeks ago

Based on AsEnumCollection class:

As today, the get method has:

$data = Json::decode($attributes[$key]);

if (! is_array($data)) {
    return;
}

$enumClass = $this->arguments[0];

return (new Collection($data))->map(function ($value) use ($enumClass) {
    return is_subclass_of($enumClass, BackedEnum::class)
        ? $enumClass::from($value)
        : constant($enumClass.'::'.$value);
});

We might be expect something like this when we pass the $value argument:

$data = Json::decode($value ?? $attributes[$key]);

if (! is_array($data)) {
    return;
}

$enumClass = $this->arguments[0];

return (new Collection($data))->map(function ($value) use ($enumClass) {
    return is_subclass_of($enumClass, BackedEnum::class)
        ? $enumClass::from($value)
        : constant($enumClass.'::'.$value);
});
github-actions[bot] commented 1 week ago

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

Lakshan-Madushanka commented 2 days ago

castAttrbute() is an internal method since its visibility is protected. However, I'm curious how you managed to call that method from outside without an exception being thrown.