analogueorm / analogue

Analogue ORM : Data Mapper ORM for Laravel/PHP
MIT License
632 stars 51 forks source link

The update $dirtyRelatedAggregates override foreighn keys to null #219

Closed Insolita closed 6 years ago

Insolita commented 6 years ago

So i have pure entities without magic, and try test

            $userRep = new UserRepository();
            $rep = new FilterTaskRepository();
            $user = $userRep->findOrFail(1);
            expect($user)->isInstanceOf(User::class);
            $task = new FilterTask($user, 'dummy', 'foobar', 100);
            $candidate1 = new Candidate('foo', 'bar');
            $candidate2 = new Candidate('bar','baz');
            $task->addCandidates([$candidate1, $candidate2]);
            $result = $rep->store($task);
            expect($result->id())->notNull();
            expect($result->userId())->equals(1);
            expect($result->candidates())->count(2);
            $result->candidates()->each(function (Candidate $candidate) use ($result) {
                expect($candidate->report_id)->equals($result->id());
            });

With user relation all right, but with candidates relations test failed; I make debug of this case, and see, that before https://github.com/analogueorm/analogue/blob/b91fd2417dc642d3c9d8c7c262061e69ba2f513b/src/Commands/Store.php#L147 candidate records was stored in database with correct foreighn keys ('report_id'); but after checking dirty attributes it detect 'report_id' as dirty, and next steps override it to null https://github.com/analogueorm/analogue/blob/b91fd2417dc642d3c9d8c7c262061e69ba2f513b/src/Commands/Store.php#L52 https://github.com/analogueorm/analogue/blob/b91fd2417dc642d3c9d8c7c262061e69ba2f513b/src/Commands/Store.php#L266

Entity and Map examples

class FilterTask
{
    private $id;
    private $userid;
    ...
    private $candidates;
    private $user;

    public function __construct(User $user, string $label, string $source, string $price)
    {
        $this->user = $user;
        $this->label = $label;
        ...
        $this->candidates = new EntityCollection();
    }

    public function addCandidate(Candidate $candidate)
    {
        $this->candidates->add($candidate);
    }
    public function addCandidates(array $candidates)
    {
        foreach ($candidates as $candidate) {
            $this->addCandidate($candidate);
        }
    }
     //... Getters/Setters
}

class Candidate
{
    private $id;
    public $report_id;
    private $task;
    private $level = 0;
    private $valid = false;
    private $name;
    private $info;
    public function __construct($name, $info)
    {
        $this->name = $name;
        $this->info = $info;
    }
   // ... Getters/Setters
}

class FilterTaskMap extends EntityMap
{
    protected $table = 'task_filters';
    protected $arrayName = null;
    protected $primaryKey = 'id';
    protected $dateFormat = 'U';
    protected $createdAtColumn = 'created';
    protected $updatedAtColumn = 'updated';
    protected $properties= [ 'id','userid','source','label','price',...,'user','candidates'];

    public function user(FilterTask $task)
    {
        return $this->belongsTo($task, User::class, 'userid');
    }

    public function candidates(FilterTask $task)
    {
        return $this->hasMany($task, Candidate::class, 'report_id', 'id');
    }
}
class CandidateMap extends EntityMap
{
    protected $table = 'task_candidates';
    protected $arrayName = null;
    protected $primaryKey = 'id';
    protected $properties = ['id', 'report_id','name',..,'task'];

   /** tried with and without inverse definition -> same result
    public function task(Candidate $candidate)
    {
        return $this->belongsTo($candidate, FilterTask::class, 'report_id');
    }
  */
`
RemiCollin commented 6 years ago

Ok I was able to reproduce the issue in tests. It happens when using a custom foreign key as object property. As a workaround until it's fixed, using "filter_task_id" as foreign key should work.

Insolita commented 6 years ago

I admit that I could incorrectly configure the properties of entity Map as there is not enough documentation I can`t change schema and field names; it is active app with legacy code and i must refactor it piece by piece,

RemiCollin commented 6 years ago

Your configuration is correct, It's definetely a bug, I'll fix it as soon as I can.

I'm aware of the documentation is clearly outdated and rewriting it is on my top priority list..

Insolita commented 6 years ago

Thanks for quick response, and good project!

RemiCollin commented 6 years ago

@Insolita I pushed a fix, can you tell me if it solves your issue ?

Insolita commented 6 years ago

@RemiCollin , yes, thanks, new release solved my issue ... but I have new issue with embedded objects...

I add in FilterTaskMap

 public function stage(FilterTask $task)
    {
        return $this->embedsOne($task, WorkStage::class, 'stage')
            ->setPrefix('')->setColumnMap(['value'=>'stage']);
    }

And got fails when try test wth storing (test with finding passed successfull and this embed was resolved)

ormerror

WorkStage is simple stdClass ValueObject (Also WorkStageMap exists, and test wtih finidng was passed)

I'll take a more detailed research a bit later

RemiCollin commented 6 years ago

Okay, just open a new issue then ;)

I'll close this one