BaseActiveRecord::bindModels() doing unnecessary save

[hvta]

What steps will reproduce the problem?

Link models

What is the expected result?

I expect the the function to link models without saving the foreign model.

Additional info

When calling the link() method that later calls the private bindModels() function, it saves the foreign model without validating it. In my case it is not a required effect, I would like to be able to save it manually.

[tunecino]

bindModels receives 2 models only 1 of them is getting saved but not the second one:

private function bindModels($link, $foreignModel, $primaryModel)
{
    ...
    $foreignModel->save(false);
}

which means $foreignModel should be the one holding the foreign key that need to get updated. I don't see the second one ($primaryModel) getting saved anywhere and bindModels() is called once in all cases inside link() so it looks correct to me. is the SQL request in your logs updating both models ? is it a One_To_Many relationship what we are talking about ?

[hvta]

The $foreignModel->save(false); line is my problem actually. In other words, the bindModels function has that saving feature unnecessarily. I use the link method in BaseActiveRecord class, but I only need to link models and not to save them. I think it should have an option to turn on and off saving manually.

[tunecino]

Sorry but cannot understand how saving is unnecessary.

the link method (source) will basically try to parse the db schema to understand what kind of relationship both models are sharing. if it is a Many_to_Many a new row will be inserted in some junction table (bindModels not called here). If it is a One_to_Many or One_to_One that means it should be expected that one of both models is holding a foreignKey pointing to the other, so the script will just try to figure out which one of them is holding it so it's value can be updated (using save() and within bindModels). which means only one of those 2 will be called depending on models relationship:

$this->bindModels(..., $this, $model);
// or
$this->bindModels(..., $model, $this);

but I only need to link models and not to save them. I think it should have an option to turn on and off saving manually.

How ? what is the model's structure or the relationship type that can be linked without updating it's foreignKey value ? what is the expected behavior of linking if it is not updating some value in db ? could you provide a use case ?

[klimov-paul]

I can see no issue heew.

[evpav]

I find this strange too. Take the guide example:

$customer = Customer::findOne(123);
$order = new Order();
$order->subtotal = 100;
// ...

$order->customer_id = $customer->id;
$order->save();

// or:
$order->link('customer', $customer);
// which internally does:
$order->customer_id = $customer->id;
$order->save(false);

Where the validation must happen in this case? Cannot be before the link, if customer_id is required. Validating the order after doesn't make sense. What is the recommended way to deal with this?

[cebe]

there is no need to call link() if you call save() and assign the ID before.

[evpav]

@cebe, that's just two examples from the guide combined here for brevity, they are not used together, but supposed to be alternatives.

[cebe]

you can call link if you just want to link records without validating other data. If you need validation, you should set the id and call save() instead.

[evpav]

That I know, the question is why link combines setting the FK value and saving the related record, but doesn't provide a way to either validate the record after setting the FK value and before the save, or, better, just skip saving it (in cases where link is just a part of a complex save)?

[cebe]

link() will always save the record to ensure consistent behavior. Many-Many relations for example need an insert into the junction table, that can not be delayed or made together with saving the model later.

I do not see a way to improve link() right now, if you have ideas, feel free to propose some changes.

[evpav]

With M:N relations link goes through a different code path, and usually doesn't require validation of the junction table records. With simple 1:N relations an extra link argument to pass through to the bindModels to decide whether to save the foreign model or not can be used.

[aleskiontherun]

Here's my use case:

I link models through setters and getters by simply assigning a linked model to an attribute.

/**
 * @property Users $user
 * @property int $user_id
 */
trait UserTrait
{
    /**
     * @return ActiveQuery
     */
    public function getUser(): ActiveQuery
    {
        /** @var ActiveRecord $this */
        return $this->hasOne(Users::class, ['id' => 'user_id']);
    }

    /**
     * @param Users $user
     */
    public function setUser(Users $user): void
    {
        if ($this->user_id !== null && $this->user_id !== $user->id) {
            throw new InvalidCallException('User ID is already set and is different from the specified one.');
        }
        $this->link('user', $user);
    }
}

class Transaction extends ActiveRecord
{
    use UserTrait;
}

Creating a new model looks like this:

$model = new Transaction;
$model->foo = 'bar';
$model->user = Yii::$app->user->identity;
$model->insert();

Problem 1 If Transaction has multiple required relations (db fields which are NOT NULL) then the first link() call will fail because other fields are not yet set:

$model = new Transaction;
$model->foo = 'bar';
$model->user = $user; // This will throw a DB Exception.
$model->anotherRelation = $anotherModel;

OK, so it can be solved by simply replacing

$model->user = $user;
$model->anotherRelation = $anotherModel;

with

$model->user_id = $user->id;
$model->another_relation_id = $anotherModel->id;
$model->insert();

which is not nice but will solve the problem until we get to the next one:

Problem 2 Now accessing a $model->user attribute will make a new query to the database to get the data.

Let's say we create a transaction and want to perform certain actions after it had been created, e. g. send an email to the user (or write something to a log, or notify 3rd party API, or whatever).

class TransactionNotifier
{
    public static function notify(Transaction $model) {
        $title = "New transaction";
        $body = "Hi {$model->user->name}! You just made a transaction for ${$model->amount}.";
        SomeMailer::send($model->user->email, $title, $body);
    }
}

With current behavior we have to assign user_id but not the user model itself which will lead to an extra query when accessing $model->user->name in the code.

$model = new Transaction;
$model->amount = 9001;
$model->user_id = $user->id;
$mode->insert();

TransactionNotifier::notify($model);

Here we can't use the setter to make the code more readable and logical. And the extra query doesn't make any sense since we already have the user model loaded but can't use it's data in the relation.

If bindModels() would not save the model, then this scenario would be much more straight forward:

$model = new Transaction;
$model->amount = 9001;
$model->user = $user;
$mode->insert();

TransactionNotifier::notify($model);

It also would not affect Many-Many relations because it doesn't actually use bindModels() and just saves the intermediate record.