Fix review comments .... (squash me into any other commit ...)

owncloud · Apr 2, 2019 · 9c9877b · 9c9877b
1 parent ec25ffb
commit 9c9877b
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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,30 +1209,19 @@ 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
  * @return IShare modified 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'])
  );
  }
  }