Merge pull request #59677 from nextcloud/fix/57340/owncloud-migration-appconfig-userconfig

fix(appconfig,userconfig): restore pre-migration fallback for ownCloud migration
pull/60324/merge
Anna 1 week ago committed by GitHub
commit e29038414d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 56
      lib/private/AppConfig.php
  2. 62
      lib/private/Config/UserConfig.php
  3. 185
      tests/lib/AppConfigMigrationFallbackTest.php
  4. 180
      tests/lib/Config/UserConfigMigrationFallbackTest.php

@ -67,6 +67,16 @@ class AppConfig implements IAppConfig {
private array $valueTypes = []; // type for all config values
private bool $fastLoaded = false;
private bool $lazyLoaded = false;
/**
* Tracks whether the NC-only columns (`type`, `lazy`) exist in the `appconfig` table.
* Set to false on first load when a DBException::REASON_INVALID_FIELD_NAME is caught,
* which happens during an ownCloud → Nextcloud migration before the schema steps have run.
*
* Every SELECT that reads those columns and every INSERT/UPDATE that writes them must
* guard with `if ($this->migrationCompleted)` so they degrade gracefully.
* If you add a new query that touches NC-only columns, add the same guard.
*/
private bool $migrationCompleted = true;
/** @var array<string, array{entries: array<string, Entry>, aliases: array<string, string>, strictness: Strictness}> ['app_id' => ['strictness' => ConfigLexiconStrictness, 'entries' => ['config_key' => ConfigLexiconEntry[]]] */
private array $configLexiconDetails = [];
private bool $ignoreLexiconAliases = false;
@ -854,10 +864,12 @@ class AppConfig implements IAppConfig {
$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) {
@ -917,10 +929,12 @@ class AppConfig implements IAppConfig {
$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();
}
@ -1383,23 +1397,39 @@ class AppConfig implements IAppConfig {
// 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');
}
}
try {
$result = $qb->executeQuery();
} catch (DBException $e) {
if ($e->getReason() !== DBException::REASON_INVALID_FIELD_NAME || !$this->migrationCompleted) {
throw $e;
}
$qb->addSelect('lazy');
// columns 'type' and 'lazy' don't exist yet (ownCloud migration)
$this->migrationCompleted = false;
$this->loadConfig($app, $lazy);
return;
}
$result = $qb->executeQuery();
$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'] ?? '';

@ -70,6 +70,16 @@ class UserConfig implements IUserConfig {
private array $configLexiconDetails = [];
private bool $ignoreLexiconAliases = false;
private array $strictnessApplied = [];
/**
* Tracks whether the NC-only columns (`type`, `lazy`, `flags`, `indexed`) exist in the
* `preferences` table. Set to false on first load when a DBException::REASON_INVALID_FIELD_NAME
* is caught, which happens during an ownCloud → Nextcloud migration before the schema steps run.
*
* Every SELECT that reads those columns and every INSERT/UPDATE that writes them must
* guard with `if ($this->migrationCompleted)` so they degrade gracefully.
* If you add a new query that touches NC-only columns, add the same guard.
*/
private bool $migrationCompleted = true;
public function __construct(
protected IDBConnection $connection,
@ -1183,12 +1193,14 @@ class UserConfig implements IUserConfig {
$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) {
@ -1240,13 +1252,15 @@ class UserConfig implements IUserConfig {
$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();
}
@ -1810,25 +1824,41 @@ class UserConfig implements IUserConfig {
$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 || !$this->migrationCompleted) {
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);

@ -0,0 +1,185 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Test;
use Doctrine\DBAL\Exception\InvalidFieldNameException;
use OC\AppConfig;
use OC\Config\ConfigManager;
use OC\Config\PresetManager;
use OC\DB\Exceptions\DbalException;
use OC\Memcache\Factory as CacheFactory;
use OCP\DB\Exception as DBException;
use OCP\DB\IResult;
use OCP\DB\QueryBuilder\IExpressionBuilder;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Security\ICrypto;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
/**
* Tests the ownCloud migration fallback in AppConfig.
*
* When migrating from ownCloud, the appconfig table lacks 'type' and 'lazy'
* columns. AppConfig::loadConfig() must catch the resulting DBException and
* retry with a query that only selects columns present in ownCloud's schema.
*/
class AppConfigMigrationFallbackTest extends TestCase {
private IConfig&MockObject $config;
private IDBConnection&MockObject $connection;
private ConfigManager&MockObject $configManager;
private PresetManager&MockObject $presetManager;
private LoggerInterface&MockObject $logger;
private ICrypto&MockObject $crypto;
private CacheFactory&MockObject $cacheFactory;
protected function setUp(): void {
parent::setUp();
$this->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');
}
}

@ -0,0 +1,180 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Test\lib\Config;
use Doctrine\DBAL\Exception\InvalidFieldNameException;
use OC\Config\ConfigManager;
use OC\Config\PresetManager;
use OC\Config\UserConfig;
use OC\DB\Exceptions\DbalException;
use OCP\DB\Exception as DBException;
use OCP\DB\IResult;
use OCP\DB\QueryBuilder\IExpressionBuilder;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Security\ICrypto;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;
/**
* Tests the ownCloud migration fallback in UserConfig.
*
* When migrating from ownCloud, the preferences table lacks 'type', 'lazy',
* 'flags', and 'indexed' columns. UserConfig::loadConfig() must catch the
* resulting DBException and retry with a query that only selects columns
* present in ownCloud's schema.
*/
class UserConfigMigrationFallbackTest extends TestCase {
private IDBConnection&MockObject $connection;
private IConfig&MockObject $config;
private ConfigManager&MockObject $configManager;
private PresetManager&MockObject $presetManager;
private LoggerInterface&MockObject $logger;
private ICrypto&MockObject $crypto;
private IEventDispatcher&MockObject $dispatcher;
protected function setUp(): void {
parent::setUp();
$this->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');
}
}
Loading…
Cancel
Save