Skip to content

Commit 0bd0244

Browse files
DerDreschnerDerDreschner
committed
feat(UserMigration): Overwork migration to include all settings (basic changes)
Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
1 parent fffd17f commit 0bd0244

7 files changed

Lines changed: 283 additions & 322 deletions

File tree

‎.github/workflows/test.yml‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,13 +304,65 @@ jobs:
304304
env:
305305
CI: true
306306

307+
user-migration-smoke-test:
308+
runs-on: ubuntu-latest
309+
strategy:
310+
matrix:
311+
php-versions: [ '8.4' ]
312+
nextcloud-versions: [ 'stable33' ]
313+
db: [ 'mysql' ]
314+
env:
315+
TEST_USER: user1
316+
EXPORT_DIRECTORY: /tmp
317+
name: Nextcloud ${{ matrix.nextcloud-versions }} php${{ matrix.php-versions }} user migration test
318+
steps:
319+
- name: Set up Nextcloud env
320+
uses: nextcloud/setup-server-action@34b73d5b0e3633f83a52227d00cc2a6c41d01d9a # v1.0.0
321+
with:
322+
nextcloud-version: ${{ matrix.nextcloud-versions }}
323+
php-version: ${{ matrix.php-versions }}
324+
patch-php-version-check: ${{ matrix.php-versions == '8.6' }}
325+
node-version: 'false'
326+
install: true
327+
- name: Checkout mail
328+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
329+
with:
330+
path: nextcloud/apps/mail
331+
fetch-depth: 2
332+
- name: Install dependencies
333+
working-directory: nextcloud/apps/mail
334+
run: composer install --no-dev
335+
- name: Install mail
336+
run: php -f nextcloud/occ app:enable mail
337+
- name: Checkout user_migration
338+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
339+
with:
340+
repository: nextcloud/user_migration
341+
path: nextcloud/apps/user_migration
342+
- name: Install dependencies
343+
working-directory: nextcloud/apps/user_migration
344+
run: composer install --no-dev
345+
- name: Install user_migration
346+
run: php -f nextcloud/occ app:enable user_migration
347+
- name: Create test user
348+
run: php -f nextcloud/occ user:add --generate-password $TEST_USER
349+
- name: Test basic mail export
350+
run: php -f nextcloud/occ user:export -t mail_account $TEST_USER $EXPORT_DIRECTORY
351+
- name: Delete exported user to avoid overwrite confirmation during import
352+
run: php -f nextcloud/occ user:delete $TEST_USER
353+
- name: Test basic mail import
354+
run: |
355+
EXPORTED_ZIP=$(find $EXPORT_DIRECTORY -name "$TEST_USER*.zip" | tail -1)
356+
php -f nextcloud/occ user:import $EXPORTED_ZIP
357+
307358
summary:
308359
runs-on: ubuntu-latest-low
309360
needs:
310361
- unit-tests
311362
- integration-tests
312363
- frontend-unit-test
313364
- frontend-e2e-tests
365+
- user-migration-smoke-test
314366

315367
if: always()
316368

@@ -325,3 +377,5 @@ jobs:
325377
run: if ${{ needs.frontend-unit-test.result != 'success' && needs.frontend-unit-test.result != 'skipped' }}; then exit 1; fi
326378
- name: Frontend E2E test status
327379
run: if ${{ needs.frontend-e2e-tests.result != 'success' && needs.frontend-e2e-tests.result != 'skipped' }}; then exit 1; fi
380+
- name: User migration smoke test status
381+
run: if ${{ needs.user-migration-smoke-test.result != 'success' && needs.user-migration-smoke-test.result != 'skipped' }}; then exit 1; fi

‎lib/Service/AccountService.php‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ public function delete(string $currentUserId, int $accountId): void {
149149
}
150150
$this->aliasesService->deleteAll($accountId);
151151
$this->mapper->delete($mailAccount);
152+
153+
// Invalidate cache to ensure deleted account is not included
154+
// in subsequent `findByUserId` and `findByUserIdAndAddress` calls
155+
unset($this->accounts[$currentUserId]);
152156
}
153157

