Merge pull request #610 from nextcloud/no-missing-preview-10

[10] Don't try to load previews when we know there is none
2016-07-28 17:48:42 +02:00 · 2016-07-28 17:48:42 +02:00 · 94f4e1009c
parent b28e6a7938 05b6850a9e
commit 94f4e1009c
13 changed files with 133 additions and 20 deletions
--- a/apps/dav/appinfo/v1/publicwebdav.php
+++ b/apps/dav/appinfo/v1/publicwebdav.php
@ -50,7 +50,8 @@ $serverFactory = new OCA\DAV\Connector\Sabre\ServerFactory(
 	\OC::$server->getUserSession(),
 	\OC::$server->getMountManager(),
 	\OC::$server->getTagManager(),
-	\OC::$server->getRequest()
+	\OC::$server->getRequest(),
+	\OC::$server->getPreviewManager()
 );

 $requestUri = \OC::$server->getRequest()->getRequestUri();
--- a/apps/dav/appinfo/v1/webdav.php
+++ b/apps/dav/appinfo/v1/webdav.php
@ -36,7 +36,8 @@ $serverFactory = new \OCA\DAV\Connector\Sabre\ServerFactory(
 	\OC::$server->getUserSession(),
 	\OC::$server->getMountManager(),
 	\OC::$server->getTagManager(),
-	\OC::$server->getRequest()
+	\OC::$server->getRequest(),
+	\OC::$server->getPreviewManager()
 );

 // Backends
--- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
@ -33,6 +33,7 @@ namespace OCA\DAV\Connector\Sabre;
 use OC\Files\View;
 use OCA\DAV\Upload\FutureFile;
 use OCP\Files\ForbiddenException;
+use OCP\IPreview;
 use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\IFile;
