Skip to content

Commit a82c1f1

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #50555 from nextcloud/backport/50503/stable31
[stable31] fix(theming): Do not throw in background color migration
2 parents c17d892 + 09b6598 commit a82c1f1

File tree

2 files changed

+220
-1
lines changed

2 files changed

+220
-1
lines changed

‎apps/theming/lib/Migration/Version2006Date20240905111627.php

+39-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,45 @@ private function restoreUserColors(IOutput $output): void {
8383
->set('configkey', $qb->createNamedParameter('primary_color'))
8484
->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID)))
8585
->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('background_color')));
86-
$qb->executeStatement();
86+
87+
try {
88+
$qb->executeStatement();
89+
} catch (\Exception) {
90+
$output->debug('Some users already configured the background color');
91+
$this->restoreUserColorsFallback($output);
92+
}
93+
8794
$output->info('Primary color of users restored');
8895
}
96+
97+
/**
98+
* Similar to restoreUserColors but also works if some users already setup a new value.
99+
* This is only called if the first approach fails as this takes much longer on the DB.
100+
*/
101+
private function restoreUserColorsFallback(IOutput $output): void {
102+
$qb = $this->connection->getQueryBuilder();
103+
$qb2 = $this->connection->getQueryBuilder();
104+
105+
$qb2->select('userid')
106+
->from('preferences')
107+
->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID)))
108+
->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('primary_color')));
109+
110+
// MySQL does not update on select of the same table, so this is a workaround:
111+
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_MYSQL) {
112+
$subquery = 'SELECT * from ( ' . $qb2->getSQL() . ' ) preferences_alias';
113+
} else {
114+
$subquery = $qb2->getSQL();
115+
}
116+
117+
$qb->update('preferences')
118+
->set('configkey', $qb->createNamedParameter('primary_color'))
119+
->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID)))
120+
->andWhere(
121+
$qb->expr()->eq('configkey', $qb->createNamedParameter('background_color')),
122+
$qb->expr()->notIn('userid', $qb->createFunction($subquery)),
123+
);
124+
125+
$qb->executeStatement();
126+
}
89127
}

‎apps/theming/tests/Migration/Version2006Date20240905111627Test.php

