laravel / octane

Supercharge your Laravel application's performance.
https://laravel.com/docs/octane
MIT License
3.78k stars 296 forks source link

User is in cache/ram when fetched from construct #379

Closed mcolominas closed 3 years ago

mcolominas commented 3 years ago

Description:

I was using laravel fortify and found that when the user information is updated, the changes are not reflected.

Before opening this issue, I have done tests to find the problem and make sure that it is not something wrong that I have done, this is what I have found:

For some reason, when the StatefulGuard bind is passed through the construct and then used in the controller method, it gets a cached user/ram.

From what I have seen, in the class 'Illuminate\Auth\SessionGuard' in the method 'user()' when checking if(!is_null($this->user)) returns the user, the problem it is, as I have commented previously, the user data, it is not updated until the octane is restarted.

Steps To Reproduce:

Packages installed:

"require": {
    "php": "^7.3|^8.0",
    "fruitcake/laravel-cors": "^2.0",
    "guzzlehttp/guzzle": "^7.0.1",
    "laravel/framework": "^8.54",
    "laravel/octane": "^1.0"
},

Service Provider:

namespace App\Providers;

use Illuminate\Contracts\Auth\StatefulGuard;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\ServiceProvider;

class AppServiceProvider extends ServiceProvider
{
    public function register()
    {
        $this->app->bind(StatefulGuard::class, function () {
            return Auth::guard();
        });
    }
}

Controller:

namespace App\Http\Controllers;

use Illuminate\Contracts\Auth\StatefulGuard;
use Illuminate\Support\Facades\Auth;

class TestController extends Controller
{
    private $guard;

    public function __construct(StatefulGuard $guard)
    {
        $this->guard = $guard;
    }

    public function test()
    {
        dd([
            $this->guard->user()->name,
            Auth::guard()->user()->name,
            app(StatefulGuard::class)->user()->name
        ]);
    }

    public function login()
    {
        Auth::loginUsingId(1);
    }
}

Routes:

Route::get('login', [TestController::class, 'login']);
Route::get('test', [TestController::class, 'test']);

Steps:

  1. Have the user system configured and have a user created.
  2. Go to /login to login with the user id 1.
  3. Go to /test to see the user's information, in my case the following will appear: username, username, username,
  4. Modify the username from the database or elsewhere (I have changed username to modified username).
  5. Go to /test to see the user's information, in my case the following will appear: username, modified username, modified username.
  6. Restart octane and go to /test to see the user's information, in my case the following will appear: modified username, modified username, modified username.

As you can see, in step 5, the username is not updated.

mcolominas commented 3 years ago

Another test that has occurred to me now is the following:

Controller:

namespace App\Http\Controllers;

use Illuminate\Contracts\Auth\StatefulGuard;
use Illuminate\Support\Facades\Auth;

class TestController extends Controller
{
    private $guard;
    private $guard2;
    private $guard3;

    public function __construct(StatefulGuard $guard)
    {
        $this->guard = $guard;
        $this->guard2 = app(StatefulGuard::class);
        $this->guard3 = Auth::guard();
    }

    public function test()
    {
        dd([
            $this->guard->user()->name,
            $this->guard2->user()->name,
            $this->guard3->user()->name,
            Auth::guard()->user()->name,
            app(StatefulGuard::class)->user()->name
        ]);
    }

    public function login()
    {
        Auth::loginUsingId(1);
    }
}

The result after modifying the username is:

array:4 [▼
  0 => "user name"
  1 => "user name"
  2 => "user name"
  3 => "user name updated"
  4 => "user name updated"
]
mcolominas commented 3 years ago

Someone can tell me something? at least if it's an octane bug or i'm doing something wrong in the constructor.

kocoten1992 commented 3 years ago

My guess is controller is not a service, it get init only once and does not get refresh on every request, https://laravel.com/docs/8.x/octane#dependency-injection-and-octane

mcolominas commented 3 years ago

So, how should the code be in the controller? Because I can't touch the Facade Auth

