Global Scopes Not Being Busted Based On Called Context

[Drewdan]

Describe the bug When calling Model::all() - the global scope is not being applied. I am getting an instance of the first cached result, not the updated one when the Auth::user() changed in the global scope.

In my model I attach a global scope on boot which is a class implementing scope:

static::addGlobalScope(new SameCompanyScope);

However when I run the below Eloquent query, it always returns the cached results instead of updating the results when the scope changes due to a different user being logged in.

Eloquent Query Please provide the complete eloquent query that caused the bug, for example:

Model::all();

Stack Trace There is no stack trace, because nothing "breaks" it just returns the incorrect information.

Is this expected behaviour?

The workarounds I have identified include using the get() method on the model instead of all, but this does not feel correct as the global scope should be applied to all.

The other is modifying ModelCaching.php trait like:

if (!$instance->isCachable() || $instance->getGlobalScopes()) {
   return parent::all($columns);
}

To disable the caching when a global scope are being applied, but this still feels wrong. Any thoughts or tips would be appreciated.

Environment

• PHP: 7.3.5
• OS: Windows 10 - Wamp
• Laravel: 6.15.1
• Model Caching: 0.7.4

[mikebronner]

@Drewdan Thanks for reporting this. We have had numerous issues with global scopes in the past. Currently the tests I have for global scopes are passing. Could you provide the full class that you use to apply the global scope, as well as the model that uses it? This will let me create another test to hopefully replicate the issue.

[Drewdan]

Hi @mikebronner - cheers for getting back to me so quickly:

Tag.php

<?php

namespace App;

use App\Scopes\OrderByScope;
use App\Scopes\SameCompanyScope;
use Illuminate\Support\Facades\Auth;
use Illuminate\Database\Eloquent\Model;
use GeneaLabs\LaravelModelCaching\Traits\Cachable;

class Tag extends Model {

    use Cachable;

    /** @var array */
    protected $guarded = [];

    /**
     * The "booting" method of the model.
     *
     * @return void
     */
    protected static function boot() {
        parent::boot();
        static::addGlobalScope(new OrderByScope('name'));
        static::addGlobalScope(new SameCompanyScope);
    }

    /**
     * Tags can be assigned to many Interactions
     *
     * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany
     */
    public function interactions() {
        return $this->belongsToMany(Interaction::class);
    }

    /**
     * Tag belongs to a company
     *
     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo
     */
    public function company() {
        return $this->belongsTo(Company::class);
    }

    /**
     * Scope a query to only include tags of the same company
     *
     * @param \Illiminate\Database\Eloquent\Builder $query
     * @return \Illiminate\Database\Eloquent\Builder
     */
    public function scopeSameCompany($query) {
        return $query->where('company_id', Auth::user()->company_id);
    }

    /**
     * @inheritdoc
     *
     * @param array $options Save Options
     * @return bool
     */
    public function save(array $options = []) {
        if (!$this->company_id) {
            $this->company_id = Auth::user()->company_id;
        }

        parent::save($options);
    }
}

SameCompanyScope.php

<?php

namespace App\Scopes;

use Illuminate\Support\Facades\Auth;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Scope;
use Illuminate\Database\Eloquent\Builder;

class SameCompanyScope implements Scope {

    /**
     * Apply the scope to a given Eloquent query builder.
     *
     * @param  \Illuminate\Database\Eloquent\Builder $builder
     * @param  \Illuminate\Database\Eloquent\Model   $model
     * @return void
     */
    public function apply(Builder $builder, Model $model): void {
        if (Auth::user()) {
            $builder->where('company_id', '=', Auth::user()->company_id);
        }
    }
}

Also used is the OrderByScope.php

<?php

namespace App\Scopes;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Scope;
use Illuminate\Database\Eloquent\Builder;

class OrderByScope implements Scope {

    private $column;
    private $direction;

    public function __construct(string $column, string $direction = 'asc') {
        $this->column = $column;
        $this->direction = $direction;
    }

