[6.x] Added creation of custom Cast classes for Eloquent

[andrey-helldar]

Recently, my #30931 PR was rejected and I understand the reason, since my proposal implies an increase in the amount of processed code. This is not entirely good.

In exchange, I suggest introducing a new type of object - custom Cast objects that allow developers to choose how to handle their values.

The application process is as follows:

1. Using the php artisan make:cast command, the developer creates a cast file.
2. Inside the created file (by default in the app/Casts directory), the developer himself determines the behavior of the get() and set() methods:

use Illuminate\Contracts\Database\Eloquent\Castable;

class MyCast implements Castable
{
    public function get($value)
    {
        return $value / 100;
    }

    public function set($value)
    {
        return $value * 100;
    }
}

3. In the model, you need to specify the created caste:

use App\Casts\MyCast;

public $casts = [
    'user_field' => MyCast::class,
];

4. Profit :)

In my opinion, this approach will not only save the amount of code for the “box” of the framework, but also significantly expand its functionality.

In addition, I removed the methods for working with castes from the hasAttributes trait, which allowed me to keep the code cleaner.

[andrey-helldar]

@mfn, yes, in the previous PR there was another attempt in which there were errors. Taylor closed the PR.
I did not leave that branch, but made changes to the existing one and unloaded it with a replacement (force push).
Current changes contain the current code. Also fixed the tests and now everything is fine.

I also want to remind you that this code is not blocking for the current functionality of the application. The code extends its capabilities. That is why I sent PR to the 6.x branch, but I can easily do this in master if it seems more correct to you like that.

PS: sorry if the text is written incomprehensibly. Translated through Google Translator.
The same goes for docblock.

[andrey-helldar]

@GrahamCampbell, I now have the time of 2 am, I can make all the necessary changes tomorrow.

I also wanted to ask: the documentation says that all new features should be sent to the master branch. Should I change the target branch of this PR?

[browner12]

I can tell you that this feature has been discussed and attempted many times over many years. I tried it once back on 5.3, and I believe Taylor even took a stab at it himself awhile back.

It's almost like a right of passage to try and get this merged, but don't get your hopes up.

[antonkomarev]

Previous PRs attempts:

• [5.2] Model casting to any object #11225
• [5.3] [WIP] Custom Type Casting #13315
• [5.3] Implement object casting #13706 (Taylor's PR)
• [5.3] Custom casting of eloquent properties #16305
• [5.5] [WIP] Eloquent object casting #18106
• [5.5] Eloquent object casting #18229
• [5.5] Added custom attribute types #21419
• [5.6] Cast model attribute to any class #24059
• [5.7] Custom Eloquent attribute casts #24124

Idea discussions:

• Improve Typecasting ideas#9
• [Proposal] Implement custom & nested casting for Eloquent Model (WIP) ideas#1354

[sisve]

Instead of the Castable class, can we perhaps reuse the Doctrine\DBAL\Types\Type class? It's meant for exactly these purposes, with (according to me) slightly easier-to-understand method names like function convertToDatabaseValue($value, AbstractPlatform $platform) and function convertToPHPValue($value, AbstractPlatform $platform). Reusing this would also allow us to reuse all the existing user-types written for Doctrine (I'm dreaming a bit here, there are probably some weird cases of the Type class that Eloquent cannot support...)

I also like the Type class for the function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform) which would allow us to generate column declarations. This could be used so that the migrator would know about string lengths, comments, etc. This, however, is just another idea for the future and way out-of-scope for this issue.

[taylorotwell]

I have absolutely no plans to couple ourselves to any Doctrine related classes for this feature. Not a knock against Doctrine but I just don't want to have to keep track of any breaking changes in that library, etc.

[andrey-helldar]

I also think that binding to the doctrine makes the application very fragile, because need to constantly monitor the performance of the code.

[andrey-helldar]

When sending the last commit with docblock adjustment, the service issued a build error.
These errors are not in my code:

[mfn]

Thanks, looks good to me 👍

[taylorotwell]

Here is another difference with my PR, which I still kinda prefer over any others 😆 ...