154158
/**
@@ -164,17 +168,31 @@ public function deleteByAccountId(int $accountId): void {
164168
}
165169
$this->aliasesService->deleteAll($accountId);
166170
$this->mapper->delete($mailAccount);
171+
172+
// Invalidate cache to ensure deleted account is not included
173+
// in subsequent `findByUserId` and `findByUserIdAndAddress` calls
174+
unset($this->accounts[$mailAccount->getUserId()]);
167175
}
168176

169177
/**
170178
* @param MailAccount $newAccount
179+
* @param bool $scheduleBackgroundJobs Optional parameter to save the mail account
180+
* without scheduling the corresponding background jobs. This can be useful if
181+
* further database modifications must be done before running any background
182+
* jobs. Defaults to `true`.
171183
* @return MailAccount
172184
*/
173-
public function save(MailAccount $newAccount): MailAccount {
185+
public function save(MailAccount $newAccount, bool $scheduleBackgroundJobs = true): MailAccount {
174186
$newAccount = $this->mapper->save($newAccount);
175187

176188
// Insert background jobs for this account
177-
$this->scheduleBackgroundJobs($newAccount->getId());
189+
if ($scheduleBackgroundJobs) {
190+
$this->scheduleBackgroundJobs($newAccount->getId());
191+
}
192+
193+
// Invalidate cache to ensure created account is being included
194+
// in subsequent `findByUserId` and `findByUserIdAndAddress` calls
195+
unset($this->accounts[$newAccount->getUserId()]);
178196

179197
return $newAccount;
180198
}
@@ -196,6 +214,10 @@ public function updateSignature(int $id, string $uid, ?string $signature = null)
196214
$mailAccount = $account->getMailAccount();
197215
$mailAccount->setSignature($signature);
198216
$this->mapper->save($mailAccount);
217+
218+
// Invalidate cache to ensure changed signature is being included
219+
// in subsequent `findByUserId` and `findByUserIdAndAddress` calls
220+
unset($this->accounts[$uid]);
199221
}
200222

201223
/**

‎lib/UserMigration/MailAccountMigrator.php‎

Lines changed: 33 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,16 @@
44

55
/**
66
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7-
* SPDX-License-Identifier: AGPL-3.0-only
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
88
*/
99

1010
namespace OCA\Mail\UserMigration;
1111

12-
use Exception;
13-
use JsonException;
14-
use OCA\Mail\Db\Alias;
15-
use OCA\Mail\Db\MailAccount;
16-
use OCA\Mail\Service\AccountService;
17-
use OCA\Mail\Service\AliasesService;
12+
use OCA\Mail\AppInfo\Application;
13+
use OCA\Mail\Exception\ClientException;
14+
use OCA\Mail\Exception\ServiceException;
15+
use OCA\Mail\UserMigration\Service\AccountMigrationService;
16+
use OCP\AppFramework\Db\DoesNotExistException;
1817
use OCP\IL10N;
1918
use OCP\IUser;
2019
use OCP\Security\ICrypto;
@@ -23,17 +22,15 @@
2322
use OCP\UserMigration\IMigrator;
2423
use OCP\UserMigration\UserMigrationException;
2524
use Symfony\Component\Console\Output\OutputInterface;
26-
use function array_map;
27-
use function json_decode;
28-
use function json_encode;
2925

