Merge pull request #22139 from owncloud/comments-files-cleanup

cleanup jobs for comments and comment read marks

apps/files/appinfo/info.xml

@@ -6,7 +6,7 @@
 	<licence>AGPL</licence>
 	<author>Robin Appelman, Vincent Petry</author>
 	<default_enable/>
-	<version>1.4.2</version>
+	<version>1.4.3</version>
 	<types>
 		<filesystem/>
 	</types>

apps/files/appinfo/install.php

@@ -21,4 +21,4 @@
 
 // Cron job for scanning user storages
 \OC::$server->getJobList()->add('OCA\Files\BackgroundJob\ScanFiles');
-\OC::$server->getJobList()->add('OCA\Files\BackgroundJob\DeleteOrphanedTagsJob');
+\OC::$server->getJobList()->add('OCA\Files\BackgroundJob\DeleteOrphanedItems');

apps/files/appinfo/update.php

@@ -99,4 +99,4 @@ if ($installedVersion === '1.1.9' && (
 
 // Add cron job for scanning user storages
 \OC::$server->getJobList()->add('OCA\Files\BackgroundJob\ScanFiles');
-\OC::$server->getJobList()->add('OCA\Files\BackgroundJob\DeleteOrphanedTagsJob');
+\OC::$server->getJobList()->add('OCA\Files\BackgroundJob\DeleteOrphanedItems');

apps/files/lib/backgroundjob/deleteorphanedtagsjob.php → apps/files/lib/backgroundjob/deleteorphaneditems.php

@@ -22,11 +22,12 @@
 namespace OCA\Files\BackgroundJob;
 
 use OC\BackgroundJob\TimedJob;
+use OCP\DB\QueryBuilder\IQueryBuilder;
 
 /**
  * Delete all share entries that have no matching entries in the file cache table.
  */
-class DeleteOrphanedTagsJob extends TimedJob {
+class DeleteOrphanedItems extends TimedJob {
 
 	/** @var \OCP\IDBConnection */
 	protected $connection;
@@ -58,6 +59,8 @@ class DeleteOrphanedTagsJob extends TimedJob {
 	public function run($argument) {
 		$this->cleanSystemTags();
 		$this->cleanUserTags();
+		$this->cleanComments();
+		$this->cleanCommentMarkers();
 	}
 
 	/**
@@ -65,19 +68,29 @@ class DeleteOrphanedTagsJob extends TimedJob {
 	 *
 	 * @return int Number of deleted entries
 	 */
-	protected function cleanSystemTags() {
+	protected function cleanUp($table, $idCol, $typeCol) {
 		$subQuery = $this->connection->getQueryBuilder();
 		$subQuery->select($subQuery->expr()->literal('1'))
 			->from('filecache', 'f')
-			->where($subQuery->expr()->eq('objectid', 'f.fileid'));
+			->where($subQuery->expr()->eq($idCol, 'f.fileid'));
 
 		$query = $this->connection->getQueryBuilder();
-		$deletedEntries = $query->delete('systemtag_object_mapping')
-			->where($query->expr()->eq('objecttype', $query->expr()->literal('files')))
+		$deletedEntries = $query->delete($table)
+			->where($query->expr()->eq($typeCol, $query->expr()->literal('files')))
 			->andWhere($query->expr()->isNull($query->createFunction('(' . $subQuery->getSql() . ')')))
 			->execute();
 
-		$this->logger->debug("$deletedEntries orphaned system tag relations deleted", ['app' => 'DeleteOrphanedTagsJob']);
+		return $deletedEntries;
+	}
+
+	/**
+	 * Deleting orphaned system tag mappings
+	 *
+	 * @return int Number of deleted entries
+	 */
+	protected function cleanSystemTags() {
+		$deletedEntries = $this->cleanUp('systemtag_object_mapping', 'objectid', 'objecttype');
+		$this->logger->debug("$deletedEntries orphaned system tag relations deleted", ['app' => 'DeleteOrphanedItems']);
 		return $deletedEntries;
 	}
 
@@ -87,18 +100,32 @@ class DeleteOrphanedTagsJob extends TimedJob {
 	 * @return int Number of deleted entries
 	 */
 	protected function cleanUserTags() {
-		$subQuery = $this->connection->getQueryBuilder();
-		$subQuery->select($subQuery->expr()->literal('1'))
-			->from('filecache', 'f')
-			->where($subQuery->expr()->eq('objid', 'f.fileid'));
+		$deletedEntries = $this->cleanUp('vcategory_to_object', 'objid', 'type');
+		$this->logger->debug("$deletedEntries orphaned user tag relations deleted", ['app' => 'DeleteOrphanedItems']);
+		return $deletedEntries;
+	}
 
-		$query = $this->connection->getQueryBuilder();
-		$deletedEntries = $query->delete('vcategory_to_object')
-			->where($query->expr()->eq('type', $query->expr()->literal('files')))
-			->andWhere($query->expr()->isNull($query->createFunction('(' . $subQuery->getSql() . ')')))
-			->execute();
+	/**
+	 * Deleting orphaned comments
+	 *
+	 * @return int Number of deleted entries
+	 */
+	protected function cleanComments() {
+		$qb = $this->connection->getQueryBuilder();
+		$deletedEntries = $this->cleanUp('comments', $qb->expr()->castColumn('object_id', IQueryBuilder::PARAM_INT), 'object_type');
+		$this->logger->debug("$deletedEntries orphaned comments deleted", ['app' => 'DeleteOrphanedItems']);
+		return $deletedEntries;
+	}
 
-		$this->logger->debug("$deletedEntries orphaned user tag relations deleted", ['app' => 'DeleteOrphanedTagsJob']);
+	/**
+	 * Deleting orphaned comment read markers
+	 *
+	 * @return int Number of deleted entries
+	 */
+	protected function cleanCommentMarkers() {
+		$qb = $this->connection->getQueryBuilder();
+		$deletedEntries = $this->cleanUp('comments_read_markers', $qb->expr()->castColumn('object_id', IQueryBuilder::PARAM_INT), 'object_type');
+		$this->logger->debug("$deletedEntries orphaned comment read marks deleted", ['app' => 'DeleteOrphanedItems']);
 		return $deletedEntries;
 	}
 

apps/files/tests/backgroundjob/DeleteOrphanedTagsJobTest.php → apps/files/tests/backgroundjob/DeleteOrphanedItemsJobTest.php

@@ -21,17 +21,17 @@
 
 namespace OCA\Files\Tests\BackgroundJob;
 
-use OCA\Files\BackgroundJob\DeleteOrphanedTagsJob;
+use OCA\Files\BackgroundJob\DeleteOrphanedItems;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 
 /**
- * Class DeleteOrphanedTagsJobTest
+ * Class DeleteOrphanedItemsJobTest
  *
  * @group DB
  *
  * @package Test\BackgroundJob
  */
-class DeleteOrphanedTagsJobTest extends \Test\TestCase {
+class DeleteOrphanedItemsJobTest extends \Test\TestCase {
 
 	/** @var \OCP\IDBConnection */
 	protected $connection;
@@ -93,7 +93,7 @@ class DeleteOrphanedTagsJobTest extends \Test\TestCase {
 		$mapping = $this->getMappings('systemtag_object_mapping');
 		$this->assertCount(2, $mapping);
 
-		$job = new DeleteOrphanedTagsJob();
+		$job = new DeleteOrphanedItems();
 		$this->invokePrivate($job, 'cleanSystemTags');
 
 		$mapping = $this->getMappings('systemtag_object_mapping');
@@ -142,7 +142,7 @@ class DeleteOrphanedTagsJobTest extends \Test\TestCase {
 		$mapping = $this->getMappings('vcategory_to_object');
 		$this->assertCount(2, $mapping);
 
-		$job = new DeleteOrphanedTagsJob();
+		$job = new DeleteOrphanedItems();
 		$this->invokePrivate($job, 'cleanUserTags');
 
 		$mapping = $this->getMappings('vcategory_to_object');
@@ -155,4 +155,104 @@ class DeleteOrphanedTagsJobTest extends \Test\TestCase {
 		$this->cleanMapping('vcategory_to_object');
 	}
 
+	/**
+	 * Test clearing orphaned system tag mappings
+	 */
+	public function testClearComments() {
+		$this->cleanMapping('comments');
+
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('filecache')
+			->values([
+				'storage' => $query->createNamedParameter(1337, IQueryBuilder::PARAM_INT),
+				'path' => $query->createNamedParameter('apps/files/tests/deleteorphanedtagsjobtest.php'),
+				'path_hash' => $query->createNamedParameter(md5('apps/files/tests/deleteorphanedtagsjobtest.php')),
+			])->execute();
+		$fileId = $query->getLastInsertId();
+
+		// Existing file
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('comments')
+			->values([
+				'object_id' => $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT),
+				'object_type' => $query->createNamedParameter('files'),
+				'actor_id' => $query->createNamedParameter('Alice', IQueryBuilder::PARAM_INT),
+				'actor_type' => $query->createNamedParameter('users'),
+			])->execute();
+
+		// Non-existing file
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('comments')
+			->values([
+				'object_id' => $query->createNamedParameter($fileId + 1, IQueryBuilder::PARAM_INT),
+				'object_type' => $query->createNamedParameter('files'),
+				'actor_id' => $query->createNamedParameter('Alice', IQueryBuilder::PARAM_INT),
+				'actor_type' => $query->createNamedParameter('users'),
+			])->execute();
+
+		$mapping = $this->getMappings('comments');
+		$this->assertCount(2, $mapping);
+
+		$job = new DeleteOrphanedItems();
+		$this->invokePrivate($job, 'cleanComments');
+
+		$mapping = $this->getMappings('comments');
+		$this->assertCount(1, $mapping);
+
+		$query = $this->connection->getQueryBuilder();
+		$query->delete('filecache')
+			->where($query->expr()->eq('fileid', $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)))
+			->execute();
+		$this->cleanMapping('comments');
+	}
+
+	/**
+	 * Test clearing orphaned system tag mappings
+	 */
+	public function testClearCommentReadMarks() {
+		$this->cleanMapping('comments_read_markers');
+
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('filecache')
+			->values([
+				'storage' => $query->createNamedParameter(1337, IQueryBuilder::PARAM_INT),
+				'path' => $query->createNamedParameter('apps/files/tests/deleteorphanedtagsjobtest.php'),
+				'path_hash' => $query->createNamedParameter(md5('apps/files/tests/deleteorphanedtagsjobtest.php')),
+			])->execute();
+		$fileId = $query->getLastInsertId();
+
+		// Existing file
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('comments_read_markers')
+			->values([
+				'object_id' => $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT),
+				'object_type' => $query->createNamedParameter('files'),
+				'user_id' => $query->createNamedParameter('Alice', IQueryBuilder::PARAM_INT),
+			])->execute();
+
+		// Non-existing file
+		$query = $this->connection->getQueryBuilder();
+		$query->insert('comments_read_markers')
+			->values([
+				'object_id' => $query->createNamedParameter($fileId + 1, IQueryBuilder::PARAM_INT),
+				'object_type' => $query->createNamedParameter('files'),
+				'user_id' => $query->createNamedParameter('Alice', IQueryBuilder::PARAM_INT),
+			])->execute();
+
+		$mapping = $this->getMappings('comments_read_markers');
+		$this->assertCount(2, $mapping);
+
+		$job = new DeleteOrphanedItems();
+		$this->invokePrivate($job, 'cleanCommentMarkers');
+
+		$mapping = $this->getMappings('comments_read_markers');
+		$this->assertCount(1, $mapping);
+
+		$query = $this->connection->getQueryBuilder();
+		$query->delete('filecache')
+			->where($query->expr()->eq('fileid', $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)))
+			->execute();
+		$this->cleanMapping('comments_read_markers');
+	}
+
 }

lib/private/db/querybuilder/expressionbuilder.php → lib/private/db/querybuilder/expressionbuilder/expressionbuilder.php

@@ -19,9 +19,13 @@
  *
  */
 
-namespace OC\DB\QueryBuilder;
+namespace OC\DB\QueryBuilder\ExpressionBuilder;
 
 use Doctrine\DBAL\Query\Expression\ExpressionBuilder as DoctrineExpressionBuilder;
+use OC\DB\QueryBuilder\CompositeExpression;
+use OC\DB\QueryBuilder\Literal;
+use OC\DB\QueryBuilder\QueryFunction;
+use OC\DB\QueryBuilder\QuoteHelper;
 use OCP\DB\QueryBuilder\IExpressionBuilder;
 use OCP\IDBConnection;
 
@@ -331,4 +335,17 @@ class ExpressionBuilder implements IExpressionBuilder {
 	public function literal($input, $type = null) {
 		return new Literal($this->expressionBuilder->literal($input, $type));
 	}
+
+	/**
+	 * Returns a IQueryFunction that casts the column to the given type
+	 *
+	 * @param string $column
+	 * @param mixed $type One of IQueryBuilder::PARAM_*
+	 * @return string
+	 */
+	public function castColumn($column, $type) {
+		return new QueryFunction(
+			$this->helper->quoteColumnName($column)
+		);
+	}
 }

lib/private/db/querybuilder/ociexpressionbuilder.php → lib/private/db/querybuilder/expressionbuilder/ociexpressionbuilder.php

@@ -19,24 +19,25 @@
  *
  */
 
-namespace OC\DB\QueryBuilder;
+namespace OC\DB\QueryBuilder\ExpressionBuilder;
 
 
+use OC\DB\QueryBuilder\QueryFunction;
 use OCP\DB\QueryBuilder\ILiteral;
 use OCP\DB\QueryBuilder\IParameter;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\DB\QueryBuilder\IQueryFunction;
 
 class OCIExpressionBuilder extends ExpressionBuilder {
 
 	/**
 	 * @param mixed $column
 	 * @param mixed|null $type
-	 * @return array|QueryFunction|string
+	 * @return array|IQueryFunction|string
 	 */
 	protected function prepareColumn($column, $type) {
 		if ($type === IQueryBuilder::PARAM_STR && !is_array($column) && !($column instanceof IParameter) && !($column instanceof ILiteral)) {
-			$column = $this->helper->quoteColumnName($column);
-			$column = new QueryFunction('to_char(' . $column . ')');
+			$column = $this->castColumn($column, $type);
 		} else {
 			$column = $this->helper->quoteColumnNames($column);
 		}
@@ -132,4 +133,20 @@ class OCIExpressionBuilder extends ExpressionBuilder {
 
 		return $this->expressionBuilder->notIn($x, $y);
 	}
+
+	/**
+	 * Returns a IQueryFunction that casts the column to the given type
+	 *
+	 * @param string $column
+	 * @param mixed $type One of IQueryBuilder::PARAM_*
+	 * @return IQueryFunction
+	 */
+	public function castColumn($column, $type) {
+		if ($type === IQueryBuilder::PARAM_STR) {
+			$column = $this->helper->quoteColumnName($column);
+			return new QueryFunction('to_char(' . $column . ')');
+		}
+
+		return parent::castColumn($column, $type);
+	}
 }

lib/private/db/querybuilder/expressionbuilder/pgsqlexpressionbuilder.php

@@ -0,0 +1,45 @@
+<?php
+/**
+ * @author Joas Schilling <nickvergessen@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\DB\QueryBuilder\ExpressionBuilder;
+
+
+use OC\DB\QueryBuilder\QueryFunction;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+
+class PgSqlExpressionBuilder extends ExpressionBuilder {
+
+	/**
+	 * Returns a IQueryFunction that casts the column to the given type
+	 *
+	 * @param string $column
+	 * @param mixed $type One of IQueryBuilder::PARAM_*
+	 * @return string
+	 */
+	public function castColumn($column, $type) {
+		if ($type === IQueryBuilder::PARAM_INT) {
+			$column = $this->helper->quoteColumnName($column);
+			return new QueryFunction('CAST(' . $column . ' AS INT)');
+		}
+
+		return parent::castColumn($column, $type);
+	}
+}

lib/private/db/querybuilder/querybuilder.php

@@ -21,7 +21,11 @@
 
 namespace OC\DB\QueryBuilder;
 
+use Doctrine\DBAL\Platforms\PostgreSqlPlatform;
 use OC\DB\OracleConnection;
+use OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder;
+use OC\DB\QueryBuilder\ExpressionBuilder\OCIExpressionBuilder;
+use OC\DB\QueryBuilder\ExpressionBuilder\PgSqlExpressionBuilder;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\DB\QueryBuilder\IQueryFunction;
 use OCP\DB\QueryBuilder\IParameter;
@@ -85,6 +89,8 @@ class QueryBuilder implements IQueryBuilder {
 	public function expr() {
 		if ($this->connection instanceof OracleConnection) {
 			return new OCIExpressionBuilder($this->connection);
+		} else if ($this->connection->getDatabasePlatform() instanceof PostgreSqlPlatform) {
+			return new PgSqlExpressionBuilder($this->connection);
 		} else {
 			return new ExpressionBuilder($this->connection);
 		}

lib/private/repair/dropoldjobs.php

@@ -71,6 +71,7 @@ class DropOldJobs extends BasicEmitter implements RepairStep {
 		return [
 			['class' => 'OC_Cache_FileGlobalGC', 'arguments' => null],
 			['class' => 'OC\Cache\FileGlobalGC', 'arguments' => null],
+			['class' => 'OCA\Files\BackgroundJob\DeleteOrphanedTagsJob', 'arguments' => null],
 		];
 	}
 

lib/public/db/querybuilder/iexpressionbuilder.php

@@ -299,4 +299,14 @@ interface IExpressionBuilder {
 	 * @since 8.2.0
 	 */
 	public function literal($input, $type = null);
+
+	/**
+	 * Returns a IQueryFunction that casts the column to the given type
+	 *
+	 * @param string $column
+	 * @param mixed $type One of IQueryBuilder::PARAM_*
+	 * @return string
+	 * @since 9.0.0
+	 */
+	public function castColumn($column, $type);
 }

tests/lib/db/querybuilder/expressionbuildertest.php

@@ -22,7 +22,7 @@
 namespace Test\DB\QueryBuilder;
 
 use Doctrine\DBAL\Query\Expression\ExpressionBuilder as DoctrineExpressionBuilder;
-use OC\DB\QueryBuilder\ExpressionBuilder;
+use OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use Test\TestCase;
 

tests/lib/share/share.php

@@ -1423,7 +1423,7 @@ class Test_Share extends \Test\TestCase {
 		$userSession->method('getUser')->willReturn($user);
 
 
-		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder')
+		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder')
 		           ->disableOriginalConstructor()
 		           ->getMock();
 		$qb = $this->getMockBuilder('\OC\DB\QueryBuilder\QueryBuilder')
@@ -1478,7 +1478,7 @@ class Test_Share extends \Test\TestCase {
 		$userSession->method('getUser')->willReturn($user);
 
 
-		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder')
+		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder')
 		           ->disableOriginalConstructor()
 		           ->getMock();
 		$qb = $this->getMockBuilder('\OC\DB\QueryBuilder\QueryBuilder')
@@ -1531,7 +1531,7 @@ class Test_Share extends \Test\TestCase {
 		$userSession->method('getUser')->willReturn($user);
 
 
-		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder')
+		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder')
 		           ->disableOriginalConstructor()
 		           ->getMock();
 		$qb = $this->getMockBuilder('\OC\DB\QueryBuilder\QueryBuilder')
@@ -1584,7 +1584,7 @@ class Test_Share extends \Test\TestCase {
 		$userSession->method('getUser')->willReturn($user);
 
 
-		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder')
+		$ex = $this->getMockBuilder('\OC\DB\QueryBuilder\ExpressionBuilder\ExpressionBuilder')
 		           ->disableOriginalConstructor()
 		           ->getMock();
 		$qb = $this->getMockBuilder('\OC\DB\QueryBuilder\QueryBuilder')