Bug: Entity can't handle column named `attributes`

[iRedds]

CodeIgniter4 Version

4.1.9

What happened?

When using attributes, as the property/attribute name of the Entity class, the Entity::setAttributes() method is called as a setter.
Names collision.

Steps to Reproduce

$e = new CodeIgniter\Entity\Entity();
$e->a = 1;
$e->attributes = [1, 2, 3];  // called the setAttributes method as a setter.
dd($e->toRawArray());

//$e->toRawArray() array (3)
//   0 => integer 1
//   1 => integer 2
//   2 => integer 3

Expected Output

//$e->toRawArray() array (2)
//    a => integer 1
//    attributes => array (3)
//        0 => integer 1
//        1 => integer 2
//        2 => integer 3

Anything else?

In Laravel prior to version 9, the custom setter and getter had the following format:
set/get + AttributeName + Attribute.

public function getFullNameAttribute()
{
    return "{$this->first_name} {$this->last_name}";
}

[the-gt99]

Until the update comes out, I offer my temporary solution. Wrap over "Entity"

<?php


namespace App\Helpers;

use CodeIgniter\Entity\Entity;

class EntityModify extends Entity
{
    private $clearFields = ["attributes","casts","datamap"];

    //Фикс метода fill дабы все не ебнулось, когда в данных пришло что-то из списка ;)
    public function fill(?array $data = null)
    {
        if(is_array($data))
        {
            foreach (array_keys($data) as $key) {
                if (in_array($key, $this->clearFields, true)) {
                    unset($data[$key]);
                }
            }
        }

        return parent::fill($data);
    }

    //Добавляем поля которые не должны быть в объекте описывающем таблицу но очень хочется их видеть в ответе  ;)
    public function addMoreInfo($moreInfo)
    {
        foreach ($moreInfo as $key => $value)
        {
            $this->attributes[$key] = $value;
        }
    }
}

[kenjis]

In Laravel prior to version 9, the custom setter and getter had the following format:
set/get + AttributeName + Attribute.

What happened in Laravel 9?

[lonnieezell]

Damn, I wish we could just rename the setAttributes method. That is by far the simplest solution.

I am wondering if that can be considered a bug, though. If others think so then I say we do that. That is preferable to any additional complexity in the logic that we would have to maintain, and it's the only method that would be affected by this.

[kenjis]

Renaming the setAttributes() method seems to be the best solution.

It seems it is undocumented.
https://codeigniter4.github.io/CodeIgniter4/search.html?q=setAttributes&check_keywords=yes&area=default

[iRedds]

@kenjis In version 9, the getter and setter is the method equivalent to the attribute name. (using php8 functionality)

    protected function firstName(): Attribute
    {
        return Attribute::make(
            get: fn ($value) => ucfirst($value),
            set: fn ($value) => strtolower($value),
        );
    }

There is a possibility that there is an application that has its own Entity implementation with the appropriate setAttributes() method.

I do not consider it correct to rename the setAttributes() method, since the name fully reflects the essence of this method. I also think that the toRawArray() method should be called getAttributes().
Otherwise, any internal method that may be added in the future will need to be adapted instead of a friendly name.

It would be more correct to change the format of the names of setters and getters.

[iRedds]

@the-gt99 Your proposal is a special case.
It will not work if the attributes attribute is required.

[lonnieezell]

It is a much larger breaking change to update the format of the getters and setters and that's not something that could happen right now, I don't think. I'm a little leery of changing the name, honestly, but it's a pretty simple fix for people to change and I think it can legitimately be called a bug.

Naming it something like setEntityAttributes still keeps the meaning clear while fixing the issue. While I agree this is not a perfect fix, and I wouldn't be opposed to updating how it is handled in the future, I think the chances that someone will be trying to set an attribute named entityAttributes is slim enough that this can buy us time until we can do a bigger refactor.

I am very interested to from @MGatner @paulbalandan and @samsonasik before we proceed with any changes though.

[kenjis]

The current setX() and getX() are natural to me.
And as @lonnieezell says, changing them to something like setXAttribute() is a much larger breaking change.

[iRedds]

The problem is that no matter how the setAttributes() method is renamed, there will be a chance that the method name will match the table field.
In any case, it will be BC.
But we need to consider all the consequences.

[kenjis]

See my PR: #5763

[iRedds]

For example, we add the Entity::setRelation() method.
It turns out that now the developer will not be able to use the relation attribute normally.
That is, with this approach, internal methods with names of the format set|get + something will be prohibited. Since this format is used for getters and setters.

[kenjis]

@iRedds Probably I got you want to say.
If we want to add setRelation() and use it to set the relationship of entities,
we can't use it like that. It's too restrictive.

[MGatner]

Removing setAttributes() seems a pretty big breaking change over simply stating in the docs that "attributes" is a reserved Entity name. I agree this should be changed in the future, but because of the collision we can't even deprecate this so it will take any dependent projects from functional to crash.

Personally I can't say specifics right now but I'm pretty sure some of my libraries extend setAttributes().