Support for Closures in Fill and Update

[Kauto]

Hi,

Daily business threw me a new minor problem in my way: I had to iterate over all models of a database table and make a column lower case. My solution was

        foreach (\App\Models\MarketRole::lazy() as $marketRole) {
            $marketRole->alias = array_map('mb_strtolower', $marketRole->alias);
            $marketRole->save();
        }

Given the features of Laravel like "tap", etc., my first instinct was to write it that way:

\App\Models\MarketRole::query()->lazy()->each->update([
    'alias' => fn($value) => array_map( 'mb_strtolower', $value)
]);

In my mind if the update (the fill function under the hood) gets a Closure as a value it will evaluate the Closure with the arguments ($value, $model). $value would be the current value of the attribute and $model the current model. This would allow to write very compact updates.

This is somehow related to https://www.reddit.com/r/laravel/comments/j08z0f/use_an_instantly_invoked_closure_to_improve/ which is a controversial topic. But in my mind this would be a cool option for everybody that wants to follow this style. For implementation only a minor code change would be needed in laravel/framework/src/Illuminate/Database/Eloquent/Model.php:360

/**
     * Fill the model with an array of attributes.
     *
     * @param  array  $attributes
     * @return $this
     *
     * @throws \Illuminate\Database\Eloquent\MassAssignmentException
     */
    public function fill(array $attributes)
    {
        $totallyGuarded = $this->totallyGuarded();

        foreach ($this->fillableFromArray($attributes) as $key => $value) {
            // The developers may choose to place some attributes in the "fillable" array
            // which means only those attributes may be set through mass assignment to
            // the model, and all others will just get ignored for security reasons.
            if ($this->isFillable($key)) {
                $this->setAttribute($key, $value instanceof \Closure ? $value($this->getAttributeValue($key), $this) : $value);
            } elseif ($totallyGuarded) {
                throw new MassAssignmentException(sprintf(
                    'Add [%s] to fillable property to allow mass assignment on [%s].',
                    $key, get_class($this)
                ));
            }
        }

        return $this;
    }

A problem could occur for a model's custom mutation that wants a closure as its value.

Btw. there should be a forceUpdate like the forceFill function.

[tben]

I guess it depends on the size of the table but why not run the query directly on the database. Most database types allow for it.

UPDATE table_name SET column_name = LOWER(column_name)

[Kauto]

Sure, that's possible with DB::statement('');. But there are lot's of reasons I want to use Laravel's "update"-function. For example the column could have a mutation setter in the model or I want to have the additional protection. This idea is about an additional, interesting Laravel syntax and not a "how to lower case a column".

[tben]

Sorry if my reply seemed insincere. It was aimed more at your use case than a disagree about the reasoning. I was only making the suggestion because sometimes we write these scripts forgetting that each loop is a single query executed which can easily bottleneck database depending on the number of records. But your right with cases where you have a mutation setter or need additional protection.

[themsaid]

What's wrong with your first solution?

foreach (\App\Models\MarketRole::lazy() as $marketRole) {
    $marketRole->alias = array_map('mb_strtolower', $marketRole->alias);
    $marketRole->save();
}

[Kauto]

Nothing. This proposal would allow the use of the update function and closures to support a more data oriented coding style.