Refactor DatabaseConfig, minor DB manager improvements, resolve todos

Notable changes:
- CreateUserWithRLSPolicies: Clarify why we're creating a custom
  DatabaseConfing instance
- HasDatabase: Clarify why we're ignoring tenancy_db_connection
- DatabaseConfig: General refactor, clarify the role of the host conn
- SQLiteDatabaseManager: Handle trailing DIRECTORY_SEPARATOR
  in static::$path
- DisallowSqliteAttach: Don't throw any exceptions, just silently fail
  since the class isn't 100% portable
- Clean up todos that are no longer relevant
- Clean up dead code or comments in some database managers

src/Bootstrappers/DatabaseTenancyBootstrapper.php

@@ -35,7 +35,7 @@ class DatabaseTenancyBootstrapper implements TenancyBootstrapper
         // Better debugging, but breaks cached lookup, so we disable this in prod
         if (app()->environment('local') || app()->environment('testing')) {
             $database = $tenant->database()->getName();
-            if (! $tenant->database()->manager()->databaseExists($database)) { // todo@dbRefactor does this call correctly use the host connection?
+            if (! $tenant->database()->manager()->databaseExists($database)) {
                 throw new TenantDatabaseDoesNotExistException($database);
             }
         }

src/Commands/CreateUserWithRLSPolicies.php

@@ -81,12 +81,19 @@ class CreateUserWithRLSPolicies extends Command
         #[\SensitiveParameter]
         string $password,
     ): DatabaseConfig {
+        // This is a bit of a hack. We want to use our existing createUser() logic.
+        // That logic needs a DatabaseConfig instance. However, we aren't really working
+        // with any specific tenant here. We also *don't* want to use anything tenant-specific
+        // here. We are creating the SHARED "RLS user". Therefore, we need a custom DatabaseConfig
+        // instance for this purpose. The easiest way to do that is to grab an empty Tenant model
+        // (we use TenantWithDatabase in RLS) and manually create the host connection, just like
+        // DatabaseConfig::manager() would. We don't call that method since we want to use our existing
+        // PermissionControlledPostgreSQLSchemaManager $manager instance, rather than the "tenant's manager".
+
         /** @var TenantWithDatabase $tenantModel */
         $tenantModel = tenancy()->model();
 
-        // Use a temporary DatabaseConfig instance to set the host connection
         $temporaryDbConfig = $tenantModel->database();
-
         $temporaryDbConfig->purgeHostConnection();
 
         $tenantHostConnectionName = $temporaryDbConfig->getTenantHostConnectionName();

src/Database/Concerns/HasDatabase.php

@@ -28,7 +28,8 @@ trait HasDatabase
                 }
 
                 if ($key === $this->internalPrefix() . 'db_connection') {
-                    // Remove DB connection because that's not used here
+                    // Remove DB connection because that's not used for the connection *contents*.
+                    // Instead the code uses getInternal('db_connection').
                     continue;
                 }
 

src/Database/DatabaseConfig.php

@@ -13,7 +13,6 @@ use Stancl\Tenancy\Database\Contracts\TenantWithDatabase as Tenant;
 use Stancl\Tenancy\Database\Exceptions\DatabaseManagerNotRegisteredException;
 use Stancl\Tenancy\Database\Exceptions\NoConnectionSetException;
 
