nextras / orm

Orm with clean object design, smart relationship loading and powerful collections.
https://nextras.org/orm
MIT License
309 stars 59 forks source link

Entity persistence uses getter methods #337

Closed echo511 closed 5 years ago

echo511 commented 5 years ago

Describe the bug Implemented Entity::getter....($value) method is used when entity is persisted. This is confusing as getter should only mutate stored value.

To Reproduce For example try to persist entity below.

/**
 * @property string        $id          {primary}
 * @property string        $question
 */
class Card extends \Nextras\Orm\Entity\Entity
{

    protected function getterQuestion($question)
    {
        return 'replaced';
    }

}

$card = new Card;
$card->id = uuid();
$card->question = 'Some question';

$repo->persist($card);

Expected behavior In DB $question should be 'Some question'. When $entity->question is accessed in code 'replaced' should be returned.

Current behavior In DB 'replaced' is stored.

Versions::

hrach commented 5 years ago

Thank you for the report. I will take a look. This seems like a serious issue, though I'm not sure when this happened. Hopefully fixing this won't break it for someone relying on this behavior :)

hrach commented 5 years ago

Ok, I have checked that this behavior is here since version 1.0 so changing it would be probably BC break. But it is definitely something we should think about for 4.0.

Actually this applies also for setters. When removed, it should be removed symmetrically, e.g. also from setters. But in setters it has the important function to validate/coerce the db value to entity property type.

But I agree that current impl is not good. Currently it is usable only in this usecases:

echo511 commented 5 years ago

Well for now the main problem for me is the documentation to be misleading (https://nextras.org/orm/docs/3.1/entity#toc-getters-and-setters)

Specifically the paragraph

Getter method receives the stored value as the first parameter and should return the optionally modified value. “Virtual getters” do not receive any value. Setter method receives the user given value and should return the modified value for storing it in the entity.

It gives an impression that only setters modify values to be stored. In reality getters and setters are IMHO quite interchangeble. I will prepare a PR for the docs for your consideration.

Thank you for looking into this.

hrach commented 5 years ago

This is fixed in master and will be a part of 4.0 release.