tests: assert json diff between import/export

Signed-off-by: Julius Härtl <jus@bitgrid.net>

lib/Db/BoardMapper.php

@@ -532,12 +532,12 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
 		if ($boardId) {
 			unset($this->boardCache[$boardId]);
 		} else {
-			$this->boardCache = null;
+			$this->boardCache = new CappedMemoryCache();
 		}
 		if ($userId) {
 			unset($this->userBoardCache[$userId]);
 		} else {
-			$this->userBoardCache = null;
+			$this->userBoardCache = new CappedMemoryCache();
 		}
 	}
 }

lib/Service/Importer/Systems/DeckJsonService.php

@@ -59,16 +59,11 @@ class DeckJsonService extends ABoardImportService {
 	}
 
 	public function validateUsers(): void {
-		if (empty($this->getImportService()->getConfig('uidRelation')) || !isset($this->getImportService()->getData()->members)) {
+		$relation = $this->getImportService()->getConfig('uidRelation');
+		if (empty($relation)) {
 			return;
 		}
-		foreach ($this->getImportService()->getConfig('uidRelation') as $exportUid => $nextcloudUid) {
+		foreach ($relation as $exportUid => $nextcloudUid) {
-			$user = array_filter($this->getImportService()->getData()->members, function (\stdClass $u) use ($exportUid) {
-				return $u->username === $exportUid;
-			});
-			if (!$user) {
-				throw new \LogicException('Trello user ' . $exportUid . ' not found in property "members" of json data');
-			}
 			if (!is_string($nextcloudUid) && !is_numeric($nextcloudUid)) {
 				throw new \LogicException('User on setting uidRelation is invalid');
 			}
@@ -77,14 +72,12 @@ class DeckJsonService extends ABoardImportService {
 			if (!$this->getImportService()->getConfig('uidRelation')->$exportUid) {
 				throw new \LogicException('User on setting uidRelation not found: ' . $nextcloudUid);
 			}
-			$user = current($user);
+			$this->members[$exportUid] = $this->getImportService()->getConfig('uidRelation')->$exportUid;
-			$this->members[$user->id] = $this->getImportService()->getConfig('uidRelation')->$exportUid;
 		}
 	}
 
 	public function mapMember($uid): ?string {
-
+		$uidCandidate = isset($this->members[$uid]) ? $this->members[$uid]?->getUID() ?? null : null;
-		$uidCandidate = $this->members[$uid]?->getUID() ?? null;
 		if ($uidCandidate) {
 			return $uidCandidate;
 		}
@@ -96,6 +89,15 @@ class DeckJsonService extends ABoardImportService {
 		return null;
 	}
 
+	public function mapOwner(string $uid): string {
+		$configOwner = $this->getImportService()->getConfig('owner');
+		if ($configOwner) {
+			return $configOwner->getUID();
+		}
+
+		return $uid;
+	}
+
 	public function getCardAssignments(): array {
 		$assignments = [];
 		foreach ($this->tmpCards as $sourceCard) {
@@ -134,7 +136,7 @@ class DeckJsonService extends ABoardImportService {
 		}
 		$importBoard = $this->getImportService()->getData();
 		$board->setTitle($importBoard->title);
-		$board->setOwner($importBoard->owner?->uid ?? $importBoard->owner);
+		$board->setOwner($this->mapOwner($importBoard->owner?->uid ?? $importBoard->owner));
 		$board->setColor($importBoard->color);
 		$board->setArchived($importBoard->archived);
 		$board->setDeletedAt($importBoard->deletedAt);
@@ -168,6 +170,7 @@ class DeckJsonService extends ABoardImportService {
 				$stack->setTitle($source->title);
 				$stack->setBoardId($this->getImportService()->getBoard()->getId());
 				$stack->setOrder($source->order);
+				$stack->setLastModified($source->lastModified);
 				$return[$source->id] = $stack;
 			}
 
@@ -191,13 +194,15 @@ class DeckJsonService extends ABoardImportService {
 			$card = new Card();
 			$card->setTitle($cardSource->title);
 			$card->setLastModified($cardSource->lastModified);
+			$card->setLastEditor($cardSource->lastEditor);
 			$card->setCreatedAt($cardSource->createdAt);
 			$card->setArchived($cardSource->archived);
 			$card->setDescription($cardSource->description);
 			$card->setStackId($this->stacks[$cardSource->stackId]->getId());
 			$card->setType('plain');
 			$card->setOrder($cardSource->order);
-			$card->setOwner($this->getBoard()->getOwner());
+			$boardOwner = $this->getBoard()->getOwner();
+			$card->setOwner($this->mapOwner(is_string($boardOwner) ? $boardOwner : $boardOwner->getUID()));
 			$card->setDuedate($cardSource->duedate);
 			$cards[$cardSource->id] = $card;
 		}
@@ -215,11 +220,18 @@ class DeckJsonService extends ABoardImportService {
 			$acl = new Acl();
 			$acl->setBoardId($this->getImportService()->getBoard()->getId());
 			$acl->setType($aclData->type);
-			$acl->setParticipant($aclData->participant?->primaryKey ?? $aclData->participant);
+			$participant = $aclData->participant?->primaryKey ?? $aclData->participant;
+			if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
+				$participant = $this->mapMember($participant);
+			}
+			$acl->setParticipant($participant);
 			$acl->setPermissionEdit($aclData->permissionEdit);
 			$acl->setPermissionShare($aclData->permissionShare);
 			$acl->setPermissionManage($aclData->permissionManage);
-			$return[] = $acl;
+			if ($participant) {
+				$return[] = $acl;
+			}
+			// TODO: Once we have error collection we should catch non-existing users
 		}
 		return $return;
 	}

lib/Service/Importer/fixtures/config-deckJson-schema.json

@@ -12,13 +12,6 @@
             "type": "string",
             "required": true,
             "comment": "Nextcloud owner username"
-        },
-        "color": {
-            "type": "string",
-            "required": true,
-            "pattern": "^[0-9a-fA-F]{6}$",
-            "comment": "Default color for the board. If you don't inform, the default color will be used.",
-            "default": "0800fd"
         }
     }
-}
+}

