Check if the offset exists before accessing

This checks if the offset exists before accessing it and also adds unit tests to this function which would have catched this before :see_no_evil:

Fixes https://github.com/owncloud/core/issues/14277

diff --git a/apps/files/controller/apicontroller.php b/apps/files/controller/apicontroller.php
index 1bb07010a274157903180895c80aa8c2d6e8bfbb..de9fa20dde01e9b54798d83299e0504cd1ee3220 100644 (file)
--- a/apps/files/controller/apicontroller.php
+++ b/apps/files/controller/apicontroller.php
@@ -1,6 +1,6 @@
 <?php
 /**
- * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
+ * Copyright (c) 2014-2015 Lukas Reschke <lukas@owncloud.com>
  * This file is licensed under the Affero General Public License version 3 or
  * later.
  * See the COPYING-README file.
@@ -17,19 +17,27 @@ use OCP\AppFramework\Http\DownloadResponse;
 use OC\Preview;
 use OCA\Files\Service\TagService;
 
+/**
+ * Class ApiController
+ *
+ * @package OCA\Files\Controller
+ */
 class ApiController extends Controller {
+       /** @var TagService */
+       private $tagService;
 
        /**
-        * @var TagService $tagService
+        * @param string $appName
+        * @param IRequest $request
+        * @param TagService $tagService
         */
-       private $tagService;
-
-       public function __construct($appName, IRequest $request, TagService $tagService){
+       public function __construct($appName,
+                                                               IRequest $request,
+                                                               TagService $tagService){
                parent::__construct($appName, $request);
                $this->tagService = $tagService;
        }
 
-
        /**
         * Gets a thumbnail of the specified file
         *
@@ -66,25 +74,31 @@ class ApiController extends Controller {
         * @CORS
         *
         * @param string $path path
-        * @param array  $tags array of tags
+        * @param array|string $tags array of tags
         * @return DataResponse
         */
        public function updateFileTags($path, $tags = null) {
-               $result = array();
+               $result = [];
                // if tags specified or empty array, update tags
                if (!is_null($tags)) {
                        try {
                                $this->tagService->updateFileTags($path, $tags);
                        } catch (\OCP\Files\NotFoundException $e) {
-                               return new DataResponse(['message' => $e->getMessage()], Http::STATUS_NOT_FOUND);
+                               return new DataResponse([
+                                       'message' => $e->getMessage()
+                               ], Http::STATUS_NOT_FOUND);
                        } catch (\OCP\Files\StorageNotAvailableException $e) {
-                               return new DataResponse(['message' => $e->getMessage()], Http::STATUS_SERVICE_UNAVAILABLE);
+                               return new DataResponse([
+                                       'message' => $e->getMessage()
+                               ], Http::STATUS_SERVICE_UNAVAILABLE);
                        } catch (\Exception $e) {
-                               return new DataResponse(['message' => $e->getMessage()], Http::STATUS_NOT_FOUND);
+                               return new DataResponse([
+                                       'message' => $e->getMessage()
+                               ], Http::STATUS_NOT_FOUND);
                        }
                        $result['tags'] = $tags;
                }
-               return new DataResponse($result, Http::STATUS_OK);
+               return new DataResponse($result);
        }
 
        /**
@@ -93,7 +107,7 @@ class ApiController extends Controller {
         * @NoAdminRequired
         * @CORS
         *
-        * @param array $tagName tag name to filter by
+        * @param array|string $tagName tag name to filter by
         * @return DataResponse
         */
        public function getFilesByTag($tagName) {
@@ -102,11 +116,15 @@ class ApiController extends Controller {
                foreach ($fileInfos as &$fileInfo) {
                        $file = \OCA\Files\Helper::formatFileInfo($fileInfo);
                        $parts = explode('/', dirname($fileInfo->getPath()), 4);
-                       $file['path'] = '/' . $parts[3];
-                       $file['tags'] = array($tagName);
+                       if(isset($parts[3])) {
+                               $file['path'] = '/' . $parts[3];
+                       } else {
+                               $file['path'] = '/';
+                       }
+                       $file['tags'] = [$tagName];
                        $files[] = $file;
                }
-               return new DataResponse(array('files' => $files), Http::STATUS_OK);
+               return new DataResponse(['files' => $files]);
        }
 
 }

diff --git a/apps/files/tests/controller/apicontrollertest.php b/apps/files/tests/controller/apicontrollertest.php
new file mode 100644 (file)
index 0000000..1be7a74
--- /dev/null
+++ b/apps/files/tests/controller/apicontrollertest.php
@@ -0,0 +1,243 @@
+<?php
+/**
+ * Copyright (c) 2015 Lukas Reschke <lukas@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OCA\Files\Controller;
+
+use OC\Files\FileInfo;
+use OCP\AppFramework\Http;
+use OC\Preview;
+use OCP\Files\NotFoundException;
+use OCP\Files\StorageNotAvailableException;
+use Test\TestCase;
+use OCP\IRequest;
+use OCA\Files\Service\TagService;
+use OCP\AppFramework\Http\DataResponse;
+
+/**
+ * Class ApiController
+ *
+ * @package OCA\Files\Controller
+ */
+class ApiControllerTest extends TestCase {
+       /** @var string */
+       private $appName = 'files';
+       /** @var IRequest */
+       private $request;
+       /** @var TagService */
+       private $tagService;
+       /** @var ApiController */
+       private $apiController;
+
+       public function setUp() {
+               $this->request = $this->getMockBuilder('\OCP\IRequest')
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $this->tagService = $this->getMockBuilder('\OCA\Files\Service\TagService')
+                       ->disableOriginalConstructor()
+                       ->getMock();
+
+               $this->apiController = new ApiController(
+                       $this->appName,
+                       $this->request,
+                       $this->tagService
+               );
+       }
+
+       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',
+                               'size' => 1234,
+                               'etag' => 'MyEtag',
+                       ],
+                       $this->getMockBuilder('\OCP\Files\Mount\IMountPoint')
+                               ->disableOriginalConstructor()
+                               ->getMock()
+               );
+               $this->tagService->expects($this->once())
+                       ->method('getFilesByTag')
+                       ->with($this->equalTo([$tagName]))
+                       ->will($this->returnValue([$fileInfo]));
+
+               $expected = new DataResponse([
+                       'files' => [
+                               [
+                                       'id' => null,
+                                       'parentId' => null,
+                                       'date' => 'January 1, 1970 at 12:00:55 AM GMT+0',
+                                       'mtime' => 55000,
+                                       'icon' => \OCA\Files\Helper::determineIcon($fileInfo),
+                                       'name' => 'root.txt',
+                                       'permissions' => null,
+                                       'mimetype' => 'application/pdf',
+                                       'size' => 1234,
+                                       'type' => 'file',
+                                       'etag' => 'MyEtag',
+                                       'path' => '/',
+                                       'tags' => [
+                                               [
+                                                       'MyTagName'
+                                               ]
+                                       ],
+                               ],
+                       ],
+               ]);
+               $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',
+                               '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',
+                               'size' => 9876,
+                               'etag' => 'SubEtag',
+                       ],
+                       $this->getMockBuilder('\OCP\Files\Mount\IMountPoint')
+                               ->disableOriginalConstructor()
+                               ->getMock()
+               );
+               $this->tagService->expects($this->once())
+                       ->method('getFilesByTag')
+                       ->with($this->equalTo([$tagName]))
+                       ->will($this->returnValue([$fileInfo1, $fileInfo2]));
+
+               $expected = new DataResponse([
+                       'files' => [
+                               [
+                                       'id' => null,
+                                       'parentId' => null,
+                                       'date' => 'January 1, 1970 at 12:00:55 AM GMT+0',
+                                       'mtime' => 55000,
+                                       'icon' => \OCA\Files\Helper::determineIcon($fileInfo1),
+                                       'name' => 'root.txt',
+                                       'permissions' => null,
+                                       'mimetype' => 'application/pdf',
+                                       'size' => 1234,
+                                       'type' => 'file',
+                                       'etag' => 'MyEtag',
+                                       'path' => '/',
+                                       'tags' => [
+                                               [
+                                                       'MyTagName'
+                                               ]
+                                       ],
+                               ],
+                               [
+                                       'id' => null,
+                                       'parentId' => null,
+                                       'date' => 'January 1, 1970 at 12:16:39 AM GMT+0',
+                                       'mtime' => 999000,
+                                       'icon' => \OCA\Files\Helper::determineIcon($fileInfo2),
+                                       'name' => 'root.txt',
+                                       'permissions' => null,
+                                       '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'));
+       }
+
+       public function testUpdateFileTagsWorking() {
+               $this->tagService->expects($this->once())
+                       ->method('updateFileTags')
+                       ->with('/path.txt', ['Tag1', 'Tag2']);
+
+               $expected = new DataResponse([
+                       'tags' => [
+                               'Tag1',
+                               'Tag2'
+                       ],
+               ]);
+               $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2']));
+       }
+
+       public function testUpdateFileTagsNotFoundException() {
+               $this->tagService->expects($this->once())
+                       ->method('updateFileTags')
+                       ->with('/path.txt', ['Tag1', 'Tag2'])
+                       ->will($this->throwException(new NotFoundException('My error message')));
+
+               $expected = new DataResponse(['message' => 'My error message'], Http::STATUS_NOT_FOUND);
+               $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2']));
+       }
+
+       public function testUpdateFileTagsStorageNotAvailableException() {
+               $this->tagService->expects($this->once())
+                       ->method('updateFileTags')
+                       ->with('/path.txt', ['Tag1', 'Tag2'])
+                       ->will($this->throwException(new StorageNotAvailableException('My error message')));
+
+               $expected = new DataResponse(['message' => 'My error message'], Http::STATUS_SERVICE_UNAVAILABLE);
+               $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2']));
+       }
+
+       public function testUpdateFileTagsStorageGenericException() {
+               $this->tagService->expects($this->once())
+                       ->method('updateFileTags')
+                       ->with('/path.txt', ['Tag1', 'Tag2'])
+                       ->will($this->throwException(new \Exception('My error message')));
+
+               $expected = new DataResponse(['message' => 'My error message'], Http::STATUS_NOT_FOUND);
+               $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2']));
+       }
+}