spatie / laravel-multitenancy

Make your Laravel app usable by multiple tenants
https://spatie.be/docs/laravel-multitenancy
MIT License
1.11k stars 153 forks source link

SwitchTenantDatabaseTask.php #285

Closed gofish543 closed 3 years ago

gofish543 commented 3 years ago

When attempting to execute the command

$FORGE_PHP artisan tenants:artisan "auth:clear-resets tenants"

I get a failed to call prepare() on null. This refers to the PDO object within Laravel's DB Connection class. PDO is set to null when the switch db task is executed and causes future pdos to return null.

I propose adding App::forgetInstance('db'); after the purge to force the singleton to be refreshed by laravel.

Thoughts?

masterix21 commented 3 years ago

Hi @gofish543,

I have tried now in a project of mine, but it works: image

Can you help me to dig inside?

Thanks

gofish543 commented 3 years ago

:( That's not fair... image

The ->prepare() call originates from Laravel's connection class

 /**
     * Run an SQL statement and get the number of rows affected.
     *
     * @param  string  $query
     * @param  array  $bindings
     * @return int
     */
    public function affectingStatement($query, $bindings = [])
    {
        return $this->run($query, $bindings, function ($query, $bindings) {
            if ($this->pretending()) {
                return 0;
            }

            // For update or delete statements, we want to get the number of rows affected
            // by the statement and return that back to the developer. We'll first need
            // to execute the statement and then we'll use PDO to fetch the affected.
            $statement = $this->getPdo()->prepare($query);

            $this->bindValues($statement, $this->prepareBindings($bindings));

            $statement->execute();

            $this->recordsHaveBeenModified(
                ($count = $statement->rowCount()) > 0
            );

            return $count;
        });
    }

I've found that $this->getPdo() returns null because in the packages SwitchTenantDatabaseTast.php file under setTenantConnectionDatabaseName the line DB::purge($tenantConnectionName); ends up calling Laravel's disconnect function.

    /**
     * Disconnect from the underlying PDO connection.
     *
     * @return void
     */
    public function disconnect()
    {
        $this->setPdo(null)->setReadPdo(null);
    }

If I add the line App::forgetInstance('db'); afterh the DB::purge($tenantConnectionName); the issue goes away.

masterix21 commented 3 years ago

Please post your config/auth.php

Thanks

gofish543 commented 3 years ago
<?php

return [
    /*
    |--------------------------------------------------------------------------
    | Authentication Defaults
    |--------------------------------------------------------------------------
    |
    | This option controls the default authentication "guard" and password
    | reset options for your application. You may change these defaults
    | as required, but they're a perfect start for most applications.
    |
    */
    'defaults' => [
        'guard' => 'landlords',
        'passwords' => 'users',
    ],

    /*
    |--------------------------------------------------------------------------
    | Authentication Guards
    |--------------------------------------------------------------------------
    |
    | Next, you may define every authentication guard for your application.
    | Of course, a great default configuration has been defined for you
    | here which uses session storage and the Eloquent user provider.
    |
    | All authentication drivers have a user provider. This defines how the
    | users are actually retrieved out of your database or other storage
    | mechanisms used by this application to persist your user's data.
    |
    | Supported: "session", "token"
    |
    */
    'guards' => [
        'landlords' => [
            'driver' => 'session',
            'provider' => 'landlords',
        ],

        'tenants' => [
            'driver' => 'session',
            'provider' => 'tenants',
        ],

        'api' => [
            'driver' => 'token',
            'provider' => 'users',
            'hash' => false,
        ],
    ],

    /*
    |--------------------------------------------------------------------------
    | User Providers
    |--------------------------------------------------------------------------
    |
    | All authentication drivers have a user provider. This defines how the
    | users are actually retrieved out of your database or other storage
    | mechanisms used by this application to persist your user's data.
    |
    | If you have multiple user tables or models you may configure multiple
    | sources which represent each model / table. These sources may then
    | be assigned to any extra authentication guards you have defined.
    |
    | Supported: "database", "eloquent"
    |
    */
    'providers' => [
        'landlords' => [
            'driver' => 'eloquent',
            'model' => App\Models\LandlordUser::class,
        ],
        'tenants' => [
            'driver' => 'eloquent',
            'model' => App\Models\TenantUser::class,
        ],
    ],

    /*
    |--------------------------------------------------------------------------
    | Resetting Passwords
    |--------------------------------------------------------------------------
    |
    | You may specify multiple password reset configurations if you have more
    | than one user table or model in the application, and you want to have
    | separate password reset settings based on the specific user types.
    |
    | The expiry time is the number of minutes that the reset token should be
    | considered valid. This security feature keeps tokens short-lived so
    | they have less time to be guessed. You may change this as needed.
    |
    */
    'passwords' => [
        'landlords' => [
            'provider' => 'landlords',
            'table' => 'password_resets',
            'connection' => 'landlord',
            'expire' => 120,
            'throttle' => 60,
        ],
        'tenants' => [
            'provider' => 'tenants',
            'table' => 'password_resets',
            'connection' => 'tenant',
            'expire' => 120,
            'throttle' => 60,
        ],
    ],

    /*
    |--------------------------------------------------------------------------
    | Password Confirmation Timeout
    |--------------------------------------------------------------------------
    |
    | Here you may define the amount of seconds before a password confirmation
    | times out and the user is prompted to re-enter their password via the
    | confirmation screen. By default, the timeout lasts for three hours.
    |
    */
    'password_timeout' => 10800,
];
masterix21 commented 3 years ago

Your auth.php file seems ok. Watching your screenshot appears that it works for the first tenant but fails for the second. Are you sure that your second tenant configuration is ok?

masterix21 commented 3 years ago

I'm closing here because I think isn't related to a package issue, but feel free to continue the discussion here.

Thanks

gofish543 commented 3 years ago

Understandable for the closure, I've fixed it in the app by overriding the SwitchDatabaseTenantTask, but want to let you know that this is not the only project we've this issue on and fresh installs also have this problem.

Thanks for your time!

masterix21 commented 3 years ago

Could you create a GitHub repository with a project that has the bug?

finchy70 commented 3 years ago

When attempting to execute the command

$FORGE_PHP artisan tenants:artisan "auth:clear-resets tenants"

I get a failed to call prepare() on null. This refers to the PDO object within Laravel's DB Connection class. PDO is set to null when the switch db task is executed and causes future pdos to return null.

I propose adding App::forgetInstance('db'); after the purge to force the singleton to be refreshed by laravel.

Thoughts?

When I call Artisan::call("tenants:artisan 'migrate --seed' --tenant=$id"); from inside my app I get the same error. Adding App::forgetInstance('db'); removes the error but the first tenant db is migrated and seeded and not the one indicated by the $id.

masterix21 commented 3 years ago

@finchy70 try with:

Artisan::call('tenants:artisan', [
    'artisanCommand' => 'migrate --seed'
    '--tenant' => $id
])
Ghostscypher commented 11 months ago

Just for anyone who might be facing this issue in your multitenancy config file make sure that the tenant_database_connection_name and landlord_database_connection_name is not null.