tests/data/config-deckJson.json

@@ -0,0 +1,7 @@
+{
+    "owner": "admin",
+    "color": "0800fd",
+    "uidRelation": {
+        "johndoe": "test-user-1"
+    }
+}

tests/integration/import/ImportExportTest.php

@@ -26,10 +26,12 @@ namespace OCA\Deck\Db;
 use OCA\Deck\Command\BoardImport;
 use OCA\Deck\Command\UserExport;
 use OCA\Deck\Service\BoardService;
+use OCA\Deck\Service\CardService;
 use OCA\Deck\Service\Importer\BoardImportService;
 use OCA\Deck\Service\Importer\Systems\DeckJsonService;
 use OCA\Deck\Service\PermissionService;
 use OCA\Deck\Service\StackService;
+use OCP\App\IAppManager;
 use OCP\AppFramework\Db\Entity;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
@@ -59,6 +61,9 @@ class ImportExportTest extends \Test\TestCase {
 		$backend = new \Test\Util\User\Dummy();
 		\OC_User::useBackend($backend);
 		Server::get(IUserManager::class)->registerBackend($backend);
+		$backend->createUser('alice', 'alice');
+		$backend->createUser('jane', 'jane');
+		$backend->createUser('johndoe', 'johndoe');
 		$backend->createUser(self::TEST_USER1, self::TEST_USER1);
 		$backend->createUser(self::TEST_USER2, self::TEST_USER2);
 		$backend->createUser(self::TEST_USER3, self::TEST_USER3);
@@ -78,68 +83,118 @@ class ImportExportTest extends \Test\TestCase {
 		$groupBackend->addToGroup(self::TEST_USER4, 'group3');
 		$groupBackend->addToGroup(self::TEST_USER2, self::TEST_GROUP1);
 		Server::get(IGroupManager::class)->addBackend($groupBackend);
+
+		Server::get(PermissionService::class)->setUserId('admin');
 	}
 
 	public function setUp(): void {
 		parent::setUp();
 
 		$this->connection = \OCP\Server::get(IDBConnection::class);
-		$this->connection->beginTransaction();
+		$this->cleanDb();
-
+		$this->cleanDb(self::TEST_USER1);
 	}
 
 	public function testImportOcc() {
-		$input = $this->createMock(InputInterface::class);
+		$this->importFromFile(__DIR__ . '/../../data/deck.json');
-		$input->expects($this->any())
-			->method('getOption')
-			->willReturnCallback(function ($arg) {
-				return match ($arg) {
-					'system' => 'DeckJson',
-					'data' => __DIR__ . '/../../data/deck.json',
-					'config' => __DIR__ . '/../../data/config-trelloJson.json',
-				};
-			});
-		$output = $this->createMock(OutputInterface::class);
-		$importer = \OCP\Server::get(BoardImport::class);
-		$application = new Application();
-		$importer->setApplication($application);
-		$importer->run($input, $output);
-
 		$this->assertDatabase();
 	}
 
 	/**
-	 * This test runs an import, export and another import to assert that multiple attempts result in the same data structure
+	 * This test runs an import, export and another import and
+	 * assert that multiple attempts result in the same data structure
+	 *
+	 * In addition, it asserts that multiple import/export runs result in the same JSON
 	 */
 	public function testReimportOcc() {
-		// initial import from test fixture json
+		$this->importFromFile(__DIR__ . '/../../data/deck.json');
+		$this->assertDatabase();
+
+		$tmpExportFile = $this->exportToTemp();
+
+		// Useful for double checking differences as there is no easy way to compare equal with skipping certain id keys, etag
+		// self::assertEquals(file_get_contents(__DIR__ . '/../../data/deck.json'), $jsonOutput);
+		self::assertEquals(
+			self::writeArrayStructure(array: json_decode(file_get_contents(__DIR__ . '/../../data/deck.json'), true)),
+			self::writeArrayStructure(array: json_decode(file_get_contents($tmpExportFile), true))
+		);
+
+		// cleanup test database
+		$this->cleanDb();
+
+		// Re-import from temporary file
+		$this->importFromFile($tmpExportFile);
+		$this->assertDatabase();
+
+		$tmpExportFile2 = $this->exportToTemp();
+		self::assertEquals(
+			self::writeArrayStructure(array: json_decode(file_get_contents($tmpExportFile), true)),
+			self::writeArrayStructure(array: json_decode(file_get_contents($tmpExportFile2), true))
+		);
+	}
+
+	public static function writeArrayStructure(string $prefix = '', array $array = [], array $skipKeyList = ['id', 'boardId', 'cardId', 'stackId', 'ETag', 'permissions', 'shared']): string {
+		$output = '';
+		$arrayIsList = array_keys($array) === range(0, count($array) - 1);
+		foreach ($array as $key => $value) {
+			$tmpPrefix = $prefix;
+			if (in_array($key, $skipKeyList)) {
+				continue;
+			}
+			if (is_array($value)) {
+				if ($key === 'participant' || $key === 'owner') {
+					$output .= $tmpPrefix . $key . ' => ' . $value['primaryKey'] . PHP_EOL;
+					continue;
+				}
+				$tmpPrefix .= (!$arrayIsList && !is_numeric($key) ? $key : '!!!') . ' => ';
+				$output .= self::writeArrayStructure($tmpPrefix, $value, $skipKeyList);
+			} else {
+				$output .= $tmpPrefix . $key . ' => ' . $value . PHP_EOL;
+			}
+		}
+		return $output;
+	}
+
+	public function cleanDb(string $owner = 'admin'): void {
+		$this->connection->executeQuery('DELETE from oc_deck_boards;');
+	}
+
+	private function importFromFile(string $filePath): void {
 		$input = $this->createMock(InputInterface::class);
 		$input->expects($this->any())
 			->method('getOption')
-			->willReturnCallback(function ($arg) {
+			->willReturnCallback(function ($arg) use ($filePath) {
 				return match ($arg) {
 					'system' => 'DeckJson',
-					'data' => __DIR__ . '/../../data/deck.json',
+					'data' => $filePath,
 					'config' => __DIR__ . '/../../data/config-trelloJson.json',
 				};
 			});
 		$output = $this->createMock(OutputInterface::class);
-		$importer = \OCP\Server::get(BoardImport::class);
+		$importer = self::getFreshService(BoardImport::class);
 		$application = new Application();
 		$importer->setApplication($application);
 		$importer->run($input, $output);
+	}
 
-		$this->assertDatabase();
+	/** Returns the path of a deck export json */
-
+	private function exportToTemp(): string {
-		self::overwriteService(BoardService::class, self::getFreshService(BoardService::class));
+		\OCP\Server::get(BoardMapper::class)->flushCache();
-		// export to a temporary file
+		$application = new Application();
 		$input = $this->createMock(InputInterface::class);
 		$input->expects($this->any())
 			->method('getArgument')
 			->with('user-id')
 			->willReturn('admin');
 		$output = new BufferedOutput();
-		$exporter = \OCP\Server::get(UserExport::class);
+		$exporter = new UserExport(
+			\OCP\Server::get(IAppManager::class),
+			self::getFreshService(BoardMapper::class),
+			self::getFreshService(BoardService::class),
+			self::getFreshService(StackMapper::class),
+			self::getFreshService(CardMapper::class),
+			self::getFreshService(AssignmentMapper::class),
+		);
 		$exporter->setApplication($application);
 		$exporter->run($input, $output);
 		$jsonOutput = $output->fetch();
@@ -147,37 +202,12 @@ class ImportExportTest extends \Test\TestCase {
 		self::assertTrue(json_last_error() === JSON_ERROR_NONE);
 		$tmpExportFile = tempnam('/tmp', 'export');
 		file_put_contents($tmpExportFile, $jsonOutput);
-
+		return $tmpExportFile;
-		// self::assertEquals(file_get_contents(__DIR__ . '/../../data/deck.json'), $jsonOutput);
-
-		// cleanup test database
-		$this->connection->rollBack();
-		$this->connection->beginTransaction();
-
-		self::overwriteService(BoardService::class, self::getFreshService(BoardService::class));
-		// Re-import from temporary file
-		$input = $this->createMock(InputInterface::class);
-		$input->expects($this->any())
-			->method('getOption')
-			->willReturnCallback(function ($arg) use ($tmpExportFile) {
-				return match ($arg) {
-					'system' => 'DeckJson',
-					'data' => $tmpExportFile,
-					'config' => __DIR__ . '/../../data/config-trelloJson.json',
-				};
-			});
-		$output = $this->createMock(OutputInterface::class);
-		$importer = \OCP\Server::get(BoardImport::class);
-		$application = new Application();
-		$importer->setApplication($application);
-		$importer->run($input, $output);
-
-		$this->assertDatabase();
 	}
 
 	public function testImport() {
-		$importer = \OCP\Server::get(BoardImportService::class);
+		$importer = self::getFreshService(BoardImportService::class);
-		$deckJsonService = \OCP\Server::get(DeckJsonService::class);
+		$deckJsonService = self::getFreshService(DeckJsonService::class);
 		$deckJsonService->setImportService($importer);
 
 		$importer->setSystem('DeckJson');
@@ -189,23 +219,65 @@ class ImportExportTest extends \Test\TestCase {
 		$this->assertDatabase();
 	}
 
+	public function testImportAsOtherUser() {
+		$importer = self::getFreshService(BoardImportService::class);
+		$deckJsonService = self::getFreshService(DeckJsonService::class);
+		$deckJsonService->setImportService($importer);
+
+		$importer->setSystem('DeckJson');
+		$importer->setImportSystem($deckJsonService);
+		$importer->setConfigInstance((object)[
+			'owner' => self::TEST_USER1
+		]);
+		$importer->setData(json_decode(file_get_contents(__DIR__ . '/../../data/deck.json')));
+		$importer->import();
+
+		$this->assertDatabase(self::TEST_USER1);
+	}
+
+	public function testImportWithRemap() {
+		$importer = self::getFreshService(BoardImportService::class);
+		$deckJsonService = self::getFreshService(DeckJsonService::class);
+		$deckJsonService->setImportService($importer);
+
+		$importer->setSystem('DeckJson');
+		$importer->setImportSystem($deckJsonService);
+		$importer->setConfigInstance((object)[
+			'owner' => self::TEST_USER1,
+			'uidRelation' => (object)[
+				'alice' => self::TEST_USER2,
+				'jane' => self::TEST_USER3,
+			],
+		]);
+		$importer->setData(json_decode(file_get_contents(__DIR__ . '/../../data/deck.json')));
+		$importer->import();
+
+		$this->assertDatabase(self::TEST_USER1);
+		$otherUserboards = self::getFreshService(BoardMapper::class)->findAllByUser(self::TEST_USER2);
+		self::assertCount(1, $otherUserboards);
+	}
+
 	/**
 	 * @template T
 	 * @param class-string<T>|string $className
 	 * @return T
 	 */
 	private function getFreshService(string $className): mixed {
-		return \OC::$server->getRegisteredAppContainer('deck')->resolve($className);
+		$fresh = \OC::$server->getRegisteredAppContainer('deck')->resolve($className);
+		self::overwriteService($className, $fresh);
+		return $fresh;
 	}
 
-	public function assertDatabase() {
+	public function assertDatabase(string $owner = 'admin') {
-		$permissionService = \OCP\Server::get(PermissionService::class);
+		$permissionService = self::getFreshService(PermissionService::class);
-		$permissionService->setUserId('admin');
+		$permissionService->setUserId($owner);
-		$boardMapper = \OCP\Server::get(BoardMapper::class);
+		self::getFreshService(BoardService::class);
-		$stackMapper = \OCP\Server::get(StackMapper::class);
+		self::getFreshService(CardService::class);
-		$cardMapper = \OCP\Server::get(CardMapper::class);
+		$boardMapper = self::getFreshService(BoardMapper::class);
+		$stackMapper = self::getFreshService(StackMapper::class);
+		$cardMapper = self::getFreshService(CardMapper::class);
 
-		$boards = $boardMapper->findAllByOwner('admin');
+		$boards = $boardMapper->findAllByOwner($owner);
 		$boardNames = array_map(fn ($board) => $board->getTitle(), $boards);
 		self::assertEquals(2, count($boards));
 
@@ -213,7 +285,7 @@ class ImportExportTest extends \Test\TestCase {
 		self::assertEntity(Board::fromRow([
 			'title' => 'My test board',
 			'color' => 'e0ed31',
-			'owner' => 'admin',
+			'owner' => $owner,
 			'lastModified' => 1689667796,
 		]), $board);
 		$boardService = $this->getFreshService(BoardService::class);
@@ -230,16 +302,19 @@ class ImportExportTest extends \Test\TestCase {
 			'title' => 'A',
 			'order' => 999,
 			'boardId' => $boards[0]->getId(),
+			'lastModified' => 1689667779,
 		]), $stacks[0]);
 		self::assertEntity(Stack::fromRow([
 			'title' => 'B',
 			'order' => 999,
 			'boardId' => $boards[0]->getId(),
+			'lastModified' => 1689667796,
 		]), $stacks[1]);
 		self::assertEntity(Stack::fromRow([
 			'title' => 'C',
 			'order' => 999,
 			'boardId' => $boards[0]->getId(),
+			'lastModified' => 0,
 		]), $stacks[2]);
 
 		$cards = $cardMapper->findAll($stacks[0]->getId());
@@ -249,7 +324,7 @@ class ImportExportTest extends \Test\TestCase {
 			'type' => 'plain',
 			'lastModified' => 1689667779,
 			'createdAt' => 1689667569,
-			'owner' => 'admin',
+			'owner' => $owner,
 			'duedate' => new \DateTime('2050-07-24T22:00:00.000000+0000'),
 			'order' => 999,
 			'stackId' => $stacks[0]->getId(),
@@ -275,10 +350,10 @@ class ImportExportTest extends \Test\TestCase {
 		self::assertEntity(Board::fromRow([
 			'title' => 'Shared board',
 			'color' => '30b6d8',
-			'owner' => 'admin',
+			'owner' => $owner,
 		]), $sharedBoard, true);
 
-		$stackService = \OCP\Server::get(StackService::class);
+		$stackService = self::getFreshService(StackService::class);
 		$stacks = $stackService->findAll($board->getId());
 		self::assertEntityInArray(Label::fromParams([
 			'title' => 'L2',
@@ -334,9 +409,8 @@ class ImportExportTest extends \Test\TestCase {
 	}
 
 	public function tearDown(): void {
-		if ($this->connection->inTransaction()) {
+		$this->cleanDb();
-			$this->connection->rollBack();
+		$this->cleanDb(self::TEST_USER1);
-		}
 		parent::tearDown();
 	}
 }

tests/unit/Service/Importer/Systems/DeckJsonServiceTest.php

@@ -65,13 +65,13 @@ class DeckJsonServiceTest extends \Test\TestCase {
 		$data = json_decode(file_get_contents(__DIR__ . '/../../../../data/deck.json'));
 		$importService->setData($data);
 
-		$configInstance = json_decode(file_get_contents(__DIR__ . '/../../../../data/config-trelloJson.json'));
+		$configInstance = json_decode(file_get_contents(__DIR__ . '/../../../../data/config-deckJson.json'));
 		$importService->setConfigInstance($configInstance);
 
 		$owner = $this->createMock(IUser::class);
 		$owner
 			->method('getUID')
-			->willReturn('owner');
+			->willReturn('admin');
 		$importService->setConfig('owner', $owner);
 
 		$this->service->setImportService($importService);