[Bug]: Incorrect connection creation

[ineersa]

What happened?

I've tried to use this driver in my application, basically I have MySQL database as main database and turso as per user local file database.

I've found several issues while trying to make this driver working:

1. https://github.com/tursodatabase/turso-driver-laravel/blob/main/src/Database/LibSQLConnectionFactory.php#L18

To implement new connector you should do something like this in ServiceProvider:

$this->app->singleton(LibSQLConnector::class, function ($app) {
    $app['db.connector.libsql'] = (new LibSQLConnector())->connect(config('database.connections.libsql'));
});

This would be the correct way to add new connector, under the hood Laravel checks if connector defined in container and if it is, it will use it.

2. https://github.com/tursodatabase/turso-driver-laravel/blob/main/src/Database/LibSQLConnectionFactory.php#L9

When you rewriting connection config, you are changing it for all connections, and in my case application was trying to create libsql connection with mysql config, which was failing.

Even if I use it with correct settings it will fail on parent call

        return parent::createConnection($config['driver'], $connection, $config['url'], $prefix, $config);

Since there is no libsql in default factory:

protected function createConnection($driver, $connection, $database, $prefix = '', array $config = [])
    {
        if ($resolver = Connection::getResolver($driver)) {
            return $resolver($connection, $database, $prefix, $config);
        }

        return match ($driver) {
            'mysql' => new MySqlConnection($connection, $database, $prefix, $config),
            'mariadb' => new MariaDbConnection($connection, $database, $prefix, $config),
            'pgsql' => new PostgresConnection($connection, $database, $prefix, $config),
            'sqlite' => new SQLiteConnection($connection, $database, $prefix, $config),
            'sqlsrv' => new SqlServerConnection($connection, $database, $prefix, $config),
            default => throw new InvalidArgumentException("Unsupported driver [{$driver}]."),
        };
    }

You shouldn't touch ConnectionFactory and correct way would be to extend DBManager with new driver (which is already implemented in https://github.com/tursodatabase/turso-driver-laravel/blob/main/src/LibSQLDriverServiceProvider.php#L45 and working)

To resolve this I've registered default connection factory back:

$this->app->singleton('db.factory', function ($app) {
    return new \Illuminate\Database\Connectors\ConnectionFactory($app);
});

And with correct connector creation this resolved issue.

3. Incorrect database url in examples:
   So in examples you provide this:

    'url' => 'file:' . env('DB_DATABASE', database_path('database.sqlite')),

which looks good, but idk why when it's parsed by DBManager driver is set to file and again it goes to ConnectionFactory which doesn't know anything about file driver.

This is my config which worked for local file:

'url' => 'libsql:dbname:'.$this->getFilename($db->db_name),

In this case driver correctly parsed to libsql.

I can create PR to change this behaviors, but I don't know why was decided to implement own ConnectionFactory.

How to reproduce the bug

1. Create default application.
2. Add turso driver
3. Set up default database to MySQL (or postgres)
4. Try to create db connection with libsql

\Config::set('database.connections.test', [
          'driver' => 'libsql',
          'url' => 'libsql:dbname:{{db file}}',
          'encryptionKey' => '',
          'authToken' => '',
          'syncUrl' => '',
          'syncInterval' => 5,
          'read_your_writes' => true,
          'remoteOnly' => false,
          'database' => null,
          'prefix' => '',
]);

you might need to create database before that:

f (! \File::exists($this->getFilename($db->db_name))) {
    $libsql = new \LibSQL(
        config: 'file:{{db file}}',
        flags: \LibSQL::OPEN_READWRITE | \LibSQL::OPEN_CREATE,
        encryption_key: ''
    );
    $libsql->close();
}

Package Version

latest

PHP Version

8.3.8

Laravel Version

11

Which operating systems does with happen with?

Linux

Notes

No response

[ineersa]

Figured out with file:, now it's working with default ConnectionFactory and 'libsql:dbname:{{db file}}' will not work, actually it will even trigger an exception - https://github.com/tursodatabase/turso-driver-laravel/blob/main/src/Database/LibSQLDatabase.php#L25 here we access uninitialized property connection mode, because it won't be resolved to local.

Also in documentation link to database file is provided like this:

'url' => 'file:'.env('DB_DATABASE', database_path('database.sqlite')),

which is confusing, because inside https://github.com/tursodatabase/turso-driver-laravel/blob/main/src/Database/LibSQLDatabase.php#L27C1-L27C77 database file is created on path inside database_path, and this make configuring some custom path (like maybe i want to put it in /var/storage) quite challenging.

[darkterminal]

Thank you for this wonderful feedback! This is 🔥 me up... I will work for that, this is really helpful to me.

[ineersa]

No problem, I'm using it for my pet project as a tenant database, and it's working for now.
Great job building this and extension.

I'll share some more observations or issues that I'll find, i can try to fix some of them, but I'm not Laravel expert, especailly in packages and it's container.

BTW with dynamic connection setting and further unwinding Laravel magic https://github.com/tursodatabase/turso-driver-laravel/blob/main/src/LibSQLDriverServiceProvider.php#L46 you are rewriting using hardcoded config string, which will not work with any connection names except libsql.

If you would change this behavior to use actual incoming config, then problem with driver file comes into play...
\Illuminate\Support\ConfigurationUrlParser::parseConfiguration will parse file:users_databases/id_166821b38418e3.sqlite into

 "scheme" => "file"
  "path" => "users_databases/id_166821b38418e3.sqlite"