-// todo@dbRefactor refactor host connection logic to make customizing the host connection easier
 class DatabaseConfig
 {
     /** The tenant whose database we're dealing with. */
@@ -115,7 +114,7 @@ class DatabaseConfig
     {
         $this->tenant->setInternal('db_name', $this->getName());
 
-        if ($this->connectionDriverManager($this->getTemplateConnectionDriver()) instanceof Contracts\ManagesDatabaseUsers) {
+        if ($this->managerForDriver($this->getTemplateConnectionDriver()) instanceof Contracts\ManagesDatabaseUsers) {
             $this->tenant->setInternal('db_username', $this->getUsername() ?? (static::$usernameGenerator)($this->tenant, $this));
             $this->tenant->setInternal('db_password', $this->getPassword() ?? (static::$passwordGenerator)($this->tenant, $this));
         }
@@ -137,7 +136,9 @@ class DatabaseConfig
         }
 
         if ($template = config('tenancy.database.template_tenant_connection')) {
-            return is_array($template) ? array_merge($this->getCentralConnection(), $template) : config("database.connections.{$template}");
+            return is_array($template)
+                ? array_merge($this->getCentralConnection(), $template)
+                : config("database.connections.{$template}");
         }
 
         return $this->getCentralConnection();
@@ -176,10 +177,10 @@ class DatabaseConfig
         $config = $this->tenantConfig;
         $templateConnection = $this->getTemplateConnection();
 
-        if ($this->connectionDriverManager($this->getTemplateConnectionDriver()) instanceof Contracts\ManagesDatabaseUsers) {
+        if ($this->managerForDriver($this->getTemplateConnectionDriver()) instanceof Contracts\ManagesDatabaseUsers) {
-            // We're removing the username and password because user with these credentials is not created yet
+            // We remove the username and password because the user with these credentials is not yet created.
-            // If you need to provide username and password when using PermissionControlledMySQLDatabaseManager,
+            // If you need to provide a username and a password when using a permission controlled database manager,
-            // consider creating a new connection and use it as `tenancy_db_connection` tenant config key
+            // consider creating a new connection and use it as `tenancy_db_connection`.
             unset($config['username'], $config['password']);
         }
 
@@ -191,7 +192,7 @@ class DatabaseConfig
     }
 
     /**
-     * Purge the previous tenant connection before opening it for another tenant.
+     * Purge the previous host connection before opening it for another tenant.
      */
     public function purgeHostConnection(): void
     {
@@ -199,20 +200,20 @@ class DatabaseConfig
     }
 
     /**
-     * Get the TenantDatabaseManager for this tenant's connection.
+     * Get the TenantDatabaseManager for this tenant's host connection.
      *
      * @throws NoConnectionSetException|DatabaseManagerNotRegisteredException
      */
     public function manager(): Contracts\TenantDatabaseManager
     {
-        // Laravel caches the previous PDO connection, so we purge it to be able to change the connection details
+        // Laravel persists the PDO connection, so we purge it to be able to change the connection details
         $this->purgeHostConnection();
 
         // Create the tenant host connection config
         $tenantHostConnectionName = $this->getTenantHostConnectionName();
         config(["database.connections.{$tenantHostConnectionName}" => $this->hostConnection()]);
 
-        $manager = $this->connectionDriverManager(config("database.connections.{$tenantHostConnectionName}.driver"));
+        $manager = $this->managerForDriver(config("database.connections.{$tenantHostConnectionName}.driver"));
 
         if ($manager instanceof Contracts\StatefulTenantDatabaseManager) {
             $manager->setConnection($tenantHostConnectionName);
@@ -222,12 +223,11 @@ class DatabaseConfig
     }
 
     /**
-     * todo@dbRefactor come up with a better name
+     * Get the TenantDatabaseManager for a given database driver.
-     * Get database manager class from the given connection config's driver.
      *
      * @throws DatabaseManagerNotRegisteredException
      */
-    protected function connectionDriverManager(string $driver): Contracts\TenantDatabaseManager
+    protected function managerForDriver(string $driver): Contracts\TenantDatabaseManager
     {
         $databaseManagers = config('tenancy.database.managers');
 

src/Database/TenantDatabaseManagers/PermissionControlledMySQLDatabaseManager.php

@@ -23,7 +23,6 @@ class PermissionControlledMySQLDatabaseManager extends MySQLDatabaseManager impl
     {
         $database = $databaseConfig->getName();
         $username = $databaseConfig->getUsername();
-        $hostname = $databaseConfig->connection()['host'];
         $password = $databaseConfig->getPassword();
 
         $this->connection()->statement("CREATE USER `{$username}`@`%` IDENTIFIED BY '{$password}'");

src/Database/TenantDatabaseManagers/PermissionControlledPostgreSQLSchemaManager.php

@@ -30,10 +30,6 @@ class PermissionControlledPostgreSQLSchemaManager extends PostgreSQLSchemaManage
         $tables = $this->connection()->select("SELECT table_name FROM information_schema.tables WHERE table_schema = '{$schema}' AND table_type = 'BASE TABLE'");
 
         // Grant permissions to any existing tables. This is used with RLS
-        // todo@dbRefactor refactor this along with the todo in TenantDatabaseManager
-        // and move the grantPermissions() call inside the condition in `ManagesPostgresUsers::createUser()`
-        // but maybe moving it inside $createUser is wrong because some central user may migrate new tables
-        // while the RLS user should STILL get access to those tables
         foreach ($tables as $table) {
             $tableName = $table->table_name;
 

src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php

@@ -14,7 +14,9 @@ use Throwable;
 class SQLiteDatabaseManager implements TenantDatabaseManager
 {
     /**
-     * SQLite Database path without ending slash.
+     * SQLite database directory path.
+     *
+     * Defaults to database_path().
      */
     public static string|null $path = null;
 
@@ -132,15 +134,10 @@ class SQLiteDatabaseManager implements TenantDatabaseManager
         return $baseConfig;
     }
 
-    public function setConnection(string $connection): void
-    {
-        //
-    }
-
     public function getPath(string $name): string
     {
         if (static::$path) {
-            return static::$path . DIRECTORY_SEPARATOR . $name;
+            return rtrim(static::$path, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . $name;
         }
 
         return database_path($name);

src/Features/DisallowSqliteAttach.php

@@ -4,7 +4,6 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Features;
 
-use Exception;
 use Illuminate\Database\Connectors\ConnectionFactory;
 use Illuminate\Database\SQLiteConnection;
 use Illuminate\Support\Facades\DB;
@@ -48,9 +47,11 @@ class DisallowSqliteAttach implements Feature
             'Linux' => 'so',
             'Windows' => 'dll',
             'Darwin' => 'dylib',
-            default => throw new Exception("The DisallowSqliteAttach feature doesn't support your operating system: " . PHP_OS_FAMILY),
+            default => 'error',
         };
 
+        if ($suffix === 'error') return false;
+
         $arch = php_uname('m');
         $arm = $arch === 'aarch64' || $arch === 'arm64';