Eloquent casting enum attributes not checking if backedEnum

[henzeb]

• Laravel Version: 9.16.0
• PHP Version: 8.1.6
• Database Driver & Version: -

Description:

I am working on my package henzeb/enumhancer And I want to make UnitEnums castable as I find it ridiculous that we have to write 'CASE ENUM='ENUM' . It is redundant.

I could of course make an adapter class for every enum I create, but I want to make it easy for people. Just add a trait to your UnitEnum and you're done.

The problem is that currently Laravel is checking if it is an enum at all. So if it is an enum, no matter if it is a BackedEnum or a UnitEnum, it will use the enum casting functionality you build. Ofcourse, with UnitEnums, it crashes.

Steps To Reproduce:

1 . create a UnitEnum (without int or string)
2. create a table and a Model using a field that is using those enum cases
3. add the enum class path to the casts property
4. run it.

All I am asking is that you change the way it verifies if it is a BackedEnum using is_subclass_of. It won't change anything on how you implement the casting, just the error message people receive when trying an UnitEnum

I'll take care of the rest.

[henzeb]

@driesvints, I hope you understand it's a minor change which is a bug anyway as you want to support backed enums and not unit enums. And that I am going to implement the support for unit enums once this bug is fixed, right?

[rodrigopedra]

This is not a "bug". Laravel expects users to read its documentation, and/or honor a method's docblock typehints, which is not the case with enums, but is a frequently reported "bug", for example when someone reports a method is crashing due to passing a null value to a method that only expects a string.

The docs on enum casting is very clear:

Eloquent also allows you to cast your attribute values to PHP "backed" Enums

https://laravel.com/docs/9.x/eloquent-mutators#enum-casting

An option you can provide to your package's users, is to offer a trait which overrides the Model@getEnumCastableAttributeValue() method and account for a Basic Enumeration (as it is called on PHP's documentation).

reference: https://www.php.net/manual/en/language.enumerations.basics.php#language.enumerations.basics

[henzeb]

It's poorly written at least. as laravel's code expects an enum, and not necessarily a backed one. It's like expecting an Eloquent Model and then tell users to add a trait that is actually being used. poorly written. The classes for checking which type it is are there since the moment enums came out. hence why I name them.

Thanks for the getEnumCastableAttributeValue suggestion, I will look in to it.

[henzeb]

This works great, except when I use toArray I get an Undefined property error. It seems that it casts the variable to the enum object before it returns it's value. which is weird if it is returning the value anyway. I'll file a bug report later, because in this case it is not using getEnumCastableAttributeValue where it should.

[rodrigopedra]

Maybe take a look into Model@setEnumCastableAttribute() too.

I guess the second if branch might needed to be changed to $value->name() when using basic enumerations.

[henzeb]

I did, both are working, but in HasAttributes line 301 the value is casted to an enum and it tries to use the value property.

[rodrigopedra]

@henzeb try using a custom cast:

<?php

namespace App\Casts;

use Illuminate\Contracts\Database\Eloquent\CastsAttributes;
use Illuminate\Contracts\Database\Eloquent\SerializesCastableAttributes;

class AsEnum implements CastsAttributes, SerializesCastableAttributes
{
    protected readonly string $enumName;

    public function __construct(string $enumName)
    {
        if (! \is_subclass_of($enumName, \UnitEnum::class)) {
            throw new \InvalidArgumentException($enumName . ' should be an Enum');
        }

        $this->enumName = $enumName;
    }

    public function get($model, string $key, $value, array $attributes): ?\UnitEnum
    {
        if (\is_null($value)) {
            return null;
        }

        if ($value instanceof $this->enumName) {
            return $value;
        }

        if (\is_subclass_of($this->enumName, \BackedEnum::class)) {
            return $this->enumName::from($value);
        }

        foreach ($this->enumName::cases() as $case) {
            if ($case->name === $value) {
                return $case;
            }
        }

        throw new \InvalidArgumentException('Invalid enum case for: ' . $this->enumName);
    }

    public function set($model, string $key, $value, array $attributes): array
    {
        return [$key => $this->serialize($model, $key, $value, $attributes)];
    }

