Merge pull request #4146 from nextcloud/unread-comments-folder

Allow getting the unread comment count for an entire folder at once

apps/dav/lib/Connector/Sabre/CommentPropertiesPlugin.php

@@ -42,6 +42,10 @@ class CommentPropertiesPlugin extends ServerPlugin {
 	/** @var IUserSession */
 	private $userSession;
 
+	private $cachedUnreadCount = [];
+
+	private $cachedFolders = [];
+
 	public function __construct(ICommentsManager $commentsManager, IUserSession $userSession) {
 		$this->commentsManager = $commentsManager;
 		$this->userSession = $userSession;
@@ -79,6 +83,18 @@ class CommentPropertiesPlugin extends ServerPlugin {
 			return;
 		}
 
+		// need prefetch ?
+		if ($node instanceof \OCA\DAV\Connector\Sabre\Directory
+			&& $propFind->getDepth() !== 0
+			&& !is_null($propFind->getStatus(self::PROPERTY_NAME_UNREAD))
+		) {
+			$unreadCounts = $this->commentsManager->getNumberOfUnreadCommentsForFolder($node->getId(), $this->userSession->getUser());
+			$this->cachedFolders[] = $node->getPath();
+			foreach ($unreadCounts as $id => $count) {
+				$this->cachedUnreadCount[$id] = $count;
+			}
+		}
+
 		$propFind->handle(self::PROPERTY_NAME_COUNT, function() use ($node) {
 			return $this->commentsManager->getNumberOfCommentsForObject('files', strval($node->getId()));
 		});
@@ -88,7 +104,20 @@ class CommentPropertiesPlugin extends ServerPlugin {
 		});
 
 		$propFind->handle(self::PROPERTY_NAME_UNREAD, function() use ($node) {
+			if (isset($this->cachedUnreadCount[$node->getId()])) {
+				return $this->cachedUnreadCount[$node->getId()];
+			} else {
+				list($parentPath,) = \Sabre\Uri\split($node->getPath());
+				if ($parentPath === '') {
+					$parentPath = '/';
+				}
+				// if we already cached the folder this file is in we know there are no shares for this file
+				if (array_search($parentPath, $this->cachedFolders) === false) {
 					return $this->getUnreadCount($node);
+				} else {
+					return 0;
+				}
+			}
 		});
 	}
 

lib/private/Comments/Manager.php

@@ -21,9 +21,11 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>
  *
  */
+
 namespace OC\Comments;
 
 use Doctrine\DBAL\Exception\DriverException;
+use Doctrine\DBAL\Platforms\MySqlPlatform;
 use OCP\Comments\CommentsEvent;
 use OCP\Comments\IComment;
 use OCP\Comments\ICommentsEventHandler;
@@ -401,6 +403,40 @@ class Manager implements ICommentsManager {
 		return intval($data[0]);
 	}
 
+	/**
+	 * Get the number of unread comments for all files in a folder
+	 *
+	 * @param int $folderId
+	 * @param IUser $user
+	 * @return array [$fileId => $unreadCount]
+	 */
+	public function getNumberOfUnreadCommentsForFolder($folderId, IUser $user) {
+		$qb = $this->dbConn->getQueryBuilder();
+		$query = $qb->select('fileid', $qb->createFunction(
+			'COUNT(' . $qb->getColumnName('c.id') . ')')
+		)->from('comments', 'c')
+			->innerJoin('c', 'filecache', 'f', $qb->expr()->andX(
+				$qb->expr()->eq('c.object_type', $qb->createNamedParameter('files')),
+				$qb->expr()->eq('f.fileid', $qb->expr()->castColumn('c.object_id', IQueryBuilder::PARAM_INT))
+			))
+			->leftJoin('c', 'comments_read_markers', 'm', $qb->expr()->andX(
+				$qb->expr()->eq('m.object_type', $qb->createNamedParameter('files')),
+				$qb->expr()->eq('m.object_id', 'c.object_id'),
+				$qb->expr()->eq('m.user_id', $qb->createNamedParameter($user->getUID()))
+			))
+			->andWhere($qb->expr()->eq('f.parent', $qb->createNamedParameter($folderId)))
+			->andWhere($qb->expr()->orX(
+				$qb->expr()->gt('c.creation_timestamp', 'marker_datetime'),
+				$qb->expr()->isNull('marker_datetime')
+			))
+			->groupBy('f.fileid');
+
+		$resultStatement = $query->execute();
+		return array_map(function ($count) {
+			return (int)$count;
+		}, $resultStatement->fetchAll(\PDO::FETCH_KEY_PAIR));
+	}
+
 	/**
 	 * creates a new comment and returns it. At this point of time, it is not
 	 * saved in the used data storage. Use save() after setting other fields

lib/private/DB/QueryBuilder/QuoteHelper.php

@@ -73,7 +73,7 @@ class QuoteHelper {
 				return $string;
 			}
 
-			return $alias . '.`' . $columnName . '`';
+			return '`' . $alias . '`.`' . $columnName . '`';
 		}
 
 		return '`' . $string . '`';

lib/public/Comments/ICommentsManager.php

@@ -23,6 +23,8 @@
  */
 namespace OCP\Comments;
 