If you say you have a custom cast to Address and then address has multiple values like city, street_address, etc... you can't do $user->address->city = 'foo'; $user->save() ... the value of city won't be synced back in because you never fall into the setAttribute logic. In my PR I performed this syncing so that it just worked.

[taylorotwell]

Personally I think if I was going to add custom casts I would just resurrect my PR. To me it has more functionality and would work more as expected. I think the discussion around it mainly got derailed by one person.

[rodrigopedra]

@taylorotwell I guess I was this person this time.

This is a feature that would simplify a lot of code already as it was originally proposed.

Sorry to everyone, was just trying to help.

[andrey-helldar]

😒

[mfn]

Actually I think I was meant: I too apologize 😞

[andrey-helldar]

@taylorotwell, However, I will make changes to my code to cover this case. And to accept it or not ...

[gareth-ib]

so is this getting merged or is the other PR mentioned getting merged? Cuz the lack of custom casts causes me a lot of problems.

[andrey-helldar]

@gareth-ib, was he. Taylor closed today. I am preparing more changes for this code in the hope that they will be accepted.

[gareth-ib]

@andrey-helldar that would be great, thanks! If you need help, lemme know

[andrey-helldar]

@gareth-ib, Now the main problem is that it is impossible to change the property of an initialized instance of a class in a cast. On pure php, I did not encounter such a problem, as I tried it today. In general, I solve this problem.

In order not to spam this PR, I created a branch from it and uploaded the latest changes to them, since I am writing code both from home and from work.

If there are thoughts on how to solve the problem, I will listen. But for today my stock of thoughts has run out.

[gareth-ib]

I'm checking it out now... I don't see a reason why that overwrite of a value would be blocked... does no error output when you do the set?

[andrey-helldar]

An error occurs on line L63 — property changes not accepted.

Moreover, if you write the code like this:

$foo = $model->field_6;
$foo->line_one = 'Modified Line 1';
$model->field_6 = $foo;

then it works without problems.

[gareth-ib]

ok I have your package in a fresh project but I don't see how to run that test, is there a command I'm not seeing?

[andrey-helldar]

@gareth-ib, usually tests are run by command phpunit with passing parameters. But I use phpStorm that does this for me.

[andrey-helldar]

@gareth-ib, If you are still interested, I found a solution to the problem.

Problem: the attribute variable stores the record from the database in a "pure" form. Upon receipt of the value, it is processed by mutators, castes, etc.

Situation: changing the code "live" we do not actually change the object stored in the variable, but a copy of it from memory. Thus, the value of the attribute does not change, since there was no actual change in the property.

I already found a solution and will upload it here soon 🙂

[andrey-helldar]

Happy New Year, guys! 🎄🌱🎅🎁🥂🥂🥂

[andrey-helldar]

@taylorotwell, @GrahamCampbell, @mfn, I solved the problem by changing the properties of the cast.

See tests and their results.

Question: should I create a new PR or will you reopen this one?

I made changes to the same branch.

[antonkomarev]

@andrey-helldar Taylor already merged same feature to the 7.x branch: #31035

[SerafimArts]

Omfg =\

[andrey-helldar]

@taylorotwell, well. If you don’t want to accept someone else’s development for you, then pay attention to the following places in my code:

1. tests: in particular line 110. In it, a test case is made to change the object with saving in the model.
2. Casting attributes from database (here and here) and to database (here).
3. getAttributeValue: here

These sections of code solve the problem that you described in #30958 (comment)

Unfortunately, at the moment your code does not work and needs to be improved. At the same time, my code is fully working and covers these problems (tests passed successfully). Therefore, I really hope that you will pay attention to this and at least make your code a little bit like the functional part of my code. Note: exactrly the functional part, not the appearance.

You can always ignore my messages and not reply to them, but I know for sure that you read them. So make your code work fine, not like #31035 (comment).

Best regards,
Andrey Helldar.

[antonkomarev]

@andrey-helldar the best way to fix any issues in already merged code - create a new PR with appropriate changes. In my experience, Taylor rarely looks into closed PRs and issues.

[taylorotwell]

My code does work. You just didn't use it right, as I commented on the other PR.