Custom attribute accessor yields "column does not exist" error

[phanan]

• Laravel Version: 8.78.1
• PHP Version: 8.0
• Database Driver & Version: Irrelevant

Description:

With reference to the method mutateAttributeMarkedAttribute found in the HasAttributes trait introduced with the new attribute casting feature in v8.77:

    protected function mutateAttributeMarkedAttribute($key, $value)
    {
        if (isset($this->attributeCastCache[$key])) {
            return $this->attributeCastCache[$key];
        }

        $value = call_user_func($this->{Str::camel($key)}()->get ?: function ($value) {
            return $value;
        }, $value, $this->attributes);

        if (! is_object($value)) {
            unset($this->attributeCastCache[$key]);
        } else {
            $this->attributeCastCache[$key] = $value;
        }

        return $value;
    }

It seems to me that the calculated $value is only cached if it's an object, which essentially means any callable Attribute that returns scalar values (string, boolean etc.) will not be cached. I'm not sure of the reason behind this, but even with this aside, it seems the logic here introduces a bug. Considering the method mergeAttributesFromAttributeCasts() in the same trait, which is called upon saving/updating a model:

    protected function mergeAttributesFromAttributeCasts()
    {
        foreach ($this->attributeCastCache as $key => $value) {
            $callback = $this->{Str::camel($key)}()->set ?: function ($value) use ($key) {
                $this->attributes[$key] = $value;
            };

            $this->attributes = array_merge(
                $this->attributes,
                $this->normalizeCastClassResponse(
                    $key, call_user_func($callback, $value, $this->attributes)
                )
            );
        }
    }

If I read it correctly, this method will merge $attributeCastCache back to the model before performing the query. Now if in the model I have an attribute accessor like this:

// User.php
protected function foo(): Attribute
{
   return Attribute::get(fn () => new Foo());
}

Once $user->foo is accessed, a 'foo' key will be stored into $attributeCastCache and merged into $attributes. If my table doesn't have a foo column, however, saving the model will yield an error saying the column foo doesn't exist.

This problem won't happen if the closure provided in Attribute::get() returns a non-object value, because 'foo' won't be registered into $attributeCastCache and as such won't exist in the $attributes array after merging.

Steps to Reproduce:

1. Checkout https://github.com/phanan/laravel-attr-bug
2. Run composer install
3. Run php artisan test. The test should break.

~/P/laravel (master) $ php artisan test

   FAIL  Tests\Unit\UserTest
  ⨯ custom attribute

  ---

  • Tests\Unit\UserTest > custom attribute
   Illuminate\Database\QueryException

  SQLSTATE[HY000]: General error: 1 no such column: foo (SQL: update "users" set "foo" = ?, "updated_at" = 2022-01-12 13:47:18 where "id" = 1)

More Information:

The bug (?) didn't exist with the old syntax i.e. getFooAttribute(). Of course, as a workaround, I can change foo() from an attribute into a method like this:

public function foo(): Foo
{
    return new Foo();
}

But this a) makes it a bit less elegant (IMHO) and b) makes it harder to add custom attributes into appends.

Of course, if this is just me not understanding Laravel, my apologies.

[bakerkretzmar]

Having the same issue, accessors with the new syntax that are intended to be getter-only are sometimes trying to be saved to the database even though there's no underlying column for them.

[driesvints]

Can you try with the latest Laravel version? This has changed a bit.

[phanan]

Is there any new version since yesterday? Because I reproduced this bug in a fresh install yesterday, as shown in the linked repository.

…

On Thu 13. Jan 2022 at 15:24 Dries Vints ***@***.***> wrote: Can you try with the latest Laravel version? This has changed a bit. — Reply to this email directly, view it on GitHub <#40364 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5O3UQSSPNSE7FXMSUOEIDUV3OAZANCNFSM5LZAXX5Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[driesvints]

Yes. Please try with Laravel v8.79.0 and verify that this version resolves it or not.

[phanan]

Ah, indeed it seems to have been fixed in 29a6692. I'll go ahead and close the issue then. Thanks!