+181
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Theming\Tests\Migration;
11+
12+
use NCU\Config\IUserConfig;
13+
use OCA\Theming\Migration\Version2006Date20240905111627;
14+
use OCP\BackgroundJob\IJobList;
15+
use OCP\IAppConfig;
16+
use OCP\IDBConnection;
17+
use OCP\IUserManager;
18+
use OCP\Migration\IOutput;
19+
use PHPUnit\Framework\MockObject\MockObject;
20+
use Test\TestCase;
21+
22+
/**
23+
* @group DB
24+
*/
25+
class Version2006Date20240905111627Test extends TestCase {
26+
27+
private IAppConfig&MockObject $appConfig;
28+
private IDBConnection&MockObject $connection;
29+
private IJobList&MockObject $jobList;
30+
private Version2006Date20240905111627 $migration;
31+
32+
protected function setUp(): void {
33+
parent::setUp();
34+
35+
$this->appConfig = $this->createMock(IAppConfig::class);
36+
$this->connection = $this->createMock(IDBConnection::class);
37+
$this->jobList = $this->createMock(IJobList::class);
38+
$this->migration = new Version2006Date20240905111627(
39+
$this->jobList,
40+
$this->appConfig,
41+
$this->connection,
42+
);
43+
}
44+
45+
public function testRestoreSystemColors(): void {
46+
$this->appConfig->expects(self::once())
47+
->method('getValueString')
48+
->with('theming', 'color', '')
49+
->willReturn('ffab00');
50+
$this->appConfig->expects(self::once())
51+
->method('getValueBool')
52+
->with('theming', 'disable-user-theming')
53+
->willReturn(true);
54+
55+
// expect the color value to be deleted
56+
$this->appConfig->expects(self::once())
57+
->method('deleteKey')
58+
->with('theming', 'color');
59+
// expect the correct calls to setValueString (setting the new values)
60+
$setValueCalls = [];
61+
$this->appConfig->expects(self::exactly(2))
62+
->method('setValueString')
63+
->willReturnCallback(function () use (&$setValueCalls) {
64+
$setValueCalls[] = func_get_args();
65+
return true;
66+
});
67+
68+
$output = $this->createMock(IOutput::class);
69+
$this->migration->changeSchema($output, fn () => null, []);
70+
71+
$this->assertEquals([
72+
['theming', 'background_color', 'ffab00', false, false],
73+
['theming', 'primary_color', 'ffab00', false, false],
74+
], $setValueCalls);
75+
}
76+
77+
/**
78+
* @group DB
79+
*/
80+
public function testRestoreUserColors(): void {
81+
$this->appConfig->expects(self::once())
82+
->method('getValueString')
83+
->with('theming', 'color', '')
84+
->willReturn('');
85+
$this->appConfig->expects(self::once())
86+
->method('getValueBool')
87+
->with('theming', 'disable-user-theming')
88+
->willReturn(false);
89+
90+
// Create a user
91+
$manager = \OCP\Server::get(IUserManager::class);
92+
$user = $manager->createUser('theming_legacy', 'theming_legacy');
93+
self::assertNotFalse($user);
94+
// Set the users theming value to legacy key
95+
$config = \OCP\Server::get(IUserConfig::class);
96+
$config->setValueString('theming_legacy', 'theming', 'background_color', 'ffab00');
97+
98+
// expect some output
99+
$output = $this->createMock(IOutput::class);
100+
$output->expects(self::exactly(3))
101+
->method('info')
102+
->willReturnCallback(fn ($txt) => match($txt) {
103+
'No custom system color configured - skipping' => true,
104+
'Restoring user primary color' => true,
105+
'Primary color of users restored' => true,
106+
default => self::fail('output.info called with unexpected argument: ' . $txt)
107+
});
108+
// Create the migration class
109+
$migration = new Version2006Date20240905111627(
110+
$this->jobList,
111+
$this->appConfig,
112+
\OCP\Server::get(IDBConnection::class),
113+
);
114+
// Run the migration
115+
$migration->changeSchema($output, fn () => null, []);
116+
117+
// See new value
118+
$config->clearCache('theming_legacy');
119+
$newValue = $config->getValueString('theming_legacy', 'theming', 'primary_color');
120+
self::assertEquals('ffab00', $newValue);
121+
122+
// cleanup
123+
$user->delete();
124+
}
125+
126+
/**
127+
* Ensure only users with background color but no primary color are migrated
128+
* @group DB
129+
*/
130+
public function testRestoreUserColorsWithConflicts(): void {
131+
$this->appConfig->expects(self::once())
132+
->method('getValueString')
133+
->with('theming', 'color', '')
134+
->willReturn('');
135+
$this->appConfig->expects(self::once())
136+
->method('getValueBool')
137+
->with('theming', 'disable-user-theming')
138+
->willReturn(false);
139+
140+
// Create a user
141+
$manager = \OCP\Server::get(IUserManager::class);
142+
$legacyUser = $manager->createUser('theming_legacy', 'theming_legacy');
143+
self::assertNotFalse($legacyUser);
144+
$user = $manager->createUser('theming_no_legacy', 'theming_no_legacy');
145+
self::assertNotFalse($user);
146+
// Set the users theming value to legacy key
147+
$config = \OCP\Server::get(IUserConfig::class);
148+
$config->setValueString($user->getUID(), 'theming', 'primary_color', '999999');
149+
$config->setValueString($user->getUID(), 'theming', 'background_color', '111111');
150+
$config->setValueString($legacyUser->getUID(), 'theming', 'background_color', 'ffab00');
151+
152+
// expect some output
153+
$output = $this->createMock(IOutput::class);
154+
$output->expects(self::exactly(3))
155+
->method('info')
156+
->willReturnCallback(fn ($txt) => match($txt) {
157+
'No custom system color configured - skipping' => true,
158+
'Restoring user primary color' => true,
159+
'Primary color of users restored' => true,
160+
default => self::fail('output.info called with unexpected argument: ' . $txt)
161+
});
162+
// Create the migration class
163+
$migration = new Version2006Date20240905111627(
164+
$this->jobList,
165+
$this->appConfig,
166+
\OCP\Server::get(IDBConnection::class),
167+
);
168+
// Run the migration
169+
$migration->changeSchema($output, fn () => null, []);
170+
171+
// See new value of only the legacy user
172+
$config->clearCacheAll();
173+
self::assertEquals('111111', $config->getValueString($user->getUID(), 'theming', 'background_color'));
174+
self::assertEquals('999999', $config->getValueString($user->getUID(), 'theming', 'primary_color'));
175+
self::assertEquals('ffab00', $config->getValueString($legacyUser->getUID(), 'theming', 'primary_color'));
176+
177+
// cleanup
178+
$legacyUser->delete();
179+
$user->delete();
180+
}
181+
}

0 commit comments

Comments
 (0)