+use OCP\IUser;
+
 /**
  * Interface ICommentsManager
  *
@@ -125,6 +127,16 @@ interface ICommentsManager {
 	 */
 	public function getNumberOfCommentsForObject($objectType, $objectId, \DateTime $notOlderThan = null);
 
+	/**
+	 * Get the number of unread comments for all files in a folder
+	 *
+	 * @param int $folderId
+	 * @param IUser $user
+	 * @return array [$fileId => $unreadCount]
+	 * @since 12.0.0
+	 */
+	public function getNumberOfUnreadCommentsForFolder($folderId, IUser $user);
+
 	/**
 	 * creates a new comment and returns it. At this point of time, it is not
 	 * saved in the used data storage. Use save() after setting other fields

tests/lib/Comments/FakeManager.php

@@ -1,6 +1,7 @@
 <?php
 
 namespace Test\Comments;
+use OCP\IUser;
 
 /**
  * Class FakeManager
@@ -44,4 +45,6 @@ class FakeManager implements \OCP\Comments\ICommentsManager {
 	public function registerDisplayNameResolver($type, \Closure $closure) {}
 
 	public function resolveDisplayName($type, $id) {}
+
+	public function getNumberOfUnreadCommentsForFolder($folderId, IUser $user) {}
 }

tests/lib/Comments/ManagerTest.php

@@ -6,7 +6,9 @@ use OC\Comments\Comment;
 use OCP\Comments\CommentsEvent;
 use OCP\Comments\ICommentsEventHandler;
 use OCP\Comments\ICommentsManager;
+use OCP\IDBConnection;
 use OCP\IUser;
+use Test\Files\Storage\DummyUser;
 use Test\TestCase;
 
 /**
@@ -15,23 +17,30 @@ use Test\TestCase;
  * @group DB
  */
 class ManagerTest extends TestCase {
+	/** @var IDBConnection */
+	private $connection;
 
 	public function setUp() {
 		parent::setUp();
 
-		$sql = \OC::$server->getDatabaseConnection()->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`');
+		$this->connection = \OC::$server->getDatabaseConnection();
-		\OC::$server->getDatabaseConnection()->prepare($sql)->execute();
+
+		$sql = $this->connection->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`');
+		$this->connection->prepare($sql)->execute();
 	}
 
-	protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = null, $latestChildDT = null) {
+	protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = null, $latestChildDT = null, $objectId = null) {
 		if (is_null($creationDT)) {
 			$creationDT = new \DateTime();
 		}
 		if (is_null($latestChildDT)) {
 			$latestChildDT = new \DateTime('yesterday');
 		}
+		if (is_null($objectId)) {
+			$objectId = 'file64';
+		}
 
-		$qb = \OC::$server->getDatabaseConnection()->getQueryBuilder();
+		$qb = $this->connection->getQueryBuilder();
 		$qb
 			->insert('comments')
 			->values([
@@ -45,7 +54,7 @@ class ManagerTest extends TestCase {
 				'creation_timestamp' => $qb->createNamedParameter($creationDT, 'datetime'),
 				'latest_child_timestamp' => $qb->createNamedParameter($latestChildDT, 'datetime'),
 				'object_type' => $qb->createNamedParameter('files'),
-					'object_id' 				=> $qb->createNamedParameter('file64'),
+				'object_id' => $qb->createNamedParameter($objectId),
 			])
 			->execute();
 
@@ -297,6 +306,52 @@ class ManagerTest extends TestCase {
 		$this->assertSame($amount, 4);
 	}
 
+	public function testGetNumberOfUnreadCommentsForFolder() {
+		// 2 comment for 1111 with 1 before read marker
+		// 2 comments for 1112 with no read marker
+		// 1 comment for 1113 before read marker
+		// 1 comment for 1114 with no read marker
+		$this->addDatabaseEntry(0, 0, null, null, '1112');
+		for ($i = 1; $i < 5; $i++) {
+			$this->addDatabaseEntry(0, 0, null, null, '111' . $i);
+		}
+		$this->addDatabaseEntry(0, 0, (new \DateTime())->modify('-2 days'), null, '1111');
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('comment_test'));
+
+		$manager = $this->getManager();
+
+		$manager->setReadMark('files', '1111', (new \DateTime())->modify('-1 days'), $user);
+		$manager->setReadMark('files', '1113', (new \DateTime()), $user);
+
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('filecache')
+			->values([
+				'fileid' => $query->createParameter('fileid'),
+				'parent' => $query->createNamedParameter(1000),
+				'size' => $query->createNamedParameter(10),
+				'mtime' => $query->createNamedParameter(10),
+				'storage_mtime' => $query->createNamedParameter(10),
+				'path' => $query->createParameter('path'),
+				'path_hash' => $query->createParameter('path'),
+			]);
+
+		for ($i = 1111; $i < 1115; $i++) {
+			$query->setParameter('path', 'path_' . $i);
+			$query->setParameter('fileid', $i);
+			$query->execute();
+		}
+
+		$amount = $manager->getNumberOfUnreadCommentsForFolder(1000, $user);
+		$this->assertEquals([
+			'1111' => 1,
+			'1112' => 2,
+			'1114' => 1,
+		], $amount);
+	}
+
 	public function invalidCreateArgsProvider() {
 		return [
 			['', 'aId-1', 'oType-1', 'oId-1'],
@@ -643,8 +698,12 @@ class ManagerTest extends TestCase {
 			->method('handle');
 
 		$manager = $this->getManager();
-		$manager->registerEventHandler(function () use ($handler1) {return $handler1; });
+		$manager->registerEventHandler(function () use ($handler1) {
-		$manager->registerEventHandler(function () use ($handler2) {return $handler2; });
+			return $handler1;
+		});
+		$manager->registerEventHandler(function () use ($handler2) {
+			return $handler2;
+		});
 
 		$comment = new Comment();
 		$comment
@@ -724,7 +783,9 @@ class ManagerTest extends TestCase {
 	public function testResolveDisplayNameDirtyResolver() {
 		$manager = $this->getManager();
 
-		$planetClosure = function() { return null; };
+		$planetClosure = function () {
+			return null;
+		};
 
 		$manager->registerDisplayNameResolver('planet', $planetClosure);
 		$this->assertTrue(is_string($manager->resolveDisplayName('planet', 'neptune')));
@@ -736,7 +797,9 @@ class ManagerTest extends TestCase {
 	public function testResolveDisplayNameInvalidType() {
 		$manager = $this->getManager();
 
-		$planetClosure = function() { return null; };
+		$planetClosure = function () {
+			return null;
+		};
 
 		$manager->registerDisplayNameResolver('planet', $planetClosure);
 		$this->assertTrue(is_string($manager->resolveDisplayName(1337, 'neptune')));

tests/lib/DB/QueryBuilder/QueryBuilderTest.php

@@ -1200,7 +1200,7 @@ class QueryBuilderTest extends \Test\TestCase {
 	public function dataGetColumnName() {
 		return [
 			['column', '', '`column`'],
-			['column', 'a', 'a.`column`'],
+			['column', 'a', '`a`.`column`'],
 		];
 	}
 

tests/lib/DB/QueryBuilder/QuoteHelperTest.php

@@ -65,7 +65,7 @@ class QuoteHelperTest extends \Test\TestCase {
 	public function dataQuoteColumnNames() {
 		return [
 			// Single case
-			['d.column', 'd.`column`'],
+			['d.column', '`d`.`column`'],
 			['column', '`column`'],
 			[new Literal('literal'), 'literal'],
 			[new Literal(1), '1'],