Model::update() is dangerous

But have you guys ever forgotten to validate input values?

Ok, this is hard to argue. This probably happened to everyone.

I can't think of a use case like this sample where you want to update all records at the same time when updating a single record.

As I said before. We have to think of it from the point of view of the entire API that comes with the Model.
Developers may write queries that don't necessarily update all the records when the null is set, like:

PHP Code:

$model->where(...)->set(...)->update(); 


So, if we suddenly start to require an $id, this will break people's code. Not sure what is more damaging.

The only workaround I see for this is checking if the where part was set before or not. Something like:

PHP Code:

    protected function doUpdate($id = null, $data = null, bool $allowNullId = false): bool
    {
        $escape      = $this->escape;
        $this->escape = [];

        $builder = $this->builder();

        if ($id === null && ! $allowNullId && empty($builder->getCompiledQBWhere())) {
            throw new DatabaseException(
                'Updates are not allowed unless they contain a "where" or "like" clause.'
            );
        }

        if ($id) {
            $builder = $builder->whereIn($this->table . '.' . $this->primaryKey, $id);
        }

        // Must use the set() method to ensure to set the correct escape flag
        foreach ($data as $key => $val) {
            $builder->set($key, $val, $escape[$key] ?? null);
        }

        return $builder->update();
    } 


It would still be a BC break, but the overall functionality would remain. Still, I'm not sure this is the best thing to do.

michalsn.dev - mostly about CodeIgniter