Increment on a field with custom cast checks for method on one class but calls another

[Sjord]

• Laravel Version: illuminate/database v9.45.1
• PHP Version: 8.2.0
• Database Driver & Version: SQLite PDO

Description:

I'm calling increment on a field that has a custom cast. The Laravel Model class checks whether a custom increment implementation exists. However, it checks on one class and then performs the call on another class.

Steps To Reproduce:

Example code here. The Euro->increment and Euro->decrement methods need to exist, but are never called.

I have a Member model with an amount field, which is of type Euro, and casting is provided by EuroCaster. The problem is that calling increment checks for Euro->increment, and if that exists calls EuroCaster->increment.

The problem is in Model.incrementOrDecrement:

        $this->{$column} = $this->isClassDeviable($column)
            ? $this->deviateClassCastableAttribute($method, $column, $amount)
            : $this->{$column} + ($method === 'increment' ? $amount : $amount * -1);

The last line won't work, since adding Euro with + isn't possible. Our column should be "class deviable". The function isClassDeviable checks whether the methods exist:

    protected function isClassDeviable($key)
    {
        return $this->isClassCastable($key) &&
            method_exists($castType = $this->parseCasterClass($this->getCasts()[$key]), 'increment') &&
            method_exists($castType, 'decrement');
    }

It checks this on the Euro class. If these methods exist, deviateClassCastableAttribute is called. This calls resolveCasterClass, which actually checks whether the class implements Castable and calls castUsing to get EuroCaster.

So it checks for Euro->increment, and then calls EuroCaster->increment. I believe this is a bug.

[github-actions[bot]]

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!

[WendellAdriel]

I worked on a small change in the isClassDeviable method for this. Let's see if this fix will be accepted. 😉

[abbood]

@Sjord I see where you're coming from. However I wouldn't call it a bug simply because the interface CastAttributes only supports two methods (get, and set). I tested those methods and it works fine (ie if you run this code

$member = new Member();
$member->amount = 2; 
$member->save();

You will see that it calls the set method defined in EuroCaster even though you didn't declare set in Euro)

However, CastAttributes doesn't include increment nor decrement in its methods. My suggestion would be to include them in the interface if we are to accept @WendellAdriel's solution and add them to the documentation as well, or not do anything at all.

What do you think @driesvints?

[WendellAdriel]

@abbood I understand your point, but I would say that this is buggy behaviour because the code is checking if a method exists in the Euro class but it actually uses the function from the EuroCaster class.

[sumihiran]

However, CastAttributes doesn't include increment nor decrement in its methods. My suggestion would be to include them in the interface if we are to accept @WendellAdriel's solution and add them to the documentation as well, or not do anything at all.

@abbood As of Laravel 9.X the deviate(increment/decrement) characteristic is segregated to another interface: DeviatesCastableAttributes

However, neither the CastsAttributes nor DeviatesCastableAttributes are used within the framework for method identification.

[WendellAdriel]

Is anything else needed here? Can you confirm if the issue was fixed @Sjord? Just asking because the PR I worked on was merged 3 weeks ago.

[Sjord]

Thanks. This now works as expected on 9.48.0. Perhaps it should also be merged into 10.x? Other than that, I am happy.

[WendellAdriel]

Oh, that's great! I think that the Core Team updates the 10.x with 9.x before release 😉