Skip to content

Use exceptions instead of booleans #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PVince81 merged 1 commit into from
Apr 14, 2022
Merged

Use exceptions instead of booleans #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions lib/ExportDestination.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,26 +56,24 @@ public function __construct($r) {
/**
* {@inheritDoc}
*/
public function addFileContents(string $path, string $content): bool {
public function addFileContents(string $path, string $content): void {
$stream = fopen('php://temp', 'r+');
fwrite($stream, $content);
rewind($stream);
$this->streamer->addFileFromStream($stream, $path);
return true;
}

/**
* {@inheritDoc}
*/
public function addFileAsStream(string $path, $stream): bool {
public function addFileAsStream(string $path, $stream): void {
$this->streamer->addFileFromStream($stream, $path);
return true;
}

/**
* {@inheritDoc}
*/
public function copyFolder(Folder $folder, string $destinationPath): bool {
public function copyFolder(Folder $folder, string $destinationPath): void {
$this->streamer->addEmptyDir($destinationPath, [
'timestamp' => $folder->getMTime(),
]);
Expand All @@ -92,22 +90,19 @@ public function copyFolder(Folder $folder, string $destinationPath): bool {
'timestamp' => $node->getMTime(),
]);
} elseif ($node instanceof Folder) {
$success = $this->copyFolder($node, $destinationPath.'/'.$node->getName());
if ($success === false) {
return false;
}
$this->copyFolder($node, $destinationPath.'/'.$node->getName());
} else {
return false;
// ignore unknown node type, shouldn't happen
continue;
}
}
return true;
}