@ -50,6 +51,7 @@ class FilesPlugin extends ServerPlugin {

 	// namespace
 	const NS_OWNCLOUD = 'http://owncloud.org/ns';
+	const NS_NEXTCLOUD = 'http://nextcloud.org/ns';
 	const FILEID_PROPERTYNAME = '{http://owncloud.org/ns}id';
 	const INTERNAL_FILEID_PROPERTYNAME = '{http://owncloud.org/ns}fileid';
 	const PERMISSIONS_PROPERTYNAME = '{http://owncloud.org/ns}permissions';
@ -62,6 +64,7 @@ class FilesPlugin extends ServerPlugin {
 	const OWNER_DISPLAY_NAME_PROPERTYNAME = '{http://owncloud.org/ns}owner-display-name';
 	const CHECKSUMS_PROPERTYNAME = '{http://owncloud.org/ns}checksums';
 	const DATA_FINGERPRINT_PROPERTYNAME = '{http://owncloud.org/ns}data-fingerprint';
+	const HAS_PREVIEW_PROPERTYNAME = '{http://nextcloud.org/ns}has-preview';

 	/**
 	 * Reference to main server object
@ -103,11 +106,17 @@ class FilesPlugin extends ServerPlugin {
 	 */
 	private $request;

+	/**
+	 * @var IPreview
+	 */
+	private $previewManager;
+
 	/**
 	 * @param Tree $tree
 	 * @param View $view
 	 * @param IConfig $config
 	 * @param IRequest $request
+	 * @param IPreview $previewManager
 	 * @param bool $isPublic
 	 * @param bool $downloadAttachment
 	 */
@ -115,6 +124,7 @@ class FilesPlugin extends ServerPlugin {
 								View $view,
 								IConfig $config,
 								IRequest $request,
+								IPreview $previewManager,
 								$isPublic = false,
 								$downloadAttachment = true) {
 		$this->tree = $tree;
@ -123,6 +133,7 @@ class FilesPlugin extends ServerPlugin {
 		$this->request = $request;
 		$this->isPublic = $isPublic;
 		$this->downloadAttachment = $downloadAttachment;
+		$this->previewManager = $previewManager;
 	}

 	/**
@ -139,6 +150,7 @@ class FilesPlugin extends ServerPlugin {
 	public function initialize(\Sabre\DAV\Server $server) {

 		$server->xml->namespaceMap[self::NS_OWNCLOUD] = 'oc';
+		$server->xml->namespaceMap[self::NS_NEXTCLOUD] = 'nc';
 		$server->protectedProperties[] = self::FILEID_PROPERTYNAME;
 		$server->protectedProperties[] = self::INTERNAL_FILEID_PROPERTYNAME;
 		$server->protectedProperties[] = self::PERMISSIONS_PROPERTYNAME;
@ -149,6 +161,7 @@ class FilesPlugin extends ServerPlugin {
 		$server->protectedProperties[] = self::OWNER_DISPLAY_NAME_PROPERTYNAME;
 		$server->protectedProperties[] = self::CHECKSUMS_PROPERTYNAME;
 		$server->protectedProperties[] = self::DATA_FINGERPRINT_PROPERTYNAME;
+		$server->protectedProperties[] = self::HAS_PREVIEW_PROPERTYNAME;

 		// normally these cannot be changed (RFC4918), but we want them modifiable through PROPPATCH
 		$allowedProperties = ['{DAV:}getetag'];
@ -316,6 +329,10 @@ class FilesPlugin extends ServerPlugin {
 					return $this->config->getSystemValue('data-fingerprint', '');
 				}
 			});
+
+			$propFind->handle(self::HAS_PREVIEW_PROPERTYNAME, function () use ($node) {
+				return json_encode($this->previewManager->isAvailable($node->getFileInfo()));
+			});
 		}

 		if ($node instanceof \OCA\DAV\Files\FilesHome) {
--- a/apps/dav/lib/Connector/Sabre/ServerFactory.php
+++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php
@ -34,6 +34,7 @@ use OCP\Files\Mount\IMountManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\ILogger;
+use OCP\IPreview;
 use OCP\IRequest;
 use OCP\ITagManager;
 use OCP\IUserSession;
@ -54,6 +55,8 @@ class ServerFactory {
 	private $tagManager;
 	/** @var IRequest */
 	private $request;
+	/** @var IPreview  */
+	private $previewManager;

 	/**
 	 * @param IConfig $config
@ -63,6 +66,7 @@ class ServerFactory {
 	 * @param IMountManager $mountManager
 	 * @param ITagManager $tagManager
 	 * @param IRequest $request
+	 * @param IPreview $previewManager
 	 */
 	public function __construct(
 		IConfig $config,
@ -71,7 +75,8 @@ class ServerFactory {
 		IUserSession $userSession,
 		IMountManager $mountManager,
 		ITagManager $tagManager,
-		IRequest $request
+		IRequest $request,
+		IPreview $previewManager
 	) {
 		$this->config = $config;
 		$this->logger = $logger;
@ -80,6 +85,7 @@ class ServerFactory {
 		$this->mountManager = $mountManager;
 		$this->tagManager = $tagManager;
 		$this->request = $request;
+		$this->previewManager = $previewManager;
 	}

 	/**
@ -145,6 +151,7 @@ class ServerFactory {
 					$view,
 					$this->config,
 					$this->request,
+					$this->previewManager,
 					false,
 					!$this->config->getSystemValue('debug', false)
 				)
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@ -158,6 +158,7 @@ class Server {
 						$view,
 						\OC::$server->getConfig(),
 						$this->request,
+						\OC::$server->getPreviewManager(),
 						false,
 						!\OC::$server->getConfig()->getSystemValue('debug', false)
 					)
--- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php
@ -48,6 +48,7 @@ class FilesPluginTest extends TestCase {
 	const OWNER_ID_PROPERTYNAME = FilesPlugin::OWNER_ID_PROPERTYNAME;
 	const OWNER_DISPLAY_NAME_PROPERTYNAME = FilesPlugin::OWNER_DISPLAY_NAME_PROPERTYNAME;
 	const DATA_FINGERPRINT_PROPERTYNAME = FilesPlugin::DATA_FINGERPRINT_PROPERTYNAME;
+	const HAS_PREVIEW_PROPERTYNAME = FilesPlugin::HAS_PREVIEW_PROPERTYNAME;

 	/**
 	 * @var \Sabre\DAV\Server | \PHPUnit_Framework_MockObject_MockObject
@ -79,6 +80,11 @@ class FilesPluginTest extends TestCase {
 	 */
 	private $request;

+	/**
+	 * @var \OCP\IPreview | \PHPUnit_Framework_MockObject_MockObject
+	 */
+	private $previewManager;
+
 	public function setUp() {
 		parent::setUp();
 		$this->server = $this->getMockBuilder('\Sabre\DAV\Server')
@ -99,12 +105,16 @@ class FilesPluginTest extends TestCase {
 		$this->request = $this->getMockBuilder('\OCP\IRequest')
 			->disableOriginalConstructor()
 			->getMock();
+		$this->previewManager = $this->getMockBuilder('\OCP\IPreview')
+			->disableOriginalConstructor()
+			->getMock();

 		$this->plugin = new FilesPlugin(
 			$this->tree,
 			$this->view,
 			$this->config,
-			$this->request
+			$this->request,
+			$this->previewManager
 		);
 		$this->plugin->initialize($this->server);
 	}
@ -139,6 +149,13 @@ class FilesPluginTest extends TestCase {
 		$node->expects($this->any())
 			->method('getDavPermissions')
 			->will($this->returnValue('DWCKMSR'));
+		$node->expects($this->any())
+			->method('getFileInfo')
+			->will($this->returnValue(
+				$this->getMockBuilder('\OCP\Files\FileInfo')
+				->disableOriginalConstructor()
+				->getMock()
+			));

 		return $node;
 	}
@ -283,6 +300,7 @@ class FilesPluginTest extends TestCase {
 			$this->getMockBuilder('\OCP\IRequest')
 				->disableOriginalConstructor()
 				->getMock(),
+			$this->previewManager,
 			true);
 		$this->plugin->initialize($this->server);

@ -555,4 +573,28 @@ class FilesPluginTest extends TestCase {

 		$this->plugin->httpGet($request, $response);
 	}
+
+	public function testHasPreview() {
+		/** @var \OCA\DAV\Connector\Sabre\Directory | \PHPUnit_Framework_MockObject_MockObject $node */
+		$node = $this->createTestNode('\OCA\DAV\Connector\Sabre\Directory');
+
+		$propFind = new PropFind(
+			'/dummyPath',
+			array(
+				self::HAS_PREVIEW_PROPERTYNAME
+			),
+			0
+		);
+
+		$this->previewManager->expects($this->once())
+			->method('isAvailable')
+			->will($this->returnValue(false));
+
+		$this->plugin->handleGetProperties(
+			$propFind,
+			$node
+		);
+
+		$this->assertEquals("false", $propFind->get(self::HAS_PREVIEW_PROPERTYNAME));
+	}
 }
--- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@ -25,6 +25,7 @@
 namespace OCA\DAV\Tests\unit\Connector\Sabre;

 use OCA\DAV\Connector\Sabre\FilesReportPlugin as FilesReportPluginImplementation;
+use OCP\IPreview;
 use Sabre\DAV\Exception\NotFound;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OC\Files\View;
@ -60,6 +61,9 @@ class FilesReportPluginTest extends \Test\TestCase {
 	/** @var Folder|\PHPUnit_Framework_MockObject_MockObject **/
 	private $userFolder;

+	/** @var IPreview|\PHPUnit_Framework_MockObject_MockObject * */
+	private $previewManager;
+
 	public function setUp() {
 		parent::setUp();
 		$this->tree = $this->getMockBuilder('\Sabre\DAV\Tree')
@ -93,6 +97,10 @@ class FilesReportPluginTest extends \Test\TestCase {
 			->disableOriginalConstructor()
 			->getMock();

+		$this->previewManager = $this->getMockBuilder('\OCP\IPreview')
+			->disableOriginalConstructor()
+			->getMock();
+
 		$user = $this->getMockBuilder('\OCP\IUser')
 			->disableOriginalConstructor()
 			->getMock();
@ -365,7 +373,8 @@ class FilesReportPluginTest extends \Test\TestCase {
 				$config,
 				$this->getMockBuilder('\OCP\IRequest')
 					->disableOriginalConstructor()
-					->getMock()
+					->getMock(),
+				$this->previewManager
 			)
 		);
 		$this->plugin->initialize($this->server);
--- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTest.php
@ -66,7 +66,8 @@ abstract class RequestTest extends TestCase {
 			\OC::$server->getTagManager(),
 			$this->getMockBuilder('\OCP\IRequest')
 				->disableOriginalConstructor()
-				->getMock()
+				->getMock(),
+			\OC::$server->getPreviewManager()
 		);
 	}

--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@ -852,7 +852,8 @@
 				type: $el.attr('data-type'),
 				size: parseInt($el.attr('data-size'), 10),
 				etag: $el.attr('data-etag'),
-				permissions: parseInt($el.attr('data-permissions'), 10)
+				permissions: parseInt($el.attr('data-permissions'), 10),
+				hasPreview: $el.attr('data-has-preview') === 'true'
 			};
 			var icon = $el.attr('data-icon');
 			if (icon) {
@ -1068,7 +1069,8 @@
 				"data-mime": mime,
 				"data-mtime": mtime,
 				"data-etag": fileData.etag,
-				"data-permissions": fileData.permissions || this.getDirectoryPermissions()
+				"data-permissions": fileData.permissions || this.getDirectoryPermissions(),
+				"data-has-preview": fileData.hasPreview !== false
 			});

 			if (dataIcon) {
@ -1330,7 +1332,7 @@
 			// display actions
 			this.fileActions.display(filenameTd, !options.silent, this);

-			if (mime !== 'httpd/unix-directory') {
+			if (mime !== 'httpd/unix-directory' && fileData.hasPreview !== false) {
 				var iconDiv = filenameTd.find('.thumbnail');
 				// lazy load / newly inserted td ?
 				// the typeof check ensures that the default value of animate is true
--- a/apps/files/js/sidebarpreviewmanager.js
+++ b/apps/files/js/sidebarpreviewmanager.js
@ -32,9 +32,18 @@
 		},

 		loadPreview: function (model, $thumbnailDiv, $thumbnailContainer) {
+			if (model.get('hasPreview') === false) {
+				var mimeIcon = OC.MimeType.getIconUrl(model.get('mimetype'));
+				$thumbnailDiv.removeClass('icon-loading icon-32');
+				$thumbnailContainer.removeClass('image'); //fall back to regular view
+				$thumbnailDiv.css({
+					'background-image': 'url("' + mimeIcon + '")'
+				});
+			} else {
 				var handler = this.getPreviewHandler(model.get('mimetype'));
 				var fallback = this.fallbackPreview.bind(this, model, $thumbnailDiv, $thumbnailContainer);
 				handler(model, $thumbnailDiv, $thumbnailContainer, fallback);
+			}
 		},

 		// previews for images and mimetype icons
--- a/apps/files/tests/js/filelistSpec.js
+++ b/apps/files/tests/js/filelistSpec.js
@ -1792,7 +1792,8 @@ describe('OCA.Files.FileList tests', function() {
 					type: 'file',
 					size: 12,
 					etag: 'abc',
-					permissions: OC.PERMISSION_ALL
+					permissions: OC.PERMISSION_ALL,
+					hasPreview: true
 				});
 				expect(files[1]).toEqual({
 					id: 3,
@ -1802,7 +1803,8 @@ describe('OCA.Files.FileList tests', function() {
 					mtime: 234560000,
 					size: 58009,
 					etag: '123',
-					permissions: OC.PERMISSION_ALL
+					permissions: OC.PERMISSION_ALL,
+					hasPreview: true
 				});
 				expect(files[2]).toEqual({
 					id: 4,
@ -1812,7 +1814,8 @@ describe('OCA.Files.FileList tests', function() {
 					mtime: 134560000,
 					size: 250,
 					etag: '456',
-					permissions: OC.PERMISSION_ALL
+					permissions: OC.PERMISSION_ALL,
+					hasPreview: true
 				});
 				expect(files[0].id).toEqual(1);
 				expect(files[0].name).toEqual('One.txt');
@ -1833,7 +1836,8 @@ describe('OCA.Files.FileList tests', function() {
 					type: 'file',
 					size: 12,
 					etag: 'abc',
-					permissions: OC.PERMISSION_ALL
+					permissions: OC.PERMISSION_ALL,
+					hasPreview: true
 				});
 				expect(files[1]).toEqual({
 					id: 4,
@ -1843,7 +1847,8 @@ describe('OCA.Files.FileList tests', function() {
 					mtime: 134560000,
 					size: 250,
 					etag: '456',
-					permissions: OC.PERMISSION_ALL
+					permissions: OC.PERMISSION_ALL,
+					hasPreview: true
 				});
 			});
 			describe('Download', function() {
--- a/core/js/files/client.js
+++ b/core/js/files/client.js
@ -47,7 +47,8 @@
 			baseUrl: this._baseUrl,
 			xmlNamespaces: {
 				'DAV:': 'd',
-				'http://owncloud.org/ns': 'oc'
+				'http://owncloud.org/ns': 'oc',
+				'http://nextcloud.org/ns': 'nc'
 			}
 		};
 		if (options.userName) {
@ -61,6 +62,7 @@
 	};

 	Client.NS_OWNCLOUD = 'http://owncloud.org/ns';
+	Client.NS_NEXTCLOUD = 'http://nextcloud.org/ns';
 	Client.NS_DAV = 'DAV:';
 	Client._PROPFIND_PROPERTIES = [
 		/**
@ -95,7 +97,11 @@
 		/**
 		 * File sizes
 		 */
-		[Client.NS_DAV, 'getcontentlength']
+		[Client.NS_DAV, 'getcontentlength'],
+		/**
+		 * Preview availability
+		 */
+		[Client.NS_NEXTCLOUD, 'has-preview']
 	];

 	/**
@ -274,6 +280,13 @@
 				data.size = parseInt(sizeProp, 10);
 			}

+			var hasPreviewProp = props['{' + Client.NS_NEXTCLOUD + '}has-preview'];
+			if (!_.isUndefined(hasPreviewProp)) {
+				data.hasPreview = hasPreviewProp === 'true';
+			} else {
+				data.hasPreview = true;
+			}
+
 			var contentType = props['{' + Client.NS_DAV + '}getcontenttype'];
 			if (!_.isUndefined(contentType)) {
 				data.mimetype = contentType;
--- a/core/js/files/fileinfo.js
+++ b/core/js/files/fileinfo.js
@ -127,7 +127,12 @@
 		 *
 		 * @type string
 		 */
-		mountType: null
+		mountType: null,
+
+		/**
+		 * @type boolean
+		 */
+		hasPreview: true
 	};

 	if (!OC.Files) {