diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php index e4da80b94d94d..d9d761835ad6b 100644 --- a/lib/private/AppConfig.php +++ b/lib/private/AppConfig.php @@ -67,6 +67,8 @@ class AppConfig implements IAppConfig { private array $valueTypes = []; // type for all config values private bool $fastLoaded = false; private bool $lazyLoaded = false; + // only needed to manage the old database structure during ownCloud-to-Nextcloud migration + private bool $migrationCompleted = true; /** @var array, aliases: array, strictness: Strictness}> ['app_id' => ['strictness' => ConfigLexiconStrictness, 'entries' => ['config_key' => ConfigLexiconEntry[]]] */ private array $configLexiconDetails = []; private bool $ignoreLexiconAliases = false; @@ -827,10 +829,12 @@ private function setTypedValue( $insert = $this->connection->getQueryBuilder(); $insert->insert('appconfig') ->setValue('appid', $insert->createNamedParameter($app)) - ->setValue('lazy', $insert->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) - ->setValue('type', $insert->createNamedParameter($type, IQueryBuilder::PARAM_INT)) ->setValue('configkey', $insert->createNamedParameter($key)) ->setValue('configvalue', $insert->createNamedParameter($value)); + if ($this->migrationCompleted) { + $insert->setValue('lazy', $insert->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) + ->setValue('type', $insert->createNamedParameter($type, IQueryBuilder::PARAM_INT)); + } $insert->executeStatement(); $inserted = true; } catch (DBException $e) { @@ -890,10 +894,12 @@ private function setTypedValue( $update = $this->connection->getQueryBuilder(); $update->update('appconfig') ->set('configvalue', $update->createNamedParameter($value)) - ->set('lazy', $update->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) - ->set('type', $update->createNamedParameter($type, IQueryBuilder::PARAM_INT)) ->where($update->expr()->eq('appid', $update->createNamedParameter($app))) ->andWhere($update->expr()->eq('configkey', $update->createNamedParameter($key))); + if ($this->migrationCompleted) { + $update->set('lazy', $update->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) + ->set('type', $update->createNamedParameter($type, IQueryBuilder::PARAM_INT)); + } $update->executeStatement(); } @@ -1347,23 +1353,39 @@ private function loadConfig(?string $app = null, bool $lazy = false): void { // Otherwise no cache available and we need to fetch from database $qb = $this->connection->getQueryBuilder(); - $qb->from('appconfig') - ->select('appid', 'configkey', 'configvalue', 'type'); + $qb->from('appconfig'); - if ($lazy === false) { - $qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + if (!$this->migrationCompleted) { + $qb->select('appid', 'configkey', 'configvalue'); } else { - if ($loadLazyOnly) { - $qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))); + $qb->select('appid', 'configkey', 'configvalue', 'type'); + + if ($lazy === false) { + $qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + } else { + if ($loadLazyOnly) { + $qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))); + } + $qb->addSelect('lazy'); } - $qb->addSelect('lazy'); } - $result = $qb->executeQuery(); + try { + $result = $qb->executeQuery(); + } catch (DBException $e) { + if ($e->getReason() !== DBException::REASON_INVALID_FIELD_NAME) { + throw $e; + } + // columns 'type' and 'lazy' don't exist yet (ownCloud migration) + $this->migrationCompleted = false; + $this->loadConfig($app, $lazy); + return; + } + $rows = $result->fetchAll(); foreach ($rows as $row) { // most of the time, 'lazy' is not in the select because its value is already known - if ($lazy && ((int)$row['lazy']) === 1) { + if ($this->migrationCompleted && $lazy && ((int)$row['lazy']) === 1) { $this->lazyCache[$row['appid']][$row['configkey']] = $row['configvalue'] ?? ''; } else { $this->fastCache[$row['appid']][$row['configkey']] = $row['configvalue'] ?? ''; diff --git a/lib/private/Config/UserConfig.php b/lib/private/Config/UserConfig.php index 1c6b819571e92..f7c157686d469 100644 --- a/lib/private/Config/UserConfig.php +++ b/lib/private/Config/UserConfig.php @@ -70,6 +70,8 @@ class UserConfig implements IUserConfig { private array $configLexiconDetails = []; private bool $ignoreLexiconAliases = false; private array $strictnessApplied = []; + // only needed to manage the old database structure during ownCloud-to-Nextcloud migration + private bool $migrationCompleted = true; public function __construct( protected IDBConnection $connection, @@ -1148,12 +1150,14 @@ private function setTypedValue( $insert->insert('preferences') ->setValue('userid', $insert->createNamedParameter($userId)) ->setValue('appid', $insert->createNamedParameter($app)) - ->setValue('lazy', $insert->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) - ->setValue('type', $insert->createNamedParameter($type->value, IQueryBuilder::PARAM_INT)) - ->setValue('flags', $insert->createNamedParameter($flags, IQueryBuilder::PARAM_INT)) - ->setValue('indexed', $insert->createNamedParameter($indexed)) ->setValue('configkey', $insert->createNamedParameter($key)) ->setValue('configvalue', $insert->createNamedParameter($value)); + if ($this->migrationCompleted) { + $insert->setValue('lazy', $insert->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) + ->setValue('type', $insert->createNamedParameter($type->value, IQueryBuilder::PARAM_INT)) + ->setValue('flags', $insert->createNamedParameter($flags, IQueryBuilder::PARAM_INT)) + ->setValue('indexed', $insert->createNamedParameter($indexed)); + } $insert->executeStatement(); $inserted = true; } catch (DBException $e) { @@ -1205,13 +1209,15 @@ private function setTypedValue( $update = $this->connection->getQueryBuilder(); $update->update('preferences') ->set('configvalue', $update->createNamedParameter($value)) - ->set('lazy', $update->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) - ->set('type', $update->createNamedParameter($type->value, IQueryBuilder::PARAM_INT)) - ->set('flags', $update->createNamedParameter($flags, IQueryBuilder::PARAM_INT)) - ->set('indexed', $update->createNamedParameter($indexed)) ->where($update->expr()->eq('userid', $update->createNamedParameter($userId))) ->andWhere($update->expr()->eq('appid', $update->createNamedParameter($app))) ->andWhere($update->expr()->eq('configkey', $update->createNamedParameter($key))); + if ($this->migrationCompleted) { + $update->set('lazy', $update->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) + ->set('type', $update->createNamedParameter($type->value, IQueryBuilder::PARAM_INT)) + ->set('flags', $update->createNamedParameter($flags, IQueryBuilder::PARAM_INT)) + ->set('indexed', $update->createNamedParameter($indexed)); + } $update->executeStatement(); } @@ -1762,25 +1768,41 @@ private function loadConfig(string $userId, ?bool $lazy = false): void { $qb = $this->connection->getQueryBuilder(); $qb->from('preferences'); - $qb->select('appid', 'configkey', 'configvalue', 'type', 'flags'); $qb->where($qb->expr()->eq('userid', $qb->createNamedParameter($userId))); - // we only need value from lazy when loadConfig does not specify it - if ($lazy !== null) { - $qb->andWhere($qb->expr()->eq('lazy', $qb->createNamedParameter($lazy ? 1 : 0, IQueryBuilder::PARAM_INT))); + if (!$this->migrationCompleted) { + $qb->select('appid', 'configkey', 'configvalue'); } else { - $qb->addSelect('lazy'); + $qb->select('appid', 'configkey', 'configvalue', 'type', 'flags'); + + // we only need value from lazy when loadConfig does not specify it + if ($lazy !== null) { + $qb->andWhere($qb->expr()->eq('lazy', $qb->createNamedParameter($lazy ? 1 : 0, IQueryBuilder::PARAM_INT))); + } else { + $qb->addSelect('lazy'); + } + } + + try { + $result = $qb->executeQuery(); + } catch (DBException $e) { + if ($e->getReason() !== DBException::REASON_INVALID_FIELD_NAME) { + throw $e; + } + // columns 'type', 'lazy', 'flags', 'indexed' don't exist yet (ownCloud migration) + $this->migrationCompleted = false; + $this->loadConfig($userId, $lazy); + return; } - $result = $qb->executeQuery(); $rows = $result->fetchAll(); foreach ($rows as $row) { - if (($row['lazy'] ?? ($lazy ?? 0) ? 1 : 0) === 1) { + if ($this->migrationCompleted && (($row['lazy'] ?? ($lazy ?? 0) ? 1 : 0) === 1)) { $this->lazyCache[$userId][$row['appid']][$row['configkey']] = $row['configvalue'] ?? ''; } else { $this->fastCache[$userId][$row['appid']][$row['configkey']] = $row['configvalue'] ?? ''; } - $this->valueDetails[$userId][$row['appid']][$row['configkey']] = ['type' => ValueType::from((int)($row['type'] ?? 0)), 'flags' => (int)$row['flags']]; + $this->valueDetails[$userId][$row['appid']][$row['configkey']] = ['type' => ValueType::from((int)($row['type'] ?? 0)), 'flags' => (int)($row['flags'] ?? 0)]; } $result->closeCursor(); $this->setAsLoaded($userId, $lazy); diff --git a/tests/lib/AppConfigMigrationFallbackTest.php b/tests/lib/AppConfigMigrationFallbackTest.php new file mode 100644 index 0000000000000..63e7a96bca27c --- /dev/null +++ b/tests/lib/AppConfigMigrationFallbackTest.php @@ -0,0 +1,185 @@ +connection = $this->createMock(IDBConnection::class); + $this->config = $this->createMock(IConfig::class); + $this->configManager = $this->createMock(ConfigManager::class); + $this->presetManager = $this->createMock(PresetManager::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->crypto = $this->createMock(ICrypto::class); + $this->cacheFactory = $this->createMock(CacheFactory::class); + } + + private function getAppConfig(): AppConfig { + $this->config->method('getSystemValueBool') + ->with('cache_app_config', true) + ->willReturn(true); + $this->cacheFactory->method('isLocalCacheAvailable')->willReturn(false); + + return new AppConfig( + $this->connection, + $this->config, + $this->configManager, + $this->presetManager, + $this->logger, + $this->crypto, + $this->cacheFactory, + ); + } + + private function createInvalidFieldNameException(): DBException { + $driverException = $this->createMock(\Doctrine\DBAL\Driver\Exception::class); + $dbalException = new InvalidFieldNameException($driverException, null); + return DbalException::wrap($dbalException); + } + + private function createMockQueryBuilder(): IQueryBuilder&MockObject { + $expression = $this->createMock(IExpressionBuilder::class); + $qb = $this->createMock(IQueryBuilder::class); + $qb->method('from')->willReturn($qb); + $qb->method('select')->willReturn($qb); + $qb->method('addSelect')->willReturn($qb); + $qb->method('where')->willReturn($qb); + $qb->method('andWhere')->willReturn($qb); + $qb->method('set')->willReturn($qb); + $qb->method('expr')->willReturn($expression); + $qb->method('insert')->willReturn($qb); + $qb->method('setValue')->willReturn($qb); + $qb->method('createNamedParameter')->willReturn('?'); + return $qb; + } + + /** + * Test that loadConfig retries without type/lazy columns on InvalidFieldNameException. + */ + public function testLoadConfigFallsBackOnMissingColumns(): void { + $exception = $this->createInvalidFieldNameException(); + + $successResult = $this->createMock(IResult::class); + $successResult->method('fetchAll')->willReturn([ + ['appid' => 'core', 'configkey' => 'vendor', 'configvalue' => 'owncloud'], + ['appid' => 'core', 'configkey' => 'installedat', 'configvalue' => '1234567890'], + ]); + + $qb = $this->createMockQueryBuilder(); + // First call throws (columns missing), second call succeeds (fallback query) + $qb->method('executeQuery') + ->willReturnOnConsecutiveCalls( + $this->throwException($exception), + $successResult, + ); + + $this->connection->method('getQueryBuilder')->willReturn($qb); + + $appConfig = $this->getAppConfig(); + + // getValueString triggers loadConfig internally + $value = $appConfig->getValueString('core', 'vendor'); + $this->assertSame('owncloud', $value); + } + + /** + * Test that non-INVALID_FIELD_NAME exceptions are re-thrown, not swallowed. + */ + public function testLoadConfigRethrowsOtherExceptions(): void { + $driverException = $this->createMock(\Doctrine\DBAL\Driver\Exception::class); + $dbalException = new \Doctrine\DBAL\Exception\SyntaxErrorException($driverException, null); + $exception = DbalException::wrap($dbalException); + + $qb = $this->createMockQueryBuilder(); + $qb->method('executeQuery')->willThrowException($exception); + + $this->connection->method('getQueryBuilder')->willReturn($qb); + + $appConfig = $this->getAppConfig(); + + $this->expectException(DBException::class); + $appConfig->getValueString('core', 'vendor'); + } + + /** + * Test that insert omits lazy/type columns when migration is not completed. + */ + public function testInsertOmitsNewColumnsInFallbackMode(): void { + $exception = $this->createInvalidFieldNameException(); + + $loadResult = $this->createMock(IResult::class); + $loadResult->method('fetchAll')->willReturn([]); + + $qb = $this->createMockQueryBuilder(); + + $callCount = 0; + $qb->method('executeQuery') + ->willReturnCallback(function () use ($exception, $loadResult, &$callCount) { + $callCount++; + if ($callCount === 1) { + throw $exception; + } + return $loadResult; + }); + + // Verify insert() is called (meaning we reached the insert path) + $qb->expects(self::once())->method('insert')->with('appconfig')->willReturn($qb); + $qb->method('executeStatement')->willReturn(1); + + // Track which columns are set via setValue + $setColumns = []; + $qb->method('setValue') + ->willReturnCallback(function (string $column) use ($qb, &$setColumns) { + $setColumns[] = $column; + return $qb; + }); + + $this->connection->method('getQueryBuilder')->willReturn($qb); + + $appConfig = $this->getAppConfig(); + $appConfig->setValueString('core', 'vendor', 'owncloud'); + + $this->assertContains('appid', $setColumns); + $this->assertContains('configkey', $setColumns); + $this->assertContains('configvalue', $setColumns); + $this->assertNotContains('lazy', $setColumns, 'lazy column should be omitted in fallback mode'); + $this->assertNotContains('type', $setColumns, 'type column should be omitted in fallback mode'); + } +} diff --git a/tests/lib/Config/UserConfigMigrationFallbackTest.php b/tests/lib/Config/UserConfigMigrationFallbackTest.php new file mode 100644 index 0000000000000..4d1ec8850cf8d --- /dev/null +++ b/tests/lib/Config/UserConfigMigrationFallbackTest.php @@ -0,0 +1,180 @@ +connection = $this->createMock(IDBConnection::class); + $this->config = $this->createMock(IConfig::class); + $this->configManager = $this->createMock(ConfigManager::class); + $this->presetManager = $this->createMock(PresetManager::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->crypto = $this->createMock(ICrypto::class); + $this->dispatcher = $this->createMock(IEventDispatcher::class); + } + + private function getUserConfig(): UserConfig { + return new UserConfig( + $this->connection, + $this->config, + $this->configManager, + $this->presetManager, + $this->logger, + $this->crypto, + $this->dispatcher, + ); + } + + private function createInvalidFieldNameException(): DBException { + $driverException = $this->createMock(\Doctrine\DBAL\Driver\Exception::class); + $dbalException = new InvalidFieldNameException($driverException, null); + return DbalException::wrap($dbalException); + } + + private function createMockQueryBuilder(): IQueryBuilder&MockObject { + $expression = $this->createMock(IExpressionBuilder::class); + $qb = $this->createMock(IQueryBuilder::class); + $qb->method('from')->willReturn($qb); + $qb->method('select')->willReturn($qb); + $qb->method('addSelect')->willReturn($qb); + $qb->method('where')->willReturn($qb); + $qb->method('andWhere')->willReturn($qb); + $qb->method('set')->willReturn($qb); + $qb->method('expr')->willReturn($expression); + $qb->method('insert')->willReturn($qb); + $qb->method('setValue')->willReturn($qb); + $qb->method('createNamedParameter')->willReturn('?'); + return $qb; + } + + /** + * Test that loadConfig retries without new columns on InvalidFieldNameException. + */ + public function testLoadConfigFallsBackOnMissingColumns(): void { + $exception = $this->createInvalidFieldNameException(); + + $successResult = $this->createMock(IResult::class); + $successResult->method('fetchAll')->willReturn([ + ['appid' => 'settings', 'configkey' => 'email', 'configvalue' => 'user@example.com'], + ]); + + $qb = $this->createMockQueryBuilder(); + $qb->method('executeQuery') + ->willReturnOnConsecutiveCalls( + $this->throwException($exception), + $successResult, + ); + + $this->connection->method('getQueryBuilder')->willReturn($qb); + + $userConfig = $this->getUserConfig(); + + $value = $userConfig->getValueString('user1', 'settings', 'email'); + $this->assertSame('user@example.com', $value); + } + + /** + * Test that non-INVALID_FIELD_NAME exceptions are re-thrown. + */ + public function testLoadConfigRethrowsOtherExceptions(): void { + $driverException = $this->createMock(\Doctrine\DBAL\Driver\Exception::class); + $dbalException = new \Doctrine\DBAL\Exception\SyntaxErrorException($driverException, null); + $exception = DbalException::wrap($dbalException); + + $qb = $this->createMockQueryBuilder(); + $qb->method('executeQuery')->willThrowException($exception); + + $this->connection->method('getQueryBuilder')->willReturn($qb); + + $userConfig = $this->getUserConfig(); + + $this->expectException(DBException::class); + $userConfig->getValueString('user1', 'settings', 'email'); + } + + /** + * Test that insert omits new columns when migration is not completed. + */ + public function testInsertOmitsNewColumnsInFallbackMode(): void { + $exception = $this->createInvalidFieldNameException(); + + $loadResult = $this->createMock(IResult::class); + $loadResult->method('fetchAll')->willReturn([]); + + $qb = $this->createMockQueryBuilder(); + + $callCount = 0; + $qb->method('executeQuery') + ->willReturnCallback(function () use ($exception, $loadResult, &$callCount) { + $callCount++; + if ($callCount === 1) { + throw $exception; + } + return $loadResult; + }); + + $qb->expects(self::once())->method('insert')->with('preferences')->willReturn($qb); + $qb->method('executeStatement')->willReturn(1); + + $setColumns = []; + $qb->method('setValue') + ->willReturnCallback(function (string $column) use ($qb, &$setColumns) { + $setColumns[] = $column; + return $qb; + }); + + $this->connection->method('getQueryBuilder')->willReturn($qb); + + $userConfig = $this->getUserConfig(); + $userConfig->setValueString('user1', 'settings', 'email', 'user@example.com'); + + $this->assertContains('userid', $setColumns); + $this->assertContains('appid', $setColumns); + $this->assertContains('configkey', $setColumns); + $this->assertContains('configvalue', $setColumns); + $this->assertNotContains('lazy', $setColumns, 'lazy column should be omitted in fallback mode'); + $this->assertNotContains('type', $setColumns, 'type column should be omitted in fallback mode'); + $this->assertNotContains('flags', $setColumns, 'flags column should be omitted in fallback mode'); + $this->assertNotContains('indexed', $setColumns, 'indexed column should be omitted in fallback mode'); + } +}