Skip to content

Commit

Permalink
Merge pull request #50554 from nextcloud/backport/50503/stable30
Browse files Browse the repository at this point in the history
[stable30] fix(theming): Do not throw in background color migration
  • Loading branch information
susnux authored Jan 31, 2025
2 parents c3c6102 + 6d3bdbc commit b58e072
Show file tree
Hide file tree
Showing 3 changed files with 233 additions and 2 deletions.
2 changes: 1 addition & 1 deletion apps/theming/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<name>Theming</name>
<summary>Adjust the Nextcloud theme</summary>
<description>Adjust the Nextcloud theme</description>
<version>2.5.0</version>
<version>2.6.0</version>
<licence>agpl</licence>
<author>Nextcloud</author>
<namespace>Theming</namespace>
Expand Down
40 changes: 39 additions & 1 deletion apps/theming/lib/Migration/Version2006Date20240905111627.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,45 @@ private function restoreUserColors(IOutput $output): void {
->set('configkey', $qb->createNamedParameter('primary_color'))
->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID)))
->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('background_color')));
$qb->executeStatement();

try {
$qb->executeStatement();
} catch (\Exception) {
$output->debug('Some users already configured the background color');
$this->restoreUserColorsFallback($output);
}

$output->info('Primary color of users restored');
}

/**
* Similar to restoreUserColors but also works if some users already setup a new value.
* This is only called if the first approach fails as this takes much longer on the DB.
*/
private function restoreUserColorsFallback(IOutput $output): void {
$qb = $this->connection->getQueryBuilder();
$qb2 = $this->connection->getQueryBuilder();

$qb2->select('userid')
->from('preferences')
->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID)))
->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('primary_color')));

// MySQL does not update on select of the same table, so this is a workaround:
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_MYSQL) {
$subquery = 'SELECT * from ( ' . $qb2->getSQL() . ' ) preferences_alias';
} else {
$subquery = $qb2->getSQL();
}

$qb->update('preferences')
->set('configkey', $qb->createNamedParameter('primary_color'))
->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID)))
->andWhere(
$qb->expr()->eq('configkey', $qb->createNamedParameter('background_color')),
$qb->expr()->notIn('userid', $qb->createFunction($subquery)),
);

$qb->executeStatement();
}
}
193 changes: 193 additions & 0 deletions apps/theming/tests/Migration/Version2006Date20240905111627Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Theming\Tests\Migration;

use OCA\Theming\Migration\Version2006Date20240905111627;
use OCP\BackgroundJob\IJobList;
use OCP\Cache\CappedMemoryCache;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IUserManager;
use OCP\Migration\IOutput;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