3026
class MailAccountMigrator implements IMigrator {
27+
public const EXPORT_ROOT = Application::APP_ID;
28+
public const FILENAME_PLACEHOLDER = '{filename}';
3129

3230
public function __construct(
33-
private AccountService $accountService,
34-
private AliasesService $aliasesService,
35-
private IL10N $l10n,
36-
private ICrypto $crypto,
31+
private readonly IL10N $l10n,
32+
private readonly ICrypto $crypto,
33+
private readonly AccountMigrationService $accountMigrationService,
3734
) {
3835
}
3936

@@ -42,140 +39,31 @@ public function export(IUser $user,
4239
IExportDestination $exportDestination,
4340
OutputInterface $output,
4441
): void {
45-
$accounts = $this->accountService->findByUserId($user->getUID());
46-
$index = [];
47-
foreach ($accounts as $account) {
48-
if ($account->getMailAccount()->getProvisioningId() !== null) {
49-
// These configuration of these accounts is owned by the admins
50-
$output->writeln("Skipping provisioned account {$account->getId()}");
51-
continue;
52-
}
53-
54-
$accountFilePath = "mail/accounts/{$account->getId()}.json";
55-
$accountData = $account->jsonSerialize();
56-
57-
if ($account->getMailAccount()->getAuthMethod() === 'password') {
58-
$encryptedInboundPassword = $account->getMailAccount()->getInboundPassword();
59-
$encryptedOutboundPassword = $account->getMailAccount()->getOutboundPassword();
60-
if ($encryptedInboundPassword !== null) {
61-
try {
62-
$accountData['inboundPassword'] = $this->crypto->decrypt($encryptedInboundPassword);
63-
} catch (Exception $e) {
64-
$output->writeln("Can not decrypt inbound password of account {$account->getId()}: " . $e->getMessage());
65-
}
66-
}
67-
if ($encryptedOutboundPassword !== null) {
68-
try {
69-
$accountData['outboundPassword'] = $this->crypto->decrypt($encryptedOutboundPassword);
70-
} catch (Exception $e) {
71-
$output->writeln("Can not decrypt outbound password of account {$account->getId()}: " . $e->getMessage());
72-
}
73-
}
74-
} elseif ($account->getMailAccount()->getAuthMethod() === 'xoauth2') {
75-
$encryptedRefreshToken = $account->getMailAccount()->getOauthRefreshToken();
76-
$encryptedAccessToken = $account->getMailAccount()->getOauthAccessToken();
77-
if ($encryptedRefreshToken !== null) {
78-
try {
79-
$accountData['oauthRefreshToken'] = $this->crypto->decrypt($encryptedRefreshToken);
80-
} catch (Exception $e) {
81-
$output->writeln("Can not decrypt oauth refresh token of account {$account->getId()}: " . $e->getMessage());
82-
}
83-
}
84-
if ($encryptedAccessToken !== null) {
85-
try {
86-
$accountData['oauthAccessToken'] = $this->crypto->decrypt($encryptedAccessToken);
87-
} catch (Exception $e) {
88-
$output->writeln("Can not decrypt oauth access token of account {$account->getId()}: " . $e->getMessage());
89-
}
90-
}
91-
$accountData['oauthTokenTtl'] = $account->getMailAccount()->getOauthTokenTtl();
92-
}
93-
94-
unset(
95-
$accountData['draftsMailboxId'],
96-
$accountData['sentMailboxId'],
97-
$accountData['trashMailboxId'],
98-
$accountData['archiveMailboxId'],
99-
$accountData['snoozeMailboxId'],
100-
$accountData['junkMailboxId'],
101-
);
102-
103-
$aliases = $this->aliasesService->findAll(
104-
$account->getId(),
105-
$account->getUserId(), // perf: this adds overhead - add dedicated method to fetch by account id only
106-
);
107-
$accountData['aliases'] = array_map(function (Alias $alias) {
108-
$data = $alias->jsonSerialize();
109-
return $data;
110-
}, $aliases);
111-
112-
$exportDestination->addFileContents($accountFilePath, json_encode($accountData));
113-
$index[$account->getId()] = $accountFilePath;
114-
}
115-
116-
$exportDestination->addFileContents('mail/accounts/index.json', json_encode($index));
42+
$output->writeln($this->l10n->t("Exporting mail accounts for user {$user->getUID()}"), OutputInterface::VERBOSITY_VERBOSE);
11743
}
11844

11945
#[\Override]
12046
public function import(IUser $user, IImportSource $importSource, OutputInterface $output): void {
121-
try {
122-
$index = json_decode($importSource->getFileContents('mail/accounts/index.json'), true, flags: JSON_THROW_ON_ERROR);
123-
} catch (JsonException $e) {
124-
throw new UserMigrationException("Invalid index content: {$e->getMessage()}", $e->getCode(), $e);
125-
}
126-
foreach ($index as $accountFilePath) {
127-
try {
128-
$accountData = json_decode($importSource->getFileContents($accountFilePath), true, flags: JSON_THROW_ON_ERROR);
129-
} catch (JsonException $e) {
130-
throw new UserMigrationException("Invalid account content: {$e->getMessage()}", $e->getCode(), $e);
131-
}
132-
133-
// Wipe the old ID(s) to prevent overwrites
134-
unset(
135-
$accountData['id'],
136-
$accountData['accountId'],
137-
);
138-
139-
$newAccount = new MailAccount($accountData);
140-
141-
// Change UID to new owner
142-
$newAccount->setUserId($user->getUID());
143-
// Map the rest of the properties that are not mapped via the constructor
144-
$newAccount->setName($accountData['name']);
145-
$newAccount->setAuthMethod($accountData['authMethod']);
146-
$newAccount->setEditorMode($accountData['editorMode'] ?? 'plain');
147-
$newAccount->setSearchBody($accountData['searchBody'] ?? false);
148-
$newAccount->setClassificationEnabled($accountData['classificationEnabled'] ?? false);
149-
$newAccount->setSignatureAboveQuote($accountData['signatureAboveQuote'] ?? false);
150-
$newAccount->setPersonalNamespace($accountData['personalNamespace'] ?? null);
151-
if (isset($accountData['inboundPassword'])) {
152-
$newAccount->setInboundPassword($this->crypto->encrypt($accountData['inboundPassword']));
153-
}
154-
if (isset($accountData['outboundPassword'])) {
155-
$newAccount->setOutboundPassword($this->crypto->encrypt($accountData['outboundPassword']));
156-
}
157-
if (isset($accountData['oauthRefreshToken'])) {
158-
$newAccount->setOauthRefreshToken($this->crypto->encrypt($accountData['oauthRefreshToken']));
159-
}
160-
if (isset($accountData['oauthAccessToken'])) {
161-
$newAccount->setOauthAccessToken($this->crypto->encrypt($accountData['oauthAccessToken']));
162-
}
163-
$newAccount->setOauthTokenTtl($accountData['oauthTokenTtl'] ?? null);
164-
165-
$mailAccount = $this->accountService->save(
166-
$newAccount
167-
);
168-
169-
// Import aliases
170-
foreach ($accountData['aliases'] as $alias) {
171-
$this->aliasesService->create(
172-
$user->getUID(),
173-
$mailAccount->getId(),
174-
$alias['alias'],
175-
$alias['name'],
176-
);
177-
}
178-
}
47+
$output->writeln($this->l10n->t("Importing mail accounts for user {$user->getUID()}"), OutputInterface::VERBOSITY_VERBOSE);
48+
49+
$this->deleteExistingData($user, $output);
50+
51+
$this->accountMigrationService->scheduleBackgroundJobs($user, $output);
52+
}
53+
54+
/**
55+
* Delete all existing user data of our app to ensure
56+
* the result of the import is always the same.
57+
*
58+
* @param IUser $user
59+
* @param OutputInterface $output
60+
* @throws ClientException
61+
* @throws DoesNotExistException
62+
* @throws ServiceException
63+
*/
64+
private function deleteExistingData(IUser $user, OutputInterface $output): void {
65+
$output->writeln($this->l10n->t("Deleting existing mail data for user {$user->getUID()}"), OutputInterface::VERBOSITY_VERBOSE);
66+
$this->accountMigrationService->deleteAllAccounts($user, $output);
17967
}
18068

18169
#[\Override]
@@ -195,7 +83,7 @@ public function getDescription(): string {
19583

19684
#[\Override]
19785
public function getVersion(): int {
198-
return 01_00_00;
86+
return 02_00_00;
19987
}
20088

20189
#[\Override]

0 commit comments

Comments
 (0)