From f50abde7ac99fd6f7563fb60b662ca21ac320f34 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 10 Apr 2018 10:50:57 +0200 Subject: [PATCH 1/3] Add proper comment offset support The offset is based on the last known comment instead of limit-offset, so new comments don't mess up requests which get the history of an object- Signed-off-by: Joas Schilling --- lib/private/Comments/Manager.php | 115 +++++++++++++++++++++++ lib/public/Comments/ICommentsManager.php | 17 ++++ tests/lib/Comments/ManagerTest.php | 42 +++++++++ 3 files changed, 174 insertions(+) diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index 2b43a27b7a..37b5f03309 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -376,6 +376,121 @@ class Manager implements ICommentsManager { return $comments; } + /** + * @param string $objectType + * @param string $objectId + * @param int $lastKnownCommentId + * @param string $sortDirection + * @param int $limit + * @return array + */ + public function getForObjectSince( + string $objectType, + string $objectId, + int $lastKnownCommentId, + string $sortDirection = 'asc', + int $limit = 30 + ): array { + $comments = []; + + $query = $this->dbConn->getQueryBuilder(); + $query->select('*') + ->from('comments') + ->where($query->expr()->eq('object_type', $query->createNamedParameter($objectType))) + ->andWhere($query->expr()->eq('object_id', $query->createNamedParameter($objectId))) + ->orderBy('creation_timestamp', $sortDirection === 'desc' ? 'DESC' : 'ASC') + ->addOrderBy('id', $sortDirection === 'desc' ? 'DESC' : 'ASC'); + + if ($limit > 0) { + $query->setMaxResults($limit); + } + + $lastKnownComment = $this->getLastKnownComment( + $objectType, + $objectId, + $lastKnownCommentId + ); + if ($lastKnownComment instanceof IComment) { + $lastKnownCommentDateTime = $lastKnownComment->getCreationDateTime(); + if ($sortDirection === 'desc') { + $query->andWhere( + $query->expr()->orX( + $query->expr()->lt( + 'creation_timestamp', + $query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE), + IQueryBuilder::PARAM_DATE + ), + $query->expr()->andX( + $query->expr()->eq( + 'creation_timestamp', + $query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE), + IQueryBuilder::PARAM_DATE + ), + $query->expr()->lt('id', $query->createNamedParameter($lastKnownCommentId)) + ) + ) + ); + } else { + $query->andWhere( + $query->expr()->orX( + $query->expr()->gt( + 'creation_timestamp', + $query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE), + IQueryBuilder::PARAM_DATE + ), + $query->expr()->andX( + $query->expr()->eq( + 'creation_timestamp', + $query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE), + IQueryBuilder::PARAM_DATE + ), + $query->expr()->gt('id', $query->createNamedParameter($lastKnownCommentId)) + ) + ) + ); + } + } + + $resultStatement = $query->execute(); + while ($data = $resultStatement->fetch()) { + $comment = new Comment($this->normalizeDatabaseData($data)); + $this->cache($comment); + $comments[] = $comment; + } + $resultStatement->closeCursor(); + + return $comments; + } + + /** + * @param string $objectType + * @param string $objectId + * @param int $id + * @return Comment|null + */ + protected function getLastKnownComment(string $objectType, + string $objectId, + int $id) { + $query = $this->dbConn->getQueryBuilder(); + $query->select('*') + ->from('comments') + ->where($query->expr()->eq('object_type', $query->createNamedParameter($objectType))) + ->andWhere($query->expr()->eq('object_id', $query->createNamedParameter($objectId))) + ->andWhere($query->expr()->eq('id', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT))); + + $result = $query->execute(); + $row = $result->fetch(); + $result->closeCursor(); + + if ($row) { + $comment = new Comment($this->normalizeDatabaseData($row)); + $this->cache($comment); + return $comment; + } + + return null; + } + /** * @param $objectType string the object type, e.g. 'files' * @param $objectId string the id of the object diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index bd9c4ff5ba..888e7df3e7 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/lib/public/Comments/ICommentsManager.php @@ -120,6 +120,23 @@ interface ICommentsManager { \DateTime $notOlderThan = null ); + /** + * @param string $objectType + * @param string $objectId + * @param int $lastKnownCommentId + * @param string $sortDirection + * @param int $limit + * @return array + * @since 14.0.0 + */ + public function getForObjectSince( + string $objectType, + string $objectId, + int $lastKnownCommentId, + string $sortDirection = 'asc', + int $limit = 30 + ): array; + /** * @param $objectType string the object type, e.g. 'files' * @param $objectId string the id of the object diff --git a/tests/lib/Comments/ManagerTest.php b/tests/lib/Comments/ManagerTest.php index 671389232e..28002ff42c 100644 --- a/tests/lib/Comments/ManagerTest.php +++ b/tests/lib/Comments/ManagerTest.php @@ -354,6 +354,48 @@ class ManagerTest extends TestCase { ], $amount); } + /** + * @dataProvider dataGetForObjectSince + * @param $lastKnown + * @param $order + * @param $limit + * @param $resultFrom + * @param $resultTo + */ + public function testGetForObjectSince($lastKnown, $order, $limit, $resultFrom, $resultTo) { + $ids = []; + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry(0, 0); + + $manager = $this->getManager(); + $comments = $manager->getForObjectSince('files', 'file64', ($lastKnown === null ? 0 : $ids[$lastKnown]), $order, $limit); + + $expected = array_slice($ids, $resultFrom, $resultTo - $resultFrom + 1); + if ($order === 'desc') { + $expected = array_reverse($expected); + } + + $this->assertSame($expected, array_map(function(IComment $c) { + return (int) $c->getId(); + }, $comments)); + } + + public function dataGetForObjectSince() { + return [ + [null, 'asc', 20, 0, 4], + [null, 'asc', 2, 0, 1], + [null, 'desc', 20, 0, 4], + [null, 'desc', 2, 3, 4], + [1, 'asc', 20, 2, 4], + [1, 'asc', 2, 2, 3], + [3, 'desc', 20, 0, 2], + [3, 'desc', 2, 1, 2], + ]; + } + public function invalidCreateArgsProvider() { return [ ['', 'aId-1', 'oType-1', 'oId-1'], From 5157349b699a46eee2f84247f90b1f155ba40cd5 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 19 Apr 2018 17:10:03 +0200 Subject: [PATCH 2/3] Fix docs Signed-off-by: Joas Schilling --- lib/private/Comments/Manager.php | 18 ++++++++++-------- lib/public/Comments/ICommentsManager.php | 13 +++++++------ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index 37b5f03309..9393cce334 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -377,11 +377,13 @@ class Manager implements ICommentsManager { } /** - * @param string $objectType - * @param string $objectId - * @param int $lastKnownCommentId - * @param string $sortDirection - * @param int $limit + * @param string $objectType the object type, e.g. 'files' + * @param string $objectId the id of the object + * @param int $lastKnownCommentId the last known comment (will be used as offset) + * @param string $sortDirection direction of the comments (`asc` or `desc`) + * @param int $limit optional, number of maximum comments to be returned. if + * set to 0, all comments are returned. + * @return IComment[] * @return array */ public function getForObjectSince( @@ -463,9 +465,9 @@ class Manager implements ICommentsManager { } /** - * @param string $objectType - * @param string $objectId - * @param int $id + * @param string $objectType the object type, e.g. 'files' + * @param string $objectId the id of the object + * @param int $id the comment to look for * @return Comment|null */ protected function getLastKnownComment(string $objectType, diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index 888e7df3e7..b3ed176b3b 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/lib/public/Comments/ICommentsManager.php @@ -121,12 +121,13 @@ interface ICommentsManager { ); /** - * @param string $objectType - * @param string $objectId - * @param int $lastKnownCommentId - * @param string $sortDirection - * @param int $limit - * @return array + * @param string $objectType the object type, e.g. 'files' + * @param string $objectId the id of the object + * @param int $lastKnownCommentId the last known comment (will be used as offset) + * @param string $sortDirection direction of the comments (`asc` or `desc`) + * @param int $limit optional, number of maximum comments to be returned. if + * set to 0, all comments are returned. + * @return IComment[] * @since 14.0.0 */ public function getForObjectSince( From cccdfaa6e9e1ac02193ecfb9d253462fc3320d05 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 19 Apr 2018 17:12:07 +0200 Subject: [PATCH 3/3] Fix FakeManager Signed-off-by: Joas Schilling --- tests/lib/Comments/FakeManager.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/lib/Comments/FakeManager.php b/tests/lib/Comments/FakeManager.php index d3dd1dfb58..3ba66e9669 100644 --- a/tests/lib/Comments/FakeManager.php +++ b/tests/lib/Comments/FakeManager.php @@ -22,6 +22,14 @@ class FakeManager implements ICommentsManager { \DateTime $notOlderThan = null ) {} + public function getForObjectSince( + string $objectType, + string $objectId, + int $lastKnownCommentId, + string $sortDirection = 'asc', + int $limit = 30 + ): array { return []; } + public function getNumberOfCommentsForObject($objectType, $objectId, \DateTime $notOlderThan = null) {} public function create($actorType, $actorId, $objectType, $objectId) {}