From 9c9877bbc1c3467baa4880ee13e7f73d099cf462 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20M=C3=BCller?= <thomas.mueller@tmit.eu>
Date: Tue, 2 Apr 2019 12:02:48 +0200
Subject: [PATCH] Fix review comments .... (squash me into any other commit
 ...)

---
 apps/dav/lib/DAV/ViewOnlyPlugin.php           |  5 ++--
 .../dav/tests/unit/DAV/ViewOnlyPluginTest.php |  8 +++----
 .../lib/Controller/Share20OcsController.php   | 24 +++++++------------
 3 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/apps/dav/lib/DAV/ViewOnlyPlugin.php b/apps/dav/lib/DAV/ViewOnlyPlugin.php
index b53631ca6171..bf26fd8b9914 100644
--- a/apps/dav/lib/DAV/ViewOnlyPlugin.php
+++ b/apps/dav/lib/DAV/ViewOnlyPlugin.php
@@ -25,8 +25,8 @@
 use OCA\DAV\Connector\Sabre\File as DavFile;
 use OCA\DAV\Meta\MetaFile;
 use OCP\Files\FileInfo;
+use OCP\Files\NotFoundException;
 use OCP\ILogger;
-use Sabre\DAV\Exception\ServiceUnavailable;
 use Sabre\DAV\Server;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
@@ -37,7 +37,7 @@
  */
 class ViewOnlyPlugin extends ServerPlugin {
 
-	/** @var \Sabre\DAV\Server $server */
+	/** @var Server $server */
 	private $server;
 
 	/** @var ILogger $logger */
@@ -75,6 +75,7 @@ public function initialize(Server $server) {
 	 * @param RequestInterface $request request object
 	 * @return boolean
 	 * @throws Forbidden
+	 * @throws NotFoundException
 	 */
 	public function checkViewOnly(
 		RequestInterface $request
diff --git a/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php b/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php
index 430b1d483a90..577935be7b07 100644
--- a/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php
+++ b/apps/dav/tests/unit/DAV/ViewOnlyPluginTest.php
@@ -57,14 +57,14 @@ public function setUp() {
 	}
 
 	public function testCanGetNonDav() {
-		$this->request->expects($this->exactly(1))->method('getPath')->willReturn('files/test/target');
+		$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
 		$this->tree->method('getNodeForPath')->willReturn(null);
 
 		$this->assertTrue($this->plugin->checkViewOnly($this->request));
 	}
 
 	public function testCanGetNonFileInfo() {
-		$this->request->expects($this->exactly(1))->method('getPath')->willReturn('files/test/target');
+		$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
 		$davNode = $this->createMock(DavFile::class);
 		$this->tree->method('getNodeForPath')->willReturn($davNode);
 
@@ -74,7 +74,7 @@ public function testCanGetNonFileInfo() {
 	}
 
 	public function testCanGetNonShared() {
-		$this->request->expects($this->exactly(1))->method('getPath')->willReturn('files/test/target');
+		$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
 		$davNode = $this->createMock(DavFile::class);
 		$this->tree->method('getNodeForPath')->willReturn($davNode);
 
@@ -103,7 +103,7 @@ public function providesDataForCanGet() {
 	 * @dataProvider providesDataForCanGet
 	 */
 	public function testCanGet($nodeInfo, $attrEnabled, $expectCanDownloadFile) {
-		$this->request->expects($this->exactly(1))->method('getPath')->willReturn('files/test/target');
+		$this->request->expects($this->once())->method('getPath')->willReturn('files/test/target');
 
 		$davNode = $this->createMock(DavFile::class);
 		$this->tree->method('getNodeForPath')->willReturn($davNode);
diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php
index c95d07252ed0..7d70de28dd3f 100644
--- a/apps/files_sharing/lib/Controller/Share20OcsController.php
+++ b/apps/files_sharing/lib/Controller/Share20OcsController.php
@@ -222,7 +222,10 @@ protected function formatShare(IShare $share, $received = false) {
 
 		$result['mail_send'] = $share->getMailSend() ? 1 : 0;
 
-		$result['attributes'] = $this->formatShareAttributes($share);
+		$result['attributes'] = null;
+		if ($attributes = $share->getAttributes()) {
+			$result['attributes'] =  \json_encode($attributes->toArray());
+		}
 
 		return $result;
 	}
@@ -1206,17 +1209,6 @@ private function getShareById($id, $recipient = null) {
 		return $share;
 	}
 
-	/**
-	 * @param IShare $share
-	 * @return string|null
-	 */
-	private function formatShareAttributes(IShare $share) {
-		if ($attributes = $share->getAttributes()) {
-			return \json_encode($attributes->toArray());
-		}
-		return null;
-	}
-
 	/**
 	 * @param IShare $share
 	 * @param string[][]|null $formattedShareAttributes
@@ -1224,12 +1216,12 @@ private function formatShareAttributes(IShare $share) {
 	 */
 	private function setShareAttributes(IShare $share, $formattedShareAttributes) {
 		$newShareAttributes = $this->shareManager->newShare()->newAttributes();
-		if ($formattedShareAttributes != null) {
+		if ($formattedShareAttributes !== null) {
 			foreach ($formattedShareAttributes as $formattedAttr) {
 				$newShareAttributes->setAttribute(
-					$formattedAttr["scope"],
-					$formattedAttr["key"],
-					(bool) \json_decode($formattedAttr["enabled"])
+					$formattedAttr['scope'],
+					$formattedAttr['key'],
+					(bool) \json_decode($formattedAttr['enabled'])
 				);
 			}
 		}