Which will further translate into driver file error in Connection factory.

Then logical decision would be to use:

'url' => 'libsql:file:'.$this->getFilename($db->db_name),

Then it's parsed into:

array:2 [
  "scheme" => "libsql"
  "path" => "file:users_databases/id_166821b38418e3.sqlite"
]

Which is awesome, but it gets corruppted here \Illuminate\Support\ConfigurationUrlParser::getDatabase

return $path && $path !== '/' ? substr($path, 1) : null;

Which will remove "f" from "file".
Okay, let's try to use this:

'url' => 'libsql::file:'.$this->getFilename($db->db_name),

Awesome now we get in parsed config:

array:9 [
  "driver" => "libsql"
  "encryptionKey" => "KEY"
  "authToken" => ""
  "syncUrl" => ""
  "syncInterval" => 5
  "read_your_writes" => true
  "remoteOnly" => false
  "database" => "file:users_databases/id_166821b38418e3.sqlite"
  "prefix" => ""
]

Which will go into our extend resolving where we can safely remove hardcoded config.
Next we will go in to connector and everything should work, we have file: and we now should be able to understand that connection mode is local, but https://github.com/tursodatabase/turso-driver-laravel/blob/main/src/Database/LibSQLDatabase.php#L22 we are getting hardcoded configs also.

I will try to put this all together into PR, but again I'm not an expert in Laravel so can get some mistakes.

[darkterminal]

If you're using tenant database with tenancy I am also have repository that implement tenancy for laravel called turso-laravel-tenancy. For now, it's only support local file connection as far I tested.

If you use this driver with Tenancy for Laravel then you need to create custom DatabaseManager and Bootstrapper in Tenancy for Laravel, cz Tenancy for Laravel didn't recognized Turso/libSQL Driver.

[ineersa]

Great I'll look into, but my use case is very simple and i don't need a lot of functionality, just per user local DB and a way to migrate them and so i don't use Stancl\Tenancy inside app.
https://github.com/ineersa/notes here is my pet project, it's on starting stage, but I did migration commands for example and some helper command to query libsql (couldn't find good free GUI) ,maybe you will find something useful there =)

[darkterminal]

So, you plan to create a tenant db per user without using stancl\tenancy. That's interesting, yes, to make local and embedded replica connections you have to use the file: prefix and don't use the database value from the configuration but instead use the url value from the configuration.

I am really thank you about this section:

$this->app->singleton(LibSQLConnector::class, function ($app) {
 $app['db.connector.libsql'] = (new LibSQLConnector())->connect(config('database.connections.libsql'));
});

[darkterminal]

This logic can resolved this issue and allow developer to set custom database directory.

src/LibSQLDatabase.php

$config = config('database.connections.libsql');

$url = str_replace('file:', '', $config['url']);
if ($this->checkPathOrFilename($config['url']) === 'filename') {
    $config['url'] = 'file:' . database_path($url);
}

$libsql = $this->checkConnectionMode($config['url'], $config['syncUrl'], $config['authToken'], $config['remoteOnly']);

If DB_DATABASE=database.sqlite at environment variable have value like this. Then our driver will fallback and using default database_path. I will create patch update.

[ineersa]

https://github.com/tursodatabase/turso-driver-laravel/pull/5/files

Merged my thoughts into PR, added tests for all types of connections.
Some things are debatable (like init() method), and I think should be changed, but it would've taken few more days for actual structure refactoring, because of how it's structured now it's quite challenging to test.

There is some removals, like facades for example, since I don't think it's actually possible to use those here etc.

Also phpstan now works)

I've tested on my app:

...
\Turso\Driver\Laravel\LibSQLDriverServiceProvider::class,
...

\Config::set('database.connections.'.self::CONNECTION_NAME, [
    'driver' => 'libsql',
    'url' => 'libsql::file:'.database_path($this->getFilename($db->db_name)),
    'encryptionKey' => $this->encrypter->decryptString($db->db_password),
    'authToken' => '',
    'syncUrl' => '',
    'syncInterval' => 5,
    'read_your_writes' => true,
    'remoteOnly' => false,
    'database' => null,
    'prefix' => '',
]);

Works ok, but maybe need more intensive testing.

Feel free to jump into discussion inside PR, or maybe it will bring up some ideas to you.

[flexchar]

I believe I came across the same problem. As soon as I installed the package, things broke down. The following terminal output points out very well where it stems from.

ErrorException 

  Trying to access array offset on null

  at vendor/tursodatabase/turso-driver-laravel/src/Database/LibSQLDatabase.php:23
     19▕ 
     20▕     public function __construct(array $config = [])
     21▕     {
     22▕         $config = config('database.connections.libsql');
  ➜  23▕         $libsql = $this->checkConnectionMode($config['url'], $config['syncUrl'], $config['authToken'], $config['remoteOnly']);
     24▕ 
     25▕         if ($this->connection_mode === 'local') {
     26▕ 
     27▕             $url = \str_replace('file:', '', database_path($config['url']));

      +25 vendor frames 

My intention is to consume turso as an additional side database, with remote only read. So almost like the OP's use case.

[darkterminal]

@flexchar Thank you for the report! Work in Progress...

[darkterminal]

Reopen if need it. Thank you.