Merge pull request #20487 from owncloud/activity-oracle-sql-error-for-favorites

Activity oracle sql error for favorites

3 files changed, 50 insertions, 18 deletions

diff --git a/apps/files/appinfo/app.php b/apps/files/appinfo/app.php
index c752b5e7d72..61ff6d748f9 100644
--- a/apps/files/appinfo/app.php
+++ b/apps/files/appinfo/app.php
@@ -65,6 +65,7 @@ $templateManager->registerTemplate('application/vnd.oasis.opendocument.spreadshe
 		new \OCA\Files\ActivityHelper(
 			\OC::$server->getTagManager()
 		),
+		\OC::$server->getDatabaseConnection(),
 		\OC::$server->getConfig()
 	);
 });
diff --git a/apps/files/lib/activity.php b/apps/files/lib/activity.php
index f3bbff48640..23e3f26e62d 100644
--- a/apps/files/lib/activity.php
+++ b/apps/files/lib/activity.php
@@ -22,7 +22,8 @@
 
 namespace OCA\Files;
 
-use OC\L10N\Factory;
+use OCP\IDBConnection;
+use OCP\L10N\IFactory;
 use OCP\Activity\IExtension;
 use OCP\Activity\IManager;
 use OCP\IConfig;
@@ -43,7 +44,7 @@ class Activity implements IExtension {
 	/** @var IL10N */
 	protected $l;
 
-	/** @var Factory */
+	/** @var IFactory */
 	protected $languageFactory;
 
 	/** @var IURLGenerator */
@@ -52,6 +53,9 @@ class Activity implements IExtension {
 	/** @var \OCP\Activity\IManager */
 	protected $activityManager;
 
+	/** @var \OCP\IDBConnection */
+	protected $connection;
+
 	/** @var \OCP\IConfig */
 	protected $config;
 
@@ -59,18 +63,20 @@ class Activity implements IExtension {
 	protected $helper;
 
 	/**
-	 * @param Factory $languageFactory
+	 * @param IFactory $languageFactory
 	 * @param IURLGenerator $URLGenerator
 	 * @param IManager $activityManager
 	 * @param ActivityHelper $helper
+	 * @param IDBConnection $connection
 	 * @param IConfig $config
 	 */
-	public function __construct(Factory $languageFactory, IURLGenerator $URLGenerator, IManager $activityManager, ActivityHelper $helper, IConfig $config) {
+	public function __construct(IFactory $languageFactory, IURLGenerator $URLGenerator, IManager $activityManager, ActivityHelper $helper, IDBConnection $connection, IConfig $config) {
 		$this->languageFactory = $languageFactory;
 		$this->URLGenerator = $URLGenerator;
 		$this->l = $this->getL10N();
 		$this->activityManager = $activityManager;
 		$this->helper = $helper;
+		$this->connection = $connection;
 		$this->config = $config;
 	}
 
@@ -379,6 +385,7 @@ class Activity implements IExtension {
 			 */
 			$parameters = $fileQueryList = [];
 			$parameters[] = self::APP_FILES;
+			$parameters[] = self::APP_FILES;
 
 			$fileQueryList[] = '(`type` <> ? AND `type` <> ?)';
 			$parameters[] = self::TYPE_SHARE_CREATED;
@@ -390,13 +397,15 @@ class Activity implements IExtension {
 			}
 			foreach ($favorites['folders'] as $favorite) {
 				$fileQueryList[] = '`file` LIKE ?';
-				$parameters[] = $favorite . '/%';
+				$parameters[] = $this->connection->escapeLikeParameter($favorite) . '/%';
 			}
 
-			$parameters[] = self::APP_FILES;
-
 			return [
-				' CASE WHEN `app` = ? THEN (' . implode(' OR ', $fileQueryList) . ') ELSE `app` <> ? END ',
+				' CASE '
+					. 'WHEN `app` <> ? THEN 1 '
+					. 'WHEN `app` = ? AND (' . implode(' OR ', $fileQueryList) . ') THEN 1 '
+					. 'ELSE 0 '
+				. 'END = 1 ',
 				$parameters,
 			];
 		}
diff --git a/apps/files/tests/activitytest.php b/apps/files/tests/activitytest.php
index cdb1d21bcd8..59c020c9042 100644
--- a/apps/files/tests/activitytest.php
+++ b/apps/files/tests/activitytest.php
@@ -25,24 +25,30 @@ namespace OCA\Files\Tests;
 use OCA\Files\Activity;
 use Test\TestCase;
 
+/**
+ * Class ActivityTest
+ *
+ * @group DB
+ * @package OCA\Files\Tests
+ */
 class ActivityTest extends TestCase {
 
 	/** @var \OC\ActivityManager */
 	private $activityManager;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \OCP\IRequest|\PHPUnit_Framework_MockObject_MockObject */
 	protected $request;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \OCP\IUserSession|\PHPUnit_Framework_MockObject_MockObject */
 	protected $session;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \OCP\IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	protected $config;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \OCA\Files\ActivityHelper|\PHPUnit_Framework_MockObject_MockObject */
 	protected $activityHelper;
 
-	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	/** @var \OCP\L10N\IFactory|\PHPUnit_Framework_MockObject_MockObject */
 	protected $l10nFactory;
 
 	/** @var \OCA\Files\Activity */
@@ -70,7 +76,7 @@ class ActivityTest extends TestCase {
 			$this->config
 		);
 
-		$this->l10nFactory = $this->getMockBuilder('OC\L10N\Factory')
+		$this->l10nFactory = $this->getMockBuilder('OCP\L10N\IFactory')
 			->disableOriginalConstructor()
 			->getMock();
 		$deL10n = $this->getMockBuilder('OC_L10N')
@@ -95,6 +101,7 @@ class ActivityTest extends TestCase {
 			$this->getMockBuilder('OCP\IURLGenerator')->disableOriginalConstructor()->getMock(),
 			$this->activityManager,
 			$this->activityHelper,
+			\OC::$server->getDatabaseConnection(),
 			$this->config
 		);
 
@@ -290,16 +297,16 @@ class ActivityTest extends TestCase {
 					'items' => [],
 					'folders' => [],
 				],
-				' CASE WHEN `app` = ? THEN ((`type` <> ? AND `type` <> ?)) ELSE `app` <> ? END ',
-				['files', Activity::TYPE_SHARE_CREATED, Activity::TYPE_SHARE_CHANGED, 'files']
+				' CASE WHEN `app` <> ? THEN 1 WHEN `app` = ? AND ((`type` <> ? AND `type` <> ?)) THEN 1 ELSE 0 END = 1 ',
+				['files', 'files', Activity::TYPE_SHARE_CREATED, Activity::TYPE_SHARE_CHANGED]
 			],
 			[
 				[
 					'items' => ['file.txt', 'folder'],
 					'folders' => ['folder'],
 				],
-				' CASE WHEN `app` = ? THEN ((`type` <> ? AND `type` <> ?) OR `file` = ? OR `file` = ? OR `file` LIKE ?) ELSE `app` <> ? END ',
-				['files', Activity::TYPE_SHARE_CREATED, Activity::TYPE_SHARE_CHANGED, 'file.txt', 'folder', 'folder/%', 'files']
+				' CASE WHEN `app` <> ? THEN 1 WHEN `app` = ? AND ((`type` <> ? AND `type` <> ?) OR `file` = ? OR `file` = ? OR `file` LIKE ?) THEN 1 ELSE 0 END = 1 ',
+				['files', 'files', Activity::TYPE_SHARE_CREATED, Activity::TYPE_SHARE_CHANGED, 'file.txt', 'folder', 'folder/%']
 			],
 		];
 	}
@@ -333,6 +340,21 @@ class ActivityTest extends TestCase {
 
 		$result = $this->activityExtension->getQueryForFilter('all');
 		$this->assertEquals([$query, $parameters], $result);
+
+		$this->executeQueryForFilter($result);
+	}
+
+	public function executeQueryForFilter(array $result) {
+		list($resultQuery, $resultParameters) = $result;
+		$resultQuery = str_replace('`file`', '`user`', $resultQuery);
+		$resultQuery = str_replace('`type`', '`key`', $resultQuery);
+
+		$connection = \OC::$server->getDatabaseConnection();
+		// Test the query on the privatedata table, because the activity table
+		// does not exist in core
+		$result = $connection->executeQuery('SELECT * FROM `*PREFIX*privatedata` WHERE ' . $resultQuery, $resultParameters);
+		$rows = $result->fetchAll();
+		$result->closeCursor();
 	}
 
 	protected function mockUserSession($user) {