Merge pull request #1796 from nextcloud/oc_fav-report

Make it possible to filter by tags with REPORT method

11 files changed, 118 insertions, 276 deletions

diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
index 5b96f4f0f64..4df5f8b80e5 100644
--- a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -39,6 +39,7 @@ use OCP\Files\Folder;
 use OCP\IGroupManager;
 use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\TagNotFoundException;
+use OCP\ITagManager;
 
 class FilesReportPlugin extends ServerPlugin {
 
@@ -75,6 +76,13 @@ class FilesReportPlugin extends ServerPlugin {
 	private $tagMapper;
 
 	/**
+	 * Manager for private tags
+	 *
+	 * @var ITagManager
+	 */
+	private $fileTagger;
+
+	/**
 	 * @var IUserSession
 	 */
 	private $userSession;
@@ -92,11 +100,18 @@ class FilesReportPlugin extends ServerPlugin {
 	/**
 	 * @param Tree $tree
 	 * @param View $view
+	 * @param ISystemTagManager $tagManager
+	 * @param ISystemTagObjectMapper $tagMapper
+	 * @param ITagManager $fileTagger manager for private tags
+	 * @param IUserSession $userSession
+	 * @param IGroupManager $groupManager
+	 * @param Folder $userfolder
 	 */
 	public function __construct(Tree $tree,
 								View $view,
 								ISystemTagManager $tagManager,
 								ISystemTagObjectMapper $tagMapper,
+								ITagManager $fileTagger,
 								IUserSession $userSession,
 								IGroupManager $groupManager,
 								Folder $userFolder
@@ -105,6 +120,7 @@ class FilesReportPlugin extends ServerPlugin {
 		$this->fileView = $view;
 		$this->tagManager = $tagManager;
 		$this->tagMapper = $tagMapper;
+		$this->fileTagger = $fileTagger;
 		$this->userSession = $userSession;
 		$this->groupManager = $groupManager;
 		$this->userFolder = $userFolder;
@@ -215,12 +231,38 @@ class FilesReportPlugin extends ServerPlugin {
 		$ns = '{' . $this::NS_OWNCLOUD . '}';
 		$resultFileIds = null;
 		$systemTagIds = [];
+		$favoriteFilter = null;
 		foreach ($filterRules as $filterRule) {
 			if ($filterRule['name'] === $ns . 'systemtag') {
 				$systemTagIds[] = $filterRule['value'];
 			}
+			if ($filterRule['name'] === $ns . 'favorite') {
+				$favoriteFilter = true;
+			}
 		}
 
+		if ($favoriteFilter !== null) {
+			$resultFileIds = $this->fileTagger->load('files')->getFavorites();
+			if (empty($resultFileIds)) {
+				return [];
+			}
+		}
+
+		if (!empty($systemTagIds)) {
+			$fileIds = $this->getSystemTagFileIds($systemTagIds);
+			if (empty($resultFileIds)) {
+				$resultFileIds = $fileIds;
+			} else {
+				$resultFileIds = array_intersect($fileIds, $resultFileIds);
+			}
+		}
+
+		return $resultFileIds;
+	}
+
+	private function getSystemTagFileIds($systemTagIds) {
+		$resultFileIds = null;
+
 		// check user permissions, if applicable
 		if (!$this->isAdmin()) {
 			// check visibility/permission
diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php
index 644f0f28f57..6d9f9b1bc8b 100644
--- a/apps/dav/lib/Connector/Sabre/ServerFactory.php
+++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php
@@ -177,6 +177,7 @@ class ServerFactory {
 					$view,
 					\OC::$server->getSystemTagManager(),
 					\OC::$server->getSystemTagObjectMapper(),
+					\OC::$server->getTagManager(),
 					$this->userSession,
 					\OC::$server->getGroupManager(),
 					$userFolder
diff --git a/apps/dav/lib/Connector/Sabre/ShareTypeList.php b/apps/dav/lib/Connector/Sabre/ShareTypeList.php
index fd9ba3f7267..4d9179b6651 100644
--- a/apps/dav/lib/Connector/Sabre/ShareTypeList.php
+++ b/apps/dav/lib/Connector/Sabre/ShareTypeList.php
@@ -66,7 +66,11 @@ class ShareTypeList implements Element {
 	static function xmlDeserialize(Reader $reader) {
 		$shareTypes = [];
 
-		foreach ($reader->parseInnerTree() as $elem) {
+		$tree = $reader->parseInnerTree();
+		if ($tree === null) {
+			return null;
+		}
+		foreach ($tree as $elem) {
 			if ($elem['name'] === '{' . self::NS_OWNCLOUD . '}share-type') {
 				$shareTypes[] = (int)$elem['value'];
 			}
diff --git a/apps/dav/lib/Connector/Sabre/TagList.php b/apps/dav/lib/Connector/Sabre/TagList.php
index 23576171262..2f514112a43 100644
--- a/apps/dav/lib/Connector/Sabre/TagList.php
+++ b/apps/dav/lib/Connector/Sabre/TagList.php
@@ -85,7 +85,11 @@ class TagList implements Element {
 	static function xmlDeserialize(Reader $reader) {
 		$tags = [];
 
-		foreach ($reader->parseInnerTree() as $elem) {
+		$tree = $reader->parseInnerTree();
+		if ($tree === null) {
+			return null;
+		}
+		foreach ($tree as $elem) {
 			if ($elem['name'] === '{' . self::NS_OWNCLOUD . '}tag') {
 				$tags[] = $elem['value'];
 			}
diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
index 336a33058bf..2097777c8fd 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -26,12 +26,15 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre;
 
 use OCA\DAV\Connector\Sabre\FilesReportPlugin as FilesReportPluginImplementation;
 use OCP\IPreview;
+use OCP\ITagManager;
+use OCP\IUserSession;
 use Sabre\DAV\Exception\NotFound;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OC\Files\View;
 use OCP\Files\Folder;
 use OCP\IGroupManager;
 use OCP\SystemTag\ISystemTagManager;
+use OCP\ITags;
 
 class FilesReportPluginTest extends \Test\TestCase {
 	/** @var \Sabre\DAV\Server|\PHPUnit_Framework_MockObject_MockObject */
@@ -46,6 +49,9 @@ class FilesReportPluginTest extends \Test\TestCase {
 	/** @var ISystemTagManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $tagManager;
 
+	/** @var ITags|\PHPUnit_Framework_MockObject_MockObject */
+	private $privateTags;
+
 	/** @var  \OCP\IUserSession */
 	private $userSession;
 
@@ -87,20 +93,20 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->disableOriginalConstructor()
 			->getMock();
 
-		$this->tagManager = $this->getMockBuilder('\OCP\SystemTag\ISystemTagManager')
-			->disableOriginalConstructor()
-			->getMock();
-		$this->tagMapper = $this->getMockBuilder('\OCP\SystemTag\ISystemTagObjectMapper')
-			->disableOriginalConstructor()
-			->getMock();
-		$this->userSession = $this->getMockBuilder('\OCP\IUserSession')
-			->disableOriginalConstructor()
-			->getMock();
-
 		$this->previewManager = $this->getMockBuilder('\OCP\IPreview')
 			->disableOriginalConstructor()
 			->getMock();
 
+		$this->tagManager = $this->createMock(ISystemTagManager::class);
+		$this->tagMapper = $this->createMock(ISystemTagObjectMapper::class);
+		$this->userSession = $this->createMock(IUserSession::class);
+		$this->privateTags = $this->createMock(ITags::class);
+		$privateTagManager = $this->createMock(ITagManager::class);
+		$privateTagManager->expects($this->any())
+			->method('load')
+			->with('files')
+			->will($this->returnValue($this->privateTags));
+
 		$user = $this->getMockBuilder('\OCP\IUser')
 			->disableOriginalConstructor()
 			->getMock();
@@ -116,6 +122,7 @@ class FilesReportPluginTest extends \Test\TestCase {
 			$this->view,
 			$this->tagManager,
 			$this->tagMapper,
+			$privateTagManager,
 			$this->userSession,
 			$this->groupManager,
 			$this->userFolder
@@ -652,4 +659,16 @@ class FilesReportPluginTest extends \Test\TestCase {
 
 		$this->assertEquals(['222'], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
 	}
+
+	public function testProcessFavoriteFilter() {
+		$rules = [
+			['name' => '{http://owncloud.org/ns}favorite', 'value' => '1'],
+		];
+
+		$this->privateTags->expects($this->once())
+			->method('getFavorites')
+			->will($this->returnValue(['456', '789']));
+
+		$this->assertEquals(['456', '789'], array_values($this->invokePrivate($this->plugin, 'processFilterRules', [$rules])));
+	}
 }
diff --git a/apps/files/appinfo/routes.php b/apps/files/appinfo/routes.php
index 6237e8413ed..49dbe553435 100644
--- a/apps/files/appinfo/routes.php
+++ b/apps/files/appinfo/routes.php
@@ -45,12 +45,6 @@ $application->registerRoutes(
 				'requirements' => array('path' => '.+'),
 			),
 			array(
-				'name' => 'API#getFilesByTag',
-				'url' => '/api/v1/tags/{tagName}/files',
-				'verb' => 'GET',
-				'requirements' => array('tagName' => '.+'),
-			),
-			array(
 				'name' => 'API#getRecentFiles',
 				'url' => '/api/v1/recent/',
 				'verb' => 'GET'
diff --git a/apps/files/js/favoritesfilelist.js b/apps/files/js/favoritesfilelist.js
index e6532ab188c..380689be10b 100644
--- a/apps/files/js/favoritesfilelist.js
+++ b/apps/files/js/favoritesfilelist.js
@@ -75,24 +75,25 @@ $(document).ready(function() {
 				// there is only root
 				this._setCurrentDir('/', false);
 
-				this._reloadCall = $.ajax({
-					url: OC.generateUrl('/apps/files/api/v1/tags/{tagName}/files', {tagName: tagName}),
-					type: 'GET',
-					dataType: 'json'
-				});
+				this._reloadCall = this.filesClient.getFilteredFiles(
+					{
+						favorite: true
+					},
+					{
+						properties: this._getWebdavProperties()
+					}
+				);
 				var callBack = this.reloadCallback.bind(this);
 				return this._reloadCall.then(callBack, callBack);
 			},
 
-			reloadCallback: function(result) {
-				delete this._reloadCall;
-				this.hideMask();
-
-				if (result.files) {
-					this.setFiles(result.files.sort(this._sortComparator));
-					return true;
+			reloadCallback: function(status, result) {
+				if (result) {
+					// prepend empty dir info because original handler
+					result.unshift({});
 				}
-				return false;
+
+				return OCA.Files.FileList.prototype.reloadCallback.call(this, status, result);
 			}
 		});
 
diff --git a/apps/files/lib/Controller/ApiController.php b/apps/files/lib/Controller/ApiController.php
index 7ce83bfca15..d6f88581b96 100644
--- a/apps/files/lib/Controller/ApiController.php
+++ b/apps/files/lib/Controller/ApiController.php
@@ -174,23 +174,6 @@ class ApiController extends Controller {
 	}
 
 	/**
-	 * Returns a list of all files tagged with the given tag.
-	 *
-	 * @NoAdminRequired
-	 *
-	 * @param string $tagName tag name to filter by
-	 * @return DataResponse
-	 */
-	public function getFilesByTag($tagName) {
-		$nodes = $this->tagService->getFilesByTag($tagName);
-		$files = $this->formatNodes($nodes);
-		foreach ($files as &$file) {
-			$file['tags'] = [$tagName];
-		}
-		return new DataResponse(['files' => $files]);
-	}
-
-	/**
 	 * Returns a list of recently modifed files.
 	 *
 	 * @NoAdminRequired
diff --git a/apps/files/lib/Service/TagService.php b/apps/files/lib/Service/TagService.php
index 04e962b5e17..4482fb45371 100644
--- a/apps/files/lib/Service/TagService.php
+++ b/apps/files/lib/Service/TagService.php
@@ -90,26 +90,5 @@ class TagService {
 		// list is up to date, in case of concurrent changes ?
 		return $tags;
 	}
-
-	/**
-	 * Get all files for the given tag
-	 *
-	 * @param string $tagName tag name to filter by
-	 * @return Node[] list of matching files
-	 * @throws \Exception if the tag does not exist
-	 */
-	public function getFilesByTag($tagName) {
-		try {
-			$fileIds = $this->tagger->getIdsForTag($tagName);
-		} catch (\Exception $e) {
-			return [];
-		}
-
-		$allNodes = [];
-		foreach ($fileIds as $fileId) {
-			$allNodes = array_merge($allNodes, $this->homeFolder->getById((int) $fileId));
-		}
-		return $allNodes;
-	}
 }
 
diff --git a/apps/files/tests/Controller/ApiControllerTest.php b/apps/files/tests/Controller/ApiControllerTest.php
index 9bfc6d6f5e8..4b7bec065a0 100644
--- a/apps/files/tests/Controller/ApiControllerTest.php
+++ b/apps/files/tests/Controller/ApiControllerTest.php
@@ -103,182 +103,6 @@ class ApiControllerTest extends TestCase {
 		);
 	}
 
-	public function testGetFilesByTagEmpty() {
-		$tagName = 'MyTagName';
-		$this->tagService->expects($this->once())
-			->method('getFilesByTag')
-			->with($this->equalTo([$tagName]))
-			->will($this->returnValue([]));
-
-		$expected = new DataResponse(['files' => []]);
-		$this->assertEquals($expected, $this->apiController->getFilesByTag([$tagName]));
-	}
-
-	public function testGetFilesByTagSingle() {
-		$tagName = 'MyTagName';
-		$fileInfo = new FileInfo(
-			'/root.txt',
-			$this->getMockBuilder('\OC\Files\Storage\Storage')
-				->disableOriginalConstructor()
-				->getMock(),
-			'/var/www/root.txt',
-			[
-				'mtime' => 55,
-				'mimetype' => 'application/pdf',
-				'permissions' => 31,
-				'size' => 1234,
-				'etag' => 'MyEtag',
-			],
-			$this->getMockBuilder('\OCP\Files\Mount\IMountPoint')
-				->disableOriginalConstructor()
-				->getMock()
-		);
-		$node = $this->getMockBuilder('\OC\Files\Node\File')
-			->disableOriginalConstructor()
-			->getMock();
-		$node->expects($this->once())
-			->method('getFileInfo')
-			->will($this->returnValue($fileInfo));
-		$this->tagService->expects($this->once())
-			->method('getFilesByTag')
-			->with($this->equalTo([$tagName]))
-			->will($this->returnValue([$node]));
-
-		$this->shareManager->expects($this->any())
-			->method('getSharesBy')
-			->with(
-				$this->equalTo('user1'),
-				$this->anything(),
-				$node,
-				$this->equalTo(false),
-				$this->equalTo(1)
-			)
-			->will($this->returnCallback(function($userId, $shareType) {
-				if ($shareType === \OCP\Share::SHARE_TYPE_USER || $shareType === \OCP\Share::SHARE_TYPE_LINK) {
-					return ['dummy_share'];
-				}
-				return [];
-			}));
-
-		$expected = new DataResponse([
-			'files' => [
-				[
-					'id' => null,
-					'parentId' => null,
-					'mtime' => 55000,
-					'name' => 'root.txt',
-					'permissions' => 31,
-					'mimetype' => 'application/pdf',
-					'size' => 1234,
-					'type' => 'file',
-					'etag' => 'MyEtag',
-					'path' => '/',
-					'tags' => [
-						[
-							'MyTagName'
-						]
-					],
-					'shareTypes' => [\OCP\Share::SHARE_TYPE_USER, \OCP\Share::SHARE_TYPE_LINK]
-				],
-			],
-		]);
-		$this->assertEquals($expected, $this->apiController->getFilesByTag([$tagName]));
-	}
-
-	public function testGetFilesByTagMultiple() {
-		$tagName = 'MyTagName';
-		$fileInfo1 = new FileInfo(
-			'/root.txt',
-			$this->getMockBuilder('\OC\Files\Storage\Storage')
-				->disableOriginalConstructor()
-				->getMock(),
-			'/var/www/root.txt',
-			[
-				'mtime' => 55,
-				'mimetype' => 'application/pdf',
-				'permissions' => 31,
-				'size' => 1234,
-				'etag' => 'MyEtag',
-			],
-			$this->getMockBuilder('\OCP\Files\Mount\IMountPoint')
-				->disableOriginalConstructor()
-				->getMock()
-		);
-		$fileInfo2 = new FileInfo(
-			'/root.txt',
-			$this->getMockBuilder('\OC\Files\Storage\Storage')
-				->disableOriginalConstructor()
-				->getMock(),
-			'/var/www/some/sub.txt',
-			[
-				'mtime' => 999,
-				'mimetype' => 'application/binary',
-				'permissions' => 31,
-				'size' => 9876,
-				'etag' => 'SubEtag',
-			],
-			$this->getMockBuilder('\OCP\Files\Mount\IMountPoint')
-				->disableOriginalConstructor()
-				->getMock()
-		);
-		$node1 = $this->getMockBuilder('\OC\Files\Node\File')
-			->disableOriginalConstructor()
-			->getMock();
-		$node1->expects($this->once())
-			->method('getFileInfo')
-			->will($this->returnValue($fileInfo1));
-		$node2 = $this->getMockBuilder('\OC\Files\Node\File')
-			->disableOriginalConstructor()
-			->getMock();
-		$node2->expects($this->once())
-			->method('getFileInfo')
-			->will($this->returnValue($fileInfo2));
-		$this->tagService->expects($this->once())
-			->method('getFilesByTag')
-			->with($this->equalTo([$tagName]))
-			->will($this->returnValue([$node1, $node2]));
-
-		$expected = new DataResponse([
-			'files' => [
-				[
-					'id' => null,
-					'parentId' => null,
-					'mtime' => 55000,
-					'name' => 'root.txt',
-					'permissions' => 31,
-					'mimetype' => 'application/pdf',
-					'size' => 1234,
-					'type' => 'file',
-					'etag' => 'MyEtag',
-					'path' => '/',
-					'tags' => [
-						[
-							'MyTagName'
-						]
-					],
-				],
-				[
-					'id' => null,
-					'parentId' => null,
-					'mtime' => 999000,
-					'name' => 'root.txt',
-					'permissions' => 31,
-					'mimetype' => 'application/binary',
-					'size' => 9876,
-					'type' => 'file',
-					'etag' => 'SubEtag',
-					'path' => '/',
-					'tags' => [
-						[
-							'MyTagName'
-						]
-					],
-				]
-			],
-		]);
-		$this->assertEquals($expected, $this->apiController->getFilesByTag([$tagName]));
-	}
-
 	public function testUpdateFileTagsEmpty() {
 		$expected = new DataResponse([]);
 		$this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt'));
diff --git a/apps/files/tests/js/favoritesfilelistspec.js b/apps/files/tests/js/favoritesfilelistspec.js
index 1c833d334e2..db890927ed4 100644
--- a/apps/files/tests/js/favoritesfilelistspec.js
+++ b/apps/files/tests/js/favoritesfilelistspec.js
@@ -41,13 +41,9 @@ describe('OCA.Files.FavoritesFileList tests', function() {
 			'</div>'
 		);
 	});
-	afterEach(function() {
-		fileList.destroy();
-		fileList = undefined;
-	});
 
 	describe('loading file list', function() {
-		var response;
+		var fetchStub;
 
 		beforeEach(function() {
 			fileList = new OCA.Files.FavoritesFileList(
@@ -55,36 +51,31 @@ describe('OCA.Files.FavoritesFileList tests', function() {
 			);
 			OCA.Files.FavoritesPlugin.attach(fileList);
 
-			fileList.reload();
-
-			/* jshint camelcase: false */
-			response = {
-				files: [{
-					id: 7,
-					name: 'test.txt',
-					path: '/somedir',
-					size: 123,
-					mtime: 11111000,
-					tags: [OC.TAG_FAVORITE],
-					permissions: OC.PERMISSION_ALL,
-					mimetype: 'text/plain'
-				}]
-			};
+			fetchStub = sinon.stub(fileList.filesClient, 'getFilteredFiles');
+		});
+		afterEach(function() {
+			fetchStub.restore();
+			fileList.destroy();
+			fileList = undefined;
 		});
 		it('render files', function() {
-			var request;
+			var deferred = $.Deferred();
+			fetchStub.returns(deferred.promise());
 
-			expect(fakeServer.requests.length).toEqual(1);
-			request = fakeServer.requests[0];
-			expect(request.url).toEqual(
-				OC.generateUrl('apps/files/api/v1/tags/{tagName}/files', {tagName: OC.TAG_FAVORITE})
-			);
+			fileList.reload();
 
-			fakeServer.requests[0].respond(
-				200,
-				{ 'Content-Type': 'application/json' },
-				JSON.stringify(response)
-			);
+			expect(fetchStub.calledOnce).toEqual(true);
+
+			deferred.resolve(207, [{
+				id: 7,
+				name: 'test.txt',
+				path: '/somedir',
+				size: 123,
+				mtime: 11111000,
+				tags: [OC.TAG_FAVORITE],
+				permissions: OC.PERMISSION_ALL,
+				mimetype: 'text/plain'
+			}]);
 
 			var $rows = fileList.$el.find('tbody tr');
 			var $tr = $rows.eq(0);