/**
* @group DB
*/
class Version2006Date20240905111627Test extends TestCase {

private IAppConfig&MockObject $appConfig;
private IDBConnection&MockObject $connection;
private IJobList&MockObject $jobList;
private Version2006Date20240905111627 $migration;

protected function setUp(): void {
parent::setUp();

$this->appConfig = $this->createMock(IAppConfig::class);
$this->connection = $this->createMock(IDBConnection::class);
$this->jobList = $this->createMock(IJobList::class);
$this->migration = new Version2006Date20240905111627(
$this->jobList,
$this->appConfig,
$this->connection,
);
}

public function testRestoreSystemColors(): void {
$this->appConfig->expects(self::once())
->method('getValueString')
->with('theming', 'color', '')
->willReturn('ffab00');
$this->appConfig->expects(self::once())
->method('getValueBool')
->with('theming', 'disable-user-theming')
->willReturn(true);

// expect the color value to be deleted
$this->appConfig->expects(self::once())
->method('deleteKey')
->with('theming', 'color');
// expect the correct calls to setValueString (setting the new values)
$setValueCalls = [];
$this->appConfig->expects(self::exactly(2))
->method('setValueString')
->willReturnCallback(function () use (&$setValueCalls) {
$setValueCalls[] = func_get_args();
return true;
});

/** @var IOutput&MockObject */
$output = $this->createMock(IOutput::class);
$this->migration->changeSchema($output, fn () => null, []);

$this->assertEquals([
['theming', 'background_color', 'ffab00', false, false],
['theming', 'primary_color', 'ffab00', false, false],
], $setValueCalls);
}

/**
* @group DB
*/
public function testRestoreUserColors(): void {
$this->appConfig->expects(self::once())
->method('getValueString')
->with('theming', 'color', '')
->willReturn('');
$this->appConfig->expects(self::once())
->method('getValueBool')
->with('theming', 'disable-user-theming')
->willReturn(false);

// Create a user
$manager = \OCP\Server::get(IUserManager::class);
$user = $manager->createUser('theming_legacy', 'theming_legacy');
self::assertNotFalse($user);
/**
* Set the users theming value to legacy key
* @var IConfig
*/
$config = \OCP\Server::get(IConfig::class);
$config->setUserValue($user->getUID(), 'theming', 'background_color', 'ffab00');

// expect some output
/** @var IOutput&MockObject */
$output = $this->createMock(IOutput::class);
$output->expects(self::exactly(3))
->method('info')
->willReturnCallback(fn ($txt) => match($txt) {
'No custom system color configured - skipping' => true,
'Restoring user primary color' => true,
'Primary color of users restored' => true,
default => self::fail('output.info called with unexpected argument: ' . $txt)
});
// Create the migration class
$migration = new Version2006Date20240905111627(
$this->jobList,
$this->appConfig,
\OCP\Server::get(IDBConnection::class),
);
// Run the migration
$migration->changeSchema($output, fn () => null, []);

// Clear cache
self::invokePrivate($config, 'userCache', [new CappedMemoryCache()]);
// See new value
$newValue = $config->getUserValue($user->getUID(), 'theming', 'primary_color');
self::assertEquals('ffab00', $newValue);

// cleanup
$user->delete();
}

/**
* Ensure only users with background color but no primary color are migrated
* @group DB
*/
public function testRestoreUserColorsWithConflicts(): void {
$this->appConfig->expects(self::once())
->method('getValueString')
->with('theming', 'color', '')
->willReturn('');
$this->appConfig->expects(self::once())
->method('getValueBool')
->with('theming', 'disable-user-theming')
->willReturn(false);

// Create a user
$manager = \OCP\Server::get(IUserManager::class);
$legacyUser = $manager->createUser('theming_legacy', 'theming_legacy');
self::assertNotFalse($legacyUser);
$user = $manager->createUser('theming_no_legacy', 'theming_no_legacy');
self::assertNotFalse($user);
/**
* Set the users theming value to legacy key
* @var IConfig
*/
$config = \OCP\Server::get(IConfig::class);
$config->setUserValue($user->getUID(), 'theming', 'primary_color', '999999');
$config->setUserValue($user->getUID(), 'theming', 'background_color', '111111');
$config->setUserValue($legacyUser->getUID(), 'theming', 'background_color', 'ffab00');

// expect some output
/** @var IOutput&MockObject */
$output = $this->createMock(IOutput::class);
$output->expects(self::exactly(3))
->method('info')
->willReturnCallback(fn ($txt) => match($txt) {
'No custom system color configured - skipping' => true,
'Restoring user primary color' => true,
'Primary color of users restored' => true,
default => self::fail('output.info called with unexpected argument: ' . $txt)
});
// Create the migration class
$migration = new Version2006Date20240905111627(
$this->jobList,
$this->appConfig,
\OCP\Server::get(IDBConnection::class),
);
// Run the migration
$migration->changeSchema($output, fn () => null, []);

// Clear cache
self::invokePrivate($config, 'userCache', [new CappedMemoryCache()]);
// See new value of only the legacy user
self::assertEquals('111111', $config->getUserValue($user->getUID(), 'theming', 'background_color'));
self::assertEquals('999999', $config->getUserValue($user->getUID(), 'theming', 'primary_color'));
self::assertEquals('ffab00', $config->getUserValue($legacyUser->getUID(), 'theming', 'primary_color'));

// cleanup
$legacyUser->delete();
$user->delete();
}
}

0 comments on commit b58e072

Please sign in to comment.