Merge pull request #31945 from nextcloud/fix/user_migration-use-exceptions

Adapt user_migration APIs to have information about failures

7 files changed, 78 insertions, 63 deletions

diff --git a/apps/dav/lib/UserMigration/CalendarMigrator.php b/apps/dav/lib/UserMigration/CalendarMigrator.php
index 908b0a564d1..d94e3ec109e 100644
--- a/apps/dav/lib/UserMigration/CalendarMigrator.php
+++ b/apps/dav/lib/UserMigration/CalendarMigrator.php
@@ -225,18 +225,20 @@ class CalendarMigrator implements IMigrator {
 			$output->writeln('No calendars to export…');
 		}
 
-		/**
-		 * @var string $name
-		 * @var VCalendar $vCalendar
-		 */
-		foreach ($calendarExports as ['name' => $name, 'vCalendar' => $vCalendar]) {
-			// Set filename to sanitized calendar name appended with the date
-			$filename = preg_replace('/[^a-zA-Z0-9-_ ]/um', '', $name) . '_' . date('Y-m-d') . CalendarMigrator::FILENAME_EXT;
-			$exportPath = CalendarMigrator::EXPORT_ROOT . $filename;
-
-			if ($exportDestination->addFileContents($exportPath, $vCalendar->serialize()) === false) {
-				throw new CalendarMigratorException('Could not export calendars');
+		try {
+			/**
+			* @var string $name
+			* @var VCalendar $vCalendar
+			*/
+			foreach ($calendarExports as ['name' => $name, 'vCalendar' => $vCalendar]) {
+				// Set filename to sanitized calendar name appended with the date
+				$filename = preg_replace('/[^a-zA-Z0-9-_ ]/um', '', $name) . '_' . date('Y-m-d') . CalendarMigrator::FILENAME_EXT;
+				$exportPath = CalendarMigrator::EXPORT_ROOT . $filename;
+
+				$exportDestination->addFileContents($exportPath, $vCalendar->serialize());
 			}
+		} catch (Throwable $e) {
+			throw new CalendarMigratorException('Could not export calendars', 0, $e);
 		}
 	}
 
diff --git a/apps/dav/lib/UserMigration/ContactsMigrator.php b/apps/dav/lib/UserMigration/ContactsMigrator.php
index 99eea2700a5..065ef05ceea 100644
--- a/apps/dav/lib/UserMigration/ContactsMigrator.php
+++ b/apps/dav/lib/UserMigration/ContactsMigrator.php
@@ -205,26 +205,26 @@ class ContactsMigrator implements IMigrator {
 			$output->writeln('No contacts to export…');
 		}
 
-		/**
-		 * @var string $name
-		 * @var string $displayName
-		 * @var ?string $description
-		 * @var VCard[] $vCards
-		 */
-		foreach ($addressBookExports as ['name' => $name, 'displayName' => $displayName, 'description' => $description, 'vCards' => $vCards]) {
-			// Set filename to sanitized address book name appended with the date
-			$basename = preg_replace('/[^a-zA-Z0-9-_ ]/um', '', $name) . '_' . date('Y-m-d');
-			$exportPath = ContactsMigrator::PATH_ROOT . $basename . '.' . ContactsMigrator::FILENAME_EXT;
-			$metadataExportPath = ContactsMigrator::PATH_ROOT . $basename . '.' . ContactsMigrator::METADATA_EXT;
-
-			if ($exportDestination->addFileContents($exportPath, $this->serializeCards($vCards)) === false) {
-				throw new ContactsMigratorException('Could not export address book');
-			}
-
-			$metadata = array_filter(['displayName' => $displayName, 'description' => $description]);
-			if ($exportDestination->addFileContents($metadataExportPath, json_encode($metadata)) === false) {
-				throw new ContactsMigratorException('Could not export address book metadata');
+		try {
+			/**
+			* @var string $name
+			* @var string $displayName
+			* @var ?string $description
+			* @var VCard[] $vCards
+			*/
+			foreach ($addressBookExports as ['name' => $name, 'displayName' => $displayName, 'description' => $description, 'vCards' => $vCards]) {
+				// Set filename to sanitized address book name appended with the date
+				$basename = preg_replace('/[^a-zA-Z0-9-_ ]/um', '', $name) . '_' . date('Y-m-d');
+				$exportPath = ContactsMigrator::PATH_ROOT . $basename . '.' . ContactsMigrator::FILENAME_EXT;
+				$metadataExportPath = ContactsMigrator::PATH_ROOT . $basename . '.' . ContactsMigrator::METADATA_EXT;
+
+				$exportDestination->addFileContents($exportPath, $this->serializeCards($vCards));
+
+				$metadata = array_filter(['displayName' => $displayName, 'description' => $description]);
+				$exportDestination->addFileContents($metadataExportPath, json_encode($metadata));
 			}
+		} catch (Throwable $e) {
+			throw new CalendarMigratorException('Could not export address book', 0, $e);
 		}
 	}
 
diff --git a/apps/files_trashbin/lib/UserMigration/TrashbinMigrator.php b/apps/files_trashbin/lib/UserMigration/TrashbinMigrator.php
index 95ed25088b1..dbc6267eb3a 100644
--- a/apps/files_trashbin/lib/UserMigration/TrashbinMigrator.php
+++ b/apps/files_trashbin/lib/UserMigration/TrashbinMigrator.php
@@ -74,18 +74,16 @@ class TrashbinMigrator implements IMigrator {
 		try {
 			$trashbinFolder = $this->root->get('/'.$uid.'/files_trashbin');
 			if (!$trashbinFolder instanceof Folder) {
-				throw new UserMigrationException('Could not export trashbin, /'.$uid.'/files_trashbin is not a folder');
+				throw new UserMigrationException('/'.$uid.'/files_trashbin is not a folder');
 			}
 			$output->writeln("Exporting trashbin files…");
-			if ($exportDestination->copyFolder($trashbinFolder, static::PATH_FILES_FOLDER) === false) {
-				throw new UserMigrationException("Could not export trashbin.");
-			}
+			$exportDestination->copyFolder($trashbinFolder, static::PATH_FILES_FOLDER);
 			$originalLocations = \OCA\Files_Trashbin\Trashbin::getLocations($uid);
-			if ($exportDestination->addFileContents(static::PATH_LOCATIONS_FILE, json_encode($originalLocations)) === false) {
-				throw new UserMigrationException("Could not export trashbin.");
-			}
+			$exportDestination->addFileContents(static::PATH_LOCATIONS_FILE, json_encode($originalLocations));
 		} catch (NotFoundException $e) {
 			$output->writeln("No trashbin to export…");
+		} catch (\Throwable $e) {
+			throw new UserMigrationException("Could not export trashbin: ".$e->getMessage(), 0, $e);
 		}
 	}
 
@@ -112,8 +110,10 @@ class TrashbinMigrator implements IMigrator {
 				$trashbinFolder = $this->root->newFolder('/'.$uid.'/files_trashbin');
 			}
 			$output->writeln("Importing trashbin files…");
-			if ($importSource->copyToFolder($trashbinFolder, static::PATH_FILES_FOLDER) === false) {
-				throw new UserMigrationException("Could not import trashbin.");
+			try {
+				$importSource->copyToFolder($trashbinFolder, static::PATH_FILES_FOLDER);
+			} catch (\Throwable $e) {
+				throw new UserMigrationException("Could not import trashbin.", 0, $e);
 			}
 			$locations = json_decode($importSource->getFileContents(static::PATH_LOCATIONS_FILE), true, 512, JSON_THROW_ON_ERROR);
 			$qb = $this->dbc->getQueryBuilder();
diff --git a/apps/settings/lib/UserMigration/AccountMigrator.php b/apps/settings/lib/UserMigration/AccountMigrator.php
index e4218d72a76..32cf898f2bc 100644
--- a/apps/settings/lib/UserMigration/AccountMigrator.php
+++ b/apps/settings/lib/UserMigration/AccountMigrator.php
@@ -74,20 +74,20 @@ class AccountMigrator implements IMigrator {
 	public function export(IUser $user, IExportDestination $exportDestination, OutputInterface $output): void {
 		$output->writeln('Exporting account information in ' . AccountMigrator::PATH_ACCOUNT_FILE . '…');
 
-		$account = $this->accountManager->getAccount($user);
-		if ($exportDestination->addFileContents(AccountMigrator::PATH_ACCOUNT_FILE, json_encode($account)) === false) {
-			throw new AccountMigratorException('Could not export account information');
-		}
+		try {
+			$account = $this->accountManager->getAccount($user);
+			$exportDestination->addFileContents(AccountMigrator::PATH_ACCOUNT_FILE, json_encode($account));
 
-		$avatar = $this->avatarManager->getAvatar($user->getUID());
-		if ($avatar->isCustomAvatar()) {
-			$avatarFile = $avatar->getFile(-1);
-			$exportPath = AccountMigrator::PATH_ROOT . AccountMigrator::AVATAR_BASENAME . '.' . $avatarFile->getExtension();
+			$avatar = $this->avatarManager->getAvatar($user->getUID());
+			if ($avatar->isCustomAvatar()) {
+				$avatarFile = $avatar->getFile(-1);
+				$exportPath = AccountMigrator::PATH_ROOT . AccountMigrator::AVATAR_BASENAME . '.' . $avatarFile->getExtension();
 
-			$output->writeln('Exporting avatar to ' . $exportPath . '…');
-			if ($exportDestination->addFileAsStream($exportPath, $avatarFile->read()) === false) {
-				throw new AccountMigratorException('Could not export avatar');
+				$output->writeln('Exporting avatar to ' . $exportPath . '…');
+				$exportDestination->addFileAsStream($exportPath, $avatarFile->read());
 			}
+		} catch (Throwable $e) {
+			throw new AccountMigratorException('Could not export account information', 0, $e);
 		}
 	}
 
diff --git a/apps/settings/tests/UserMigration/AccountMigratorTest.php b/apps/settings/tests/UserMigration/AccountMigratorTest.php
index 759e7d8a842..573d18380e5 100644
--- a/apps/settings/tests/UserMigration/AccountMigratorTest.php
+++ b/apps/settings/tests/UserMigration/AccountMigratorTest.php
@@ -152,14 +152,12 @@ class AccountMigratorTest extends TestCase {
 		$this->exportDestination
 			->expects($this->once())
 			->method('addFileContents')
-			->with($this->matchesRegularExpression(self::REGEX_ACCOUNT_FILE), json_encode($exportData))
-			->willReturn(true);
+			->with($this->matchesRegularExpression(self::REGEX_ACCOUNT_FILE), json_encode($exportData));
 
 		$this->exportDestination
 			->expects($this->once())
 			->method('addFileAsStream')
-			->with($this->matchesRegularExpression(self::REGEX_AVATAR_FILE), $this->isType('resource'))
-			->willReturn(true);
+			->with($this->matchesRegularExpression(self::REGEX_AVATAR_FILE), $this->isType('resource'));
 
 		$this->migrator->export($user, $this->exportDestination, $this->output);
 	}
diff --git a/lib/public/UserMigration/IExportDestination.php b/lib/public/UserMigration/IExportDestination.php
index a721efcdf93..65d228faeb9 100644
--- a/lib/public/UserMigration/IExportDestination.php
+++ b/lib/public/UserMigration/IExportDestination.php
@@ -38,44 +38,47 @@ interface IExportDestination {
 	 *
 	 * @param string $path Full path to the file in the export archive. Parent directories will be created if needed.
 	 * @param string $content The full content of the file.
-	 * @return bool whether the file contents were successfully added.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
-	public function addFileContents(string $path, string $content): bool;
+	public function addFileContents(string $path, string $content): void;
 
 	/**
 	 * Adds a file to the export as a stream
 	 *
 	 * @param string $path Full path to the file in the export archive. Parent directories will be created if needed.
 	 * @param resource $stream A stream resource to read from to get the file content.
-	 * @return bool whether the file stream was successfully added.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
-	public function addFileAsStream(string $path, $stream): bool;
+	public function addFileAsStream(string $path, $stream): void;
 
 	/**
 	 * Copy a folder to the export
 	 *
 	 * @param Folder $folder folder to copy to the export archive.
 	 * @param string $destinationPath Full path to the folder in the export archive. Parent directories will be created if needed.
-	 * @return bool whether the folder was successfully added.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
-	public function copyFolder(Folder $folder, string $destinationPath): bool;
+	public function copyFolder(Folder $folder, string $destinationPath): void;
 
 	/**
 	 * @param array<string,int> $versions Migrators and their versions.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
-	public function setMigratorVersions(array $versions): bool;
+	public function setMigratorVersions(array $versions): void;
 
 	/**
 	 * Called after export is complete
 	 *
+	 * @throws UserMigrationException
+	 *
 	 * @since 24.0.0
 	 */
 	public function close(): void;
diff --git a/lib/public/UserMigration/IImportSource.php b/lib/public/UserMigration/IImportSource.php
index 3816afdd033..da2c87ba241 100644
--- a/lib/public/UserMigration/IImportSource.php
+++ b/lib/public/UserMigration/IImportSource.php
@@ -39,6 +39,7 @@ interface IImportSource {
 	 *
 	 * @param string $path Full path to the file in the export archive.
 	 * @return string The full content of the file.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
@@ -49,6 +50,7 @@ interface IImportSource {
 	 *
 	 * @param string $path Full path to the file in the export archive.
 	 * @return resource A stream resource to read from to get the file content.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
@@ -59,6 +61,7 @@ interface IImportSource {
 	 *
 	 * @param string $path Full path to the folder in the export archive.
 	 * @return array The list of files.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
@@ -67,6 +70,8 @@ interface IImportSource {
 	/**
 	 * Test if a path exists, which may be a file or a folder
 	 *
+	 * @throws UserMigrationException
+	 *
 	 * @since 24.0.0
 	 */
 	public function pathExists(string $path): bool;
@@ -77,12 +82,15 @@ interface IImportSource {
 	 * Folder $destination folder to copy into
 	 * string $sourcePath path in the export archive
 	 *
+	 * @throws UserMigrationException
+	 *
 	 * @since 24.0.0
 	 */
-	public function copyToFolder(Folder $destination, string $sourcePath): bool;
+	public function copyToFolder(Folder $destination, string $sourcePath): void;
 
 	/**
 	 * @return array<string,int> Migrators and their versions from the export archive.
+	 * @throws UserMigrationException
 	 *
 	 * @since 24.0.0
 	 */
@@ -90,7 +98,7 @@ interface IImportSource {
 
 	/**
 	 * @return ?int Version for this migrator from the export archive. Null means migrator missing.
-	 *
+	 * @throws UserMigrationException
 	 * @param string $migrator Migrator id (as returned by IMigrator::getId)
 	 *
 	 * @since 24.0.0
@@ -100,6 +108,8 @@ interface IImportSource {
 	/**
 	 * Get original uid of the imported account
 	 *
+	 * @throws UserMigrationException
+	 *
 	 * @since 24.0.0
 	 */
 	public function getOriginalUid(): string;
@@ -107,6 +117,8 @@ interface IImportSource {
 	/**
 	 * Called after import is complete
 	 *
+	 * @throws UserMigrationException
+	 *
 	 * @since 24.0.0
 	 */
 	public function close(): void;