Yii2: Count for relation with via() fails in 2.0.16

[ddinchev]

Some of my tests started failing after upgrading to Yii 2.0.16. Same work OK on 2.0.15.1. Case 2 is the problematic one.

What steps will reproduce the problem?

class Vehicle extends ActiveRecord {
    public function getEntries(): ActiveQuery
    {
        return $this->hasMany(Entry::class, ['VehicleID' => 'VehicleID'])->inverseOf('vehicle');
    }

    public function getFuelingCount(): int
    {
        return $this->hasMany(Fueling::class, ['EntryID' => 'EntryID'])->via('entries')->count('FuelingID');
    }
}


// CASE 1 - OK
$vehicle = Vehicle::findOne(1);
// insert 3 Fueling::class related to $vehicle
$vehicle->fuelingsCount; // 3
// When debugging this, it seems to produce a RAW SQL like `SELECT COUNT(FuelingID) FROM Fueling WHERE EntryID IN (1,2,3)` 


// CASE 2 - NOT OK
$vehicle = Vehicle::findOne(1);
$vehicle->fuelingsCount; // 0
// insert 3 Fueling::class related to $vehicle
$vehicle->fuelingsCount; // 0 <- this would be fine in Yii2 2.0.15.1
// The RAW query is `SELECT COUNT(FuelingID) FROM Fueling WHERE 1=0`

Additional info

| Q | A
| ---------------- | ---
| Yii version | 2.0.16
| PHP version | 7.2.24
| Operating system | Ubuntu 16.04
| MySQL | 8.0

[samdark]

Is it similar to https://github.com/yiisoft/yii2/issues/17089?

[ddinchev]

Seems to be the same issue. I'll try to revert this commit to confirm that this is the one causing the bug.

[rugabarbo]

@ddinchev how exactly do you insert 3 Fueling::class related to $vehicle in your cases?

[ddinchev]

One particular test that passes in 2.0.15.1 but fails in 2.0.16 is this. It's not really a unit test, more of an integration test as it hits the db. There is no cleanup between the two specify blocks - there is some cache left for the relations after the first getFuelingCount() call.

class VehicleTest extends Unit
{
    use Specify;

    /**
     * @var Vehicle
     */
    private $vehicle;

    /**
     * @var \tests\common\UnitTester;
     */
    protected $tester;

    public function _before()
    {
        $this->vehicle = $this->tester->have(Vehicle::class);
    }

    public function testGetFuelingCount()
    {
        $this->specify("By default fuelings are 0.", function () {
            verify($this->vehicle->getFuelingCount())->equals(0);
            verify($this->vehicle->fuelingCount)->equals(0);
            verify($this->vehicle->fuelingCount)->internalType('int');
        });

        $this->specify("When there are fuelings, their count is correct.", function () {
            $this->seedFuelings(3);
            verify($this->vehicle->getFuelingCount())->equals(3);
        });
    }

    private function seedFuelings($count)
    {
        $entries = $this->tester->haveMultiple(Entry::class, $count, [
            'VehicleID' => $this->vehicle->VehicleID
        ]);
        foreach ($entries as $entry) {
            $this->tester->have(Fueling::class, ['EntryID' => $entry->EntryID]);
        }
    }
}

[samdark]

@ddinchev please check if your issue is fixed in master.

[rugabarbo]

@samdark reopen it, please.
My PR #17117 fixed problems of using via() with callable.
In this case, some other problem.... I'll explore it separately.

[rugabarbo]

@ddinchev I need to know how exactly do you insert 3 Fueling::class related to $vehicle in your cases?

$this->tester->have(Fueling::class, ['EntryID' => $entry->EntryID]);

How does this call work?

It looks like your code worked correctly due to framework code worked incorrectly :-) That's why I want to understand exactly how the data was filled in your code.

[ddinchev]

These have calls are part of Codeception's Data Module. They just insert rows of test data in the database with certain properties provided by Faker (and overwritten with optional params argument).

[rugabarbo]

OK, thanks for the additional information. I reproduced this problem with local tests. It's really related to PR #16696. It takes time to think about how to fix it...

[rugabarbo]

I'll just write down everything that has accumulated in thoughts on this issue. This case is not as trivial as it seems. This is an architecture level problem.

If we want to solve the problem correctly in terms of architecture, then any update of the related data in the ActiveRecord layer should lead to a reset of the cached relational data in the related objects. But at the moment the reset does not occur in all parts of the code.

Related problems have already been solved by different patches:

• Issue #13567 (closed) – "afterSave dont refresh relational model"
• PR #13618 (merged) – "Reset outdated related models after AR update"
• Issue #14233 (closed) – "ActiveRecord link() / unlink() methods do not update model relation with saved data"
• Issue #15875 (closed) – "afterSave for new models flushes unsaved data"
• Issue #15954 (open) – "Eager loaded relation isn't reloaded"
• PR #16085 (merged) – "Bug: afterSave for new models flushes unsaved data"
• Issue #16552 (closed) – "Cached relatation is not used in relation through via"
• PR #16588 (merged) – "Bug #16552: Cached relatation is not used in relation through via"
• Issue #17089 (closed) – "Callable in model relation via junction table stop working on 2.0.16"
• PR #17117 (merged) – "Fixes #17089: fixed caching of related records when via() using with callable"

Now I will think about how to solve this particular problem.

[rugabarbo]

After researching the issue, I made the following conclusions:

1. According to the current framework design it isn't a bug, it's a feature (there is caching of related objects in AR layer).
2. If you want to get the expected result in the second case, use refresh() method:

$vehicle = Vehicle::findOne(1);
$vehicle->fuelingsCount;
// insert 3 Fueling::class related to $vehicle
$vehicle->refresh(); // <- use it
$vehicle->fuelingsCount;

1. So, before fix #16588, your code worked due to bug #16552.
2. My test case #17143 is also incorrect for the current framework architecture. An implementation of refresh() method is already covered by other tests. Therefore, new tests are not needed.

If we want to change the framework architecture, then this should be done in 3.0 version. In version 2.0 in such cases, we should use refresh() method to get actual data from the database.