From 07c44d2113ee2fe67d6bdaf0018b321b1a6ebc9f Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Mon, 19 Dec 2016 22:27:13 -0500 Subject: Avoid code duplication in PR #2891 --- ChangeLog | 1 + app/Controller/FileViewerController.php | 60 +++++++++++++++------------------ app/Helper/FileHelper.php | 20 ++++------- app/Model/FileModel.php | 4 +-- app/Template/task_file/files.php | 5 +++ app/functions.php | 11 ++++++ tests/units/Helper/FileHelperTest.php | 43 +++++++++++++++++++++++ tests/units/Helper/FileHelperText.php | 36 -------------------- 8 files changed, 95 insertions(+), 85 deletions(-) create mode 100644 tests/units/Helper/FileHelperTest.php delete mode 100644 tests/units/Helper/FileHelperText.php diff --git a/ChangeLog b/ChangeLog index 1a42aab8..628a183c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -8,6 +8,7 @@ New features: Improvements: +* Open PDF attachments in browser tab (preview) * Handle username with dots in user mentions * Replace Chosen jQuery plugin by custom UI component * Rewrite UI component that change user/group roles diff --git a/app/Controller/FileViewerController.php b/app/Controller/FileViewerController.php index 3e0182ba..49568912 100644 --- a/app/Controller/FileViewerController.php +++ b/app/Controller/FileViewerController.php @@ -15,11 +15,11 @@ class FileViewerController extends BaseController /** * Get file content from object storage * - * @access private + * @access protected * @param array $file * @return string */ - private function getFileContent(array $file) + protected function getFileContent(array $file) { $content = ''; @@ -34,6 +34,30 @@ class FileViewerController extends BaseController return $content; } + /** + * Output file with cache + * + * @param array $file + * @param $mimetype + */ + protected function renderFileWithCache(array $file, $mimetype) + { + $etag = md5($file['path']); + + if ($this->request->getHeader('If-None-Match') === '"'.$etag.'"') { + $this->response->status(304); + } else { + try { + $this->response->withContentType($mimetype); + $this->response->withCache(5 * 86400, $etag); + $this->response->send(); + $this->objectStorage->output($file['path']); + } catch (ObjectStorageException $e) { + $this->logger->error($e->getMessage()); + } + } + } + /** * Show file content in a popover * @@ -65,21 +89,7 @@ class FileViewerController extends BaseController public function image() { $file = $this->getFile(); - $etag = md5($file['path']); - $this->response->withContentType($this->helper->file->getImageMimeType($file['name'])); - $this->response->withCache(5 * 86400, $etag); - - if ($this->request->getHeader('If-None-Match') === '"'.$etag.'"') { - $this->response->status(304); - } else { - - try { - $this->response->send(); - $this->objectStorage->output($file['path']); - } catch (ObjectStorageException $e) { - $this->logger->error($e->getMessage()); - } - } + $this->renderFileWithCache($file, $this->helper->file->getImageMimeType($file['name'])); } /** @@ -90,21 +100,7 @@ class FileViewerController extends BaseController public function browser() { $file = $this->getFile(); - $etag = md5($file['path']); - $this->response->withContentType($this->helper->file->getBrowserViewType($file['name'])); - $this->response->withCache(5 * 86400, $etag); - - if ($this->request->getHeader('If-None-Match') === '"'.$etag.'"') { - $this->response->status(304); - } else { - - try { - $this->response->send(); - $this->objectStorage->output($file['path']); - } catch (ObjectStorageException $e) { - $this->logger->error($e->getMessage()); - } - } + $this->renderFileWithCache($file, $this->helper->file->getBrowserViewType($file['name'])); } /** diff --git a/app/Helper/FileHelper.php b/app/Helper/FileHelper.php index 82b444d0..06589124 100644 --- a/app/Helper/FileHelper.php +++ b/app/Helper/FileHelper.php @@ -21,9 +21,7 @@ class FileHelper extends Base */ public function icon($filename) { - $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION)); - - switch ($extension) { + switch (get_file_extension($filename)) { case 'jpeg': case 'jpg': case 'png': @@ -70,9 +68,7 @@ class FileHelper extends Base */ public function getImageMimeType($filename) { - $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION)); - - switch ($extension) { + switch (get_file_extension($filename)) { case 'jpeg': case 'jpg': return 'image/jpeg'; @@ -94,9 +90,7 @@ class FileHelper extends Base */ public function getPreviewType($filename) { - $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION)); - - switch ($extension) { + switch (get_file_extension($filename)) { case 'md': case 'markdown': return 'markdown'; @@ -108,17 +102,15 @@ class FileHelper extends Base } /** - * Return the browser view mimetype based on the file extension. + * Return the browser view mime-type based on the file extension. * + * @access public * @param $filename - * * @return string */ public function getBrowserViewType($filename) { - $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION)); - - switch ($extension) { + switch (get_file_extension($filename)) { case 'pdf': return 'application/pdf'; } diff --git a/app/Model/FileModel.php b/app/Model/FileModel.php index 98032f9d..b5852b08 100644 --- a/app/Model/FileModel.php +++ b/app/Model/FileModel.php @@ -210,9 +210,7 @@ abstract class FileModel extends Base */ public function isImage($filename) { - $extension = strtolower(pathinfo($filename, PATHINFO_EXTENSION)); - - switch ($extension) { + switch (get_file_extension($filename)) { case 'jpeg': case 'jpg': case 'png': diff --git a/app/Template/task_file/files.php b/app/Template/task_file/files.php index 94c26f73..32bebdcb 100644 --- a/app/Template/task_file/files.php +++ b/app/Template/task_file/files.php @@ -18,6 +18,11 @@ url->link(t('View file'), 'FileViewerController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'file_id' => $file['id']), false, 'popover') ?> + file->getBrowserViewType($file['name']) !== null): ?> +
  • + + url->link(t('View file'), 'FileViewerController', 'browser', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'file_id' => $file['id']), false, '', '', true) ?> +
  • diff --git a/app/functions.php b/app/functions.php index 9dd054fb..7cd59c41 100644 --- a/app/functions.php +++ b/app/functions.php @@ -145,6 +145,17 @@ function get_upload_max_size() return min(ini_get('upload_max_filesize'), ini_get('post_max_size')); } +/** + * Get file extension + * + * @param $filename + * @return string + */ +function get_file_extension($filename) +{ + return strtolower(pathinfo($filename, PATHINFO_EXTENSION)); +} + /** * Translate a string * diff --git a/tests/units/Helper/FileHelperTest.php b/tests/units/Helper/FileHelperTest.php new file mode 100644 index 00000000..7db43460 --- /dev/null +++ b/tests/units/Helper/FileHelperTest.php @@ -0,0 +1,43 @@ +container); + $this->assertEquals('fa-file-image-o', $helper->icon('test.png')); + $this->assertEquals('fa-file-o', $helper->icon('test')); + } + + public function testGetMimeType() + { + $helper = new FileHelper($this->container); + + $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File.JPG')); + $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File.jpeg')); + $this->assertEquals('image/png', $helper->getImageMimeType('My File.PNG')); + $this->assertEquals('image/gif', $helper->getImageMimeType('My File.gif')); + $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File.bmp')); + $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File')); + } + + public function testGetPreviewType() + { + $helper = new FileHelper($this->container); + $this->assertEquals('text', $helper->getPreviewType('test.txt')); + $this->assertEquals('markdown', $helper->getPreviewType('test.markdown')); + $this->assertEquals('markdown', $helper->getPreviewType('test.md')); + $this->assertEquals(null, $helper->getPreviewType('test.doc')); + } + + public function testGetBrowserViewType() + { + $fileHelper = new FileHelper($this->container); + $this->assertSame('application/pdf', $fileHelper->getBrowserViewType('SomeFile.PDF')); + $this->assertSame(null, $fileHelper->getBrowserViewType('SomeFile.doc')); + } +} diff --git a/tests/units/Helper/FileHelperText.php b/tests/units/Helper/FileHelperText.php deleted file mode 100644 index 215b024b..00000000 --- a/tests/units/Helper/FileHelperText.php +++ /dev/null @@ -1,36 +0,0 @@ -container); - $this->assertEquals('fa-file-image-o', $helper->icon('test.png')); - $this->assertEquals('fa-file-o', $helper->icon('test')); - } - - public function testGetMimeType() - { - $helper = new FileHelper($this->container); - - $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File.JPG')); - $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File.jpeg')); - $this->assertEquals('image/png', $helper->getImageMimeType('My File.PNG')); - $this->assertEquals('image/gif', $helper->getImageMimeType('My File.gif')); - $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File.bmp')); - $this->assertEquals('image/jpeg', $helper->getImageMimeType('My File')); - } - - public function testGetPreviewType() - { - $helper = new FileHelper($this->container); - $this->assertEquals('text', $helper->getPreviewType('test.txt')); - $this->assertEquals('markdown', $helper->getPreviewType('test.markdown')); - $this->assertEquals('md', $helper->getPreviewType('test.md')); - $this->assertEquals(null, $helper->getPreviewType('test.doc')); - } -} -- cgit v1.2.3