Prevent data changes via Builder to ensure model events fire

[eilerth]

Summary
I'd like the ability to prevent the query builder from making changes to the data (insert/update/delete).

Reasoning
I like that I can use model events to track data changes. However, using some of the framework's functionality to make data changes will avoid loading the model. In those scenarios, model changes do not fire, and I'm then unable to track the data changes.

Of course, I can adjust how I'm working with things to make sure I'm working with models and not the query builder. However, the difference between working with the query builder vs. a loaded model can sometimes be quite subtle (especially to junior developers or developers not experienced with Eloquent).

Simple Example
Post::find(1)->update(['status' => $status]);
Post::where('slug', $slug)->update(['status' => $status]);

Both of these will update the status of a post. However, only the first will fire model events.
This is because find() will return a model, but where() returns a query builder.
A naive glance might see they're both starting from the "Post" model and might not catch the difference.

Of course, I could add "->first()" to the second one to actually retrieve the model, and then model events will fire:
Post::where('slug', $slug)->first()->update(['status' => $status]);

Another Example
Some examples can easily sneak up on you, especially if you're not familiar with the distinction between what is being called on a loaded model vs. a query builder.

Consider this:
$post->comments()->increment('views')

Assuming $post->comments() is returning a relation (such as HasMany), which relies on the query builder, it will increment the views column in the comments table for all the comments related to the $post.

This happens efficiently, but the post models are never loaded, and therefore no model events are fired. This will cause issues if I'm trying to track data changes, so I can troubleshoot issues, or roll back changes, or provide transparency into the journey of the data.

I acknowledge that in many cases it is more efficient to allow the query builder to do it's thing rather than loading all the models and working through one at a time. As someone who consider's themself to be a data person, it is sometimes painful for me to see things go the less efficient route.

Implementation Thoughts
I may not be the best person to tackle this, but I'm more than happy to submit a PR with my attempt at implementation if this is something that would be considered.

My first thought is to add a static property such as $allowDataChanges to Illuminate/Database/Query/Builder (defaulted to "true").
Then add a simple check to the functions that make data changes (such as insert, insertOrIgnore, insertGetId, insertUsing, update, updateFrom, upsert, delete, truncate), to first verify data changes are allowed before attempting to make the changes.

Of course, since calling ->save() on a model ultimately passes through the query builder, I'd need to update the save() function in Illuminate/Database/Eloquent/Model to update the builder's static $allowDataChanges to allow it, and then revert it back to it's original state after making the data change.

I'd also consider creating a function where you could pass a callback to override the behavior in one-off scenarios where you really do want to take advantage of the query builder's efficiency, or whatever other reason, and consciously handle (or ignore) the things that model events would be doing.
For example, something like Builder::withAllowedDataChanges(function () { ... });

Apologies if this is an undesirable approach to implementation. Working on a framework is a bit outside of my experience, so I'd also be happy to hear other suggestions on implementation. I also wouldn't be offended if someone crafted a PR of their own for this.

Wrap-up
This is something that has been bugging me (and I know I'm not exactly alone here) for a couple years now and I'd love to see it handled in the framework.
Again, I'm happy to put together a PR with something I can pull together and see where it goes from there. I'm posting this here before I go through that effort to get a sense for whether or not it has a chance to get included in the framework.

[ovp87]

I'd second this. its a pitfall i can easily see junior engineers falling into. I'd also be happy to look at making a PR for it, assuming its not a waste of my time.

[ghost]

That is why it is stated in the documentation that mass updates or raw queries are not triggering events.
Please, if you do implement something like this, make the default work as it is now (if you can achieve what you want). I remember a similar need in avoiding orWhere leaking data that is not doable (#48339).

[ovp87]

I dont think anyone is suggesting changing how it works by default, but some way of flagging that this model relies on model logic, and to prevent accessing the underlaying table via the query builder seems prudent.

[vincentcavanna]

One potential idea to solve this problem is to make a builder for that model and then override the update function for that model to throw an error instead of updating.

It's not perfect because you wouldn't have intellisense yell at you when you invoke the update method. It would just fail. But presuming good test coverage you could catch those cases.

[ovp87]

It also doesn't tackle operations being performed via relationships, as i believe the would be handled by the original models query builder, not the one defined for the "protected" model.

[ovp87]

A year later, this is still an issue, one i'm a bit surprised it hasn't been tackled.

I've personally ran into multiple issue related to this, common examples include;

• Business logic is built around eloquent events on model X being triggered, but in places, the query was performed via the model, but it wasn't resolved beforehand, so the query builder triggered the update, hence no event was fired.

• Model had some specific cast / mutator it needed to perform before save, but again, the operation was done via the query builder, resulting in this logic never being triggered.

At one point, i considered a simple trait, that overwrote the query builder, and limited the operations that way, a good solution. But even then, you run into things like a model being written to via a relationship, bypassing the model entirely, and making the model specific query-builder useless.

It feels like something that would need to be handled in the framework, rather than in userland.

[macropay-solutions]

// Eloquent builder
    public function update(array $values)
    {
        return $this->toBase()->update($this->addUpdatedAtColumn($values));
    }

toBase returns the Query Builder so also the Eloquent Builder's updates are going through the Query builder update.

[macropay-solutions]

Also the query builder does not have the model like eloquent builder does. We see no easy way of doing this.

[macropay-solutions]

Based on the Model's update function:

    public function update(array $attributes = [], array $options = [])
    {
        if (! $this->exists) {
            return false;
        }

        return $this->fill($attributes)->save($options);
    }

the eloquent builder's update function could be overwritten in a child class to check for the existence of the model

    public function update(array $values)
    {
        if (! $this->getModel()->exists) {
            return false;
        }

        return parent:update(values);
  }

But this would prevent mass updates like this only:

Post::where('slug', $slug)->update(['status' => $status]);

The query builder could still be used to mass update.

Post::where('slug', $slug)->->toBase()->update(['status' => $status]);

[macropay-solutions]

A solution can be patching all calls to \Illuminate\Database\Connection::affectingStatement by enclosing them in a DB transaction and check the affected rows. If different than 1 rollback via throwing exception.

7 places in laravel 10.

\Illuminate\Database\Query\Builder::delete
\Illuminate\Database\Query\Builder::update
\Illuminate\Database\Query\Builder::updateFrom
\Illuminate\Database\Query\Builder::insertOrIgnore
\Illuminate\Database\Query\Builder::insertOrIgnoreUsing
\Illuminate\Database\Query\Builder::insertUsing
\Illuminate\Database\Query\Builder::upsert

BUT still a mass update/delete can have just 1 affected row so this is also a dead end.

Still it can just log the debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS) case in which only the \Illuminate\Database\Connection::affectingStatement needs change so that it logs when the global flag sais to not allow mass updates/deletes and the affected rows is not 1.

[macropay-solutions]

Another idea would be to pass down to these 7 functions from the query builder, the modelExists flag from Eloquent builder's model and then to \Illuminate\Database\Connection::affectingStatement where the global flag should be checked and if the model does not exists, then throw.
But these would be breaking changes even if the new param would have default value.

But still, using directly the query builder without eloquent would not prevent the mass updates/deletes.