From ca04efb736c69c3053f3a55119224533c48bf228 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 4 Oct 2021 16:19:45 +0200 Subject: [PATCH 1/3] apply date search filter in SQL queries instead of computing it in php Signed-off-by: Julien Veyssier --- lib/Db/BoardMapper.php | 44 ++++++++++++++++++++++++++--------- lib/Service/BoardService.php | 4 ++-- lib/Service/SearchService.php | 7 ++---- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 8f21f5dc0..9852bf0ae 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -94,15 +94,15 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return $board; } - public function findAllForUser(string $userId, int $since = -1, $includeArchived = true): array { - $useCache = ($since === -1 && $includeArchived === true); + public function findAllForUser(string $userId, int $since = -1, $includeArchived = true, ?int $before = null): array { + $useCache = ($since === -1 && $includeArchived === true && $before === null); if (!isset($this->userBoardCache[$userId]) || !$useCache) { $groups = $this->groupManager->getUserGroupIds( $this->userManager->get($userId) ); - $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived); - $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived); - $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived); + $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived, $before); + $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before); + $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before); $allBoards = array_unique(array_merge($userBoards, $groupBoards, $circleBoards)); if ($useCache) { $this->userBoardCache[$userId] = $allBoards; @@ -120,19 +120,29 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { * @param null $offset * @return array */ - public function findAllByUser($userId, $limit = null, $offset = null, $since = -1, $includeArchived = true) { + public function findAllByUser($userId, $limit = null, $offset = null, $since = -1, $includeArchived = true, ?int $before = null) { // FIXME: One moving to QBMapper we should allow filtering the boards probably by method chaining for additional where clauses $sql = 'SELECT id, title, owner, color, archived, deleted_at, 0 as shared, last_modified FROM `*PREFIX*deck_boards` WHERE owner = ? AND last_modified > ?'; + $params = [$userId, $since]; if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } + if ($before !== null) { + $sql .= ' AND last_modified < ?'; + $params[] = $before; + } $sql .= ' UNION ' . 'SELECT boards.id, title, owner, color, archived, deleted_at, 1 as shared, last_modified FROM `*PREFIX*deck_boards` as boards ' . 'JOIN `*PREFIX*deck_board_acl` as acl ON boards.id=acl.board_id WHERE acl.participant=? AND acl.type=? AND boards.owner != ? AND last_modified > ?'; + array_push($params, $userId, Acl::PERMISSION_TYPE_USER, $userId, $since); if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } - $entries = $this->findEntities($sql, [$userId, $since, $userId, Acl::PERMISSION_TYPE_USER, $userId, $since], $limit, $offset); + if ($before !== null) { + $sql .= ' AND last_modified < ?'; + $params[] = $before; + } + $entries = $this->findEntities($sql, $params, $limit, $offset); /* @var Board $entry */ foreach ($entries as $entry) { $acl = $this->aclMapper->findAll($entry->id); @@ -155,12 +165,13 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { * @param null $offset * @return array */ - public function findAllByGroups($userId, $groups, $limit = null, $offset = null, $since = -1,$includeArchived = true) { + public function findAllByGroups($userId, $groups, $limit = null, $offset = null, $since = -1, $includeArchived = true, ?int $before = null) { if (count($groups) <= 0) { return []; } $sql = 'SELECT boards.id, title, owner, color, archived, deleted_at, 2 as shared, last_modified FROM `*PREFIX*deck_boards` as boards ' . 'INNER JOIN `*PREFIX*deck_board_acl` as acl ON boards.id=acl.board_id WHERE owner != ? AND type=? AND ('; + $params = [$userId, Acl::PERMISSION_TYPE_GROUP]; for ($i = 0, $iMax = count($groups); $i < $iMax; $i++) { $sql .= 'acl.participant = ? '; if (count($groups) > 1 && $i < count($groups) - 1) { @@ -168,10 +179,15 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { } } $sql .= ')'; + array_push($params, ...$groups); if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } - $entries = $this->findEntities($sql, array_merge([$userId, Acl::PERMISSION_TYPE_GROUP], $groups), $limit, $offset); + if ($before !== null) { + $sql .= ' AND last_modified < ?'; + $params[] = $before; + } + $entries = $this->findEntities($sql, $params, $limit, $offset); /* @var Board $entry */ foreach ($entries as $entry) { $acl = $this->aclMapper->findAll($entry->id); @@ -180,7 +196,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return $entries; } - public function findAllByCircles($userId, $limit = null, $offset = null, $since = -1,$includeArchived = true) { + public function findAllByCircles($userId, $limit = null, $offset = null, $since = -1, $includeArchived = true, ?int $before = null) { if (!$this->circlesEnabled) { return []; } @@ -193,6 +209,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $sql = 'SELECT boards.id, title, owner, color, archived, deleted_at, 2 as shared, last_modified FROM `*PREFIX*deck_boards` as boards ' . 'INNER JOIN `*PREFIX*deck_board_acl` as acl ON boards.id=acl.board_id WHERE owner != ? AND type=? AND ('; + $params = [$userId, Acl::PERMISSION_TYPE_CIRCLE]; for ($i = 0, $iMax = count($circles); $i < $iMax; $i++) { $sql .= 'acl.participant = ? '; if (count($circles) > 1 && $i < count($circles) - 1) { @@ -200,10 +217,15 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { } } $sql .= ')'; + array_push($params, ...$circles); if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } - $entries = $this->findEntities($sql, array_merge([$userId, Acl::PERMISSION_TYPE_CIRCLE], $circles), $limit, $offset); + if ($before !== null) { + $sql .= ' AND last_modified < ?'; + $params[] = $before; + } + $entries = $this->findEntities($sql, $params, $limit, $offset); /* @var Board $entry */ foreach ($entries as $entry) { $acl = $this->aclMapper->findAll($entry->id); diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index afd45721c..6c7d43a08 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -118,8 +118,8 @@ class BoardService { /** * Get all boards that are shared with a user, their groups or circles */ - public function getUserBoards(int $since = -1, bool $includeArchived = true): array { - return $this->boardMapper->findAllForUser($this->userId, $since, $includeArchived); + public function getUserBoards(int $since = -1, bool $includeArchived = true, ?int $before = null): array { + return $this->boardMapper->findAllForUser($this->userId, $since, $includeArchived, $before); } /** diff --git a/lib/Service/SearchService.php b/lib/Service/SearchService.php index d960a8678..9fb7140a3 100644 --- a/lib/Service/SearchService.php +++ b/lib/Service/SearchService.php @@ -90,14 +90,11 @@ class SearchService { } public function searchBoards(string $term, ?int $limit, ?int $cursor): array { - $boards = $this->boardService->getUserBoards(); + $boards = $this->boardService->getUserBoards(-1, true, $cursor); // get boards that have a lastmodified date which is lower than the cursor // and which match the search term $filteredBoards = array_filter($boards, static function (Board $board) use ($term, $cursor) { - return ( - ($cursor === null || $board->getLastModified() < $cursor) - && mb_stripos(mb_strtolower($board->getTitle()), mb_strtolower($term)) > -1 - ); + return mb_stripos(mb_strtolower($board->getTitle()), mb_strtolower($term)) > -1; }); // sort the boards, recently modified first usort($filteredBoards, function ($boardA, $boardB) { From 7425d00ba5915dc1d52fec3bc14f51be8d895f6f Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 4 Oct 2021 17:10:34 +0200 Subject: [PATCH 2/3] harmonize since and before search params Signed-off-by: Julien Veyssier --- lib/Db/BoardMapper.php | 35 +++++++++++++++++++++------ lib/Service/BoardService.php | 2 +- lib/Service/FullTextSearchService.php | 4 +-- lib/Service/SearchService.php | 2 +- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 9852bf0ae..fb05034a9 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -94,7 +94,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return $board; } - public function findAllForUser(string $userId, int $since = -1, $includeArchived = true, ?int $before = null): array { + public function findAllForUser(string $userId, ?int $since = null, bool $includeArchived = true, ?int $before = null): array { $useCache = ($since === -1 && $includeArchived === true && $before === null); if (!isset($this->userBoardCache[$userId]) || !$useCache) { $groups = $this->groupManager->getUserGroupIds( @@ -120,24 +120,33 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { * @param null $offset * @return array */ - public function findAllByUser($userId, $limit = null, $offset = null, $since = -1, $includeArchived = true, ?int $before = null) { + public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = null, ?int $since = null, + bool $includeArchived = true, ?int $before = null) { // FIXME: One moving to QBMapper we should allow filtering the boards probably by method chaining for additional where clauses - $sql = 'SELECT id, title, owner, color, archived, deleted_at, 0 as shared, last_modified FROM `*PREFIX*deck_boards` WHERE owner = ? AND last_modified > ?'; - $params = [$userId, $since]; + $sql = 'SELECT id, title, owner, color, archived, deleted_at, 0 as shared, last_modified FROM `*PREFIX*deck_boards` WHERE owner = ?'; + $params = [$userId]; if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } + if ($since !== null) { + $sql .= ' AND last_modified > ?'; + $params[] = $since; + } if ($before !== null) { $sql .= ' AND last_modified < ?'; $params[] = $before; } $sql .= ' UNION ' . 'SELECT boards.id, title, owner, color, archived, deleted_at, 1 as shared, last_modified FROM `*PREFIX*deck_boards` as boards ' . - 'JOIN `*PREFIX*deck_board_acl` as acl ON boards.id=acl.board_id WHERE acl.participant=? AND acl.type=? AND boards.owner != ? AND last_modified > ?'; - array_push($params, $userId, Acl::PERMISSION_TYPE_USER, $userId, $since); + 'JOIN `*PREFIX*deck_board_acl` as acl ON boards.id=acl.board_id WHERE acl.participant=? AND acl.type=? AND boards.owner != ?'; + array_push($params, $userId, Acl::PERMISSION_TYPE_USER, $userId); if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } + if ($since !== null) { + $sql .= ' AND last_modified > ?'; + $params[] = $since; + } if ($before !== null) { $sql .= ' AND last_modified < ?'; $params[] = $before; @@ -165,7 +174,8 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { * @param null $offset * @return array */ - public function findAllByGroups($userId, $groups, $limit = null, $offset = null, $since = -1, $includeArchived = true, ?int $before = null) { + public function findAllByGroups(string $userId, array $groups, ?int $limit = null, ?int $offset = null, ?int $since = null, + bool $includeArchived = true, ?int $before = null) { if (count($groups) <= 0) { return []; } @@ -183,6 +193,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } + if ($since !== null) { + $sql .= ' AND last_modified > ?'; + $params[] = $since; + } if ($before !== null) { $sql .= ' AND last_modified < ?'; $params[] = $before; @@ -196,7 +210,8 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return $entries; } - public function findAllByCircles($userId, $limit = null, $offset = null, $since = -1, $includeArchived = true, ?int $before = null) { + public function findAllByCircles(string $userId, ?int $limit = null, ?int $offset = null, ?int $since = null, + bool $includeArchived = true, ?int $before = null) { if (!$this->circlesEnabled) { return []; } @@ -221,6 +236,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { if (!$includeArchived) { $sql .= ' AND NOT archived AND deleted_at = 0'; } + if ($since !== null) { + $sql .= ' AND last_modified > ?'; + $params[] = $since; + } if ($before !== null) { $sql .= ' AND last_modified < ?'; $params[] = $before; diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 6c7d43a08..f6f55ece4 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -118,7 +118,7 @@ class BoardService { /** * Get all boards that are shared with a user, their groups or circles */ - public function getUserBoards(int $since = -1, bool $includeArchived = true, ?int $before = null): array { + public function getUserBoards(?int $since = null, bool $includeArchived = true, ?int $before = null): array { return $this->boardMapper->findAllForUser($this->userId, $since, $includeArchived, $before); } diff --git a/lib/Service/FullTextSearchService.php b/lib/Service/FullTextSearchService.php index 504d8a11d..77a66d87e 100644 --- a/lib/Service/FullTextSearchService.php +++ b/lib/Service/FullTextSearchService.php @@ -59,7 +59,7 @@ class FullTextSearchService { /** @var CardMapper */ private $cardMapper; - + public function __construct( BoardMapper $boardMapper, StackMapper $stackMapper, CardMapper $cardMapper ) { @@ -187,6 +187,6 @@ class FullTextSearchService { * @return Board[] */ private function getBoardsFromUser(string $userId): array { - return $this->boardMapper->findAllByUser($userId, null, null, -1); + return $this->boardMapper->findAllByUser($userId, null, null, null); } } diff --git a/lib/Service/SearchService.php b/lib/Service/SearchService.php index 9fb7140a3..c38147fd5 100644 --- a/lib/Service/SearchService.php +++ b/lib/Service/SearchService.php @@ -90,7 +90,7 @@ class SearchService { } public function searchBoards(string $term, ?int $limit, ?int $cursor): array { - $boards = $this->boardService->getUserBoards(-1, true, $cursor); + $boards = $this->boardService->getUserBoards(null, true, $cursor); // get boards that have a lastmodified date which is lower than the cursor // and which match the search term $filteredBoards = array_filter($boards, static function (Board $board) use ($term, $cursor) { From dd0a22ba04bd745b3f6939f1719c4da4764c0ed5 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 4 Oct 2021 18:23:16 +0200 Subject: [PATCH 3/3] use term in DB query instead of filtering in php Signed-off-by: Julien Veyssier --- lib/Db/BoardMapper.php | 33 +++++++++++++++++++++++++-------- lib/Service/BoardService.php | 5 +++-- lib/Service/SearchService.php | 13 +++++-------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index fb05034a9..d585b3ba1 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -94,15 +94,16 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { return $board; } - public function findAllForUser(string $userId, ?int $since = null, bool $includeArchived = true, ?int $before = null): array { - $useCache = ($since === -1 && $includeArchived === true && $before === null); + public function findAllForUser(string $userId, ?int $since = null, bool $includeArchived = true, ?int $before = null, + ?string $term = null): array { + $useCache = ($since === -1 && $includeArchived === true && $before === null && $term === null); if (!isset($this->userBoardCache[$userId]) || !$useCache) { $groups = $this->groupManager->getUserGroupIds( $this->userManager->get($userId) ); - $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived, $before); - $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before); - $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before); + $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived, $before, $term); + $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before, $term); + $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before, $term); $allBoards = array_unique(array_merge($userBoards, $groupBoards, $circleBoards)); if ($useCache) { $this->userBoardCache[$userId] = $allBoards; @@ -121,7 +122,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { * @return array */ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = null, ?int $since = null, - bool $includeArchived = true, ?int $before = null) { + bool $includeArchived = true, ?int $before = null, ?string $term = null) { // FIXME: One moving to QBMapper we should allow filtering the boards probably by method chaining for additional where clauses $sql = 'SELECT id, title, owner, color, archived, deleted_at, 0 as shared, last_modified FROM `*PREFIX*deck_boards` WHERE owner = ?'; $params = [$userId]; @@ -136,6 +137,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $sql .= ' AND last_modified < ?'; $params[] = $before; } + if ($term !== null) { + $sql .= ' AND lower(title) LIKE ?'; + $params[] = '%' . $term . '%'; + } $sql .= ' UNION ' . 'SELECT boards.id, title, owner, color, archived, deleted_at, 1 as shared, last_modified FROM `*PREFIX*deck_boards` as boards ' . 'JOIN `*PREFIX*deck_board_acl` as acl ON boards.id=acl.board_id WHERE acl.participant=? AND acl.type=? AND boards.owner != ?'; @@ -151,6 +156,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $sql .= ' AND last_modified < ?'; $params[] = $before; } + if ($term !== null) { + $sql .= ' AND lower(title) LIKE ?'; + $params[] = '%' . $term . '%'; + } $entries = $this->findEntities($sql, $params, $limit, $offset); /* @var Board $entry */ foreach ($entries as $entry) { @@ -175,7 +184,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { * @return array */ public function findAllByGroups(string $userId, array $groups, ?int $limit = null, ?int $offset = null, ?int $since = null, - bool $includeArchived = true, ?int $before = null) { + bool $includeArchived = true, ?int $before = null, ?string $term = null) { if (count($groups) <= 0) { return []; } @@ -201,6 +210,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $sql .= ' AND last_modified < ?'; $params[] = $before; } + if ($term !== null) { + $sql .= ' AND lower(title) LIKE ?'; + $params[] = '%' . $term . '%'; + } $entries = $this->findEntities($sql, $params, $limit, $offset); /* @var Board $entry */ foreach ($entries as $entry) { @@ -211,7 +224,7 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { } public function findAllByCircles(string $userId, ?int $limit = null, ?int $offset = null, ?int $since = null, - bool $includeArchived = true, ?int $before = null) { + bool $includeArchived = true, ?int $before = null, ?string $term = null) { if (!$this->circlesEnabled) { return []; } @@ -244,6 +257,10 @@ class BoardMapper extends DeckMapper implements IPermissionMapper { $sql .= ' AND last_modified < ?'; $params[] = $before; } + if ($term !== null) { + $sql .= ' AND lower(title) LIKE ?'; + $params[] = '%' . $term . '%'; + } $entries = $this->findEntities($sql, $params, $limit, $offset); /* @var Board $entry */ foreach ($entries as $entry) { diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index f6f55ece4..71cf6172a 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -118,8 +118,9 @@ class BoardService { /** * Get all boards that are shared with a user, their groups or circles */ - public function getUserBoards(?int $since = null, bool $includeArchived = true, ?int $before = null): array { - return $this->boardMapper->findAllForUser($this->userId, $since, $includeArchived, $before); + public function getUserBoards(?int $since = null, bool $includeArchived = true, ?int $before = null, + ?string $term = null): array { + return $this->boardMapper->findAllForUser($this->userId, $since, $includeArchived, $before, $term); } /** diff --git a/lib/Service/SearchService.php b/lib/Service/SearchService.php index c38147fd5..11bfedf33 100644 --- a/lib/Service/SearchService.php +++ b/lib/Service/SearchService.php @@ -90,22 +90,19 @@ class SearchService { } public function searchBoards(string $term, ?int $limit, ?int $cursor): array { - $boards = $this->boardService->getUserBoards(null, true, $cursor); - // get boards that have a lastmodified date which is lower than the cursor - // and which match the search term - $filteredBoards = array_filter($boards, static function (Board $board) use ($term, $cursor) { - return mb_stripos(mb_strtolower($board->getTitle()), mb_strtolower($term)) > -1; - }); + $boards = $this->boardService->getUserBoards(null, true, $cursor, mb_strtolower($term)); + // sort the boards, recently modified first - usort($filteredBoards, function ($boardA, $boardB) { + usort($boards, function ($boardA, $boardB) { $ta = $boardA->getLastModified(); $tb = $boardB->getLastModified(); return $ta === $tb ? 0 : ($ta > $tb ? -1 : 1); }); + // limit the number of results - return array_slice($filteredBoards, 0, $limit); + return array_slice($boards, 0, $limit); } public function searchComments(string $term, ?int $limit = null, ?int $cursor = null): array {