/**
* {@inheritDoc}
*/
public function setMigratorVersions(array $versions): bool {
return $this->addFileContents("migrator_versions.json", json_encode($versions));
public function setMigratorVersions(array $versions): void {
$this->addFileContents("migrator_versions.json", json_encode($versions));
}

/**
Expand Down
17 changes: 5 additions & 12 deletions lib/ImportSource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public function pathExists(string $path): bool {
/**
* {@inheritDoc}
*/
public function copyToFolder(Folder $destination, string $sourcePath): bool {
public function copyToFolder(Folder $destination, string $sourcePath): void {
// TODO log errors to ease debugging
$sourcePath = rtrim($sourcePath, '/').'/';
$files = $this->archive->getFolder($sourcePath);
Expand All @@ -99,9 +99,7 @@ public function copyToFolder(Folder $destination, string $sourcePath): bool {
foreach ($files as $path) {
$stat = $this->archive->getStat($sourcePath . $path);
if ($stat === null) {
// TODO: use exception
echo "Stat information not found for " . $sourcePath . $path . "\n";
return false;
throw new UserMigrationException("Failed to get stat information from archive for " . $sourcePath . $path . "\"");
}
if (str_ends_with($path, '/')) {
try {
Expand All @@ -113,16 +111,12 @@ public function copyToFolder(Folder $destination, string $sourcePath): bool {
} catch (NotFoundException $e) {
$folder = $destination->newFolder($path);
}
if ($this->copyToFolder($folder, $sourcePath.$path) === false) {
// TODO: use exception
echo "copy to $sourcePath.$path failed\n";
return false;
}
$this->copyToFolder($folder, $sourcePath.$path);
$folder->touch($stat['mtime']);
} else {
$stream = $this->archive->getStream($sourcePath.$path, 'r');
if ($stream === false) {
return false;
throw new UserMigrationException("Failed to get " . $sourcePath . $path . " from archive\"");
}
try {
$file = $destination->get($path);
Expand All @@ -139,9 +133,8 @@ public function copyToFolder(Folder $destination, string $sourcePath): bool {
}
}
} catch (NotPermittedException $e) {
return false;
throw new UserMigrationException("Could not import files due to permission issue", 0, $e);
}
return true;
}

/**
Expand Down
42 changes: 28 additions & 14 deletions lib/Migrator/FilesMigrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,19 @@ public function export(
$uid = $user->getUID();
$userFolder = $this->root->getUserFolder($uid);

if ($exportDestination->copyFolder($userFolder, static::PATH_FILES) === false) {
throw new UserMigrationException("Could not export files.");
try {
$exportDestination->copyFolder($userFolder, static::PATH_FILES);
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export files.", 0, $e);
}

try {
$versionsFolder = $this->root->get('/'.$uid.'/'.FilesVersionsStorage::VERSIONS_ROOT);
$output->writeln("Exporting file versions…");
if ($exportDestination->copyFolder($versionsFolder, static::PATH_VERSIONS) === false) {
throw new UserMigrationException("Could not export files versions.");
try {
$exportDestination->copyFolder($versionsFolder, static::PATH_VERSIONS);
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export files versions.", 0, $e);
}
} catch (NotFoundException $e) {
$output->writeln("No file versions to export…");
Expand All @@ -121,8 +125,10 @@ public function export(
$tagger = $this->tagManager->load(Application::APP_ID, [], false, $uid);
$tags = $tagger->getTagsForObjects(array_values($objectIds));
$taggedFiles = array_filter(array_map(fn ($id) => $tags[$id] ?? [], $objectIds));
if ($exportDestination->addFileContents(static::PATH_TAGS, json_encode($taggedFiles)) === false) {
throw new UserMigrationException("Could not export tagged files information.");
try {
$exportDestination->addFileContents(static::PATH_TAGS, json_encode($taggedFiles));
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export tagged files information.", 0, $e);
}

$output->writeln("Exporting file systemtags…");
Expand All @@ -136,8 +142,10 @@ public function export(
$systemTags
);
$systemTaggedFiles = array_filter(array_map(fn ($id) => $systemTags[$id] ?? [], $objectIds));
if ($exportDestination->addFileContents(static::PATH_SYSTEMTAGS, json_encode($systemTaggedFiles)) === false) {
throw new UserMigrationException("Could not export systemtagged files information.");
try {
$exportDestination->addFileContents(static::PATH_SYSTEMTAGS, json_encode($systemTaggedFiles));
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export systemtagged files information.", 0, $e);
}

$output->writeln("Exporting file comments…");
Expand All @@ -160,8 +168,10 @@ function (IComment $comment): array {
);
}
}
if ($exportDestination->addFileContents(static::PATH_COMMENTS, json_encode($comments)) === false) {
throw new UserMigrationException("Could not export file comments.");
try {
$exportDestination->addFileContents(static::PATH_COMMENTS, json_encode($comments));
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export file comments.", 0, $e);
}

// TODO other files metadata should be exported as well if relevant.
Expand Down Expand Up @@ -197,8 +207,10 @@ public function import(

$uid = $user->getUID();

if ($importSource->copyToFolder($this->root->getUserFolder($uid), static::PATH_FILES) === false) {
throw new UserMigrationException("Could not import files.");
try {
$importSource->copyToFolder($this->root->getUserFolder($uid), static::PATH_FILES);
} catch (\Throwable $e) {
throw new UserMigrationException("Could not import files.", 0, $e);
}

$userFolder = $this->root->getUserFolder($uid);
Expand All @@ -210,8 +222,10 @@ public function import(
$versionsFolder = $this->root->newFolder('/'.$uid.'/'.FilesVersionsStorage::VERSIONS_ROOT);
}
$output->writeln("Importing file versions…");
if ($importSource->copyToFolder($versionsFolder, static::PATH_VERSIONS) === false) {
throw new UserMigrationException("Could not import files versions.");
try {
$importSource->copyToFolder($versionsFolder, static::PATH_VERSIONS);
} catch (\Throwable $e) {
throw new UserMigrationException("Could not import files versions.", 0, $e);
}
} else {
$output->writeln("No file versions to import…");
Expand Down
24 changes: 16 additions & 8 deletions lib/Service/UserMigrationService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ public function export(IExportDestination $exportDestination, IUser $user, ?arra
$migrator->export($user, $exportDestination, $output);
$migratorVersions[$migrator->getId()] = $migrator->getVersion();
}
if ($exportDestination->setMigratorVersions($migratorVersions) === false) {
throw new UserMigrationException("Could not export user information.");
try {
$exportDestination->setMigratorVersions($migratorVersions);
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export user information.", 0, $e);
}

$exportDestination->close();
Expand Down Expand Up @@ -169,8 +171,10 @@ protected function exportUserInformation(IUser $user,
'enabled' => $user->isEnabled(),
];

if ($exportDestination->addFileContents(IImportSource::PATH_USER, json_encode($userinfo)) === false) {
throw new UserMigrationException("Could not export user information.");
try {
$exportDestination->addFileContents(IImportSource::PATH_USER, json_encode($userinfo));
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export user information.", 0, $e);
}
}

Expand Down Expand Up @@ -214,8 +218,10 @@ protected function exportVersions(string $uid,
\OC_App::getAppVersions()
);

if ($exportDestination->addFileContents("versions.json", json_encode($versions)) === false) {
throw new UserMigrationException("Could not export versions.");
try {
$exportDestination->addFileContents("versions.json", json_encode($versions));
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export versions.", 0, $e);
}
}

Expand All @@ -229,8 +235,10 @@ protected function exportAppsSettings(string $uid,

$data = $this->config->getAllUserValues($uid);

if ($exportDestination->addFileContents("settings.json", json_encode($data)) === false) {
throw new UserMigrationException("Could not export settings.");
try {
$exportDestination->addFileContents("settings.json", json_encode($data));
} catch (\Throwable $e) {
throw new UserMigrationException("Could not export settings.", 0, $e);
}
}

Expand Down