From 85aa554013c7287acf84c72c1adb6935d3229f00 Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Fri, 10 Jun 2022 15:55:15 +0200 Subject: [PATCH] BookmarkMapper#findAll: Implement recursive query (#1764) --- lib/Controller/FoldersController.php | 2 +- lib/Db/BookmarkMapper.php | 245 ++++++++++++++++++--------- psalm-baseline.xml | 3 + tests/BookmarkControllerTest.php | 8 +- tests/BookmarkMapperTest.php | 27 +-- tests/FindTest.php | 8 + tests/TagMapperTest.php | 2 + 7 files changed, 202 insertions(+), 93 deletions(-) diff --git a/lib/Controller/FoldersController.php b/lib/Controller/FoldersController.php index f7dad942..1ecd6328 100644 --- a/lib/Controller/FoldersController.php +++ b/lib/Controller/FoldersController.php @@ -633,7 +633,7 @@ class FoldersController extends ApiController { return $share->toArray(); }, $shares)]); } - if (Authorizer::hasPermission(Authorizer::PERM_READ, $permissions)) { + if (Authorizer::hasPermission(Authorizer::PERM_READ, $permissions) && $this->authorizer->getUserId() !== null) { try { $this->folderMapper->find($folderId); $share = $this->shareMapper->findByFolderAndUser($folderId, $this->authorizer->getUserId()); diff --git a/lib/Db/BookmarkMapper.php b/lib/Db/BookmarkMapper.php index e6048feb..082d9e8b 100644 --- a/lib/Db/BookmarkMapper.php +++ b/lib/Db/BookmarkMapper.php @@ -18,6 +18,7 @@ use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\QBMapper; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\DB\QueryBuilder\IQueryFunction; use OCP\EventDispatcher\IEventDispatcher; @@ -190,14 +191,19 @@ class BookmarkMapper extends QBMapper { /** * @param string $userId - * @param QueryParameters $params + * @param QueryParameters $queryParams * * @return Entity[] * * @throws UrlParseError * @throws \OC\DB\Exceptions\DbalException + * @throws Exception */ - public function findAll(string $userId, QueryParameters $params, $withGroupBy = true): array { + public function findAll(string $userId, QueryParameters $queryParams, bool $withGroupBy = true): array { + $rootFolder = $this->folderMapper->findRootFolder($userId); + // gives us all bookmarks in this folder, recursively + [$cte, $params, $paramTypes] = $this->_generateCTE($rootFolder->getId()); + $qb = $this->db->getQueryBuilder(); $bookmark_cols = array_map(static function ($c) { return 'b.' . $c; @@ -210,46 +216,44 @@ class BookmarkMapper extends QBMapper { $this->_selectFolders($qb); $this->_selectTags($qb); } + $qb->automaticTablePrefix(false); $qb - ->from('bookmarks', 'b') - ->leftJoin('b', 'bookmarks_tree', 'tr', 'tr.id = b.id AND tr.type = '.$qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK)) - ->leftJoin('tr', 'bookmarks_shared_folders', 'sf', $qb->expr()->eq('tr.parent_folder', 'sf.folder_id')) - ->leftJoin('tr', 'bookmarks_tree', 'tr2', 'tr2.id = tr.parent_folder AND tr2.type = '. $qb->createPositionalParameter(TreeMapper::TYPE_FOLDER)) - ->leftJoin('tr2', 'bookmarks_shared_folders', 'sf2', $qb->expr()->eq('tr2.parent_folder', 'sf.folder_id')) - ->where( - $qb->expr()->andX( - $qb->expr()->orX( - // This is only really used, when not adding folder=XXX in the controller - // as that causes $userId to be set to the folder's owner - $qb->expr()->eq('b.user_id', $qb->createPositionalParameter($userId)), - $qb->expr()->eq('sf.user_id', $qb->createPositionalParameter($userId)), - $qb->expr()->eq('sf2.user_id', $qb->createPositionalParameter($userId)) - ), - $qb->expr()->in('b.user_id', array_map([$qb, 'createPositionalParameter'], array_merge($this->_findSharersFor($userId), [$userId]))) - ) - ); + ->from('*PREFIX*bookmarks', 'b') + ->join('b', 'folder_tree', 'tree', 'tree.item_id = b.id AND tree.type = '.$qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK)); - $this->_filterUrl($qb, $params); - $this->_filterArchived($qb, $params); - $this->_filterUnavailable($qb, $params); - $this->_filterDuplicated($qb, $params); - $this->_filterFolder($qb, $params); - $this->_filterTags($qb, $params); - $this->_filterUntagged($qb, $params); - $this->_filterSearch($qb, $params); - $this->_sortAndPaginate($qb, $params); + $this->_filterUrl($qb, $queryParams); + $this->_filterArchived($qb, $queryParams); + $this->_filterUnavailable($qb, $queryParams); + $this->_filterDuplicated($qb, $queryParams); + $this->_filterFolder($qb, $queryParams); + $this->_filterTags($qb, $queryParams); + $this->_filterUntagged($qb, $queryParams); + $this->_filterSearch($qb, $queryParams); + $this->_sortAndPaginate($qb, $queryParams); - try { - return $this->findEntities($qb); - } catch (\OC\DB\Exceptions\DbalException $e) { - if (!$withGroupBy) { - throw $e; - } - // If mysql sort_buffer_size is too small, the group by caused by selecting tags and folders can cause issues - // in this case, we repeat the query without groupBys and rely on the BookmarkController to add tags and folders entries - return $this->findAll($userId, $params, false); + $finalQuery = $cte . ' ' . $qb->getSQL(); + + $params = array_merge($params, $qb->getParameters()); + $paramTypes = array_merge($paramTypes, $qb->getParameterTypes()); + return $this->findEntitiesWithRawQuery($finalQuery, $params, $paramTypes); + } + + /** + * @throws \OCP\DB\Exception + */ + protected function findEntitiesWithRawQuery(string $query, array $params, array $types) { + $cursor = $this->db->executeQuery($query, $params, $types); + + $entities = []; + + while ($row = $cursor->fetch()) { + $entities[] = $this->mapRowToEntity($row); } + + $cursor->closeCursor(); + + return $entities; } /** @@ -271,14 +275,106 @@ class BookmarkMapper extends QBMapper { return (int)$count; } + /** + * Common table expression that lists all items in a given folder, recursively + * @param int $folderId + * @return array + */ + private function _generateCTE(int $folderId) : array { + // The base case of the recursion is just the folder we're given + $baseCase = $this->db->getQueryBuilder(); + $baseCase + ->selectAlias($baseCase->createFunction($this->getDbType() === 'mysql'? 'cast('.$baseCase->createPositionalParameter($folderId, IQueryBuilder::PARAM_INT).' as UNSIGNED)' : 'cast('.$baseCase->createPositionalParameter($folderId, IQueryBuilder::PARAM_INT).' as BIGINT)'), 'item_id') + ->selectAlias($baseCase->createFunction($this->getDbType() === 'mysql'? 'cast(0 as UNSIGNED)' : 'cast(0 as BIGINT)'), 'parent_folder') + ->selectAlias($baseCase->createFunction($this->getDbType() === 'mysql'? 'cast('.$baseCase->createPositionalParameter(TreeMapper::TYPE_FOLDER).' as CHAR(20))' : 'cast('.$baseCase->createPositionalParameter(TreeMapper::TYPE_FOLDER).' as TEXT)'), 'type') + ->selectAlias($baseCase->createFunction($this->getDbType() === 'mysql'? 'cast(0 as UNSIGNED)' : 'cast(0 as BIGINT)'), 'idx'); + + // The first recursive case lists all children of folders we've already found + $recursiveCase = $this->db->getQueryBuilder(); + $recursiveCase->automaticTablePrefix(false); + $recursiveCase + ->selectAlias('tr.id', 'item_id') + ->selectAlias('tr.parent_folder', 'parent_folder') + ->selectAlias('tr.type', 'type') + ->selectAlias('tr.index', 'idx') + ->from('*PREFIX*bookmarks_tree', 'tr') + ->join('tr', $this->getDbType() === 'mysql'? 'folder_tree' : 'inner_folder_tree', 'e', 'e.item_id = tr.parent_folder AND e.type = '.$recursiveCase->createPositionalParameter(TreeMapper::TYPE_FOLDER)); + + // The second recursive case lists all children of shared folders we've already found + $recursiveCaseShares = $this->db->getQueryBuilder(); + $recursiveCaseShares->automaticTablePrefix(false); + $recursiveCaseShares + ->selectAlias('s.folder_id', 'item_id') + ->addSelect('e.parent_folder') + ->selectAlias($recursiveCaseShares->createFunction($recursiveCaseShares->createPositionalParameter(TreeMapper::TYPE_FOLDER)), 'type') + ->selectAlias('e.idx', 'idx') + ->from(($this->getDbType() === 'mysql'? 'folder_tree' : 'second_folder_tree'), 'e') + ->join('e', '*PREFIX*bookmarks_shared_folders', 's', 's.id = e.item_id AND e.type = '.$recursiveCaseShares->createPositionalParameter(TreeMapper::TYPE_SHARE)); + + if ($this->getDbType() === 'mysql') { + // For mysql we can just throw these three queries together in a CTE + $withRecursiveQuery = 'WITH RECURSIVE folder_tree(item_id, parent_folder, type, idx) AS ( ' . + $baseCase->getSQL() . ' UNION ALL ' . $recursiveCase->getSQL() . + ' UNION ALL ' . $recursiveCaseShares->getSQL() . ')'; + } else { + // Postgres loves us dearly and doesn't allow two recursive references in one CTE, aaah. + // So we nest them: + + $secondBaseCase = $this->db->getQueryBuilder(); + $secondBaseCase->automaticTablePrefix(false); + $secondBaseCase + ->select('item_id', 'parent_folder', 'type', 'idx') + ->from('inner_folder_tree'); + + $thirdBaseCase = $this->db->getQueryBuilder(); + $thirdBaseCase->automaticTablePrefix(false); + $thirdBaseCase + ->select('item_id', 'parent_folder', 'type', 'idx') + ->from('second_folder_tree'); + + $secondRecursiveCase = $this->db->getQueryBuilder(); + $secondRecursiveCase->automaticTablePrefix(false); + $secondRecursiveCase + ->selectAlias('tr.id', 'item_id') + ->selectAlias('tr.parent_folder', 'parent_folder') + ->selectAlias('tr.type', 'type') + ->selectAlias('tr.index', 'idx') + ->from('*PREFIX*bookmarks_tree', 'tr') + ->join('tr', 'folder_tree', 'e', 'e.item_id = tr.parent_folder AND e.type = '.$secondRecursiveCase->createPositionalParameter(TreeMapper::TYPE_FOLDER)); + + // First the base case together with the normal recurisve case + // Then the second helper base case together with the recursive shares case + // then we need another instance of the first recursive case, duplicated here as secondRecursive case + // to recurse into child folders of shared folders + // Note: This doesn't cover cases where a shared folder is inside a shared folder. + $withRecursiveQuery = 'WITH RECURSIVE folder_tree(item_id, parent_folder, type, idx) AS ( ' . + 'WITH RECURSIVE second_folder_tree(item_id, parent_folder, type, idx) AS (' . + 'WITH RECURSIVE inner_folder_tree(item_id, parent_folder, type, idx) AS ( ' . + $baseCase->getSQL() . ' UNION ALL ' . $recursiveCase->getSQL() . ')' . + ' ' . $secondBaseCase->getSQL() . ' UNION ALL '. $recursiveCaseShares->getSQL() .')'. + ' ' . $thirdBaseCase->getSQL() . ' UNION ALL ' . $secondRecursiveCase->getSQL(). ')'; + } + + // Now we need to concatenate the params of all these queries for downstream assembly of the greater query + if ($this->getDbType() === 'mysql') { + $params = array_merge($baseCase->getParameters(), $recursiveCase->getParameters(), $recursiveCaseShares->getParameters()); + $paramTypes = array_merge($baseCase->getParameterTypes(), $recursiveCase->getParameterTypes(), $recursiveCaseShares->getParameterTypes()); + } else { + $params = array_merge($baseCase->getParameters(), $recursiveCase->getParameters(), $secondBaseCase->getParameters(), $recursiveCaseShares->getParameters(), $thirdBaseCase->getParameters(), $secondRecursiveCase->getParameters()); + $paramTypes = array_merge($baseCase->getParameterTypes(), $recursiveCase->getParameterTypes(), $secondBaseCase->getParameterTypes(), $recursiveCaseShares->getParameterTypes(), $thirdBaseCase->getParameterTypes(), $secondRecursiveCase->getParameterTypes()); + } + + return [$withRecursiveQuery, $params, $paramTypes]; + } + private function _sortAndPaginate(IQueryBuilder $qb, QueryParameters $params): void { $sqlSortColumn = $params->getSortBy('lastmodified', $this->getSortByColumns()); if ($sqlSortColumn === 'title') { $qb->addOrderBy($qb->createFunction('UPPER(`b`.`title`)'), 'ASC'); } elseif ($sqlSortColumn === 'index') { - $qb->addOrderBy('tr.'.$sqlSortColumn, 'ASC'); - $qb->addGroupBy('tr.'.$sqlSortColumn); + $qb->addOrderBy('tree.idx', 'ASC'); + $qb->addGroupBy('tree.idx'); } else { $qb->addOrderBy('b.'.$sqlSortColumn, 'DESC'); } @@ -371,9 +467,9 @@ class BookmarkMapper extends QBMapper { if ($params->getDuplicated()) { $subQuery = $this->db->getQueryBuilder(); $subQuery->select('trdup.parent_folder') - ->from('bookmarks_tree', 'trdup') + ->from('*PREFIX*bookmarks_tree', 'trdup') ->where($subQuery->expr()->eq('b.id', 'trdup.id')) - ->andWhere($subQuery->expr()->neq('trdup.parent_folder', 'tr.parent_folder')) + ->andWhere($subQuery->expr()->neq('trdup.parent_folder', 'tree.parent_folder')) ->andWhere($subQuery->expr()->eq('trdup.type', $qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK))); $qb->andWhere($qb->createFunction('EXISTS('.$subQuery->getSQL().')')); } @@ -385,8 +481,7 @@ class BookmarkMapper extends QBMapper { */ private function _filterFolder(IQueryBuilder $qb, QueryParameters $params): void { if ($params->getFolder() !== null) { - $qb->andWhere($qb->expr()->eq('tr.parent_folder', $qb->createPositionalParameter($params->getFolder(), IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('tr.type', $qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK))); + $qb->andWhere($qb->expr()->eq('tree.parent_folder', $qb->createPositionalParameter($params->getFolder(), IQueryBuilder::PARAM_INT))); } } @@ -394,7 +489,7 @@ class BookmarkMapper extends QBMapper { * @param IQueryBuilder $qb */ private function _selectTags(IQueryBuilder $qb): void { - $qb->leftJoin('b', 'bookmarks_tags', 't', $qb->expr()->eq('t.bookmark_id', 'b.id')); + $qb->leftJoin('b', '*PREFIX*bookmarks_tags', 't', $qb->expr()->eq('t.bookmark_id', 'b.id')); $qb->selectAlias($this->_getTagsColumn($qb), 'tags'); } @@ -415,7 +510,7 @@ class BookmarkMapper extends QBMapper { private function _filterTags(IQueryBuilder $qb, QueryParameters $params): void { if (count($params->getTags())) { foreach ($params->getTags() as $i => $tag) { - $qb->leftJoin('b', 'bookmarks_tags', 'tg'.$i, $qb->expr()->eq('tg'.$i.'.bookmark_id', 'b.id')); + $qb->leftJoin('b', '*PREFIX*bookmarks_tags', 'tg'.$i, $qb->expr()->eq('tg'.$i.'.bookmark_id', 'b.id')); $qb->andWhere($qb->expr()->eq('tg'.$i.'.tag', $qb->createPositionalParameter($tag))); } } @@ -507,22 +602,26 @@ class BookmarkMapper extends QBMapper { /** * @param string $token - * @param QueryParameters $params + * @param QueryParameters $queryParams * * * @return Entity[] * * @throws DoesNotExistException * @throws MultipleObjectsReturnedException + * @throws Exception * * @psalm-return array */ - public function findAllInPublicFolder(string $token, QueryParameters $params, $withGroupBy = true): array { + public function findAllInPublicFolder(string $token, QueryParameters $queryParams, $withGroupBy = true): array { /** @var PublicFolder $publicFolder */ $publicFolder = $this->publicMapper->find($token); /** @var Folder $folder */ $folder = $this->folderMapper->find($publicFolder->getFolderId()); + // gives us all bookmarks in this folder, recursively + [$cte, $params, $paramTypes] = $this->_generateCTE($folder->getId()); + $qb = $this->db->getQueryBuilder(); $bookmark_cols = array_map(static function ($c) { return 'b.' . $c; @@ -538,38 +637,24 @@ class BookmarkMapper extends QBMapper { $qb ->from('bookmarks', 'b') - ->leftJoin('b', 'bookmarks_tree', 'tr', 'tr.id = b.id AND tr.type = '.$qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK)) - ->leftJoin('tr', 'bookmarks_tree', 'tr2', 'tr2.id = tr.parent_folder AND tr2.type = '.$qb->createPositionalParameter(TreeMapper::TYPE_FOLDER)) - ->where( - $qb->expr()->andX( - $qb->expr()->orX( - $qb->expr()->eq('tr.parent_folder', $qb->createPositionalParameter($publicFolder->getFolderId(), IQueryBuilder::PARAM_INT)), - $qb->expr()->eq('tr2.parent_folder', $qb->createPositionalParameter($publicFolder->getFolderId(), IQueryBuilder::PARAM_INT)) - ), - $qb->expr()->in('b.user_id', array_map([$qb, 'createPositionalParameter'], array_merge($this->_findSharersFor($folder->getUserId()), [$folder->getUserId()]))) - ) - ); + ->join('b', 'folder_tree', 'tree', 'tree.item_id = b.id AND tree.type = '.$qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK)); - $this->_filterUrl($qb, $params); - $this->_filterArchived($qb, $params); - $this->_filterUnavailable($qb, $params); - $this->_filterDuplicated($qb, $params); - $this->_filterFolder($qb, $params); - $this->_filterTags($qb, $params); - $this->_filterUntagged($qb, $params); - $this->_filterSearch($qb, $params); - $this->_sortAndPaginate($qb, $params); - try { - return $this->findEntities($qb); - } catch (\OC\DB\Exceptions\DbalException $e) { - if (!$withGroupBy) { - throw $e; - } - // If mysql sort_buffer_size is too small, the group by caused by selecting tags and folders can cause issues - // in this case, we repeat the query without groupBys and rely on the BookmarkController to add tags and folders entries - return $this->findAllInPublicFolder($token, $params, false); - } + $this->_filterUrl($qb, $queryParams); + $this->_filterArchived($qb, $queryParams); + $this->_filterUnavailable($qb, $queryParams); + $this->_filterDuplicated($qb, $queryParams); + $this->_filterFolder($qb, $queryParams); + $this->_filterTags($qb, $queryParams); + $this->_filterUntagged($qb, $queryParams); + $this->_filterSearch($qb, $queryParams); + $this->_sortAndPaginate($qb, $queryParams); + + $finalQuery = $cte . ' '. $qb->getSQL(); + $params = array_merge($params, $qb->getParameters()); + $paramTypes = array_merge($paramTypes, $qb->getParameterTypes()); + + return $this->findEntitiesWithRawQuery($finalQuery, $params, $paramTypes); } /** @@ -717,11 +802,11 @@ class BookmarkMapper extends QBMapper { * @param IQueryBuilder $qb */ private function _selectFolders(IQueryBuilder $qb): void { - $qb->leftJoin('b', 'bookmarks_tree', 'tree', 'b.id =tree.id AND tree.type = '.$qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK)); + $qb->leftJoin('b', 'bookmarks_tree', 'tr2', 'b.id = tr2.id AND tr2.type = '.$qb->createPositionalParameter(TreeMapper::TYPE_BOOKMARK)); if ($this->getDbType() === 'pgsql') { - $folders = $qb->createFunction('array_to_string(array_agg(' . $qb->getColumnName('tree.parent_folder') . "), ',')"); + $folders = $qb->createFunction('array_to_string(array_agg(' . $qb->getColumnName('tr2.parent_folder') . "), ',')"); } else { - $folders = $qb->createFunction('GROUP_CONCAT(' . $qb->getColumnName('tree.parent_folder') . ')'); + $folders = $qb->createFunction('GROUP_CONCAT(' . $qb->getColumnName('tr2.parent_folder') . ')'); } $qb->selectAlias($folders, 'folders'); } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 4d0b94d1..fe5d9472 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -38,6 +38,9 @@ \OC\DB\Exceptions\DbalException + + OCP\DB\Exception + diff --git a/tests/BookmarkControllerTest.php b/tests/BookmarkControllerTest.php index 13f3e31d..dafcbbcc 100644 --- a/tests/BookmarkControllerTest.php +++ b/tests/BookmarkControllerTest.php @@ -232,6 +232,9 @@ class BookmarkControllerTest extends TestCase { $this->tagMapper->addTo(['four'], $bookmark2->getId()); $this->bookmark1Id = $bookmark1->getId(); $this->bookmark2Id = $bookmark2->getId(); + $rootFolderId = $this->folderMapper->findRootFolder($this->userId)->getId(); + $this->treeMapper->addToFolders(TreeMapper::TYPE_BOOKMARK, $this->bookmark1Id, [$rootFolderId]); + $this->treeMapper->addToFolders(TreeMapper::TYPE_BOOKMARK, $this->bookmark2Id, [$rootFolderId]); } /** @@ -250,6 +253,7 @@ class BookmarkControllerTest extends TestCase { $this->folder1->setTitle('foo'); $this->folder1->setUserId($this->userId); $this->folderMapper->insert($this->folder1); + $this->treeMapper->move(TreeMapper::TYPE_FOLDER, $this->folder1->getId(), $this->folderMapper->findRootFolder($this->userId)->getId()); $this->folder2 = new Folder(); $this->folder2->setTitle('bar'); @@ -622,7 +626,7 @@ class BookmarkControllerTest extends TestCase { $this->authorizer->setUserId($this->otherUserId); $output = $this->otherController->getBookmarks(); $data = $output->getData(); - $this->assertCount(1, $data['data'], var_export($data, true)); // TODO: 1 level search Limit + $this->assertCount(2, $data['data'], var_export($data, true)); } /** @@ -655,7 +659,7 @@ class BookmarkControllerTest extends TestCase { $this->authorizer->setUserId($this->otherUserId); $output = $this->otherController->getBookmarks(); $data = $output->getData(); - $this->assertCount(2, $data['data']); // TODO: 1 level search limit + $this->assertCount(3, $data['data']); } /** diff --git a/tests/BookmarkMapperTest.php b/tests/BookmarkMapperTest.php index 404bcf61..b4105dc2 100644 --- a/tests/BookmarkMapperTest.php +++ b/tests/BookmarkMapperTest.php @@ -8,7 +8,6 @@ use OCA\Bookmarks\Db; use OCA\Bookmarks\Exception\AlreadyExistsError; use OCA\Bookmarks\Exception\UrlParseError; use OCA\Bookmarks\Exception\UserLimitExceededError; -use OCA\Bookmarks\QueryParameters; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -21,6 +20,16 @@ class BookmarkMapperTest extends TestCase { */ private $bookmarkMapper; + /** + * @var Db\TreeMapper + */ + private $treeMapper; + + /** + * @var Db\FolderMapper + */ + private $folderMapper; + /** * @var string */ @@ -41,7 +50,12 @@ class BookmarkMapperTest extends TestCase { parent::setUp(); $this->cleanUp(); + /** + * @var Db\BookmarkMapper + */ $this->bookmarkMapper = OC::$server->get(Db\BookmarkMapper::class); + $this->treeMapper = OC::$server->get(Db\TreeMapper::class); + $this->folderMapper = OC::$server->get(Db\FolderMapper::class); $this->userManager = OC::$server->get(IUserManager::class); $this->user = 'test'; @@ -85,10 +99,7 @@ class BookmarkMapperTest extends TestCase { $bookmark->setUserId($this->userId); $bookmark = $this->bookmarkMapper->insert($bookmark); - $params = new QueryParameters(); - $entities = $this->bookmarkMapper->findAll($this->userId, $params->setUrl($bookmark->getUrl())); - $this->assertCount(1, $entities); - $entity = $entities[0]; + $entity = $this->bookmarkMapper->find($bookmark->getId()); $entity->setTitle('foobar'); $this->bookmarkMapper->update($entity); $foundEntity = $this->bookmarkMapper->find($entity->getId()); @@ -110,11 +121,7 @@ class BookmarkMapperTest extends TestCase { $bookmark->setUserId($this->userId); $bookmark = $this->bookmarkMapper->insert($bookmark); - $params = new QueryParameters(); - - $foundEntities = $this->bookmarkMapper->findAll($this->userId, $params->setUrl($bookmark->getUrl())); - $this->assertCount(1, $foundEntities); - $foundEntity = $foundEntities[0]; + $foundEntity = $this->bookmarkMapper->find($bookmark->getId()); $this->bookmarkMapper->delete($foundEntity); $this->expectException(DoesNotExistException::class); $this->bookmarkMapper->find($foundEntity->getId()); diff --git a/tests/FindTest.php b/tests/FindTest.php index 2ec72adb..60b69f94 100644 --- a/tests/FindTest.php +++ b/tests/FindTest.php @@ -27,6 +27,11 @@ class FindTest extends TestCase { */ private $folderMapper; + /** + * @var Db\TreeMapper + */ + private $treeMapper; + /** * @var string */ @@ -54,6 +59,7 @@ class FindTest extends TestCase { $this->bookmarkMapper = \OC::$server->query(Db\BookmarkMapper::class); $this->tagMapper = \OC::$server->query(Db\TagMapper::class); $this->folderMapper = \OC::$server->query(Db\FolderMapper::class); + $this->treeMapper = \OC::$server->query(Db\TreeMapper::class); $this->userManager = \OC::$server->getUserManager(); $this->user = 'test'; @@ -61,11 +67,13 @@ class FindTest extends TestCase { $this->userManager->createUser($this->user, 'password'); } $this->userId = $this->userManager->get($this->user)->getUID(); + $rootFolder = $this->folderMapper->findRootFolder($this->userId); foreach ($this->singleBookmarksProvider() as $bookmarkEntry) { $bookmarkEntry[1]->setUserId($this->userId); $bookmark = $this->bookmarkMapper->insertOrUpdate($bookmarkEntry[1]); $this->tagMapper->addTo($bookmarkEntry[0], $bookmark->getId()); + $this->treeMapper->addToFolders(Db\TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), [$rootFolder->getId()]); } } diff --git a/tests/TagMapperTest.php b/tests/TagMapperTest.php index 443b5f36..898bfe60 100644 --- a/tests/TagMapperTest.php +++ b/tests/TagMapperTest.php @@ -44,6 +44,7 @@ class TagMapperTest extends TestCase { $this->bookmarkMapper = \OC::$server->get(Db\BookmarkMapper::class); $this->tagMapper = \OC::$server->get(Db\TagMapper::class); $this->folderMapper = \OC::$server->get(Db\FolderMapper::class); + $this->treeMapper = \OC::$server->get(Db\TreeMapper::class); $this->userManager = \OC::$server->get(IUserManager::class); $this->user = 'test'; @@ -66,6 +67,7 @@ class TagMapperTest extends TestCase { $bookmark->setUserId($this->userId); $bookmark = $this->bookmarkMapper->insertOrUpdate($bookmark); $this->tagMapper->addTo($tags, $bookmark->getId()); + $this->treeMapper->addToFolders(Db\TreeMapper::TYPE_BOOKMARK, $bookmark->getId(), [$this->folderMapper->findRootFolder($this->userId)->getId()]); $actualTags = $this->tagMapper->findByBookmark($bookmark->getId()); foreach ($tags as $tag) {