From 8e720f1147ce67093a8b7fdc3346daf34588f1ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 11 Aug 2023 18:09:25 +0200 Subject: [PATCH] fix: Add output for individual failures or skipped parts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Db/Assignment.php | 13 ++ .../Importer/BoardImportCommandService.php | 69 ++++++---- lib/Service/Importer/BoardImportService.php | 120 +++++++++++++----- tests/stub.phpstub | 2 + .../Importer/BoardImportServiceTest.php | 7 +- 5 files changed, 157 insertions(+), 54 deletions(-) diff --git a/lib/Db/Assignment.php b/lib/Db/Assignment.php index 56207d36a..282581d58 100644 --- a/lib/Db/Assignment.php +++ b/lib/Db/Assignment.php @@ -41,4 +41,17 @@ class Assignment extends RelationalEntity implements JsonSerializable { $this->addType('type', 'integer'); $this->addResolvable('participant'); } + + public function getTypeString(): string { + switch ($this->getType()) { + case self::TYPE_USER: + return 'user'; + case self::TYPE_GROUP: + return 'group'; + case self::TYPE_CIRCLE: + return 'circle'; + } + + return 'unknown'; + } } diff --git a/lib/Service/Importer/BoardImportCommandService.php b/lib/Service/Importer/BoardImportCommandService.php index 23257e499..64d57622c 100644 --- a/lib/Service/Importer/BoardImportCommandService.php +++ b/lib/Service/Importer/BoardImportCommandService.php @@ -25,7 +25,6 @@ namespace OCA\Deck\Service\Importer; use OCA\Deck\Exceptions\ConflictException; use OCA\Deck\NotFoundException; -use OCA\Deck\Service\Importer\Systems\DeckJsonService; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -78,11 +77,12 @@ class BoardImportCommandService extends BoardImportService { protected function validateConfig(): void { // FIXME: Make config optional for deck plain importer (but use a call on the importer insterad) - if ($this->getImportSystem() instanceof DeckJsonService) { - return; - } try { $config = $this->getInput()->getOption('config'); + if (!$config) { + return; + } + if (is_string($config)) { if (!is_file($config)) { throw new NotFoundException('It\'s not a valid config file.'); @@ -191,33 +191,56 @@ class BoardImportCommandService extends BoardImportService { public function bootstrap(): void { $this->setSystem($this->getInput()->getOption('system')); parent::bootstrap(); + + $this->registerErrorCollector(function ($error, $exception) { + $message = $error; + if ($exception instanceof \Throwable) { + $message .= ': ' . $exception->getMessage(); + } + $this->getOutput()->writeln('' . $message . ''); + if ($exception instanceof \Throwable && $this->getOutput()->isVeryVerbose()) { + $this->getOutput()->writeln($exception->getTraceAsString()); + } + }); + + $this->registerOutputCollector(function ($info) { + if ($this->getOutput()->isVerbose()) { + $this->getOutput()->writeln('' . $info . '', ); + } + }); } public function import(): void { $this->getOutput()->writeln('Starting import...'); $this->bootstrap(); + $this->validateSystem(); + $this->validateConfig(); $boards = $this->getImportSystem()->getBoards(); foreach ($boards as $board) { - $this->reset(); - $this->setData($board); - $this->getOutput()->writeln('Importing board "' . $this->getBoard()->getTitle() . '".'); - $this->importBoard(); - $this->getOutput()->writeln('Assign users to board...'); - $this->importAcl(); - $this->getOutput()->writeln('Importing labels...'); - $this->importLabels(); - $this->getOutput()->writeln('Importing stacks...'); - $this->importStacks(); - $this->getOutput()->writeln('Importing cards...'); - $this->importCards(); - $this->getOutput()->writeln('Assign cards to labels...'); - $this->assignCardsToLabels(); - $this->getOutput()->writeln('Importing comments...'); - $this->importComments(); - $this->getOutput()->writeln('Importing participants...'); - $this->importCardAssignments(); - $this->getOutput()->writeln('Finished board import of "' . $this->getBoard()->getTitle() . '"'); + try { + $this->reset(); + $this->setData($board); + $this->getOutput()->writeln('Importing board "' . $board->title . '".'); + $this->importBoard(); + $this->getOutput()->writeln('Assign users to board...'); + $this->importAcl(); + $this->getOutput()->writeln('Importing labels...'); + $this->importLabels(); + $this->getOutput()->writeln('Importing stacks...'); + $this->importStacks(); + $this->getOutput()->writeln('Importing cards...'); + $this->importCards(); + $this->getOutput()->writeln('Assign cards to labels...'); + $this->assignCardsToLabels(); + $this->getOutput()->writeln('Importing comments...'); + $this->importComments(); + $this->getOutput()->writeln('Importing participants...'); + $this->importCardAssignments(); + $this->getOutput()->writeln('Finished board import of "' . $this->getBoard()->getTitle() . '"'); + } catch (\Exception $e) { + $this->output->writeln('Import failed for board ' . $board->title . ': ' . $e->getMessage() . ''); + } } } } diff --git a/lib/Service/Importer/BoardImportService.php b/lib/Service/Importer/BoardImportService.php index ba08c7a93..b4468b7b7 100644 --- a/lib/Service/Importer/BoardImportService.php +++ b/lib/Service/Importer/BoardImportService.php @@ -49,6 +49,7 @@ use OCP\Comments\NotFoundException as CommentNotFoundException; use OCP\EventDispatcher\IEventDispatcher; use OCP\IUserManager; use OCP\Server; +use Psr\Log\LoggerInterface; class BoardImportService { private string $system = ''; @@ -70,6 +71,11 @@ class BoardImportService { private $data; private Board $board; + /** @var callable[] */ + private array $errorCollectors = []; + /** @var callable[] */ + private array $outputCollectors = []; + public function __construct( private IUserManager $userManager, private BoardMapper $boardMapper, @@ -80,7 +86,8 @@ class BoardImportService { private AttachmentMapper $attachmentMapper, private CardMapper $cardMapper, private ICommentsManager $commentsManager, - private IEventDispatcher $eventDispatcher + private IEventDispatcher $eventDispatcher, + private LoggerInterface $logger ) { $this->board = new Board(); $this->disableCommentsEvents(); @@ -88,6 +95,28 @@ class BoardImportService { $this->config = new \stdClass(); } + public function registerErrorCollector(callable $errorCollector): void { + $this->errorCollectors[] = $errorCollector; + } + + public function registerOutputCollector(callable $outputCollector): void { + $this->outputCollectors[] = $outputCollector; + } + + private function addError(string $message, $exception): void { + $message .= ' (on board ' . $this->getBoard()->getTitle() . ')'; + foreach ($this->errorCollectors as $errorCollector) { + $errorCollector($message, $exception); + } + $this->logger->error($message, ['exception' => $exception]); + } + + private function addOutput(string $message): void { + foreach ($this->outputCollectors as $outputCollector) { + $outputCollector($message); + } + } + private function disableCommentsEvents(): void { if (defined('PHPUNIT_RUN')) { return; @@ -117,6 +146,7 @@ class BoardImportService { $this->importComments(); $this->importCardAssignments(); } catch (\Throwable $th) { + $this->logger->error('Failed to import board', ['exception' => $th]); throw new BadRequestException($th->getMessage()); } } @@ -195,6 +225,10 @@ class BoardImportService { public function importBoard(): void { $board = $this->getImportSystem()->getBoard(); + if (!$this->userManager->userExists($board->getOwner())) { + throw new \Exception('Target owner ' . $board->getOwner() . ' not found. Please provide a mapping through the import config.'); + } + if ($board) { $this->boardMapper->insert($board); $this->board = $board; @@ -211,8 +245,13 @@ class BoardImportService { public function importAcl(): void { $aclList = $this->getImportSystem()->getAclList(); foreach ($aclList as $code => $acl) { - $this->aclMapper->insert($acl); - $this->getImportSystem()->updateAcl($code, $acl); + try { + $this->aclMapper->insert($acl); + $this->getImportSystem()->updateAcl($code, $acl); + } catch (\Exception $e) { + $this->addError('Failed to import acl rule for ' . $acl->getParticipant(), $e); + + } } $this->getBoard()->setAcl($aclList); } @@ -220,8 +259,12 @@ class BoardImportService { public function importLabels(): void { $labels = $this->getImportSystem()->getLabels(); foreach ($labels as $code => $label) { - $this->labelMapper->insert($label); - $this->getImportSystem()->updateLabel($code, $label); + try { + $this->labelMapper->insert($label); + $this->getImportSystem()->updateLabel($code, $label); + } catch (\Exception $e) { + $this->addError('Failed to import label ' . $label->getTitle(), $e); + } } $this->getBoard()->setLabels($labels); } @@ -229,8 +272,12 @@ class BoardImportService { public function importStacks(): void { $stacks = $this->getImportSystem()->getStacks(); foreach ($stacks as $code => $stack) { - $this->stackMapper->insert($stack); - $this->getImportSystem()->updateStack($code, $stack); + try { + $this->stackMapper->insert($stack); + $this->getImportSystem()->updateStack($code, $stack); + } catch (\Exception $e) { + $this->addError('Failed to import list ' . $stack->getTitle(), $e); + } } $this->getBoard()->setStacks(array_values($stacks)); } @@ -238,22 +285,26 @@ class BoardImportService { public function importCards(): void { $cards = $this->getImportSystem()->getCards(); foreach ($cards as $code => $card) { - $createdAt = $card->getCreatedAt(); - $lastModified = $card->getLastModified(); - $this->cardMapper->insert($card); - $updateDate = false; - if ($createdAt && $createdAt !== $card->getCreatedAt()) { - $card->setCreatedAt($createdAt); - $updateDate = true; + try { + $createdAt = $card->getCreatedAt(); + $lastModified = $card->getLastModified(); + $this->cardMapper->insert($card); + $updateDate = false; + if ($createdAt && $createdAt !== $card->getCreatedAt()) { + $card->setCreatedAt($createdAt); + $updateDate = true; + } + if ($lastModified && $lastModified !== $card->getLastModified()) { + $card->setLastModified($lastModified); + $updateDate = true; + } + if ($updateDate) { + $this->cardMapper->update($card, false); + } + $this->getImportSystem()->updateCard($code, $card); + } catch (\Exception $e) { + $this->addError('Failed to import card ' . $card->getTitle(), $e); } - if ($lastModified && $lastModified !== $card->getLastModified()) { - $card->setLastModified($lastModified); - $updateDate = true; - } - if ($updateDate) { - $this->cardMapper->update($card, false); - } - $this->getImportSystem()->updateCard($code, $card); } } @@ -274,11 +325,15 @@ class BoardImportService { $data = $this->getImportSystem()->getCardLabelAssignment(); foreach ($data as $cardId => $assignemnt) { foreach ($assignemnt as $assignmentId => $labelId) { - $this->assignCardToLabel( - $cardId, - $labelId - ); - $this->getImportSystem()->updateCardLabelsAssignment($cardId, $assignmentId, $labelId); + try { + $this->assignCardToLabel( + $cardId, + $labelId + ); + $this->getImportSystem()->updateCardLabelsAssignment($cardId, $assignmentId, $labelId); + } catch (\Exception $e) { + $this->addError('Failed to assign label ' . $labelId . ' to ' . $cardId, $e); + } } } } @@ -320,9 +375,14 @@ class BoardImportService { public function importCardAssignments(): void { $allAssignments = $this->getImportSystem()->getCardAssignments(); foreach ($allAssignments as $cardId => $assignments) { - foreach ($assignments as $assignmentId => $assignment) { - $this->assignmentMapper->insert($assignment); - $this->getImportSystem()->updateCardAssignment($cardId, $assignmentId, $assignment); + foreach ($assignments as $assignment) { + try { + $assignment = $this->assignmentMapper->insert($assignment); + $this->getImportSystem()->updateCardAssignment($cardId, (string)$assignment->getId(), $assignment); + $this->addOutput('Assignment ' . $assignment->getParticipant() . ' added'); + } catch (NotFoundException $e) { + $this->addError('No origin or mapping found for card "' . $cardId . '" and ' . $assignment->getTypeString() .' assignment "' . $assignment->getParticipant(), $e); + } } } } diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 30338e083..d09bcb153 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -193,6 +193,8 @@ namespace Symfony\Component\Console\Output { class OutputInterface { public const VERBOSITY_VERBOSE = 1; public function writeln($text, int $flat = 0) {} + public function isVerbose(): bool {} + public function isVeryVerbose(): bool {} } } diff --git a/tests/unit/Service/Importer/BoardImportServiceTest.php b/tests/unit/Service/Importer/BoardImportServiceTest.php index c652599b0..d367178ac 100644 --- a/tests/unit/Service/Importer/BoardImportServiceTest.php +++ b/tests/unit/Service/Importer/BoardImportServiceTest.php @@ -43,6 +43,7 @@ use OCP\IDBConnection; use OCP\IUser; use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; class BoardImportServiceTest extends \Test\TestCase { /** @var IDBConnection|MockObject */ @@ -92,7 +93,8 @@ class BoardImportServiceTest extends \Test\TestCase { $this->attachmentMapper, $this->cardMapper, $this->commentsManager, - $this->eventDispatcher + $this->eventDispatcher, + $this->createMock(LoggerInterface::class), ); $this->boardImportService->setSystem('trelloJson'); @@ -145,6 +147,9 @@ class BoardImportServiceTest extends \Test\TestCase { } public function testImportSuccess() { + $this->userManager->method('userExists') + ->willReturn(true); + $this->boardMapper ->expects($this->once()) ->method('insert');