kocoten1992 commented 3 years ago

So, how should the code be in the controller? Because I can't touch the Facade Auth

Give up on octane - I guess, or you can do this:

    public function test()
    {
        $this->guard = 'something else';
        // continue your code here
        // ...
    }
mcolominas commented 3 years ago

From what you say, is it impossible to pass it through the constructor? so laravel fortify is incompatible with laravel octane as it uses StatefulGuard in constructor

kocoten1992 commented 3 years ago

From what you say, is it impossible to pass it through the constructor? so laravel fortify is incompatible with laravel octane as it uses StatefulGuard in constructor

No I didnt say that, I say it is incompatible with the way you code your application, laravel octane is essentially cache every thing in memory to serve at high speed. Hence you need to carefully craft your application to make it compatible. The only place you should do it is in service or you can manually config which classes get refresh when a request is done (via config/octane.php) but that also mean give up on octane power - That is what I meant.

laravel fortify is incompatible with laravel octane as it uses StatefulGuard in constructor

And which tutorial show you that laravel fortify should be injected into controller constructor ? I cannot found it on laravel official docs.

mcolominas commented 3 years ago

I do not know how to insert the code of other projects, therefore, I will put the link: https://github.com/laravel/fortify/blob/1.x/src/FortifyServiceProvider.php <- line 55

I must also add that old versions of fortify used $this->guard->user() to get the user, therefore in my profile controller I also used this method, and when using laravel octane, the user information in the database was updated but not in the application, now I'm changing it to $request->user() and that seems to work, but from what I see, can't the guard be passed by the constructor? I have tried config/octane.php, but I have not been able to update it for each request cycle.

From what I have commented previously, is the class Illuminate\Auth\SessionGuard that would have to be reset in each request cycle to avoid these user problems in ram, specifically, the user variable.

kocoten1992 commented 3 years ago

Fortify service is register as a service, so it will be reset on each request, almost everything else is not, $request also special because it have to (otherwise you got leak info from one user to another).

Illuminate\Auth\SessionGuard may or maynot get refresh, but your controller is not, check it out here https://laravel.com/docs/8.x/octane#container-injection, that mean except it a service and you bind via bind, say goodbye to constructor.

kocoten1992 commented 3 years ago

For your extra info, normal php-fpm for each request, it gonna rebuild the world from scratch (that why you have like 20ms minimum for each request).

With swoole (octane), it init the world only once and keep everything in memory, so the minimum request time is WELL below 1ms, nothing come free though, because now the first request gonna affect the second one (like your example). Need to carefully now because every object after octane start up will exists forever (service is an exception - we need exception like these to survice).

This is a mindshift change, not just drop in performance boost, for example, you open a database transaction and forgot to close when the request is finish, the second request going to open a nested transaction two layer depth, a third request gonna open triple layer transaction and so on.

mcolominas commented 3 years ago

Yes, I understand that perfectly, that's why I ask, if there is some way of configuring the config/octane.php in the listeners section to reset that class, something listener like FlushGuardState or FlushAuthState, the problem is that I have not found any method to delete the user, only to assign a new one.

Also with service is an exception, what do you mean by service? to the ServiceProvider? because if it is that, I have done some tests and the ServiceProvider are only executed once, when laravel octane starts.

Another question, for the queue bbdd and the commands (app / Console / kernel) it has to be used as a normal laravel (crontab + supervisor) or does octane take care of it?

kocoten1992 commented 3 years ago

No, sorry, I've never try to manual reset any class, so can't really help you, you can experimenting with that.

what do you mean by service? to the ServiceProvider?

Yes.

Supervisor and (or) crontab is different thing than http server, it actually start a new php script everytime it run (create new world from scratch), so none of the rule apply here, you can write it normally. However, becareful if you force it, for examle:

while (true) {
    // if somecommand never crash
    // then we never exit the script to rebuild the world
    Artisan::call('somecommand');
}
mcolominas commented 3 years ago

ok, because supposedly as this is not an "issue" I am going to close it and I will carry out more tests.