    /**
     * Apply the scope to a given Eloquent query builder.
     *
     * @param  \Illuminate\Database\Eloquent\Builder  $builder
     * @param  \Illuminate\Database\Eloquent\Model  $model
     * @return void
     */
    public function apply(Builder $builder, Model $model) {
        $builder->orderBy($this->column, $this->direction);
    }
}

Just to clarify, the issue occurs when the user account changes and the Tag model returns the cached result and not the expected new result.

Is that everything you need?

Thanks again :)

[mikebronner]

Thanks @Drewdan, I will try to get a new test implemented that mimics this. I'll probably ask for your feedback on the test to make sure my implementation matches yours.

[Drewdan]

Thanks @mikebronner - happy to help any way I can :)

[mikebronner]

@Drewdan One more thing, can you provide the code you use to call the tag? For example the switching process you mentioned above? I just reviewed the global scopes tests, and they are set up the same way, I wonder if they are being triggered differently?

[mikebronner]

See the tests here: https://github.com/GeneaLabs/laravel-model-caching/blob/master/tests/Integration/CachedBuilder/ScopeTest.php#L59

[Drewdan]

I would l1ogin to the application with an account, for example we will say they have an id of 1 and a company_id of 1 in the database. They then load a page which is controlled by the TagController:

/**
     * Display a listing of the resource.
     *
     * @return \Illuminate\View\View;
     */
    public function index(): View {
        return view('tags.index', ['tags' => Tag::all()]);
    }

They then see tags scoped to their company displayed.

If that user was to log out, and log back in using a different account say with an id of 2, and a company_id of 2, and they viewed the same page. They would see the tags which belong to company_id of 1, rather than seeing the tags which belong to company_id =2 - if you then ran php artisan modelCache:clear you would see the correct tags appearing.

[Drewdan]

Thinking about it, my title was not quite accurate, on first run, the GlobalScope works perfectly, the cache does not get busted when the global scope changes though

[mikebronner]

@Drewdan Thanks for the clarification. I will see if I can recreate a test for that. No worries about the title.

[mikebronner]

I created another test for this use-case, but am unable to replicate the issue. The results are as expected on my end: https://github.com/GeneaLabs/laravel-model-caching/blob/master/tests/Integration/CachedBuilder/ScopeTest.php#L106 Would you mind checking if I'm doing something wrong?

[Drewdan]

I will take a look now and get back to you :)

[Drewdan]

Hi @mikebronner The test does indeed pass, however, I have managed to make it fail with:

public function testGlobalScopesWhenSwitchingContext()
    {
        factory(Author::class, 200)->create();
        $user = factory(User::class)->create(["name" => "Anton Junior"]);
        $this->actingAs($user);
        $authorsA = (new AuthorBeginsWithScoped)
            ->all() //using all instead of get
            ->map(function ($author) {
                return (new Str)->substr($author->name, 0, 1);
            })
            ->unique();
        $user = factory(User::class)->create(["name" => "Burli Burli Burli"]);
        $this->actingAs($user);
        $authorsB = (new AuthorBeginsWithScoped)
            ->all() //using all instead of get
            ->map(function ($author) {
                return (new Str)->substr($author->name, 0, 1);
            })
            ->unique();

        $this->assertCount(1, $authorsA);
        $this->assertCount(1, $authorsB);
        $this->assertEquals("A", $authorsA->first());
        $this->assertEquals("B", $authorsB->first());
    }

By calling all() instead of get() the test will fail. My query is that yes using get() would work, but I feel like all() should still work too.

I hope that helps!

[mikebronner]

Could you give this a test with release 0.8.4 and let me know how it works! Thanks so much for your PR on this :) Very much appreciated!

[Drewdan]

I will pull it in and try it out now. My pleasure, glad I could contribute :)

[Drewdan]

I jury rigged the changes as our codebase is still on Laravel 6, so could not install the latest version through composer. It seemed to work ok, but we are due to upgrade to laravel 7 in the next few weeks, so any problems I will update you then :) thanks for acting on the PR so quickly. Much appreciated