From 162f5cb3b4e1856b46af0a2f225868adcb9258ab Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 30 Oct 2026 12:17:18 +0100 Subject: [PATCH 1/4] feat(import): keep references of imported objects An importing app that runs again, or follows changes in its source, must find the boards, stacks, labels, cards and shares it wrote before. deck_import_refs maps the app's own reference (by its SHA-256) to the Deck object, with the hash of what was last written, so later imports can tell whether somebody changed the object in Deck since. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- appinfo/info.xml | 2 +- lib/Db/ImportRef.php | 76 +++++++++++++++ lib/Db/ImportRefMapper.php | 96 +++++++++++++++++++ .../Version11002Date20261030120000.php | 92 ++++++++++++++++++ 4 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 lib/Db/ImportRef.php create mode 100644 lib/Db/ImportRefMapper.php create mode 100644 lib/Migration/Version11002Date20261030120000.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 8f696b737b..490974539d 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -20,7 +20,7 @@ - 🚀 Get your project organized - 2.0.0-dev.0 + 2.0.0-dev.1 agpl Julius Härtl Deck diff --git a/lib/Db/ImportRef.php b/lib/Db/ImportRef.php new file mode 100644 index 0000000000..f8cee2622a --- /dev/null +++ b/lib/Db/ImportRef.php @@ -0,0 +1,76 @@ +addType('id', 'integer'); + $this->addType('boardId', 'integer'); + $this->addType('objectId', 'integer'); + $this->addType('state', 'integer'); + $this->addType('createdAt', 'integer'); + $this->addType('updatedAt', 'integer'); + } + + public static function hashRef(string $ref): string { + return hash('sha256', $ref); + } +} diff --git a/lib/Db/ImportRefMapper.php b/lib/Db/ImportRefMapper.php new file mode 100644 index 0000000000..f956198330 --- /dev/null +++ b/lib/Db/ImportRefMapper.php @@ -0,0 +1,96 @@ + */ +class ImportRefMapper extends QBMapper { + public function __construct(IDBConnection $db) { + parent::__construct($db, 'deck_import_refs', ImportRef::class); + } + + public function findByRef(string $app, string $kind, int $boardId, string $ref): ?ImportRef { + return $this->findByRefs($app, $kind, $boardId, [$ref])[$ref] ?? null; + } + + /** + * @param list $refs + * @return array by ref + */ + public function findByRefs(string $app, string $kind, int $boardId, array $refs): array { + $found = []; + foreach (array_chunk(array_values(array_unique($refs)), 1000) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('app', $qb->createNamedParameter($app))) + ->andWhere($qb->expr()->eq('kind', $qb->createNamedParameter($kind))) + ->andWhere($qb->expr()->eq('board_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->in('ref_hash', $qb->createNamedParameter(array_map(ImportRef::hashRef(...), $chunk), IQueryBuilder::PARAM_STR_ARRAY))); + foreach ($this->findEntities($qb) as $importRef) { + // The hash selects the row, the stored reference confirms it + if (in_array($importRef->getRef(), $chunk, true)) { + $found[$importRef->getRef()] = $importRef; + } + } + } + return $found; + } + + /** + * @return list + */ + public function findAllOfKind(string $app, string $kind, int $boardId): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('app', $qb->createNamedParameter($app))) + ->andWhere($qb->expr()->eq('kind', $qb->createNamedParameter($kind))) + ->andWhere($qb->expr()->eq('board_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) + ->orderBy('id'); + return $this->findEntities($qb); + } + + /** + * The reference of a board an importing app created, whichever app it was + */ + public function findBoard(int $boardId): ?ImportRef { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('kind', $qb->createNamedParameter(ImportRef::KIND_BOARD))) + ->andWhere($qb->expr()->eq('object_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) + ->setMaxResults(1); + $entities = $this->findEntities($qb); + return $entities[0] ?? null; + } + + /** + * Active card references of a board in the order they were first written + * + * @return list + */ + public function findCardPage(string $app, int $boardId, int $afterId, int $limit): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where($qb->expr()->eq('app', $qb->createNamedParameter($app))) + ->andWhere($qb->expr()->eq('kind', $qb->createNamedParameter(ImportRef::KIND_CARD))) + ->andWhere($qb->expr()->eq('board_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('state', $qb->createNamedParameter(ImportRef::STATE_ACTIVE, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->gt('id', $qb->createNamedParameter($afterId, IQueryBuilder::PARAM_INT))) + ->orderBy('id') + ->setMaxResults($limit); + return $this->findEntities($qb); + } +} diff --git a/lib/Migration/Version11002Date20261030120000.php b/lib/Migration/Version11002Date20261030120000.php new file mode 100644 index 0000000000..b15024fdc5 --- /dev/null +++ b/lib/Migration/Version11002Date20261030120000.php @@ -0,0 +1,92 @@ +hasTable('deck_import_refs')) { + return null; + } + + $table = $schema->createTable('deck_import_refs'); + $table->addColumn('id', Types::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 20, + ]); + $table->addColumn('app', Types::STRING, [ + 'notnull' => true, + 'length' => 32, + ]); + $table->addColumn('kind', Types::STRING, [ + 'notnull' => true, + 'length' => 8, + ]); + // The board an object belongs to; 0 for boards themselves + $table->addColumn('board_id', Types::BIGINT, [ + 'notnull' => true, + 'length' => 20, + 'default' => 0, + ]); + $table->addColumn('ref', Types::STRING, [ + 'notnull' => true, + 'length' => 255, + ]); + $table->addColumn('ref_hash', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('object_id', Types::BIGINT, [ + 'notnull' => true, + 'length' => 20, + ]); + $table->addColumn('source_hash', Types::STRING, [ + 'notnull' => false, + 'length' => 64, + ]); + $table->addColumn('stored_hash', Types::STRING, [ + 'notnull' => false, + 'length' => 64, + ]); + $table->addColumn('state', Types::SMALLINT, [ + 'notnull' => true, + 'default' => 0, + ]); + $table->addColumn('created_at', Types::BIGINT, [ + 'notnull' => true, + 'length' => 20, + 'default' => 0, + ]); + $table->addColumn('updated_at', Types::BIGINT, [ + 'notnull' => true, + 'length' => 20, + 'default' => 0, + ]); + $table->setPrimaryKey(['id']); + $table->addUniqueIndex(['app', 'kind', 'board_id', 'ref_hash'], 'deck_import_refs_uq'); + $table->addIndex(['kind', 'object_id'], 'deck_import_refs_obj'); + + return $schema; + } +} From fe743a9cb5b6cbeb55b18426a2ae52cfb92635a1 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 30 Oct 2026 12:17:18 +0100 Subject: [PATCH 2/4] feat(import): quiet board import API for migration tools occ deck:import creates a new board on every run and keeps no map from source IDs, so a migration can't run again, follow changes or verify what it wrote. IBoardImporter lets an app create boards with labels and stacks, write cards with their original creation, start, due and done times, assignments, labels and existing files as attachments, list, stat and soft-delete them and manage the board's shares, all keyed by its own references and safe to repeat. It writes through the mappers, so no activity, notification, mention or Deck event is created; full-text search, collaboration resources, share access, etags and the audit log are kept right through Deck's own listeners. Cards that were due before the import never announce themselves as overdue. A card somebody changed in Deck answers "changed" and keeps their version. Both sides hash a canonical form of each card (CardCanonicalForm) to verify it. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/AppInfo/Application.php | 4 + lib/Import/BoardContext.php | 47 ++ lib/Import/BoardImporter.php | 1095 +++++++++++++++++++++++++++++ lib/Import/CardCanonicalForm.php | 70 ++ lib/Import/EntryException.php | 32 + lib/Import/IBoardImporter.php | 106 +++ lib/Import/ImportAccess.php | 40 ++ lib/Import/ImportCard.php | 53 ++ lib/Import/ImportCardStat.php | 46 ++ lib/Import/ImportException.php | 41 ++ lib/Import/ImportLabel.php | 29 + lib/Import/ImportResult.php | 82 +++ lib/Import/ImportStack.php | 28 + lib/Service/PermissionService.php | 4 +- 14 files changed, 1675 insertions(+), 2 deletions(-) create mode 100644 lib/Import/BoardContext.php create mode 100644 lib/Import/BoardImporter.php create mode 100644 lib/Import/CardCanonicalForm.php create mode 100644 lib/Import/EntryException.php create mode 100644 lib/Import/IBoardImporter.php create mode 100644 lib/Import/ImportAccess.php create mode 100644 lib/Import/ImportCard.php create mode 100644 lib/Import/ImportCardStat.php create mode 100644 lib/Import/ImportException.php create mode 100644 lib/Import/ImportLabel.php create mode 100644 lib/Import/ImportResult.php create mode 100644 lib/Import/ImportStack.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 6b6db49c84..3397f0ccb4 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -29,6 +29,8 @@ use OCA\Deck\Event\SessionClosedEvent; use OCA\Deck\Event\SessionCreatedEvent; use OCA\Deck\Federation\DeckFederationProvider; +use OCA\Deck\Import\BoardImporter; +use OCA\Deck\Import\IBoardImporter; use OCA\Deck\Listeners\AclCreatedRemovedListener; use OCA\Deck\Listeners\BeforeTemplateRenderedListener; use OCA\Deck\Listeners\CommentEventListener; @@ -138,6 +140,8 @@ public function register(IRegistrationContext $context): void { $context->registerMiddleWare(ExceptionMiddleware::class); $context->registerMiddleWare(DefaultBoardMiddleware::class); + $context->registerServiceAlias(IBoardImporter::class, BoardImporter::class); + $context->registerService('databaseType', static function (ContainerInterface $c) { return $c->get(IConfig::class)->getSystemValue('dbtype', 'sqlite'); }); diff --git a/lib/Import/BoardContext.php b/lib/Import/BoardContext.php new file mode 100644 index 0000000000..946de61247 --- /dev/null +++ b/lib/Import/BoardContext.php @@ -0,0 +1,47 @@ + $stackIds by ref + * @param array $labelIds by ref + */ + public function __construct( + public readonly Board $board, + public readonly array $stackIds, + public readonly array $labelIds, + ) { + } + + /** + * The reference of a stack, or a stand-in for one the importer didn't write + */ + public function stackRef(int $stackId): string { + $ref = array_search($stackId, $this->stackIds, true); + return $ref === false ? 'deck:' . $stackId : (string)$ref; + } + + /** + * The reference of a label, or a stand-in for one the importer didn't write + */ + public function labelRef(int $labelId): string { + $ref = array_search($labelId, $this->labelIds, true); + return $ref === false ? 'deck:' . $labelId : (string)$ref; + } +} diff --git a/lib/Import/BoardImporter.php b/lib/Import/BoardImporter.php new file mode 100644 index 0000000000..da923226cb --- /dev/null +++ b/lib/Import/BoardImporter.php @@ -0,0 +1,1095 @@ +checkApp($app); + $this->checkRef('ref', $ref); + $this->checkTitle('title', $title, self::MAX_BOARD_TITLE); + $this->checkColor('color', $color); + $this->checkLabels($labels); + $this->checkStacks($stacks); + if (!$this->userManager->userExists($ownerUid)) { + throw EntryException::error(ImportResult::ERROR_UNKNOWN_USER, 'The owner is not a user'); + } + } catch (EntryException $e) { + return [ + 'status' => ImportResult::STATUS_ERROR, + 'board_id' => 0, + 'created' => false, + 'labels' => [], + 'stacks' => [], + 'changed' => [], + 'error' => ['code' => (string)$e->result->errorCode, 'message' => $e->getMessage()], + ]; + } + + return $this->retryOnUniqueViolation(fn () => $this->atomic( + fn () => $this->writeBoard($app, $ref, $ownerUid, $title, $color, $labels, $stacks), + $this->db, + )); + } + + /** + * @param list $labels + * @param list $stacks + * @return array{status: string, board_id: int, created: bool, labels: array, stacks: array, changed: list, error?: array{code: string, message: string}} + */ + private function writeBoard(string $app, string $ref, string $ownerUid, string $title, string $color, array $labels, array $stacks): array { + $now = $this->timeFactory->getTime(); + $changed = []; + $created = false; + $boardRef = $this->refMapper->findByRef($app, ImportRef::KIND_BOARD, 0, $ref); + if ($boardRef === null) { + $board = new Board(); + $board->setTitle($title); + $board->setOwner($ownerUid); + $board->setColor($color); + $board->setArchived(false); + $board->setDeletedAt(0); + $board->setLastModified($now); + $board = $this->boardMapper->insert($board); + $this->insertRef($app, ImportRef::KIND_BOARD, 0, $ref, $board->getId(), self::valuesHash($title, $color)); + $created = true; + } else { + try { + $board = $this->boardMapper->find($boardRef->getObjectId(), false, false, true); + } catch (DoesNotExistException) { + $board = null; + } + if ($board === null || $board->getDeletedAt() > 0) { + return [ + 'status' => ImportResult::STATUS_CONFLICT, + 'board_id' => $boardRef->getObjectId(), + 'created' => false, + 'labels' => [], + 'stacks' => [], + 'changed' => [$ref], + 'error' => ['code' => ImportResult::ERROR_CHANGED, 'message' => 'The board was deleted in Deck'], + ]; + } + $current = self::valuesHash($board->getTitle(), $board->getColor()); + if ($current !== $boardRef->getStoredHash()) { + $changed[] = $ref; + } elseif ($current !== self::valuesHash($title, $color)) { + $board->setTitle($title); + $board->setColor($color); + $board->setLastModified($now); + $this->boardMapper->update($board); + $this->updateRef($boardRef, self::valuesHash($title, $color)); + } + } + $boardId = $board->getId(); + + [$labelIds, $labelsChanged] = $this->ensureLabels($app, $boardId, $labels); + [$stackIds, $stacksChanged] = $this->ensureStacks($app, $boardId, $stacks); + + $this->boardMapper->flushCache($boardId); + $this->changeHelper->boardChanged($boardId); + if ($created) { + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'Board %s of "%s" was imported by %s', + [ + 'boardId' => $boardId, + 'owner' => $ownerUid, + 'importSystem' => $app, + ], + false, + Operation::BoardImported->value, + )); + } + + return [ + 'status' => ImportResult::STATUS_OK, + 'board_id' => $boardId, + 'created' => $created, + 'labels' => $labelIds, + 'stacks' => $stackIds, + 'changed' => array_merge($changed, $labelsChanged, $stacksChanged), + ]; + } + + /** + * @param list $labels + * @return array{0: array, 1: list} the label IDs by ref and the refs whose Deck version was kept + */ + private function ensureLabels(string $app, int $boardId, array $labels): array { + $refs = $this->refMapper->findByRefs($app, ImportRef::KIND_LABEL, $boardId, array_map(fn (ImportLabel $label) => $label->ref, $labels)); + $existing = []; + foreach ($this->labelMapper->findAll($boardId) as $label) { + $existing[$label->getId()] = $label; + } + + $ids = []; + $changed = []; + foreach ($labels as $wanted) { + $labelRef = $refs[$wanted->ref] ?? null; + if ($labelRef === null) { + $adopted = null; + foreach ($existing as $label) { + if ($label->getTitle() === $wanted->title) { + $adopted = $label; + break; + } + } + if ($adopted !== null) { + // A label somebody made with the same title is used as it is + $this->insertRef($app, ImportRef::KIND_LABEL, $boardId, $wanted->ref, $adopted->getId(), ''); + $ids[$wanted->ref] = $adopted->getId(); + $changed[] = $wanted->ref; + continue; + } + $label = new Label(); + $label->setTitle($wanted->title); + $label->setColor($wanted->color); + $label->setBoardId($boardId); + $label = $this->labelMapper->insert($label); + $existing[$label->getId()] = $label; + $this->insertRef($app, ImportRef::KIND_LABEL, $boardId, $wanted->ref, $label->getId(), self::valuesHash($wanted->title, $wanted->color)); + $ids[$wanted->ref] = $label->getId(); + continue; + } + + $label = $existing[$labelRef->getObjectId()] ?? null; + if ($label === null) { + $changed[] = $wanted->ref; + continue; + } + $ids[$wanted->ref] = $label->getId(); + $current = self::valuesHash($label->getTitle(), $label->getColor()); + $next = self::valuesHash($wanted->title, $wanted->color); + if ($current !== $labelRef->getStoredHash() + || ($label->getTitle() !== $wanted->title && $this->labelMapper->existsByBoardIdAndTitle($boardId, $wanted->title, $label->getId()))) { + $changed[] = $wanted->ref; + } elseif ($current !== $next) { + $label->setTitle($wanted->title); + $label->setColor($wanted->color); + $this->labelMapper->update($label); + $this->updateRef($labelRef, $next); + } + } + return [$ids, $changed]; + } + + /** + * @param list $stacks + * @return array{0: array, 1: list} the stack IDs by ref and the refs whose Deck version was kept + */ + private function ensureStacks(string $app, int $boardId, array $stacks): array { + $refs = $this->refMapper->findByRefs($app, ImportRef::KIND_STACK, $boardId, array_map(fn (ImportStack $stack) => $stack->ref, $stacks)); + $existing = []; + foreach ($this->stackMapper->findAll($boardId) as $stack) { + $existing[$stack->getId()] = $stack; + } + + $ids = []; + $changed = []; + foreach ($stacks as $wanted) { + $stackRef = $refs[$wanted->ref] ?? null; + if ($stackRef === null) { + $stack = new Stack(); + $stack->setTitle($wanted->title); + $stack->setBoardId($boardId); + $stack->setOrder($wanted->order); + $stack->setLastModified($this->timeFactory->getTime()); + $stack = $this->stackMapper->insert($stack); + $this->insertRef($app, ImportRef::KIND_STACK, $boardId, $wanted->ref, $stack->getId(), self::valuesHash($wanted->title, $wanted->order)); + $ids[$wanted->ref] = $stack->getId(); + continue; + } + + $stack = $existing[$stackRef->getObjectId()] ?? null; + if ($stack === null) { + $changed[] = $wanted->ref; + continue; + } + $ids[$wanted->ref] = $stack->getId(); + $current = self::valuesHash($stack->getTitle(), $stack->getOrder()); + $next = self::valuesHash($wanted->title, $wanted->order); + if ($current !== $stackRef->getStoredHash()) { + $changed[] = $wanted->ref; + } elseif ($current !== $next) { + $stack->setTitle($wanted->title); + $stack->setOrder($wanted->order); + $stack->setLastModified($this->timeFactory->getTime()); + $this->stackMapper->update($stack); + $this->updateRef($stackRef, $next); + } + } + return [$ids, $changed]; + } + + #[\Override] + public function importCards(string $app, int $boardId, array $cards): array { + $context = $this->context($app, $boardId); + $seen = []; + $results = []; + foreach ($cards as $card) { + try { + if (!$card instanceof ImportCard) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'Not a card'); + } + $this->checkCard($card, $context, $seen); + $results[] = $this->retryOnUniqueViolation(fn () => $this->atomic( + fn () => $this->writeCard($app, $context, $card), + $this->db, + )); + } catch (EntryException $e) { + $results[] = $e->result; + } + } + return $results; + } + + /** + * @param array $seen the refs of the call so far + * @throws EntryException + */ + private function checkCard(ImportCard $card, BoardContext $context, array &$seen): void { + $this->checkRef('ref', $card->ref); + if (isset($seen[$card->ref])) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The ref appears twice'); + } + $seen[$card->ref] = true; + if (!isset($context->stackIds[$card->stackRef])) { + throw EntryException::error(ImportResult::ERROR_UNKNOWN_STACK, 'The stack is not on the board'); + } + $this->checkTitle('title', $card->title, Card::TITLE_MAX_LENGTH); + if ($card->mode !== ImportCard::MODE_CREATE && $card->mode !== ImportCard::MODE_NEW_VERSION) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The mode is invalid'); + } + if (preg_match('/^[0-9a-f]{64}$/', $card->sha256) !== 1) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The sha256 is not a lowercase hex SHA-256'); + } + if ($card->createdBy !== '' && !$this->userManager->userExists($card->createdBy)) { + throw EntryException::error(ImportResult::ERROR_UNKNOWN_USER, 'The creator is not a user'); + } + $this->checkList('assignees', $card->assignees, 'is_string'); + foreach ($card->assignees as $uid) { + if (!$this->userManager->userExists($uid)) { + throw EntryException::error(ImportResult::ERROR_UNKNOWN_USER, 'An assignee is not a user'); + } + } + $this->checkList('labels', $card->labels, 'is_string'); + foreach ($card->labels as $labelRef) { + if (!isset($context->labelIds[$labelRef])) { + throw EntryException::error(ImportResult::ERROR_UNKNOWN_LABEL, 'A label is not on the board'); + } + } + $this->checkList('files', $card->files, fn ($id) => is_int($id) && $id > 0); + } + + /** + * @throws EntryException + */ + private function writeCard(string $app, BoardContext $context, ImportCard $card): ImportResult { + $boardId = $context->board->getId(); + $cardRef = $this->refMapper->findByRef($app, ImportRef::KIND_CARD, $boardId, $card->ref); + if ($cardRef === null) { + if ($card->mode === ImportCard::MODE_NEW_VERSION) { + throw EntryException::error(ImportResult::ERROR_NOT_FOUND, 'The card was not imported'); + } + $entity = $this->insertCard($context, $card); + $cardRef = $this->insertRef($app, ImportRef::KIND_CARD, $boardId, $card->ref, $entity->getId(), null); + $this->writeRelations($context, $entity->getId(), $card); + $stored = $this->storedHash($context, $card->ref, $this->cardMapper->find($entity->getId(), false)); + $this->updateRef($cardRef, $stored, $card->sha256); + $this->cardWritten($entity, true); + return ImportResult::ok($entity->getId(), $stored); + } + + $entity = $this->findCard($cardRef->getObjectId()); + if ($entity === null) { + throw EntryException::conflict(ImportResult::ERROR_CHANGED, 'The card was deleted in Deck', $cardRef->getObjectId()); + } + $current = $this->storedHash($context, $card->ref, $entity); + if ($cardRef->getState() === ImportRef::STATE_REMOVED) { + if ($entity->getDeletedAt() === 0 || $current !== $cardRef->getStoredHash()) { + throw EntryException::conflict(ImportResult::ERROR_CHANGED, 'The card was restored or changed in Deck', $entity->getId(), $current); + } + $entity->setDeletedAt(0); + } else { + if ($this->changedInDeck($entity, $current, $cardRef)) { + throw EntryException::conflict(ImportResult::ERROR_CHANGED, 'The card was changed in Deck', $entity->getId(), $current); + } + if ($card->sha256 === $cardRef->getSourceHash()) { + return ImportResult::ok($entity->getId(), $current); + } + if ($card->mode === ImportCard::MODE_CREATE) { + throw EntryException::conflict(ImportResult::ERROR_EXISTS, 'The card exists with other content', $entity->getId(), $current); + } + } + + $this->updateCard($context, $entity, $card); + $this->writeRelations($context, $entity->getId(), $card); + $stored = $this->storedHash($context, $card->ref, $this->cardMapper->find($entity->getId(), false)); + $cardRef->setState(ImportRef::STATE_ACTIVE); + $this->updateRef($cardRef, $stored, $card->sha256); + $this->cardWritten($entity, false); + return ImportResult::ok($entity->getId(), $stored); + } + + private function insertCard(BoardContext $context, ImportCard $card): Card { + $entity = new Card(); + $entity->setTitle($card->title); + $entity->setDescription($card->description); + $entity->setStackId($context->stackIds[$card->stackRef]); + $entity->setType('plain'); + $entity->setOrder($card->order); + $entity->setOwner($card->createdBy !== '' ? $card->createdBy : $context->board->getOwner()); + $entity->setStartdate(self::mutable($card->start)); + $entity->setDuedate(self::mutable($card->due)); + $entity->setDone(self::mutable($card->done)); + $entity->setArchived(false); + $entity->setDeletedAt(0); + $entity->setNotified($this->isPast($card->due)); + $entity = $this->cardMapper->insert($entity); + + // The mapper stamps the insert time, the import keeps the source's + $entity->resetUpdatedFields(); + $entity->setCreatedAt($card->created->getTimestamp()); + $this->cardMapper->update($entity, false); + return $entity; + } + + private function updateCard(BoardContext $context, Card $entity, ImportCard $card): void { + $entity->setTitle($card->title); + $entity->setDescription($card->description); + $entity->setStackId($context->stackIds[$card->stackRef]); + $entity->setOrder($card->order); + $entity->setOwner($card->createdBy !== '' ? $card->createdBy : $context->board->getOwner()); + $entity->setCreatedAt($card->created->getTimestamp()); + // Only differing times are set: the mapper resets the due notification when the due date is set + if (self::time($entity->getStartdate()) !== self::time($card->start)) { + $entity->setStartdate(self::mutable($card->start)); + } + if (self::time($entity->getDuedate()) !== self::time($card->due)) { + $entity->setDuedate(self::mutable($card->due)); + } + if (self::time($entity->getDone()) !== self::time($card->done)) { + $entity->setDone(self::mutable($card->done)); + } + $this->cardMapper->update($entity); + if ($this->isPast($card->due)) { + $this->cardMapper->markNotified($entity); + } + } + + /** + * Labels, assignments and file attachments + * + * @throws EntryException + */ + private function writeRelations(BoardContext $context, int $cardId, ImportCard $card): void { + $wantedLabels = array_map(fn (string $ref) => $context->labelIds[$ref], $card->labels); + $currentLabels = array_map(fn (Label $label) => $label->getId(), $this->labelMapper->findAssignedLabelsForCard($cardId)); + foreach (array_diff($wantedLabels, $currentLabels) as $labelId) { + $this->cardMapper->assignLabel($cardId, $labelId); + } + foreach (array_diff($currentLabels, $wantedLabels) as $labelId) { + $this->cardMapper->removeLabel($cardId, $labelId); + } + + $currentAssignments = []; + foreach ($this->assignmentMapper->findAll($cardId) as $assignment) { + $currentAssignments[self::assigneeKey($assignment)] = $assignment; + } + foreach ($card->assignees as $uid) { + if (isset($currentAssignments[$uid])) { + unset($currentAssignments[$uid]); + continue; + } + $assignment = new Assignment(); + $assignment->setCardId($cardId); + $assignment->setParticipant($uid); + $assignment->setType(Assignment::TYPE_USER); + $this->assignmentMapper->insert($assignment); + } + foreach ($currentAssignments as $assignment) { + $this->assignmentMapper->delete($assignment); + } + + $currentFiles = $this->cardFiles($cardId); + foreach ($card->files as $fileId) { + if (isset($currentFiles[$fileId])) { + unset($currentFiles[$fileId]); + continue; + } + $this->attachFile($context->board, $cardId, $fileId); + } + foreach ($currentFiles as $shareId) { + $this->shareManager->deleteShare($this->shareProvider->getShareById((string)$shareId)); + } + } + + /** + * Shares an existing file with the card as its owner, which makes it a + * Deck attachment of type `file` + * + * @throws EntryException + */ + private function attachFile(Board $board, int $cardId, int $fileId): void { + try { + $node = $this->rootFolder->getFirstNodeById($fileId); + $sharer = $node?->getOwner()?->getUID(); + $userNode = $sharer === null ? null : $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId); + if ($userNode === null) { + // A file without an owner is shared by the board owner when they can see it + $sharer = $board->getOwner(); + $userNode = $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId); + } + } catch (\Exception) { + $userNode = null; + } + if ($userNode === null || $sharer === null) { + throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' was not found'); + } + + $share = $this->shareManager->newShare(); + $share->setNode($userNode); + $share->setShareType(IShare::TYPE_DECK); + $share->setSharedWith((string)$cardId); + $share->setPermissions(Constants::PERMISSION_READ); + $share->setSharedBy($sharer); + $share->setMailSend(false); + + // The Deck share provider checks that its user may edit the card's board + $previousUser = $this->permissionService->getUserId(); + $this->permissionService->setUserId($board->getOwner()); + try { + $this->shareManager->createShare($share); + } catch (\Exception $e) { + throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' could not be shared: ' . $e->getMessage()); + } finally { + $this->permissionService->setUserId($previousUser); + } + } + + /** + * The files attached to a card as Deck shares + * + * @return array share IDs by file ID + */ + private function cardFiles(int $cardId): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('s.id', 's.file_source', 'f.fileid', 'f.path') + ->selectAlias('st.id', 'storage_string_id') + ->from('share', 's') + ->leftJoin('s', 'filecache', 'f', $qb->expr()->eq('s.file_source', 'f.fileid')) + ->leftJoin('f', 'storages', 'st', $qb->expr()->eq('f.storage', 'st.numeric_id')) + ->where($qb->expr()->eq('s.share_type', $qb->createNamedParameter(IShare::TYPE_DECK, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('s.share_with', $qb->createNamedParameter((string)$cardId))) + ->andWhere($qb->expr()->isNull('s.parent')) + ->andWhere($qb->expr()->in('s.item_type', $qb->createNamedParameter(['file', 'folder'], IQueryBuilder::PARAM_STR_ARRAY))); + $files = []; + $result = $qb->executeQuery(); + while ($row = $result->fetch()) { + if ($this->shareProvider->isAccessibleResult($row)) { + $files[(int)$row['file_source']] = (int)$row['id']; + } + } + $result->closeCursor(); + return $files; + } + + /** + * The SHA-256 of the card's canonical form as Deck holds it + */ + private function storedHash(BoardContext $context, string $ref, Card $card): string { + $labelRefs = array_map(fn (Label $label) => $context->labelRef($label->getId()), $this->labelMapper->findAssignedLabelsForCard($card->getId())); + $assignees = array_map(self::assigneeKey(...), $this->assignmentMapper->findAll($card->getId())); + return CardCanonicalForm::hash( + $ref, + $context->stackRef($card->getStackId()), + $card->getTitle(), + (string)$card->getDescription(), + (int)$card->getOrder(), + $card->getStartdate(), + $card->getDuedate(), + $card->getDone(), + array_values($assignees), + array_values($labelRefs), + array_keys($this->cardFiles($card->getId())), + ); + } + + private function changedInDeck(Card $card, string $currentHash, ImportRef $cardRef): bool { + return $currentHash !== $cardRef->getStoredHash() || $card->getArchived() || $card->getDeletedAt() > 0; + } + + private function cardWritten(Card $card, bool $created): void { + $this->changeHelper->cardChanged($card->getId(), false); + $this->fullTextSearch->handle($created ? new CardCreatedEvent($card) : new CardUpdatedEvent($card)); + } + + #[\Override] + public function statCards(string $app, int $boardId, array $refs, bool $hash): array { + $context = $this->context($app, $boardId); + $found = $this->refMapper->findByRefs($app, ImportRef::KIND_CARD, $boardId, $refs); + $stats = []; + foreach ($refs as $ref) { + $cardRef = $found[$ref] ?? null; + if ($cardRef === null) { + $stats[] = new ImportCardStat($ref, false); + continue; + } + $card = $this->findCard($cardRef->getObjectId()); + $removed = $cardRef->getState() === ImportRef::STATE_REMOVED; + if ($card === null) { + $stats[] = new ImportCardStat($ref, false, null, null, !$removed); + continue; + } + $current = $this->storedHash($context, $ref, $card); + $changed = $removed + ? $card->getDeletedAt() === 0 || $current !== $cardRef->getStoredHash() + : $this->changedInDeck($card, $current, $cardRef); + $stats[] = new ImportCardStat($ref, !$removed, $card->getId(), $hash ? $current : null, $changed); + } + return $stats; + } + + #[\Override] + public function listCards(string $app, int $boardId, string $cursor, int $limit): array { + $this->context($app, $boardId); + if ($cursor !== '' && preg_match('/^[1-9][0-9]{0,18}$/', $cursor) !== 1) { + throw new ImportException(ImportException::INVALID, 'The cursor is invalid'); + } + if ($limit < 1 || $limit > self::MAX_LIST_LIMIT) { + throw new ImportException(ImportException::INVALID, 'The limit must be between 1 and ' . self::MAX_LIST_LIMIT); + } + $page = $this->refMapper->findCardPage($app, $boardId, $cursor === '' ? 0 : (int)$cursor, $limit + 1); + $next = ''; + if (count($page) > $limit) { + array_pop($page); + $next = (string)$page[$limit - 1]->getId(); + } + return [ + 'entries' => array_map(fn (ImportRef $ref) => $ref->getRef(), $page), + 'next_cursor' => $next, + ]; + } + + #[\Override] + public function removeCards(string $app, int $boardId, array $refs): array { + $context = $this->context($app, $boardId); + $results = []; + foreach ($refs as $ref) { + try { + $results[] = $this->atomic(fn () => $this->removeCard($app, $context, $ref), $this->db); + } catch (EntryException $e) { + $results[] = $e->result; + } + } + return $results; + } + + /** + * @throws EntryException + */ + private function removeCard(string $app, BoardContext $context, string $ref): ImportResult { + $cardRef = $this->refMapper->findByRef($app, ImportRef::KIND_CARD, $context->board->getId(), $ref); + if ($cardRef === null) { + throw EntryException::error(ImportResult::ERROR_NOT_FOUND, 'The card was not imported'); + } + if ($cardRef->getState() === ImportRef::STATE_REMOVED) { + return ImportResult::ok($cardRef->getObjectId()); + } + $card = $this->findCard($cardRef->getObjectId()); + if ($card === null) { + throw EntryException::conflict(ImportResult::ERROR_CHANGED, 'The card was deleted in Deck', $cardRef->getObjectId()); + } + $current = $this->storedHash($context, $ref, $card); + if ($this->changedInDeck($card, $current, $cardRef)) { + throw EntryException::conflict(ImportResult::ERROR_CHANGED, 'The card was changed in Deck', $card->getId(), $current); + } + + $card->setDeletedAt($this->timeFactory->getTime()); + $this->cardMapper->update($card); + $cardRef->setState(ImportRef::STATE_REMOVED); + $this->updateRef($cardRef, $current); + $this->changeHelper->cardChanged($card->getId(), false); + $this->fullTextSearch->handle(new CardDeletedEvent($card)); + return ImportResult::ok($card->getId()); + } + + #[\Override] + public function setAccess(string $app, int $boardId, array $acl): array { + $context = $this->context($app, $boardId); + $board = $context->board; + $managed = []; + foreach ($this->refMapper->findAllOfKind($app, ImportRef::KIND_ACL, $boardId) as $aclRef) { + $managed[$aclRef->getRef()] = $aclRef; + } + + $results = []; + $wanted = []; + foreach ($acl as $access) { + try { + if (!$access instanceof ImportAccess) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'Not a share'); + } + if (isset($wanted[$access->ref()])) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The share appears twice'); + } + $wanted[$access->ref()] = true; + $results[] = $this->atomic(fn () => $this->writeAccess($app, $board, $access, $managed[$access->ref()] ?? null), $this->db); + } catch (EntryException $e) { + $results[] = $e->result; + } + } + + foreach ($managed as $ref => $aclRef) { + if (!isset($wanted[$ref])) { + $this->atomic(fn () => $this->removeAccess($aclRef), $this->db); + } + } + + $this->markPastDueNotified($app, $boardId); + $this->boardMapper->flushCache($boardId); + $this->changeHelper->boardChanged($boardId); + return $results; + } + + /** + * @throws EntryException + */ + private function writeAccess(string $app, Board $board, ImportAccess $access, ?ImportRef $aclRef): ImportResult { + if ($access->type === ImportAccess::TYPE_USER) { + $type = Acl::PERMISSION_TYPE_USER; + if (!$this->userManager->userExists($access->id)) { + throw EntryException::error(ImportResult::ERROR_UNKNOWN_USER, 'The user does not exist'); + } + if ($access->id === $board->getOwner()) { + return ImportResult::ok(); + } + } elseif ($access->type === ImportAccess::TYPE_GROUP) { + $type = Acl::PERMISSION_TYPE_GROUP; + if (!$this->groupManager->groupExists($access->id)) { + throw EntryException::error(ImportResult::ERROR_UNKNOWN_GROUP, 'The group does not exist'); + } + } else { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The type must be user or group'); + } + + $next = self::valuesHash($access->edit, $access->share, $access->manage); + $existing = null; + foreach ($this->aclMapper->findAll($board->getId()) as $acl) { + if ($acl->getType() === $type && $acl->getParticipant() === $access->id) { + $existing = $acl; + } + } + if ($existing === null && $aclRef !== null) { + throw EntryException::conflict(ImportResult::ERROR_CHANGED, 'The share was removed in Deck'); + } + if ($existing === null) { + $entity = new Acl(); + $entity->setBoardId($board->getId()); + $entity->setType($type); + $entity->setParticipant($access->id); + $entity->setPermissionEdit($access->edit); + $entity->setPermissionShare($access->share); + $entity->setPermissionManage($access->manage); + $entity = $this->aclMapper->insert($entity); + $this->insertRef($app, ImportRef::KIND_ACL, $board->getId(), $access->ref(), $entity->getId(), $next); + $this->accessChanged(new AclCreatedEvent($entity), Operation::BoardShareCreated); + return ImportResult::ok(); + } + + if ($aclRef === null || $aclRef->getObjectId() !== $existing->getId()) { + throw EntryException::conflict(ImportResult::ERROR_EXISTS, 'The board is already shared with them in Deck'); + } + $current = self::valuesHash($existing->getPermissionEdit(), $existing->getPermissionShare(), $existing->getPermissionManage()); + if ($current !== $aclRef->getStoredHash()) { + throw EntryException::conflict(ImportResult::ERROR_CHANGED, 'The share was changed in Deck'); + } + if ($current !== $next) { + $existing->setPermissionEdit($access->edit); + $existing->setPermissionShare($access->share); + $existing->setPermissionManage($access->manage); + $this->aclMapper->update($existing); + $this->updateRef($aclRef, $next); + $this->accessChanged(new AclUpdatedEvent($existing), Operation::BoardShareUpdated); + } + return ImportResult::ok(); + } + + /** + * Removes a share the importer made unless somebody changed it in Deck + */ + private function removeAccess(ImportRef $aclRef): void { + try { + /** @var Acl $existing */ + $existing = $this->aclMapper->find($aclRef->getObjectId()); + } catch (DoesNotExistException) { + $this->refMapper->delete($aclRef); + return; + } + $current = self::valuesHash($existing->getPermissionEdit(), $existing->getPermissionShare(), $existing->getPermissionManage()); + if ($current !== $aclRef->getStoredHash()) { + return; + } + $this->aclMapper->delete($existing); + $this->refMapper->delete($aclRef); + $this->accessChanged(new AclDeletedEvent($existing), Operation::BoardShareDeleted); + } + + /** + * Updates share access, collaboration resources, full-text search and + * the audit log through Deck's own listeners, without dispatching the + * event: its live update listener would push to open sessions + */ + private function accessChanged(AclCreatedEvent|AclUpdatedEvent|AclDeletedEvent $event, Operation $operation): void { + $this->shareAccess->handle($event); + $this->resources->handle($event); + $this->fullTextSearch->handle($event); + + $acl = $event->getAcl(); + $permissions = ['read']; + if ($acl->getPermissionEdit()) { + $permissions[] = 'edit'; + } + if ($acl->getPermissionShare()) { + $permissions[] = 'share'; + } + if ($acl->getPermissionManage()) { + $permissions[] = 'manage'; + } + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'Share %s of board %s with %s "%s" was set by an import (permissions: %s)', + [ + 'aclId' => $acl->getId(), + 'boardId' => $acl->getBoardId(), + 'shareType' => $acl->getType() === Acl::PERMISSION_TYPE_USER ? 'user' : 'group', + 'sharedWith' => $acl->getParticipant(), + 'permissions' => implode(', ', $permissions), + ], + false, + $operation->value, + )); + } + + /** + * Imported cards that were already due when they were shared don't + * announce themselves as overdue to the people who get the board + */ + private function markPastDueNotified(string $app, int $boardId): void { + $cardIds = array_map(fn (ImportRef $ref) => $ref->getObjectId(), $this->refMapper->findAllOfKind($app, ImportRef::KIND_CARD, $boardId)); + foreach (array_chunk($cardIds, 1000) as $chunk) { + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_cards') + ->set('notified', $qb->createNamedParameter(true, IQueryBuilder::PARAM_BOOL)) + ->where($qb->expr()->in('id', $qb->createNamedParameter($chunk, IQueryBuilder::PARAM_INT_ARRAY))) + ->andWhere($qb->expr()->lte('duedate', $qb->createNamedParameter($this->timeFactory->now(), IQueryBuilder::PARAM_DATETIME_IMMUTABLE))) + ->andWhere($qb->expr()->eq('notified', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))); + $qb->executeStatement(); + } + } + + /** + * The board with the imported stacks and labels that still exist + * + * @throws ImportException + */ + private function context(string $app, int $boardId): BoardContext { + try { + $this->checkApp($app); + } catch (EntryException $e) { + throw new ImportException(ImportException::INVALID, $e->getMessage()); + } + $boardRef = $this->refMapper->findBoard($boardId); + try { + $board = $this->boardMapper->find($boardId); + } catch (DoesNotExistException) { + throw new ImportException(ImportException::NOT_FOUND, 'The board does not exist'); + } + if ($boardRef === null || $boardRef->getApp() !== $app) { + throw new ImportException(ImportException::NOT_IMPORTER, 'The board was not imported by ' . $app); + } + + $stackIds = []; + $stacks = []; + foreach ($this->stackMapper->findAll($boardId) as $stack) { + $stacks[$stack->getId()] = true; + } + foreach ($this->refMapper->findAllOfKind($app, ImportRef::KIND_STACK, $boardId) as $stackRef) { + if (isset($stacks[$stackRef->getObjectId()])) { + $stackIds[$stackRef->getRef()] = $stackRef->getObjectId(); + } + } + $labelIds = []; + $labels = []; + foreach ($this->labelMapper->findAll($boardId) as $label) { + $labels[$label->getId()] = true; + } + foreach ($this->refMapper->findAllOfKind($app, ImportRef::KIND_LABEL, $boardId) as $labelRef) { + if (isset($labels[$labelRef->getObjectId()])) { + $labelIds[$labelRef->getRef()] = $labelRef->getObjectId(); + } + } + return new BoardContext($board, $stackIds, $labelIds); + } + + private function findCard(int $cardId): ?Card { + try { + return $this->cardMapper->find($cardId, false); + } catch (DoesNotExistException) { + return null; + } + } + + private function insertRef(string $app, string $kind, int $boardId, string $ref, int $objectId, ?string $storedHash): ImportRef { + $now = $this->timeFactory->getTime(); + $importRef = new ImportRef(); + $importRef->setApp($app); + $importRef->setKind($kind); + $importRef->setBoardId($boardId); + $importRef->setRef($ref); + $importRef->setRefHash(ImportRef::hashRef($ref)); + $importRef->setObjectId($objectId); + $importRef->setStoredHash($storedHash); + $importRef->setState(ImportRef::STATE_ACTIVE); + $importRef->setCreatedAt($now); + $importRef->setUpdatedAt($now); + return $this->refMapper->insert($importRef); + } + + private function updateRef(ImportRef $importRef, string $storedHash, ?string $sourceHash = null): void { + $importRef->setStoredHash($storedHash); + if ($sourceHash !== null) { + $importRef->setSourceHash($sourceHash); + } + $importRef->setUpdatedAt($this->timeFactory->getTime()); + $this->refMapper->update($importRef); + } + + /** + * Runs a write once more when a concurrent import of the same reference + * won the race; the second run finds its rows + * + * @template T + * @param callable(): T $write + * @return T + */ + private function retryOnUniqueViolation(callable $write): mixed { + try { + return $write(); + } catch (DbException $e) { + if ($e->getReason() !== DbException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + return $write(); + } + } + + /** + * @throws EntryException + */ + private function checkApp(string $app): void { + if (preg_match('/^[a-z0-9_]{1,32}$/', $app) !== 1) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The app must match [a-z0-9_]{1,32}'); + } + } + + /** + * @throws EntryException + */ + private function checkRef(string $field, string $ref): void { + if ($ref === '' || strlen($ref) > self::MAX_REF_BYTES) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The ' . $field . ' must have 1 to ' . self::MAX_REF_BYTES . ' bytes'); + } + } + + /** + * @throws EntryException + */ + private function checkTitle(string $field, string $title, int $max): void { + $length = mb_strlen($title, 'UTF-8'); + if ($length === 0 || $length > $max) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The ' . $field . ' must have 1 to ' . $max . ' characters'); + } + } + + /** + * @throws EntryException + */ + private function checkColor(string $field, string $color): void { + if (preg_match('/^[0-9a-fA-F]{6}$/', $color) !== 1) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The ' . $field . ' must be rrggbb'); + } + } + + /** + * Lists of distinct values that each pass $valid + * + * @throws EntryException + */ + private function checkList(string $field, array $values, callable $valid): void { + if (!array_is_list($values) || count($values) !== count(array_unique($values, SORT_REGULAR))) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The ' . $field . ' must be a list of distinct values'); + } + foreach ($values as $value) { + if (!$valid($value)) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The ' . $field . ' hold an invalid value'); + } + } + } + + /** + * @throws EntryException + */ + private function checkLabels(array $labels): void { + $refs = []; + $titles = []; + foreach ($labels as $label) { + if (!$label instanceof ImportLabel) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The labels must be ImportLabel objects'); + } + $this->checkRef('label ref', $label->ref); + $this->checkTitle('label title', $label->title, self::MAX_LABEL_TITLE); + $this->checkColor('label color', $label->color); + if (isset($refs[$label->ref]) || isset($titles[$label->title])) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'Label refs and titles must be distinct'); + } + $refs[$label->ref] = true; + $titles[$label->title] = true; + } + } + + /** + * @throws EntryException + */ + private function checkStacks(array $stacks): void { + $refs = []; + foreach ($stacks as $stack) { + if (!$stack instanceof ImportStack) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'The stacks must be ImportStack objects'); + } + $this->checkRef('stack ref', $stack->ref); + $this->checkTitle('stack title', $stack->title, self::MAX_STACK_TITLE); + if (isset($refs[$stack->ref])) { + throw EntryException::error(ImportResult::ERROR_INVALID, 'Stack refs must be distinct'); + } + $refs[$stack->ref] = true; + } + } + + private function isPast(?\DateTimeImmutable $time): bool { + return $time !== null && $time <= $this->timeFactory->now(); + } + + private static function assigneeKey(Assignment $assignment): string { + return $assignment->getType() === Assignment::TYPE_USER + ? $assignment->getParticipant() + : $assignment->getType() . ':' . $assignment->getParticipant(); + } + + /** + * Fingerprint of the values the importer last wrote to a board, label, + * stack or share, to tell whether somebody changed them since + */ + private static function valuesHash(string|int|bool ...$values): string { + return hash('sha256', json_encode($values, JSON_THROW_ON_ERROR)); + } + + private static function time(?\DateTimeInterface $time): ?string { + return CardCanonicalForm::time($time); + } + + private static function mutable(?\DateTimeImmutable $time): ?\DateTime { + return $time === null ? null : \DateTime::createFromImmutable($time->setTimezone(new \DateTimeZone('UTC'))); + } +} diff --git a/lib/Import/CardCanonicalForm.php b/lib/Import/CardCanonicalForm.php new file mode 100644 index 0000000000..165fadbac6 --- /dev/null +++ b/lib/Import/CardCanonicalForm.php @@ -0,0 +1,70 @@ +` and `&` as + * `\u003c`, `\u003e` and `\u0026`, U+2028 and U+2029 as `\u2028` and + * `\u2029`, other control characters as `\b`, `\f`, `\n`, `\r`, `\t` or + * `\u00XX` (lowercase hex). + * + * @since 2.0.0 + */ +final class CardCanonicalForm { + /** + * @param list $assignees + * @param list $labelRefs + * @param list $fileIds + */ + public static function hash(string $ref, string $stackRef, string $title, string $description, int $order, + ?\DateTimeInterface $start, ?\DateTimeInterface $due, ?\DateTimeInterface $done, + array $assignees, array $labelRefs, array $fileIds): string { + sort($assignees, SORT_STRING); + sort($labelRefs, SORT_STRING); + sort($fileIds, SORT_NUMERIC); + $values = [ + $ref, + $stackRef, + $title, + $description, + $order, + self::time($start), + self::time($due), + self::time($done), + $assignees, + $labelRefs, + $fileIds, + ]; + return hash('sha256', implode("\n", array_map(self::encode(...), $values))); + } + + public static function time(?\DateTimeInterface $time): ?string { + if ($time === null) { + return null; + } + return \DateTimeImmutable::createFromInterface($time) + ->setTimezone(new \DateTimeZone('UTC')) + ->format('Y-m-d\TH:i:s\Z'); + } + + private static function encode(mixed $value): string { + $json = json_encode($value, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + // These characters can only occur inside strings + return strtr($json, ['<' => '\u003c', '>' => '\u003e', '&' => '\u0026']); + } +} diff --git a/lib/Import/EntryException.php b/lib/Import/EntryException.php new file mode 100644 index 0000000000..0758dd14db --- /dev/null +++ b/lib/Import/EntryException.php @@ -0,0 +1,32 @@ +errorMessage); + } + + public static function error(string $code, string $message): self { + return new self(ImportResult::error($code, $message)); + } + + public static function conflict(string $code, string $message, ?int $cardId = null, ?string $sha256 = null): self { + return new self(ImportResult::conflict($code, $message, $cardId, $sha256)); + } +} diff --git a/lib/Import/IBoardImporter.php b/lib/Import/IBoardImporter.php new file mode 100644 index 0000000000..20a1c092ff --- /dev/null +++ b/lib/Import/IBoardImporter.php @@ -0,0 +1,106 @@ + $labels + * @param list $stacks + * @return array{status: string, board_id: int, created: bool, labels: array, stacks: array, changed: list, error?: array{code: string, message: string}} + */ + public function ensureBoard(string $app, string $ref, string $ownerUid, string $title, string $color, + \DateTimeImmutable $created, array $labels, array $stacks): array; + + /** + * Writes cards with their original creation, start, due and done times, + * assignments, labels and file attachments. + * + * @param list $cards + * @return list one per card, in the same order + * @throws ImportException when the board is missing or another app created it + */ + public function importCards(string $app, int $boardId, array $cards): array; + + /** + * What Deck holds for each reference, with the SHA-256 of the canonical + * form (CardCanonicalForm) when $hash is set. + * + * @param list $refs + * @return list one per reference, in the same order + * @throws ImportException + */ + public function statCards(string $app, int $boardId, array $refs, bool $hash): array; + + /** + * Pages through the cards the app wrote to a board and didn't remove. + * + * @param string $cursor '' for the first page, then the previous `next_cursor` + * @param int $limit 1 to 1000 + * @return array{entries: list, next_cursor: string} the card refs; `next_cursor` is '' on the last page + * @throws ImportException + */ + public function listCards(string $app, int $boardId, string $cursor, int $limit): array; + + /** + * Soft-deletes cards the importer wrote (Deck's deleted cards). + * + * @param list $refs + * @return list one per reference, in the same order + * @throws ImportException + */ + public function removeCards(string $app, int $boardId, array $refs): array; + + /** + * The complete list of shares the importer manages; no notification. + * + * Shares the importer made that are missing from $acl are removed; + * shares others made stay as they are. + * + * @param list $acl + * @return list one per share, in the same order + * @throws ImportException + */ + public function setAccess(string $app, int $boardId, array $acl): array; +} diff --git a/lib/Import/ImportAccess.php b/lib/Import/ImportAccess.php new file mode 100644 index 0000000000..c4f7d47bba --- /dev/null +++ b/lib/Import/ImportAccess.php @@ -0,0 +1,40 @@ +type . ':' . $this->id; + } +} diff --git a/lib/Import/ImportCard.php b/lib/Import/ImportCard.php new file mode 100644 index 0000000000..bdf0ea97d4 --- /dev/null +++ b/lib/Import/ImportCard.php @@ -0,0 +1,53 @@ + $assignees user IDs + * @param list $labels label refs of the board + * @param list $files IDs of existing files, attached as Deck file attachments + * @param string $sha256 hex SHA-256 of the card's canonical form (CardCanonicalForm) as the importing app computed it + * @param self::MODE_* $mode + */ + public function __construct( + public readonly string $ref, + public readonly string $stackRef, + public readonly string $title, + public readonly string $description, + public readonly int $order, + public readonly ?\DateTimeImmutable $start, + public readonly ?\DateTimeImmutable $due, + public readonly ?\DateTimeImmutable $done, + public readonly \DateTimeImmutable $created, + public readonly string $createdBy, + public readonly array $assignees, + public readonly array $labels, + public readonly array $files, + public readonly string $sha256, + public readonly string $mode = self::MODE_CREATE, + ) { + } +} diff --git a/lib/Import/ImportCardStat.php b/lib/Import/ImportCardStat.php new file mode 100644 index 0000000000..433d1ea479 --- /dev/null +++ b/lib/Import/ImportCardStat.php @@ -0,0 +1,46 @@ + $this->ref, 'exists' => $this->exists]; + if ($this->cardId !== null) { + $json['card_id'] = $this->cardId; + } + if ($this->sha256 !== null) { + $json['sha256'] = $this->sha256; + } + $json['changed'] = $this->changed; + return $json; + } +} diff --git a/lib/Import/ImportException.php b/lib/Import/ImportException.php new file mode 100644 index 0000000000..7044308815 --- /dev/null +++ b/lib/Import/ImportException.php @@ -0,0 +1,41 @@ +reason; + } +} diff --git a/lib/Import/ImportLabel.php b/lib/Import/ImportLabel.php new file mode 100644 index 0000000000..e303a5260d --- /dev/null +++ b/lib/Import/ImportLabel.php @@ -0,0 +1,29 @@ + $this->status]; + if ($this->cardId !== null) { + $json['card_id'] = $this->cardId; + } + if ($this->sha256 !== null) { + $json['sha256'] = $this->sha256; + } + if ($this->errorCode !== null) { + $json['error'] = ['code' => $this->errorCode, 'message' => (string)$this->errorMessage]; + } + return $json; + } +} diff --git a/lib/Import/ImportStack.php b/lib/Import/ImportStack.php new file mode 100644 index 0000000000..17ffb16dc8 --- /dev/null +++ b/lib/Import/ImportStack.php @@ -0,0 +1,28 @@ +userId = $userId; $this->permissionCache->clear(); } From d9c639e9b50f963caff8839f156031d1f0a02d76 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 30 Oct 2026 12:17:18 +0100 Subject: [PATCH 3/4] test(import): board import API, idempotency and quietness The quiet test imports and shares a plan with overdue, assigned and attached cards, runs the due and description cron jobs and counts the notification, activity, mail and comment tables, the activity and mail managers and Deck's events; a second test shows the same probes see Deck's normal services. The canonical form is pinned to hashes Go's encoding/json produces. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- .../integration/import/BoardImporterTest.php | 626 ++++++++++++++++++ tests/unit/Import/CardCanonicalFormTest.php | 54 ++ 2 files changed, 680 insertions(+) create mode 100644 tests/integration/import/BoardImporterTest.php create mode 100644 tests/unit/Import/CardCanonicalFormTest.php diff --git a/tests/integration/import/BoardImporterTest.php b/tests/integration/import/BoardImporterTest.php new file mode 100644 index 0000000000..eeda59a25b --- /dev/null +++ b/tests/integration/import/BoardImporterTest.php @@ -0,0 +1,626 @@ + */ + private array $deckEvents = []; + private int $activityEvents = 0; + private int $mails = 0; + private int $fileId; + + public static function setUpBeforeClass(): void { + parent::setUpBeforeClass(); + // The test bootstrap loads the server's own apps only; these deliver what the quiet test counts + foreach (['deck', 'activity', 'notifications'] as $app) { + Server::get(IAppManager::class)->loadApp($app); + } + + $backend = new \Test\Util\User\Dummy(); + Server::get(IUserManager::class)->registerBackend($backend); + foreach ([self::OWNER, self::ALICE, self::BOB, self::CAROL] as $uid) { + $backend->createUser($uid, $uid); + } + $groupBackend = new \Test\Util\Group\Dummy(); + $groupBackend->createGroup(self::GROUP); + $groupBackend->addToGroup(self::ALICE, self::GROUP); + $groupBackend->addToGroup(self::BOB, self::GROUP); + Server::get(IGroupManager::class)->addBackend($groupBackend); + + // The test cleanup empties the file cache but not the mount cache, which then points to storages of earlier runs + $qb = Server::get(IDBConnection::class)->getQueryBuilder(); + $qb->delete('mounts') + ->where($qb->expr()->in('user_id', $qb->createNamedParameter([self::OWNER, self::ALICE, self::BOB, self::CAROL], IQueryBuilder::PARAM_STR_ARRAY))) + ->executeStatement(); + } + + protected function setUp(): void { + parent::setUp(); + $this->importer = Server::get(IBoardImporter::class); + $this->db = Server::get(IDBConnection::class); + $this->cardMapper = Server::get(CardMapper::class); + $this->boardRef = 'plan-' . $this->getUniqueID(); + + $folder = Server::get(IRootFolder::class)->getUserFolder(self::ALICE); + $this->fileId = $folder->newFile('Budget ' . $this->getUniqueID() . '.txt', 'numbers')->getId(); + // Imports run without a user, like a bridge request + Server::get(IUserSession::class)->setUser(null); + + $dispatcher = Server::get(IEventDispatcher::class); + foreach ([CardCreatedEvent::class, CardUpdatedEvent::class, CardDeletedEvent::class, AclCreatedEvent::class, + AclUpdatedEvent::class, AclDeletedEvent::class, BoardUpdatedEvent::class] as $eventClass) { + $dispatcher->addListener($eventClass, function () use ($eventClass): void { + $this->deckEvents[] = $eventClass; + }); + } + Server::get(IActivityManager::class)->registerConsumer(function () { + return new class($this) implements IConsumer { + public function __construct( + private BoardImporterTest $test, + ) { + } + + public function receive(IActivityEvent $event): void { + $this->test->countActivity(); + } + }; + }); + $mailer = $this->createMock(IMailer::class); + $mailer->method('createMessage')->willReturn($this->createMock(IMessage::class)); + $mailer->method('send')->willReturnCallback(function () { + $this->mails++; + return []; + }); + $this->overwriteService(IMailer::class, $mailer); + } + + protected function tearDown(): void { + $this->restoreService(IMailer::class); + parent::tearDown(); + } + + public function countActivity(): void { + $this->activityEvents++; + } + + public function testEnsureBoardCreatesBoardLabelsAndStacksOnce(): void { + $first = $this->ensureBoard(); + $this->assertSame(ImportResult::STATUS_OK, $first['status']); + $this->assertTrue($first['created']); + $this->assertSame(['category1', 'priority'], array_keys($first['labels'])); + $this->assertSame(['bucket-todo', 'bucket-done'], array_keys($first['stacks'])); + + $board = Server::get(BoardMapper::class)->find($first['board_id']); + $this->assertSame('Launch plan', $board->getTitle()); + $this->assertSame(self::OWNER, $board->getOwner()); + $this->assertSame('0082c9', $board->getColor()); + $this->assertCount(2, Server::get(LabelMapper::class)->findAll($first['board_id']), 'no default labels'); + $this->assertCount(2, Server::get(StackMapper::class)->findAll($first['board_id'])); + + $again = $this->ensureBoard(); + $this->assertFalse($again['created']); + $this->assertSame($first['board_id'], $again['board_id']); + $this->assertSame($first['labels'], $again['labels']); + $this->assertSame($first['stacks'], $again['stacks']); + $this->assertSame([], $again['changed']); + $this->assertCount(2, Server::get(LabelMapper::class)->findAll($first['board_id'])); + } + + public function testEnsureBoardUpdatesWhatNobodyChangedAndKeepsDeckChanges(): void { + $board = $this->ensureBoard(); + $stackMapper = Server::get(StackMapper::class); + $stack = $stackMapper->find($board['stacks']['bucket-todo']); + $stack->setTitle('Renamed in Deck'); + $stackMapper->update($stack); + + $result = $this->ensureBoard('Launch plan 2026', [ + new ImportLabel('category1', 'Marketing', 'ff0000'), + new ImportLabel('priority', 'Urgent', 'e9322d'), + new ImportLabel('category2', 'Sales', '00ff00'), + ], [ + new ImportStack('bucket-todo', 'Backlog', 0), + new ImportStack('bucket-done', 'Finished', 1), + ]); + + $this->assertSame(ImportResult::STATUS_OK, $result['status']); + $this->assertSame(['bucket-todo'], $result['changed']); + $this->assertSame('Launch plan 2026', Server::get(BoardMapper::class)->find($board['board_id'])->getTitle()); + $stackMapper = Server::get(StackMapper::class); + $this->assertSame('Renamed in Deck', $stackMapper->find($board['stacks']['bucket-todo'])->getTitle()); + $this->assertSame('Finished', $stackMapper->find($board['stacks']['bucket-done'])->getTitle()); + $this->assertSame('ff0000', Server::get(LabelMapper::class)->find($board['labels']['category1'])->getColor()); + $this->assertArrayHasKey('category2', $result['labels']); + } + + public function testEnsureBoardRefusesInvalidInput(): void { + $result = $this->importer->ensureBoard(self::APP, $this->boardRef, 'nobody-' . $this->getUniqueID(), 'Plan', '0082c9', + new \DateTimeImmutable(), [], []); + $this->assertSame(ImportResult::STATUS_ERROR, $result['status']); + $this->assertSame(ImportResult::ERROR_UNKNOWN_USER, $result['error']['code']); + + $result = $this->importer->ensureBoard(self::APP, $this->boardRef, self::OWNER, 'Plan', 'blue', + new \DateTimeImmutable(), [], []); + $this->assertSame(ImportResult::ERROR_INVALID, $result['error']['code']); + + $result = $this->importer->ensureBoard(self::APP, $this->boardRef, self::OWNER, 'Plan', '0082c9', + new \DateTimeImmutable(), [new ImportLabel('a', 'Same', '000000'), new ImportLabel('b', 'Same', '111111')], []); + $this->assertSame(ImportResult::ERROR_INVALID, $result['error']['code']); + } + + public function testImportCardKeepsOriginalTimesAndRelations(): void { + $board = $this->ensureBoard(); + $created = new \DateTimeImmutable('2025-11-03T09:15:00Z'); + $card = $this->card('task-1', [ + 'description' => "Prepare the launch.\n\n- [ ] Draft\n- [x] Review\n\n[Spec](https://example.com/spec)", + 'start' => new \DateTimeImmutable('2025-11-04T08:00:00Z'), + 'due' => new \DateTimeImmutable('2025-11-20T17:00:00+01:00'), + 'done' => new \DateTimeImmutable('2025-11-19T12:30:45Z'), + 'created' => $created, + 'createdBy' => self::CAROL, + 'assignees' => [self::BOB, self::ALICE], + 'labels' => ['priority', 'category1'], + 'files' => [$this->fileId], + ]); + + [$result] = $this->importer->importCards(self::APP, $board['board_id'], [$card]); + + $this->assertSame(ImportResult::STATUS_OK, $result->status, (string)$result->errorMessage); + $this->assertSame($card->sha256, $result->sha256, 'Deck hashes what it stored to what the importer computed'); + $stored = $this->cardMapper->find($result->cardId, false); + $this->assertSame('Ship it', $stored->getTitle()); + $this->assertSame($card->description, $stored->getDescription()); + $this->assertSame($board['stacks']['bucket-todo'], $stored->getStackId()); + $this->assertSame($created->getTimestamp(), $stored->getCreatedAt()); + $this->assertSame('2025-11-19T12:30:45Z', CardCanonicalForm::time($stored->getDone())); + $this->assertSame('2025-11-20T16:00:00Z', CardCanonicalForm::time($stored->getDuedate())); + $this->assertSame('2025-11-04T08:00:00Z', CardCanonicalForm::time($stored->getStartdate())); + $this->assertSame(self::CAROL, $stored->getOwner()); + $this->assertTrue($stored->getNotified(), 'a due date in the past is never announced'); + + $assignees = array_map(fn ($a) => $a->getParticipant(), Server::get(AssignmentMapper::class)->findAll($result->cardId)); + sort($assignees); + $this->assertSame([self::ALICE, self::BOB], $assignees); + $labels = array_map(fn ($l) => $l->getId(), Server::get(LabelMapper::class)->findAssignedLabelsForCard($result->cardId)); + sort($labels); + $expected = [$board['labels']['category1'], $board['labels']['priority']]; + sort($expected); + $this->assertSame($expected, $labels); + + $shares = $this->deckShares($result->cardId); + $this->assertCount(1, $shares); + $this->assertSame($this->fileId, (int)$shares[0]['file_source']); + $this->assertSame(self::ALICE, $shares[0]['uid_initiator'], 'the file owner shares the file with the card'); + } + + public function testRepeatedImportWritesNothing(): void { + $board = $this->ensureBoard(); + $card = $this->card('task-1', ['assignees' => [self::ALICE], 'labels' => ['category1'], 'files' => [$this->fileId]]); + [$first] = $this->importer->importCards(self::APP, $board['board_id'], [$card]); + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_cards') + ->set('last_modified', $qb->createNamedParameter(1000, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($first->cardId, IQueryBuilder::PARAM_INT))) + ->executeStatement(); + $rows = $this->rowCounts(['deck_cards', 'deck_assigned_users', 'deck_assigned_labels', 'share', 'deck_import_refs']); + + [$second] = $this->importer->importCards(self::APP, $board['board_id'], [$card]); + + $this->assertSame(ImportResult::STATUS_OK, $second->status); + $this->assertSame($first->cardId, $second->cardId); + $this->assertSame($first->sha256, $second->sha256); + $this->assertSame($rows, $this->rowCounts(['deck_cards', 'deck_assigned_users', 'deck_assigned_labels', 'share', 'deck_import_refs'])); + $this->assertSame(1000, $this->cardMapper->find($first->cardId, false)->getLastModified(), 'the card was not written again'); + } + + public function testNewVersionUpdatesTheCard(): void { + $board = $this->ensureBoard(); + $this->importer->importCards(self::APP, $board['board_id'], [ + $this->card('task-1', ['assignees' => [self::ALICE], 'labels' => ['category1'], 'files' => [$this->fileId]]), + ]); + + $next = $this->card('task-1', [ + 'title' => 'Ship it on Monday', + 'stackRef' => 'bucket-done', + 'assignees' => [self::BOB], + 'labels' => ['priority'], + 'files' => [], + 'mode' => ImportCard::MODE_NEW_VERSION, + ]); + [$result] = $this->importer->importCards(self::APP, $board['board_id'], [$next]); + + $this->assertSame(ImportResult::STATUS_OK, $result->status, (string)$result->errorMessage); + $this->assertSame($next->sha256, $result->sha256); + $stored = $this->cardMapper->find($result->cardId, false); + $this->assertSame('Ship it on Monday', $stored->getTitle()); + $this->assertSame($board['stacks']['bucket-done'], $stored->getStackId()); + $this->assertSame([self::BOB], array_map(fn ($a) => $a->getParticipant(), Server::get(AssignmentMapper::class)->findAll($result->cardId))); + $this->assertSame([], $this->deckShares($result->cardId)); + } + + public function testCreateOfAnExistingRefWithOtherContentConflicts(): void { + $board = $this->ensureBoard(); + [$first] = $this->importer->importCards(self::APP, $board['board_id'], [$this->card('task-1')]); + + [$result] = $this->importer->importCards(self::APP, $board['board_id'], [$this->card('task-1', ['title' => 'Other'])]); + + $this->assertSame(ImportResult::STATUS_CONFLICT, $result->status); + $this->assertSame(ImportResult::ERROR_EXISTS, $result->errorCode); + $this->assertSame($first->cardId, $result->cardId); + $this->assertSame('Ship it', $this->cardMapper->find($first->cardId, false)->getTitle()); + } + + public function testCardEditedInDeckIsReportedAndKept(): void { + $board = $this->ensureBoard(); + [$first, $second] = $this->importer->importCards(self::APP, $board['board_id'], [$this->card('task-1'), $this->card('task-2')]); + + $card = $this->cardMapper->find($first->cardId, false); + $card->setTitle('Edited by Alice'); + $this->cardMapper->update($card); + $archived = $this->cardMapper->find($second->cardId, false); + $archived->setArchived(true); + $this->cardMapper->update($archived); + + $stats = $this->importer->statCards(self::APP, $board['board_id'], ['task-1', 'task-2', 'task-unknown'], true); + $this->assertTrue($stats[0]->exists); + $this->assertTrue($stats[0]->changed); + $this->assertNotSame($first->sha256, $stats[0]->sha256); + $this->assertTrue($stats[1]->changed, 'archiving is a change'); + $this->assertFalse($stats[2]->exists); + + [$result] = $this->importer->importCards(self::APP, $board['board_id'], [ + $this->card('task-1', ['title' => 'Newer in the source', 'mode' => ImportCard::MODE_NEW_VERSION]), + ]); + $this->assertSame(ImportResult::STATUS_CONFLICT, $result->status); + $this->assertSame(ImportResult::ERROR_CHANGED, $result->errorCode); + $this->assertSame('Edited by Alice', $this->cardMapper->find($first->cardId, false)->getTitle()); + + [$removed] = $this->importer->removeCards(self::APP, $board['board_id'], ['task-1']); + $this->assertSame(ImportResult::ERROR_CHANGED, $removed->errorCode); + $this->assertSame(0, $this->cardMapper->find($first->cardId, false)->getDeletedAt()); + } + + public function testUnchangedCardStatsWithoutHash(): void { + $board = $this->ensureBoard(); + [$first] = $this->importer->importCards(self::APP, $board['board_id'], [$this->card('task-1')]); + + [$stat] = $this->importer->statCards(self::APP, $board['board_id'], ['task-1'], false); + + $this->assertEquals(new ImportCardStat('task-1', true, $first->cardId, null, false), $stat); + } + + public function testRemoveCardsSoftDeletesAndCanRestore(): void { + $board = $this->ensureBoard(); + [$first] = $this->importer->importCards(self::APP, $board['board_id'], [$this->card('task-1'), $this->card('task-2')]); + + $results = $this->importer->removeCards(self::APP, $board['board_id'], ['task-1', 'task-unknown']); + + $this->assertSame(ImportResult::STATUS_OK, $results[0]->status); + $this->assertSame(ImportResult::ERROR_NOT_FOUND, $results[1]->errorCode); + $this->assertGreaterThan(0, $this->cardMapper->find($first->cardId, false)->getDeletedAt()); + $deleted = array_map(fn ($c) => $c->getId(), $this->cardMapper->findDeleted($board['board_id'])); + $this->assertContains($first->cardId, $deleted, 'the card is in Deck\'s deleted cards'); + $this->assertSame(['task-2'], $this->importer->listCards(self::APP, $board['board_id'], '', 10)['entries']); + $this->assertFalse($this->importer->statCards(self::APP, $board['board_id'], ['task-1'], false)[0]->exists); + $this->assertSame(ImportResult::STATUS_OK, $this->importer->removeCards(self::APP, $board['board_id'], ['task-1'])[0]->status); + + [$restored] = $this->importer->importCards(self::APP, $board['board_id'], [$this->card('task-1')]); + $this->assertSame(ImportResult::STATUS_OK, $restored->status); + $this->assertSame($first->cardId, $restored->cardId); + $this->assertSame(0, $this->cardMapper->find($first->cardId, false)->getDeletedAt()); + } + + public function testListCardsPages(): void { + $board = $this->ensureBoard(); + $refs = ['task-a', 'task-b', 'task-c', 'task-d', 'task-e']; + $this->importer->importCards(self::APP, $board['board_id'], array_map(fn ($ref) => $this->card($ref), $refs)); + + $listed = []; + $cursor = ''; + $pages = 0; + do { + $page = $this->importer->listCards(self::APP, $board['board_id'], $cursor, 2); + $listed = array_merge($listed, $page['entries']); + $cursor = $page['next_cursor']; + $pages++; + } while ($cursor !== ''); + + $this->assertSame($refs, $listed); + $this->assertSame(3, $pages); + } + + public function testInvalidCardsAreRefusedAndWriteNothing(): void { + $board = $this->ensureBoard(); + $rows = $this->rowCounts(['deck_cards', 'deck_import_refs', 'share']); + + $results = $this->importer->importCards(self::APP, $board['board_id'], [ + $this->card('a', ['stackRef' => 'bucket-unknown']), + $this->card('b', ['labels' => ['unknown']]), + $this->card('c', ['assignees' => ['nobody-' . $this->getUniqueID()]]), + $this->card('d', ['createdBy' => 'nobody-' . $this->getUniqueID()]), + $this->card('e', ['sha256' => 'ABC']), + $this->card('f', ['mode' => ImportCard::MODE_NEW_VERSION]), + $this->card('g', ['files' => [PHP_INT_MAX]]), + $this->card('h', ['title' => '']), + $this->card('i', ['assignees' => [self::ALICE, self::ALICE]]), + ]); + + $this->assertSame([ + ImportResult::ERROR_UNKNOWN_STACK, + ImportResult::ERROR_UNKNOWN_LABEL, + ImportResult::ERROR_UNKNOWN_USER, + ImportResult::ERROR_UNKNOWN_USER, + ImportResult::ERROR_INVALID, + ImportResult::ERROR_NOT_FOUND, + ImportResult::ERROR_FILE_UNAVAILABLE, + ImportResult::ERROR_INVALID, + ImportResult::ERROR_INVALID, + ], array_map(fn (ImportResult $r) => $r->errorCode, $results)); + $this->assertSame($rows, $this->rowCounts(['deck_cards', 'deck_import_refs', 'share']), 'a failed card leaves nothing behind'); + } + + public function testBoardsOfOthersAreRefused(): void { + $this->actAs(self::ALICE); + $own = Server::get(BoardService::class)->create('Alice\'s board', self::ALICE, '000000'); + $this->actAs(null); + $imported = $this->ensureBoard(); + + foreach ([ + [self::APP, $own->getId(), ImportException::NOT_IMPORTER], + ['other_app', $imported['board_id'], ImportException::NOT_IMPORTER], + [self::APP, PHP_INT_MAX, ImportException::NOT_FOUND], + ['Not An App', $imported['board_id'], ImportException::INVALID], + ] as [$app, $boardId, $reason]) { + try { + $this->importer->importCards($app, $boardId, [$this->card('task-1')]); + $this->fail('Expected ' . $reason); + } catch (ImportException $e) { + $this->assertSame($reason, $e->getReason()); + } + } + } + + public function testSetAccessManagesItsOwnShares(): void { + $board = $this->ensureBoard(); + $aclMapper = Server::get(AclMapper::class); + $acl = function (int $type, string $participant) use ($aclMapper, $board): ?Acl { + foreach ($aclMapper->findAll($board['board_id']) as $acl) { + if ($acl->getType() === $type && $acl->getParticipant() === $participant) { + return $acl; + } + } + return null; + }; + $this->actAs(self::OWNER); + Server::get(BoardService::class)->addAcl($board['board_id'], Acl::PERMISSION_TYPE_USER, self::CAROL, false, false, false); + $this->actAs(null); + + $results = $this->importer->setAccess(self::APP, $board['board_id'], [ + new ImportAccess(ImportAccess::TYPE_GROUP, self::GROUP, true, false, false), + new ImportAccess(ImportAccess::TYPE_USER, self::ALICE, true, true, true), + new ImportAccess(ImportAccess::TYPE_USER, self::CAROL, true, true, true), + new ImportAccess(ImportAccess::TYPE_GROUP, 'no-such-group', true, false, false), + ]); + + $this->assertSame([ImportResult::STATUS_OK, ImportResult::STATUS_OK, ImportResult::STATUS_CONFLICT, ImportResult::STATUS_ERROR], + array_map(fn (ImportResult $r) => $r->status, $results)); + $this->assertSame(ImportResult::ERROR_EXISTS, $results[2]->errorCode); + $this->assertSame(ImportResult::ERROR_UNKNOWN_GROUP, $results[3]->errorCode); + $group = $acl(Acl::PERMISSION_TYPE_GROUP, self::GROUP); + $this->assertTrue($group->getPermissionEdit()); + $this->assertFalse($group->getPermissionManage()); + $this->assertFalse($acl(Acl::PERMISSION_TYPE_USER, self::CAROL)->getPermissionEdit()); + + $results = $this->importer->setAccess(self::APP, $board['board_id'], [ + new ImportAccess(ImportAccess::TYPE_GROUP, self::GROUP, true, true, false), + ]); + + $this->assertSame(ImportResult::STATUS_OK, $results[0]->status); + $this->assertTrue($acl(Acl::PERMISSION_TYPE_GROUP, self::GROUP)->getPermissionShare()); + $this->assertNull($acl(Acl::PERMISSION_TYPE_USER, self::ALICE)); + $this->assertNotNull($acl(Acl::PERMISSION_TYPE_USER, self::CAROL), 'a share somebody else made stays'); + } + + /** + * A whole plan is imported and shared, then the cron jobs that announce + * due cards and description changes run: nobody hears about any of it. + */ + public function testImportIsQuiet(): void { + $before = $this->rowCounts(self::QUIET_TABLES); + + $board = $this->ensureBoard(); + $results = $this->importer->importCards(self::APP, $board['board_id'], [ + $this->card('overdue', ['due' => new \DateTimeImmutable('-3 days'), 'assignees' => [self::ALICE, self::BOB], 'labels' => ['priority'], 'files' => [$this->fileId]]), + $this->card('due-soon', ['due' => new \DateTimeImmutable('+1 hour'), 'assignees' => [self::BOB]]), + $this->card('done', ['done' => new \DateTimeImmutable('-1 day'), 'assignees' => [self::ALICE]]), + ]); + $results = array_merge($results, $this->importer->setAccess(self::APP, $board['board_id'], [ + new ImportAccess(ImportAccess::TYPE_GROUP, self::GROUP, true, false, false), + new ImportAccess(ImportAccess::TYPE_USER, self::CAROL, true, true, true), + ])); + $results = array_merge($results, $this->importer->importCards(self::APP, $board['board_id'], [ + $this->card('due-soon', ['title' => 'Moved', 'due' => new \DateTimeImmutable('-1 hour'), 'assignees' => [self::ALICE], 'mode' => ImportCard::MODE_NEW_VERSION]), + ])); + $results = array_merge($results, $this->importer->removeCards(self::APP, $board['board_id'], ['done'])); + $results = array_merge($results, $this->importer->setAccess(self::APP, $board['board_id'], [ + new ImportAccess(ImportAccess::TYPE_GROUP, self::GROUP, true, true, false), + ])); + $this->runJob(ScheduledNotifications::class); + $this->runJob(CardDescriptionActivity::class); + + $this->assertSame(array_fill(0, 8, ImportResult::STATUS_OK), array_map(fn (ImportResult $r) => $r->status, $results)); + $this->assertCount(1, $this->deckShares($results[0]->cardId), 'the attachment was shared'); + $this->assertSame($before, $this->rowCounts(self::QUIET_TABLES)); + $this->assertSame([], $this->deckEvents, 'no Deck event reaches live updates or other apps'); + $this->assertSame(0, $this->activityEvents); + $this->assertSame(0, $this->mails); + } + + /** + * The same steps through Deck's services do announce themselves, so the + * quiet test would see them + */ + public function testProbesSeeDeckServices(): void { + $before = $this->rowCounts(self::QUIET_TABLES); + $this->actAs(self::OWNER); + $board = Server::get(BoardService::class)->create('Visible', self::OWNER, '000000'); + Server::get(BoardService::class)->addAcl($board->getId(), Acl::PERMISSION_TYPE_USER, self::ALICE, true, false, false); + $stack = Server::get(StackService::class)->create('To do', $board->getId(), 0); + $card = Server::get(CardService::class)->create('Task', $stack->getId(), 'plain', 0, self::OWNER); + Server::get(AssignmentService::class)->assignUser($card->getId(), self::ALICE); + $this->actAs(null); + + $after = $this->rowCounts(self::QUIET_TABLES); + foreach (['notifications', 'activity'] as $table) { + if ($this->db->tableExists($table)) { + $this->assertGreaterThan($before[$table], $after[$table], $table); + } + } + $this->assertNotSame([], $this->deckEvents); + $this->assertGreaterThan(0, $this->activityEvents); + } + + /** + * Deck's services act for the user their permission service holds + */ + private function actAs(?string $uid): void { + Server::get(IUserSession::class)->setUser($uid === null ? null : Server::get(IUserManager::class)->get($uid)); + Server::get(PermissionService::class)->setUserId($uid); + } + + /** + * @param ?list $labels + * @param ?list $stacks + */ + private function ensureBoard(string $title = 'Launch plan', ?array $labels = null, ?array $stacks = null): array { + return $this->importer->ensureBoard(self::APP, $this->boardRef, self::OWNER, $title, '0082c9', + new \DateTimeImmutable('2025-10-01T10:00:00Z'), + $labels ?? [new ImportLabel('category1', 'Marketing', 'f1db50'), new ImportLabel('priority', 'Urgent', 'e9322d')], + $stacks ?? [new ImportStack('bucket-todo', 'To do', 0), new ImportStack('bucket-done', 'Done', 1)], + ); + } + + /** + * A card as an importing app sends it, with the hash it computes + */ + private function card(string $ref, array $fields = []): ImportCard { + $fields += [ + 'stackRef' => 'bucket-todo', + 'title' => 'Ship it', + 'description' => '', + 'order' => 0, + 'start' => null, + 'due' => null, + 'done' => null, + 'created' => new \DateTimeImmutable('2025-11-01T08:00:00Z'), + 'createdBy' => '', + 'assignees' => [], + 'labels' => [], + 'files' => [], + 'mode' => ImportCard::MODE_CREATE, + ]; + $fields['sha256'] ??= CardCanonicalForm::hash($ref, $fields['stackRef'], $fields['title'], $fields['description'], + $fields['order'], $fields['start'], $fields['due'], $fields['done'], $fields['assignees'], $fields['labels'], $fields['files']); + return new ImportCard($ref, $fields['stackRef'], $fields['title'], $fields['description'], $fields['order'], + $fields['start'], $fields['due'], $fields['done'], $fields['created'], $fields['createdBy'], + $fields['assignees'], $fields['labels'], $fields['files'], $fields['sha256'], $fields['mode']); + } + + /** + * @return list> + */ + private function deckShares(int $cardId): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_DECK, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter((string)$cardId))); + return $qb->executeQuery()->fetchAll(); + } + + /** + * @param list $tables + * @return array + */ + private function rowCounts(array $tables): array { + $counts = []; + foreach ($tables as $table) { + if (!$this->db->tableExists($table)) { + $counts[$table] = 0; + continue; + } + $qb = $this->db->getQueryBuilder(); + $qb->select($qb->func()->count('*'))->from($table); + $counts[$table] = (int)$qb->executeQuery()->fetchOne(); + } + return $counts; + } + + /** + * @param class-string<\OCP\BackgroundJob\TimedJob> $class + */ + private function runJob(string $class): void { + $job = Server::get($class); + $this->invokePrivate($job, 'run', [null]); + } +} diff --git a/tests/unit/Import/CardCanonicalFormTest.php b/tests/unit/Import/CardCanonicalFormTest.php new file mode 100644 index 0000000000..a1d274c82d --- /dev/null +++ b/tests/unit/Import/CardCanonicalFormTest.php @@ -0,0 +1,54 @@ + & \"launch\" \u{2028} ünïcødé 🚀", + "Notes\r\n\n- [ ] one\n- [x] two\t\x01\x7f\x08\x0c\\ /path", + 8, + null, + new \DateTimeImmutable('2026-03-04T17:30:00+01:00'), + new \DateTimeImmutable('2026-03-05T08:00:09.5Z'), + ['bob', 'Alice', 'älex'], + ['category2', 'category10', 'priority:urgent'], + [42, 7, 1000], + ); + + $this->assertSame('64303043128b3af48f9572ccba442a4bb91519c7d5c963a35ac4913cdda24231', $hash); + } + + public function testEmptyListsAreArrays(): void { + $hash = CardCanonicalForm::hash('r', 's', 't', '', 0, null, null, null, [], [], []); + + $this->assertSame('3012ac82cf824f37fe7c854d91ccb0a6d0e9b6681606e5940cf9fa19b249b5ce', $hash); + } + + public function testOrderOfListsDoesNotMatter(): void { + $this->assertSame( + CardCanonicalForm::hash('r', 's', 't', '', 0, null, null, null, ['a', 'b'], ['x', 'y'], [2, 1]), + CardCanonicalForm::hash('r', 's', 't', '', 0, null, null, null, ['b', 'a'], ['y', 'x'], [1, 2]), + ); + } + + public function testTimesAreUtcSeconds(): void { + $this->assertSame('2026-01-02T02:04:05Z', CardCanonicalForm::time(new \DateTime('2026-01-02T03:04:05.999+01:00'))); + $this->assertNull(CardCanonicalForm::time(null)); + } +} From d84e54da1b420c88c4a6249c06e553f3e81afd77 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 30 Oct 2026 12:30:55 +0100 Subject: [PATCH 4/4] fix(import): share attachments outside the card transaction Sharing a file runs the server's share and mount listeners, and one of them recovers from a failed insert into the mount cache. On PostgreSQL that failure aborts the surrounding transaction, so every card with an attachment failed there. Cards, labels and assignments are now written in one transaction and attachments after it; until they are written the reference has no source hash, so a repeat completes the card instead of reporting a conflict. Files are checked before anything is written, and share and audit side effects run after commit as well. A repeated ensureBoard no longer bumps the board's change time, and board IDs beyond Deck's 32-bit IDs are not found instead of failing in the database. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/Import/BoardImporter.php | 284 ++++++++++++------ .../integration/import/BoardImporterTest.php | 60 +++- 2 files changed, 248 insertions(+), 96 deletions(-) diff --git a/lib/Import/BoardImporter.php b/lib/Import/BoardImporter.php index da923226cb..657438f843 100644 --- a/lib/Import/BoardImporter.php +++ b/lib/Import/BoardImporter.php @@ -44,12 +44,14 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IRootFolder; +use OCP\Files\Node; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserManager; use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Share\IManager as IShareManager; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; /** * The board import API, written on Deck's mappers @@ -68,6 +70,8 @@ class BoardImporter implements IBoardImporter { private const MAX_STACK_TITLE = 100; private const MAX_LABEL_TITLE = 100; private const MAX_LIST_LIMIT = 1000; + /** Deck's IDs are 32-bit integers */ + private const MAX_DECK_ID = 2147483647; public function __construct( private IDBConnection $db, @@ -90,6 +94,7 @@ public function __construct( private ResourceListener $resources, private IEventDispatcher $eventDispatcher, private ITimeFactory $timeFactory, + private LoggerInterface $logger, ) { } @@ -118,10 +123,23 @@ public function ensureBoard(string $app, string $ref, string $ownerUid, string $ ]; } - return $this->retryOnUniqueViolation(fn () => $this->atomic( + $result = $this->retryOnUniqueViolation(fn () => $this->atomic( fn () => $this->writeBoard($app, $ref, $ownerUid, $title, $color, $labels, $stacks), $this->db, )); + if ($result['created']) { + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( + 'Board %s of "%s" was imported by %s', + [ + 'boardId' => $result['board_id'], + 'owner' => $ownerUid, + 'importSystem' => $app, + ], + false, + Operation::BoardImported->value, + )); + } + return $result; } /** @@ -133,6 +151,7 @@ private function writeBoard(string $app, string $ref, string $ownerUid, string $ $now = $this->timeFactory->getTime(); $changed = []; $created = false; + $wrote = false; $boardRef = $this->refMapper->findByRef($app, ImportRef::KIND_BOARD, 0, $ref); if ($boardRef === null) { $board = new Board(); @@ -145,6 +164,7 @@ private function writeBoard(string $app, string $ref, string $ownerUid, string $ $board = $this->boardMapper->insert($board); $this->insertRef($app, ImportRef::KIND_BOARD, 0, $ref, $board->getId(), self::valuesHash($title, $color)); $created = true; + $wrote = true; } else { try { $board = $this->boardMapper->find($boardRef->getObjectId(), false, false, true); @@ -171,26 +191,17 @@ private function writeBoard(string $app, string $ref, string $ownerUid, string $ $board->setLastModified($now); $this->boardMapper->update($board); $this->updateRef($boardRef, self::valuesHash($title, $color)); + $wrote = true; } } $boardId = $board->getId(); - [$labelIds, $labelsChanged] = $this->ensureLabels($app, $boardId, $labels); - [$stackIds, $stacksChanged] = $this->ensureStacks($app, $boardId, $stacks); + [$labelIds, $labelsChanged, $labelsWritten] = $this->ensureLabels($app, $boardId, $labels); + [$stackIds, $stacksChanged, $stacksWritten] = $this->ensureStacks($app, $boardId, $stacks); - $this->boardMapper->flushCache($boardId); - $this->changeHelper->boardChanged($boardId); - if ($created) { - $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent( - 'Board %s of "%s" was imported by %s', - [ - 'boardId' => $boardId, - 'owner' => $ownerUid, - 'importSystem' => $app, - ], - false, - Operation::BoardImported->value, - )); + if ($wrote || $labelsWritten || $stacksWritten) { + $this->boardMapper->flushCache($boardId); + $this->changeHelper->boardChanged($boardId); } return [ @@ -205,7 +216,7 @@ private function writeBoard(string $app, string $ref, string $ownerUid, string $ /** * @param list $labels - * @return array{0: array, 1: list} the label IDs by ref and the refs whose Deck version was kept + * @return array{0: array, 1: list, 2: bool} the label IDs by ref, the refs whose Deck version was kept and whether a label was written */ private function ensureLabels(string $app, int $boardId, array $labels): array { $refs = $this->refMapper->findByRefs($app, ImportRef::KIND_LABEL, $boardId, array_map(fn (ImportLabel $label) => $label->ref, $labels)); @@ -216,6 +227,7 @@ private function ensureLabels(string $app, int $boardId, array $labels): array { $ids = []; $changed = []; + $wrote = false; foreach ($labels as $wanted) { $labelRef = $refs[$wanted->ref] ?? null; if ($labelRef === null) { @@ -231,6 +243,7 @@ private function ensureLabels(string $app, int $boardId, array $labels): array { $this->insertRef($app, ImportRef::KIND_LABEL, $boardId, $wanted->ref, $adopted->getId(), ''); $ids[$wanted->ref] = $adopted->getId(); $changed[] = $wanted->ref; + $wrote = true; continue; } $label = new Label(); @@ -241,6 +254,7 @@ private function ensureLabels(string $app, int $boardId, array $labels): array { $existing[$label->getId()] = $label; $this->insertRef($app, ImportRef::KIND_LABEL, $boardId, $wanted->ref, $label->getId(), self::valuesHash($wanted->title, $wanted->color)); $ids[$wanted->ref] = $label->getId(); + $wrote = true; continue; } @@ -260,14 +274,15 @@ private function ensureLabels(string $app, int $boardId, array $labels): array { $label->setColor($wanted->color); $this->labelMapper->update($label); $this->updateRef($labelRef, $next); + $wrote = true; } } - return [$ids, $changed]; + return [$ids, $changed, $wrote]; } /** * @param list $stacks - * @return array{0: array, 1: list} the stack IDs by ref and the refs whose Deck version was kept + * @return array{0: array, 1: list, 2: bool} the stack IDs by ref, the refs whose Deck version was kept and whether a stack was written */ private function ensureStacks(string $app, int $boardId, array $stacks): array { $refs = $this->refMapper->findByRefs($app, ImportRef::KIND_STACK, $boardId, array_map(fn (ImportStack $stack) => $stack->ref, $stacks)); @@ -278,6 +293,7 @@ private function ensureStacks(string $app, int $boardId, array $stacks): array { $ids = []; $changed = []; + $wrote = false; foreach ($stacks as $wanted) { $stackRef = $refs[$wanted->ref] ?? null; if ($stackRef === null) { @@ -289,6 +305,7 @@ private function ensureStacks(string $app, int $boardId, array $stacks): array { $stack = $this->stackMapper->insert($stack); $this->insertRef($app, ImportRef::KIND_STACK, $boardId, $wanted->ref, $stack->getId(), self::valuesHash($wanted->title, $wanted->order)); $ids[$wanted->ref] = $stack->getId(); + $wrote = true; continue; } @@ -308,9 +325,10 @@ private function ensureStacks(string $app, int $boardId, array $stacks): array { $stack->setLastModified($this->timeFactory->getTime()); $this->stackMapper->update($stack); $this->updateRef($stackRef, $next); + $wrote = true; } } - return [$ids, $changed]; + return [$ids, $changed, $wrote]; } #[\Override] @@ -324,10 +342,7 @@ public function importCards(string $app, int $boardId, array $cards): array { throw EntryException::error(ImportResult::ERROR_INVALID, 'Not a card'); } $this->checkCard($card, $context, $seen); - $results[] = $this->retryOnUniqueViolation(fn () => $this->atomic( - fn () => $this->writeCard($app, $context, $card), - $this->db, - )); + $results[] = $this->importCard($app, $context, $card); } catch (EntryException $e) { $results[] = $e->result; } @@ -335,6 +350,46 @@ public function importCards(string $app, int $boardId, array $cards): array { return $results; } + /** + * Writes the card with its labels and assignments in one transaction, + * then its attachments: sharing a file runs the server's share and + * mount listeners, which must not run inside a transaction (a failed + * statement they recover from aborts it on PostgreSQL). Until the + * attachments are written the reference has no source hash, so a + * repeat completes the card. + * + * @throws EntryException + */ + private function importCard(string $app, BoardContext $context, ImportCard $card): ImportResult { + $cardRef = $this->refMapper->findByRef($app, ImportRef::KIND_CARD, $context->board->getId(), $card->ref); + $attached = $cardRef === null ? [] : $this->cardFiles($cardRef->getObjectId()); + $files = $this->resolveFiles($context->board, array_values(array_diff($card->files, array_keys($attached)))); + + $write = $this->retryOnUniqueViolation(fn () => $this->atomic( + fn () => $this->writeCard($app, $context, $card), + $this->db, + )); + if ($write instanceof ImportResult) { + return $write; + } + [$entity, $cardRef, $created] = $write; + $cardId = $entity->getId(); + + try { + $this->writeFiles($context->board, $cardId, $card->files, $files); + } catch (EntryException $e) { + $stored = $this->storedHash($context, $card->ref, $this->cardMapper->find($cardId, false)); + $this->updateRef($cardRef, $stored); + throw new EntryException(new ImportResult(ImportResult::STATUS_ERROR, $cardId, $stored, $e->result->errorCode, $e->result->errorMessage)); + } + + $stored = $this->storedHash($context, $card->ref, $this->cardMapper->find($cardId, false)); + $this->updateRef($cardRef, $stored, $card->sha256); + $this->changeHelper->cardChanged($cardId, false); + $this->fullTextSearch->handle($created ? new CardCreatedEvent($entity) : new CardUpdatedEvent($entity)); + return ImportResult::ok($cardId, $stored); + } + /** * @param array $seen the refs of the call so far * @throws EntryException @@ -374,9 +429,12 @@ private function checkCard(ImportCard $card, BoardContext $context, array &$seen } /** + * Writes the card, its labels and assignments, or answers without a write + * + * @return ImportResult|array{0: Card, 1: ImportRef, 2: bool} the result, or the written card, its reference and whether it is new * @throws EntryException */ - private function writeCard(string $app, BoardContext $context, ImportCard $card): ImportResult { + private function writeCard(string $app, BoardContext $context, ImportCard $card): ImportResult|array { $boardId = $context->board->getId(); $cardRef = $this->refMapper->findByRef($app, ImportRef::KIND_CARD, $boardId, $card->ref); if ($cardRef === null) { @@ -385,11 +443,8 @@ private function writeCard(string $app, BoardContext $context, ImportCard $card) } $entity = $this->insertCard($context, $card); $cardRef = $this->insertRef($app, ImportRef::KIND_CARD, $boardId, $card->ref, $entity->getId(), null); - $this->writeRelations($context, $entity->getId(), $card); - $stored = $this->storedHash($context, $card->ref, $this->cardMapper->find($entity->getId(), false)); - $this->updateRef($cardRef, $stored, $card->sha256); - $this->cardWritten($entity, true); - return ImportResult::ok($entity->getId(), $stored); + $this->writeLabelsAndAssignees($context, $entity->getId(), $card); + return [$entity, $cardRef, true]; } $entity = $this->findCard($cardRef->getObjectId()); @@ -409,18 +464,18 @@ private function writeCard(string $app, BoardContext $context, ImportCard $card) if ($card->sha256 === $cardRef->getSourceHash()) { return ImportResult::ok($entity->getId(), $current); } - if ($card->mode === ImportCard::MODE_CREATE) { + if ($card->mode === ImportCard::MODE_CREATE && $cardRef->getSourceHash() !== null) { throw EntryException::conflict(ImportResult::ERROR_EXISTS, 'The card exists with other content', $entity->getId(), $current); } } $this->updateCard($context, $entity, $card); - $this->writeRelations($context, $entity->getId(), $card); - $stored = $this->storedHash($context, $card->ref, $this->cardMapper->find($entity->getId(), false)); + $this->writeLabelsAndAssignees($context, $entity->getId(), $card); $cardRef->setState(ImportRef::STATE_ACTIVE); - $this->updateRef($cardRef, $stored, $card->sha256); - $this->cardWritten($entity, false); - return ImportResult::ok($entity->getId(), $stored); + $cardRef->setSourceHash(null); + $cardRef->setUpdatedAt($this->timeFactory->getTime()); + $this->refMapper->update($cardRef); + return [$entity, $cardRef, false]; } private function insertCard(BoardContext $context, ImportCard $card): Card { @@ -469,12 +524,7 @@ private function updateCard(BoardContext $context, Card $entity, ImportCard $car } } - /** - * Labels, assignments and file attachments - * - * @throws EntryException - */ - private function writeRelations(BoardContext $context, int $cardId, ImportCard $card): void { + private function writeLabelsAndAssignees(BoardContext $context, int $cardId, ImportCard $card): void { $wantedLabels = array_map(fn (string $ref) => $context->labelIds[$ref], $card->labels); $currentLabels = array_map(fn (Label $label) => $label->getId(), $this->labelMapper->findAssignedLabelsForCard($cardId)); foreach (array_diff($wantedLabels, $currentLabels) as $labelId) { @@ -502,45 +552,76 @@ private function writeRelations(BoardContext $context, int $cardId, ImportCard $ foreach ($currentAssignments as $assignment) { $this->assignmentMapper->delete($assignment); } + } - $currentFiles = $this->cardFiles($cardId); - foreach ($card->files as $fileId) { - if (isset($currentFiles[$fileId])) { - unset($currentFiles[$fileId]); - continue; + /** + * Finds each file and the user who shares it with a card: its owner, + * or the board owner for a file without one when they can see it + * + * @param list $fileIds + * @return array the file in the sharer's view and the sharer, by file ID + * @throws EntryException + */ + private function resolveFiles(Board $board, array $fileIds): array { + $files = []; + foreach ($fileIds as $fileId) { + $sharer = null; + $node = null; + try { + $sharer = $this->rootFolder->getFirstNodeById($fileId)?->getOwner()?->getUID(); + $node = $sharer === null ? null : $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId); + if ($node === null) { + $sharer = $board->getOwner(); + $node = $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId); + } + } catch (\Exception) { + $node = null; } - $this->attachFile($context->board, $cardId, $fileId); - } - foreach ($currentFiles as $shareId) { - $this->shareManager->deleteShare($this->shareProvider->getShareById((string)$shareId)); + if ($node === null || $sharer === null) { + throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' was not found'); + } + if (!$node->isShareable()) { + throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' may not be shared'); + } + $files[$fileId] = [$node, $sharer]; } + return $files; } /** - * Shares an existing file with the card as its owner, which makes it a - * Deck attachment of type `file` + * Makes $fileIds the card's file attachments * + * @param list $fileIds + * @param array $resolved the files that weren't attached yet * @throws EntryException */ - private function attachFile(Board $board, int $cardId, int $fileId): void { - try { - $node = $this->rootFolder->getFirstNodeById($fileId); - $sharer = $node?->getOwner()?->getUID(); - $userNode = $sharer === null ? null : $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId); - if ($userNode === null) { - // A file without an owner is shared by the board owner when they can see it - $sharer = $board->getOwner(); - $userNode = $this->rootFolder->getUserFolder($sharer)->getFirstNodeById($fileId); + private function writeFiles(Board $board, int $cardId, array $fileIds, array $resolved): void { + $attached = $this->cardFiles($cardId); + foreach ($fileIds as $fileId) { + if (!isset($attached[$fileId])) { + [$node, $sharer] = $resolved[$fileId] ?? $this->resolveFiles($board, [$fileId])[$fileId]; + $this->attachFile($board, $cardId, $fileId, $node, $sharer); } - } catch (\Exception) { - $userNode = null; } - if ($userNode === null || $sharer === null) { - throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' was not found'); + foreach (array_diff_key($attached, array_flip($fileIds)) as $fileId => $shareId) { + try { + $this->shareManager->deleteShare($this->shareProvider->getShareById((string)$shareId)); + } catch (\Exception $e) { + $this->logger->info('Imported file attachment could not be removed', ['exception' => $e, 'fileId' => $fileId, 'cardId' => $cardId]); + throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' could not be detached'); + } } + } + /** + * Shares an existing file with the card, which makes it a Deck + * attachment of type `file` + * + * @throws EntryException + */ + private function attachFile(Board $board, int $cardId, int $fileId, Node $node, string $sharer): void { $share = $this->shareManager->newShare(); - $share->setNode($userNode); + $share->setNode($node); $share->setShareType(IShare::TYPE_DECK); $share->setSharedWith((string)$cardId); $share->setPermissions(Constants::PERMISSION_READ); @@ -553,7 +634,8 @@ private function attachFile(Board $board, int $cardId, int $fileId): void { try { $this->shareManager->createShare($share); } catch (\Exception $e) { - throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' could not be shared: ' . $e->getMessage()); + $this->logger->info('Imported file attachment could not be shared', ['exception' => $e, 'fileId' => $fileId, 'cardId' => $cardId]); + throw EntryException::error(ImportResult::ERROR_FILE_UNAVAILABLE, 'File ' . $fileId . ' could not be shared'); } finally { $this->permissionService->setUserId($previousUser); } @@ -607,13 +689,15 @@ private function storedHash(BoardContext $context, string $ref, Card $card): str ); } + /** + * Whether somebody changed, archived or deleted a card since the importer wrote it + */ private function changedInDeck(Card $card, string $currentHash, ImportRef $cardRef): bool { - return $currentHash !== $cardRef->getStoredHash() || $card->getArchived() || $card->getDeletedAt() > 0; - } - - private function cardWritten(Card $card, bool $created): void { - $this->changeHelper->cardChanged($card->getId(), false); - $this->fullTextSearch->handle($created ? new CardCreatedEvent($card) : new CardUpdatedEvent($card)); + if ($card->getArchived() || $card->getDeletedAt() > 0) { + return true; + } + // A write the importer didn't finish differs from its last hash by design + return $cardRef->getSourceHash() !== null && $currentHash !== $cardRef->getStoredHash(); } #[\Override] @@ -669,7 +753,11 @@ public function removeCards(string $app, int $boardId, array $refs): array { $results = []; foreach ($refs as $ref) { try { - $results[] = $this->atomic(fn () => $this->removeCard($app, $context, $ref), $this->db); + [$result, $removed] = $this->atomic(fn () => $this->removeCard($app, $context, $ref), $this->db); + if ($removed !== null) { + $this->fullTextSearch->handle(new CardDeletedEvent($removed)); + } + $results[] = $result; } catch (EntryException $e) { $results[] = $e->result; } @@ -678,15 +766,16 @@ public function removeCards(string $app, int $boardId, array $refs): array { } /** + * @return array{0: ImportResult, 1: ?Card} the result and the card this call removed * @throws EntryException */ - private function removeCard(string $app, BoardContext $context, string $ref): ImportResult { + private function removeCard(string $app, BoardContext $context, string $ref): array { $cardRef = $this->refMapper->findByRef($app, ImportRef::KIND_CARD, $context->board->getId(), $ref); if ($cardRef === null) { throw EntryException::error(ImportResult::ERROR_NOT_FOUND, 'The card was not imported'); } if ($cardRef->getState() === ImportRef::STATE_REMOVED) { - return ImportResult::ok($cardRef->getObjectId()); + return [ImportResult::ok($cardRef->getObjectId()), null]; } $card = $this->findCard($cardRef->getObjectId()); if ($card === null) { @@ -702,14 +791,14 @@ private function removeCard(string $app, BoardContext $context, string $ref): Im $cardRef->setState(ImportRef::STATE_REMOVED); $this->updateRef($cardRef, $current); $this->changeHelper->cardChanged($card->getId(), false); - $this->fullTextSearch->handle(new CardDeletedEvent($card)); - return ImportResult::ok($card->getId()); + return [ImportResult::ok($card->getId()), $card]; } #[\Override] public function setAccess(string $app, int $boardId, array $acl): array { $context = $this->context($app, $boardId); $board = $context->board; + $this->markPastDueNotified($app, $boardId); $managed = []; foreach ($this->refMapper->findAllOfKind($app, ImportRef::KIND_ACL, $boardId) as $aclRef) { $managed[$aclRef->getRef()] = $aclRef; @@ -726,7 +815,11 @@ public function setAccess(string $app, int $boardId, array $acl): array { throw EntryException::error(ImportResult::ERROR_INVALID, 'The share appears twice'); } $wanted[$access->ref()] = true; - $results[] = $this->atomic(fn () => $this->writeAccess($app, $board, $access, $managed[$access->ref()] ?? null), $this->db); + [$result, $change] = $this->atomic(fn () => $this->writeAccess($app, $board, $access, $managed[$access->ref()] ?? null), $this->db); + if ($change !== null) { + $this->accessChanged($change[0], $change[1]); + } + $results[] = $result; } catch (EntryException $e) { $results[] = $e->result; } @@ -734,27 +827,30 @@ public function setAccess(string $app, int $boardId, array $acl): array { foreach ($managed as $ref => $aclRef) { if (!isset($wanted[$ref])) { - $this->atomic(fn () => $this->removeAccess($aclRef), $this->db); + $change = $this->atomic(fn () => $this->removeAccess($aclRef), $this->db); + if ($change !== null) { + $this->accessChanged($change[0], $change[1]); + } } } - $this->markPastDueNotified($app, $boardId); $this->boardMapper->flushCache($boardId); $this->changeHelper->boardChanged($boardId); return $results; } /** + * @return array{0: ImportResult, 1: ?array{0: AclCreatedEvent|AclUpdatedEvent, 1: Operation}} the result and the change to announce * @throws EntryException */ - private function writeAccess(string $app, Board $board, ImportAccess $access, ?ImportRef $aclRef): ImportResult { + private function writeAccess(string $app, Board $board, ImportAccess $access, ?ImportRef $aclRef): array { if ($access->type === ImportAccess::TYPE_USER) { $type = Acl::PERMISSION_TYPE_USER; if (!$this->userManager->userExists($access->id)) { throw EntryException::error(ImportResult::ERROR_UNKNOWN_USER, 'The user does not exist'); } if ($access->id === $board->getOwner()) { - return ImportResult::ok(); + return [ImportResult::ok(), null]; } } elseif ($access->type === ImportAccess::TYPE_GROUP) { $type = Acl::PERMISSION_TYPE_GROUP; @@ -785,8 +881,7 @@ private function writeAccess(string $app, Board $board, ImportAccess $access, ?I $entity->setPermissionManage($access->manage); $entity = $this->aclMapper->insert($entity); $this->insertRef($app, ImportRef::KIND_ACL, $board->getId(), $access->ref(), $entity->getId(), $next); - $this->accessChanged(new AclCreatedEvent($entity), Operation::BoardShareCreated); - return ImportResult::ok(); + return [ImportResult::ok(), [new AclCreatedEvent($entity), Operation::BoardShareCreated]]; } if ($aclRef === null || $aclRef->getObjectId() !== $existing->getId()) { @@ -802,29 +897,31 @@ private function writeAccess(string $app, Board $board, ImportAccess $access, ?I $existing->setPermissionManage($access->manage); $this->aclMapper->update($existing); $this->updateRef($aclRef, $next); - $this->accessChanged(new AclUpdatedEvent($existing), Operation::BoardShareUpdated); + return [ImportResult::ok(), [new AclUpdatedEvent($existing), Operation::BoardShareUpdated]]; } - return ImportResult::ok(); + return [ImportResult::ok(), null]; } /** * Removes a share the importer made unless somebody changed it in Deck + * + * @return ?array{0: AclDeletedEvent, 1: Operation} the change to announce */ - private function removeAccess(ImportRef $aclRef): void { + private function removeAccess(ImportRef $aclRef): ?array { try { /** @var Acl $existing */ $existing = $this->aclMapper->find($aclRef->getObjectId()); } catch (DoesNotExistException) { $this->refMapper->delete($aclRef); - return; + return null; } $current = self::valuesHash($existing->getPermissionEdit(), $existing->getPermissionShare(), $existing->getPermissionManage()); if ($current !== $aclRef->getStoredHash()) { - return; + return null; } $this->aclMapper->delete($existing); $this->refMapper->delete($aclRef); - $this->accessChanged(new AclDeletedEvent($existing), Operation::BoardShareDeleted); + return [new AclDeletedEvent($existing), Operation::BoardShareDeleted]; } /** @@ -890,6 +987,9 @@ private function context(string $app, int $boardId): BoardContext { } catch (EntryException $e) { throw new ImportException(ImportException::INVALID, $e->getMessage()); } + if ($boardId < 1 || $boardId > self::MAX_DECK_ID) { + throw new ImportException(ImportException::NOT_FOUND, 'The board does not exist'); + } $boardRef = $this->refMapper->findBoard($boardId); try { $board = $this->boardMapper->find($boardId); diff --git a/tests/integration/import/BoardImporterTest.php b/tests/integration/import/BoardImporterTest.php index eeda59a25b..8f5ced1656 100644 --- a/tests/integration/import/BoardImporterTest.php +++ b/tests/integration/import/BoardImporterTest.php @@ -158,6 +158,13 @@ public function testEnsureBoardCreatesBoardLabelsAndStacksOnce(): void { $this->assertCount(2, Server::get(LabelMapper::class)->findAll($first['board_id']), 'no default labels'); $this->assertCount(2, Server::get(StackMapper::class)->findAll($first['board_id'])); + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_boards') + ->set('last_modified', $qb->createNamedParameter(1000, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($first['board_id'], IQueryBuilder::PARAM_INT))) + ->executeStatement(); + Server::get(BoardMapper::class)->flushCache(); + $again = $this->ensureBoard(); $this->assertFalse($again['created']); $this->assertSame($first['board_id'], $again['board_id']); @@ -165,6 +172,7 @@ public function testEnsureBoardCreatesBoardLabelsAndStacksOnce(): void { $this->assertSame($first['stacks'], $again['stacks']); $this->assertSame([], $again['changed']); $this->assertCount(2, Server::get(LabelMapper::class)->findAll($first['board_id'])); + $this->assertSame(1000, Server::get(BoardMapper::class)->find($first['board_id'])->getLastModified(), 'a repeat writes nothing'); } public function testEnsureBoardUpdatesWhatNobodyChangedAndKeepsDeckChanges(): void { @@ -262,14 +270,14 @@ public function testRepeatedImportWritesNothing(): void { ->set('last_modified', $qb->createNamedParameter(1000, IQueryBuilder::PARAM_INT)) ->where($qb->expr()->eq('id', $qb->createNamedParameter($first->cardId, IQueryBuilder::PARAM_INT))) ->executeStatement(); - $rows = $this->rowCounts(['deck_cards', 'deck_assigned_users', 'deck_assigned_labels', 'share', 'deck_import_refs']); + $rows = $this->rowCounts(['deck_cards', 'deck_assigned_users', 'deck_assigned_labels', 'deck_import_refs']) + ['attachments' => $this->deckShareCount()]; [$second] = $this->importer->importCards(self::APP, $board['board_id'], [$card]); $this->assertSame(ImportResult::STATUS_OK, $second->status); $this->assertSame($first->cardId, $second->cardId); $this->assertSame($first->sha256, $second->sha256); - $this->assertSame($rows, $this->rowCounts(['deck_cards', 'deck_assigned_users', 'deck_assigned_labels', 'share', 'deck_import_refs'])); + $this->assertSame($rows, $this->rowCounts(['deck_cards', 'deck_assigned_users', 'deck_assigned_labels', 'deck_import_refs']) + ['attachments' => $this->deckShareCount()]); $this->assertSame(1000, $this->cardMapper->find($first->cardId, false)->getLastModified(), 'the card was not written again'); } @@ -340,6 +348,32 @@ public function testCardEditedInDeckIsReportedAndKept(): void { $this->assertSame(0, $this->cardMapper->find($first->cardId, false)->getDeletedAt()); } + public function testUnfinishedWriteIsCompletedByARepeat(): void { + $board = $this->ensureBoard(); + $card = $this->card('task-1', ['files' => [$this->fileId]]); + [$first] = $this->importer->importCards(self::APP, $board['board_id'], [$card]); + // A write that stopped before its attachments: no source hash, no share + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_import_refs') + ->set('source_hash', $qb->createNamedParameter(null)) + ->where($qb->expr()->eq('object_id', $qb->createNamedParameter($first->cardId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('kind', $qb->createNamedParameter('card'))) + ->executeStatement(); + $qb = $this->db->getQueryBuilder(); + $qb->delete('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_DECK, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('share_with', $qb->createNamedParameter((string)$first->cardId))) + ->executeStatement(); + + [$stat] = $this->importer->statCards(self::APP, $board['board_id'], ['task-1'], true); + $this->assertFalse($stat->changed); + [$repeat] = $this->importer->importCards(self::APP, $board['board_id'], [$card]); + + $this->assertSame(ImportResult::STATUS_OK, $repeat->status, (string)$repeat->errorMessage); + $this->assertSame($card->sha256, $repeat->sha256); + $this->assertCount(1, $this->deckShares($first->cardId)); + } + public function testUnchangedCardStatsWithoutHash(): void { $board = $this->ensureBoard(); [$first] = $this->importer->importCards(self::APP, $board['board_id'], [$this->card('task-1')]); @@ -391,7 +425,7 @@ public function testListCardsPages(): void { public function testInvalidCardsAreRefusedAndWriteNothing(): void { $board = $this->ensureBoard(); - $rows = $this->rowCounts(['deck_cards', 'deck_import_refs', 'share']); + $rows = $this->rowCounts(['deck_cards', 'deck_import_refs']) + ['attachments' => $this->deckShareCount()]; $results = $this->importer->importCards(self::APP, $board['board_id'], [ $this->card('a', ['stackRef' => 'bucket-unknown']), @@ -416,7 +450,7 @@ public function testInvalidCardsAreRefusedAndWriteNothing(): void { ImportResult::ERROR_INVALID, ImportResult::ERROR_INVALID, ], array_map(fn (ImportResult $r) => $r->errorCode, $results)); - $this->assertSame($rows, $this->rowCounts(['deck_cards', 'deck_import_refs', 'share']), 'a failed card leaves nothing behind'); + $this->assertSame($rows, $this->rowCounts(['deck_cards', 'deck_import_refs']) + ['attachments' => $this->deckShareCount()], 'a failed card leaves nothing behind'); } public function testBoardsOfOthersAreRefused(): void { @@ -479,6 +513,13 @@ public function testSetAccessManagesItsOwnShares(): void { $this->assertTrue($acl(Acl::PERMISSION_TYPE_GROUP, self::GROUP)->getPermissionShare()); $this->assertNull($acl(Acl::PERMISSION_TYPE_USER, self::ALICE)); $this->assertNotNull($acl(Acl::PERMISSION_TYPE_USER, self::CAROL), 'a share somebody else made stays'); + + $aclMapper->delete($acl(Acl::PERMISSION_TYPE_GROUP, self::GROUP)); + $results = $this->importer->setAccess(self::APP, $board['board_id'], [ + new ImportAccess(ImportAccess::TYPE_GROUP, self::GROUP, true, true, false), + ]); + $this->assertSame(ImportResult::ERROR_CHANGED, $results[0]->errorCode, 'a share removed in Deck is not made again'); + $this->assertNull($acl(Acl::PERMISSION_TYPE_GROUP, self::GROUP)); } /** @@ -598,6 +639,17 @@ private function deckShares(int $cardId): array { return $qb->executeQuery()->fetchAll(); } + /** + * Card attachments; the per-user shares Deck adds when somebody's files are set up are not counted + */ + private function deckShareCount(): int { + $qb = $this->db->getQueryBuilder(); + $qb->select($qb->func()->count('*')) + ->from('share') + ->where($qb->expr()->eq('share_type', $qb->createNamedParameter(IShare::TYPE_DECK, IQueryBuilder::PARAM_INT))); + return (int)$qb->executeQuery()->fetchOne(); + } + /** * @param list $tables * @return array