diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index c837e74952171..2ac39d475d4f3 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3874,12 +3874,6 @@ dbPort)]]> - - dbprettyname]]> - dbprettyname]]> - dbprettyname]]> - dbprettyname]]> - diff --git a/lib/private/Setup/AbstractDatabase.php b/lib/private/Setup/AbstractDatabase.php index d07d4af91a6c0..c5616d0a0cee3 100644 --- a/lib/private/Setup/AbstractDatabase.php +++ b/lib/private/Setup/AbstractDatabase.php @@ -34,52 +34,116 @@ public function __construct( ) { } + /** + * Returns the display name of the database system (e.g., "MySQL/MariaDB", "PostgreSQL") + */ + abstract protected function getDisplayName(): string; + + /** + * Validates the database configuration + * + * @param array $config Configuration array containing database credentials and settings + * @return array Array of validation error messages (empty if valid) + */ public function validate(array $config): array { $errors = []; - if (empty($config['dbuser']) && empty($config['dbname'])) { - $errors[] = $this->trans->t('Enter the database Login and name for %s', [$this->dbprettyname]); - } elseif (empty($config['dbuser'])) { - $errors[] = $this->trans->t('Enter the database Login for %s', [$this->dbprettyname]); - } elseif (empty($config['dbname'])) { - $errors[] = $this->trans->t('Enter the database name for %s', [$this->dbprettyname]); + + $errors = array_merge($errors, $this->validateRequiredFields($config)); + $errors = array_merge($errors, $this->validateDatabaseName($config)); + + return $errors; + } + + protected function validateRequiredFields(array $config): array { + $errors = []; + + $dbUser = $config['dbuser'] ?? ''; + $dbName = $config['dbname'] ?? ''; + $displayName = $this->getDisplayName(); + + // Check for missing required parameters + if (empty($dbUser) && empty($dbName)) { + $errors[] = $this->trans->t('Enter the database Login and name for %s', [$displayName]); + } elseif (empty($dbUser)) { + $errors[] = $this->trans->t('Enter the database Login for %s', [$displayName]); + } elseif (empty($dbName)) { + $errors[] = $this->trans->t('Enter the database name for %s', [$displayName]); } - if (substr_count($config['dbname'], '.') >= 1) { - $errors[] = $this->trans->t('You cannot use dots in the database name %s', [$this->dbprettyname]); + + return $errors; + } + + protected function validateDatabaseName(array $config): array { + $errors = []; + + $dbName = $config['dbname'] ?? ''; + + if (empty($dbName)) { + return $errors; } + + // Avoid downsides of supporting database names with dots (`.`) + if (str_contains($dbName, '.')) { + $errors[] = $this->trans->t('You cannot use dots in the database name %s', [$this->getDisplayName()]); + } + + // Note: Child classes should implement db specific name validations + // (optionally still calling this parent for default validations) + // (e.g. length, characters, casing, starting character, reserved words) + return $errors; } public function initialize(array $config): void { - $dbUser = $config['dbuser']; - $dbPass = $config['dbpass']; - $dbName = $config['dbname']; - $dbHost = !empty($config['dbhost']) ? $config['dbhost'] : 'localhost'; - $dbPort = !empty($config['dbport']) ? $config['dbport'] : ''; - $dbTablePrefix = $config['dbtableprefix'] ?? 'oc_'; + $dbParams = $this->extractDatabaseParameters($config); - $createUserConfig = $this->config->getValue('setup_create_db_user', true); - // accept `false` both as bool and string, since setting config values from env will result in a string - $this->tryCreateDbUser = $createUserConfig !== false && $createUserConfig !== 'false'; + // Should a database user/credential set be created automatically? + $this->tryCreateDbUser = $this->shouldCreateDbUser(); + // Persist database configuration to config.php $this->config->setValues([ - 'dbname' => $dbName, - 'dbhost' => $dbHost, - 'dbtableprefix' => $dbTablePrefix, + 'dbname' => $dbParams['name'], + 'dbhost' => $dbParams['host'], + 'dbtableprefix' => $dbParams['tablePrefix'], ]); - $this->dbUser = $dbUser; - $this->dbPassword = $dbPass; - $this->dbName = $dbName; - $this->dbHost = $dbHost; - $this->dbPort = $dbPort; - $this->tablePrefix = $dbTablePrefix; + // Set instance properties from database parameters for subsequent operations (e.g. connecting) + $this->setInstanceProperties($dbParams); + } + + protected function extractDatabaseParameters(array $config): array { + return [ + // Guaranteed to exist after validate() (exceptions override initialize() - i.e. SQLite) + 'user' => $config['dbuser'] ?? throw new \InvalidArgumentException('dbuser is required'), + 'name' => $config['dbname'] ?? throw new \InvalidArgumentException('dbname is required'), + // Password can be empty for some setups / code paths + 'pass' => $config['dbpass'] ?? '', + 'host' => !empty($config['dbhost']) ? $config['dbhost'] : 'localhost', + 'port' => !empty($config['dbport']) ? $config['dbport'] : '', + 'tablePrefix' => $config['dbtableprefix'] ?? 'oc_', + ]; + } + + protected function shouldCreateDbUser(): bool { + $createUserConfig = $this->config->getValue('setup_create_db_user', true); + // Accept `false` both as bool and string, since env vars result in strings + return $createUserConfig !== false && $createUserConfig !== 'false'; + } + + protected function setInstanceProperties(array $dbParams): void { + $this->dbUser = $dbParams['user']; + $this->dbPassword = $dbParams['pass']; + $this->dbName = $dbParams['name']; + $this->dbHost = $dbParams['host']; + $this->dbPort = $dbParams['port']; + $this->tablePrefix = $dbParams['tablePrefix']; } /** - * @param array $configOverwrite - * @return \OC\DB\Connection + * @param array $configOverwrite Optional parameters to override (e.g., ['dbname' => null]) */ protected function connect(array $configOverwrite = []): Connection { + // Build base connection parameters $connectionParams = [ 'host' => $this->dbHost, 'user' => $this->dbUser, @@ -88,38 +152,85 @@ protected function connect(array $configOverwrite = []): Connection { 'dbname' => $this->dbName ]; - // adding port support through installer + // Apply port and socket configuration + $connectionParams = $this->applyPortAndSocketConfig($connectionParams); + + // Apply any caller-provided overrides (e.g., dbname => null) + $connectionParams = array_merge($connectionParams, $configOverwrite); + + // Configure for primary/replica topology (both point to same server during install) + $connectionParams['primary'] = $connectionParams; + $connectionParams['replica'] = [$connectionParams]; + + $dbType = $this->config->getValue('dbtype', 'sqlite'); + + $this->logger->debug('Creating database connection', [ + 'dbtype' => $dbType, + 'host' => $connectionParams['host'] ?? 'unknown', + 'port' => $connectionParams['port'] ?? $connectionParams['unix_socket'] ?? 'default', + 'dbname' => $connectionParams['dbname'] ?? 'none', + ]); + + // Create and return the connection + $cf = new ConnectionFactory($this->config); + $connection = $cf->getConnection($dbType, $connectionParams); + $connection->ensureConnectedToPrimary(); + return $connection; + } + + protected function applyPortAndSocketConfig(array $params): array { + // Check if port/socket is specified in the dedicated port field (only used by installer) if (!empty($this->dbPort)) { if (ctype_digit($this->dbPort)) { - $connectionParams['port'] = $this->dbPort; + $params['port'] = $this->dbPort; } else { - $connectionParams['unix_socket'] = $this->dbPort; + $params['unix_socket'] = $this->dbPort; } - } elseif (strpos($this->dbHost, ':')) { - // Host variable may carry a port or socket. + return $params; + } + + // Check if port/socket is embedded in the hostname (e.g., "localhost:3306") + if (str_contains($this->dbHost, ':')) { [$host, $portOrSocket] = explode(':', $this->dbHost, 2); + $params['host'] = $host; + // Host variable may carry a port or socket. if (ctype_digit($portOrSocket)) { - $connectionParams['port'] = $portOrSocket; + $params['port'] = $portOrSocket; } else { - $connectionParams['unix_socket'] = $portOrSocket; + $params['unix_socket'] = $portOrSocket; } - $connectionParams['host'] = $host; } - $connectionParams = array_merge($connectionParams, $configOverwrite); - $connectionParams = array_merge($connectionParams, ['primary' => $connectionParams, 'replica' => [$connectionParams]]); - $cf = new ConnectionFactory($this->config); - $connection = $cf->getConnection($this->config->getValue('dbtype', 'sqlite'), $connectionParams); - $connection->ensureConnectedToPrimary(); - return $connection; + return $params; } - abstract public function setupDatabase(); + /** + * Sets up the database (creates database, users, etc.) + * + * Must be implemented by database-specific child classes + */ + abstract public function setupDatabase(): void; - public function runMigrations(?IOutput $output = null) { - if (!is_dir(\OC::$SERVERROOT . '/core/Migrations')) { - return; + /** + * @throws \Exception If migration fails (handled by caller) + */ + public function runMigrations(?IOutput $output = null): void { + $migrationsPath = \OC::$SERVERROOT . '/core/Migrations'; + + if (!is_dir($migrationsPath)) { + $this->logger->debug('Skipping migrations - directory not found: {path}', [ + 'path' => $migrationsPath, + ]); + return; // @todo: should we throw an Exception here instead to let caller decide? } - $ms = new MigrationService('core', Server::get(Connection::class), $output); - $ms->migrate('latest', true); + + $this->logger->info('Starting core database migrations'); + + $migrationService = new MigrationService('core', Server::get(Connection::class), $output); + + // Migrate to latest version, applying schema changes only + // (no data migrations needed for fresh install) + $migrationService->migrate('latest', true); + + $this->logger->info('Core database migrations completed successfully'); } } diff --git a/lib/private/Setup/MySQL.php b/lib/private/Setup/MySQL.php index 24093ae97a40c..af0f59780a3c8 100644 --- a/lib/private/Setup/MySQL.php +++ b/lib/private/Setup/MySQL.php @@ -16,7 +16,10 @@ use OCP\Security\ISecureRandom; class MySQL extends AbstractDatabase { - public string $dbprettyname = 'MySQL/MariaDB'; + + protected function getDisplayName(): string { + return 'MySQL/MariaDB'; + } public function setupDatabase(): void { //check if the database user has admin right diff --git a/lib/private/Setup/OCI.php b/lib/private/Setup/OCI.php index 7bde42c1ace75..73cba1f1c46de 100644 --- a/lib/private/Setup/OCI.php +++ b/lib/private/Setup/OCI.php @@ -10,10 +10,12 @@ use OC\DatabaseSetupException; class OCI extends AbstractDatabase { - public $dbprettyname = 'Oracle'; - protected $dbtablespace; + protected function getDisplayName(): string { + return 'Oracle'; + } + public function initialize(array $config): void { parent::initialize($config); if (array_key_exists('dbtablespace', $config)) { @@ -30,16 +32,9 @@ public function initialize(array $config): void { ]); } - public function validate(array $config): array { - $errors = []; - if (empty($config['dbuser']) && empty($config['dbname'])) { - $errors[] = $this->trans->t('Enter the database Login and name for %s', [$this->dbprettyname]); - } elseif (empty($config['dbuser'])) { - $errors[] = $this->trans->t('Enter the database Login for %s', [$this->dbprettyname]); - } elseif (empty($config['dbname'])) { - $errors[] = $this->trans->t('Enter the database name for %s', [$this->dbprettyname]); - } - return $errors; + protected function validateDatabaseName(array $config): array { + // Override default checks; allow dots in service name for oracle autosetup + return []; } public function setupDatabase(): void { diff --git a/lib/private/Setup/PostgreSQL.php b/lib/private/Setup/PostgreSQL.php index 5ace7f6bcbee3..570d1680396e2 100644 --- a/lib/private/Setup/PostgreSQL.php +++ b/lib/private/Setup/PostgreSQL.php @@ -15,7 +15,10 @@ use OCP\Server; class PostgreSQL extends AbstractDatabase { - public $dbprettyname = 'PostgreSQL'; + + protected function getDisplayName(): string { + return 'PostgreSQL'; + } /** * @throws DatabaseSetupException diff --git a/lib/private/Setup/Sqlite.php b/lib/private/Setup/Sqlite.php index 325a26ce89680..95050abe128e2 100644 --- a/lib/private/Setup/Sqlite.php +++ b/lib/private/Setup/Sqlite.php @@ -10,7 +10,10 @@ use OC\DB\ConnectionFactory; class Sqlite extends AbstractDatabase { - public string $dbprettyname = 'Sqlite'; + + protected function getDisplayName(): string { + return 'SQLite'; + } public function validate(array $config): array { return [];