    public function serialize($model, string $key, $value, array $attributes)
    {
        if (\is_null($value)) {
            return null;
        }

        ['value' => $enumValue] = ((array) $value) + ['value' => $value->name];

        return $enumValue;
    }
}

Usage:

<?php // ./routes/console.php

use App\Casts\AsEnum;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Facades\Artisan;

enum Foo
{
    case BAR;
    case BAZ;
}

enum Qux: int
{
    case BAR = 1;
    case BAZ = 2;
}

class Dummy extends Model
{
    protected $attributes = [
        'unit' => Foo::BAR,
        'backed' => Qux::BAR,
        'backed_value' => '1',
    ];

    protected $casts = [
        'unit' => AsEnum::class . ':' . Foo::class,
        'backed' => AsEnum::class . ':' . Qux::class,
        'backed_value' => AsEnum::class . ':' . Qux::class,
    ];
}

Artisan::command('test', function () {
    $model = new Dummy();

    \dump(
        $model->unit,
        $model->unit instanceof Foo,
        $model->backed,
        $model->backed instanceof Qux,
        $model->backed_value,
        $model->backed_value instanceof Qux,
        $model->toArray(),
    );

    $model->unit = Foo::BAZ;
    $model->backed = Qux::BAZ;
    $model->backed_value = Qux::BAZ;

    \dump(
        $model->unit,
        $model->unit instanceof Foo,
        $model->backed,
        $model->backed instanceof Qux,
        $model->backed_value,
        $model->backed_value instanceof Qux,
        $model->toArray(),
    );
});

Output:

$ php artisan test
^ Foo^ {#532
  +name: "BAR"
}
^ true
^ Qux^ {#544
  +name: "BAR"
  +value: 1
}
^ true
^ Qux^ {#544
  +name: "BAR"
  +value: 1
}
^ true
^ array:3 [
  "unit" => "BAR"
  "backed" => 1
  "backed_value" => 1
]
^ Foo^ {#880
  +name: "BAZ"
}
^ true
^ Qux^ {#876
  +name: "BAZ"
  +value: 2
}
^ true
^ Qux^ {#876
  +name: "BAZ"
  +value: 2
}
^ true
^ array:3 [
  "unit" => "BAZ"
  "backed" => 2
  "backed_value" => 2
]

references:

• https://laravel.com/docs/9.x/eloquent-mutators#custom-casts
• https://laravel.com/docs/9.x/eloquent-mutators#cast-parameters
• https://laravel.com/docs/9.x/eloquent-mutators#array-json-serialization

If you believe a custom cast like this belongs to the core, try to PR it

[rodrigopedra]

@henzeb, I just updated the code to show how it working with backed enums. take a look

[henzeb]

That also works, but I don't see the point. It's not a fast solution. It's a very cloggy solution to have a class FQCN concatenated in a string with another. What belongs to the core of PHP is a basic enumeration object with a value that returns the name or a lowercase version of this name when asking for a value.What belongs to the core of Laravel is the acceptance of basic enumerations and use their names.

a PR would most likely not make it unless the team behind laravel agrees with it. As such a change would also require to accept such enums in validation or routing (consistency), I doubt it.

The bug in #42658 is, as far as I can see, just a problem in the toArray method. If they won't fix it, I already have an idea for a workaround.

[rodrigopedra]

What belongs to the core of PHP is a basic enumeration object with a value that returns the name or a lowercase version of this name when asking for a value

Just a note here, many people advocate for using PascalCase for enum cases, like proposed in the original PR. There was a thread this week on twitter with mixed opinions:

https://twitter.com/brendt_gd/status/1531134317539299328

So, in my opinion, if ever PHP auto-serializes an enum, it should preserve the case it was written. At least this is what I would expect from a serialization library like you are proposing. If I wanted different casing for a enum's serialization form, I would use a backed enum instead.

[henzeb]

Thanks for the note, I will keep the name as is and add an option to change that behavior, as it still does not make sense to me to type ENUM=enum or MyEnum=myenum or MyEnum=MyEnum.