stefanzweifel / laravel-backup-restore

A package to restore database backups made with spatie/laravel-backup.
https://stefanzweifel.dev/posts/2023/06/15/introducing-laravel-backup-restore
MIT License
165 stars 15 forks source link

DbImporterFactory uses connection name instead of driver to create DbImporter #22

Closed shez1983 closed 1 year ago

shez1983 commented 1 year ago

so i have the default mysql connection, but i created a new connection for external mysql db


'restore-db' => [
            'driver' => 'mysql',
            'url' => env('DATABASE_RESTORE_URL'),
            'host' => env('DB_RESTORE_HOST', '127.0.0.1'),
            'port' => env('DB_PORT', '3306'),
            'database' => env('DB_RESTORE_DATABASE', 'forge'),
            'username' => env('DB_RESTORE_USERNAME', 'forge'),
            'password' => env('DB_RESTORE_PASSWORD', ''),
            'unix_socket' => env('DB_SOCKET', ''),
            'charset' => 'utf8mb4',
            'collation' => 'utf8mb4_unicode_ci',
            'prefix' => '',
            'prefix_indexes' => true,
            'strict' => true,
            'engine' => null,
            'options' => extension_loaded('pdo_mysql') ? array_filter([
                PDO::MYSQL_ATTR_SSL_CA => env('MYSQL_ATTR_SSL_CA'),
            ]) : [],
        ],

the issue is this file: vendor/wnx/laravel-backup-restore/src/DbImporterFactory.php

the function createFromConnection, checks

if (config("database.connections.$dbConnectionName") === null) {
            throw CannotCreateDbImporter::unsupportedDriver($dbConnectionName);
        }

and then proceeds to do this:

             return static::forDriver($dbConnectionName);

which seems wrong, surely you should be checking for the driver inside that config?

So for now I have changed this to:

 public static function createFromConnection(string $dbConnectionName): DbImporter
    {
        $config = config("database.connections.$dbConnectionName");

        if ($config === null) {
            throw CannotCreateDbImporter::unsupportedDriver($dbConnectionName);
        }

        return static::forDriver($config['driver']);
    }

but obv this will break if you decide to update your repo..

stefanzweifel commented 1 year ago

Howdy.

Took me a while to parse your issue here to figure out, what you're trying to tell me, but I see the issue now. :)

Can confirm that we have a bug here, as we assume that the database connection name equals the database driver name.

That obviously doesn't work if someone adds a connection name like restore-db. Will fix this right away.