mikebronner / laravel-model-caching

Eloquent model-caching made easy.
MIT License
2.25k stars 213 forks source link

Constrained eager loading does not detect change to where constraint #295

Open saernz opened 4 years ago

saernz commented 4 years ago

Describe the bug When using constrained eager loading the first cached result is always return regardless of any changes to the constraint. I could not find any other issues relating to this and the docs only mentioned lazy-loaded relationships are unsupported at the moment so I thought it wise to raise this as a bug.

Eloquent Query I've provided a test that you can copy and paste into tests/Integration/CachedBuilderRelationshipsTest.php which should show my problem.

public function testConstrainedEagerLoads()
{
    factory(Book::class)
        ->create([
            "author_id" => 1,
            "publisher_id" => 1,
            "title" => "Jason Bourne",
        ]);
    factory(Book::class)
        ->create([
            "author_id" => 1,
            "publisher_id" => 1,
            "title" => "Bason Journe",
        ]);

    $jasonBourneBooks = Publisher::with(['books' => function ($q) {
        $q->where('title', 'Jason Bourne');
    }])->get()->pluck('books')->flatten();

    $this->assertCount(1, $jasonBourneBooks);
    $this->assertEquals("Jason Bourne", $jasonBourneBooks->first()->title);

    $basonJournBooks = Publisher::with(['books' => function ($q) {
        $q->where('title', 'Bason Journe');
    }])->get()->pluck('books')->flatten();

    $this->assertCount(1, $basonJournBooks);
    $this->assertEquals("Bason Journe", $basonJournBooks->first()->title);
}

Then when running ./vendor/bin/phpunit --filter CachedBuilderRelationshipsTest I get the following result:

There was 1 failure:

1) GeneaLabs\LaravelModelCaching\Tests\Integration\CachedBuilderRelationshipsTest::testConstrainedEagerLoads
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Bason Journe'
+'Jason Bourne'

/Users/some-path/laravel-model-caching/tests/Integration/CachedBuilderRelationshipsTest.php:80

FAILURES!
Tests: 3, Assertions: 7, Failures: 1.

Environment

Additional context Can use:

$result = app("model-cache")->runDisabled(function () {
    return Publisher::with(['books' => function ($q) {
        $q->where('title', 'Bason Journe');
    }])->get();
});

to workaround the problem for my specific query at the moment, but for some reason ->disableCache() did not work.

Thanks for the great library, has been working great so far. Wish I had more time to look into this more or do a PR but even as it is I have probably got a little too side tracked :)

mikebronner commented 4 years ago

@saernz Thanks for this! I will look into this over the next few days, time permitting.

mikebronner commented 4 years ago

@saernz Will have to work on this some more. Tried coming up with a solution today, but having a bit of trouble trying to extract the key of with with query. WIP

mikebronner commented 4 years ago

Related to #291