From c005da2e6df57ef8958ec90b73176ba77a057de3 Mon Sep 17 00:00:00 2001 From: Marcin Kurczewski Date: Tue, 6 May 2014 15:49:02 +0200 Subject: [PATCH] Refactored post content edit jobs; added unit test --- data/config.ini | 2 +- src/Api/Jobs/EditPostContentJob.php | 15 +- src/Api/Jobs/EditPostJob.php | 1 - src/Api/Jobs/EditPostUrlJob.php | 29 --- src/Controllers/PostController.php | 10 +- src/Helpers/TransferHelper.php | 24 ++- src/Models/Entities/PostEntity.php | 14 +- src/Models/Enums/Privilege.php | 2 +- src/Views/post-edit.phtml | 2 +- tests/JobTests/EditPostContentJobTest.php | 225 ++++++++++++++++++++++ tests/TestFiles/image.gif | Bin 0 -> 426 bytes tests/TestFiles/image.jpg | Bin 0 -> 734 bytes tests/TestFiles/image.png | Bin 0 -> 317 bytes tests/TestFiles/text.txt | 1 + 14 files changed, 278 insertions(+), 47 deletions(-) delete mode 100644 src/Api/Jobs/EditPostUrlJob.php create mode 100644 tests/JobTests/EditPostContentJobTest.php create mode 100644 tests/TestFiles/image.gif create mode 100644 tests/TestFiles/image.jpg create mode 100644 tests/TestFiles/image.png create mode 100644 tests/TestFiles/text.txt diff --git a/data/config.ini b/data/config.ini index 0197164a..962b38dd 100644 --- a/data/config.ini +++ b/data/config.ini @@ -91,7 +91,7 @@ editPostThumb=moderator editPostSource=moderator editPostRelations.own=registered editPostRelations.all=moderator -editPostFile=moderator +editPostContent=moderator massTag.own=registered massTag.all=power-user hidePost=moderator diff --git a/src/Api/Jobs/EditPostContentJob.php b/src/Api/Jobs/EditPostContentJob.php index 239e0032..72c5caef 100644 --- a/src/Api/Jobs/EditPostContentJob.php +++ b/src/Api/Jobs/EditPostContentJob.php @@ -2,13 +2,22 @@ class EditPostContentJob extends AbstractPostEditJob { const POST_CONTENT = 'post-content'; + const POST_CONTENT_URL = 'post-content-url'; public function execute() { $post = $this->post; - $file = $this->getArgument(self::POST_CONTENT); - $post->setContentFromPath($file->filePath, $file->fileName); + if ($this->hasArgument(self::POST_CONTENT_URL)) + { + $url = $this->getArgument(self::POST_CONTENT_URL); + $post->setContentFromUrl($url); + } + else + { + $file = $this->getArgument(self::POST_CONTENT); + $post->setContentFromPath($file->filePath, $file->fileName); + } if (!$this->skipSaving) PostModel::save($post); @@ -23,7 +32,7 @@ class EditPostContentJob extends AbstractPostEditJob public function requiresPrivilege() { return new Privilege( - Privilege::EditPostFile, + Privilege::EditPostContent, Access::getIdentity($this->post->getUploader())); } } diff --git a/src/Api/Jobs/EditPostJob.php b/src/Api/Jobs/EditPostJob.php index 59dd938b..a832e48b 100644 --- a/src/Api/Jobs/EditPostJob.php +++ b/src/Api/Jobs/EditPostJob.php @@ -14,7 +14,6 @@ class EditPostJob extends AbstractPostEditJob new EditPostSourceJob(), new EditPostRelationsJob(), new EditPostContentJob(), - new EditPostUrlJob(), new EditPostThumbJob(), ]; diff --git a/src/Api/Jobs/EditPostUrlJob.php b/src/Api/Jobs/EditPostUrlJob.php deleted file mode 100644 index cd2869f8..00000000 --- a/src/Api/Jobs/EditPostUrlJob.php +++ /dev/null @@ -1,29 +0,0 @@ -post; - $url = $this->getArgument(self::POST_CONTENT_URL); - - $post->setContentFromUrl($url); - - if (!$this->skipSaving) - PostModel::save($post); - - Logger::log('{user} changed contents of {post}', [ - 'user' => TextHelper::reprUser(Auth::getCurrentUser()), - 'post' => TextHelper::reprPost($post)]); - - return $post; - } - - public function requiresPrivilege() - { - return new Privilege( - Privilege::EditPostFile, - Access::getIdentity($this->post->getUploader())); - } -} diff --git a/src/Controllers/PostController.php b/src/Controllers/PostController.php index 513fcb2f..12dc954a 100644 --- a/src/Controllers/PostController.php +++ b/src/Controllers/PostController.php @@ -96,7 +96,7 @@ class PostController if (!empty(InputHelper::get('url'))) { - $jobArgs[EditPostUrlJob::POST_CONTENT_URL] = InputHelper::get('url'); + $jobArgs[EditPostContentJob::POST_CONTENT_URL] = InputHelper::get('url'); } elseif (!empty($_FILES['file']['name'])) { @@ -106,6 +106,8 @@ class PostController $jobArgs[EditPostContentJob::POST_CONTENT] = new ApiFileInput( $file['tmp_name'], $file['name']); + + TransferHelper::remove($file['tmp_name']); } Api::run(new AddPostJob(), $jobArgs); @@ -138,7 +140,7 @@ class PostController if (!empty(InputHelper::get('url'))) { - $jobArgs[EditPostUrlJob::POST_CONTENT_URL] = InputHelper::get('url'); + $jobArgs[EditPostContentJob::POST_CONTENT_URL] = InputHelper::get('url'); } elseif (!empty($_FILES['file']['name'])) { @@ -148,6 +150,8 @@ class PostController $jobArgs[EditPostContentJob::POST_CONTENT] = new ApiFileInput( $file['tmp_name'], $file['name']); + + TransferHelper::remove($file['tmp_name']); } if (!empty($_FILES['thumb']['name'])) @@ -158,6 +162,8 @@ class PostController $jobArgs[EditPostThumbJob::THUMB_CONTENT] = new ApiFileInput( $file['tmp_name'], $file['name']); + + TransferHelper::remove($file['tmp_name']); } Api::run(new EditPostJob(), $jobArgs); diff --git a/src/Helpers/TransferHelper.php b/src/Helpers/TransferHelper.php index 55c6d8d0..5e561225 100644 --- a/src/Helpers/TransferHelper.php +++ b/src/Helpers/TransferHelper.php @@ -1,8 +1,17 @@ getThumbCustomPath(); - TransferHelper::moveUpload($srcPath, $dstPath); + TransferHelper::copy($srcPath, $dstPath); } public function generateThumb($width = null, $height = null) @@ -332,7 +332,7 @@ class PostEntity extends AbstractEntity implements IValidatable $dstPath = $this->getFullPath(); - TransferHelper::moveUpload($srcPath, $dstPath); + TransferHelper::copy($srcPath, $dstPath); $thumbPath = $this->getThumbDefaultPath(); if (file_exists($thumbPath)) @@ -370,20 +370,20 @@ class PostEntity extends AbstractEntity implements IValidatable return; } - $srcPath = tempnam(sys_get_temp_dir(), 'upload') . '.dat'; + $tmpPath = tempnam(sys_get_temp_dir(), 'upload') . '.dat'; try { $maxBytes = TextHelper::stripBytesUnits(ini_get('upload_max_filesize')); - TransferHelper::download($srcUrl, $srcPath, $maxBytes); + TransferHelper::download($srcUrl, $tmpPath, $maxBytes); - $this->setContentFromPath($srcPath, basename($srcUrl)); + $this->setContentFromPath($tmpPath, basename($srcUrl)); } finally { - if (file_exists($srcPath)) - unlink($srcPath); + if (file_exists($tmpPath)) + unlink($tmpPath); } } diff --git a/src/Models/Enums/Privilege.php b/src/Models/Enums/Privilege.php index 4b269d34..f8840c88 100644 --- a/src/Models/Enums/Privilege.php +++ b/src/Models/Enums/Privilege.php @@ -11,7 +11,7 @@ class Privilege extends Enum const EditPostThumb = 8; const EditPostSource = 26; const EditPostRelations = 30; - const EditPostFile = 36; + const EditPostContent = 36; const HidePost = 9; const DeletePost = 10; const FeaturePost = 25; diff --git a/src/Views/post-edit.phtml b/src/Views/post-edit.phtml index b5a0ebe8..6bbab574 100644 --- a/src/Views/post-edit.phtml +++ b/src/Views/post-edit.phtml @@ -86,7 +86,7 @@ context->transport->post->getUploader())))): ?>
diff --git a/tests/JobTests/EditPostContentJobTest.php b/tests/JobTests/EditPostContentJobTest.php new file mode 100644 index 00000000..88903a21 --- /dev/null +++ b/tests/JobTests/EditPostContentJobTest.php @@ -0,0 +1,225 @@ +prepare(); + $this->grantAccess('editPostContent'); + $post = $this->uploadFromFile('image.jpg'); + $this->assert->doesNotThrow(function() use ($post) + { + PostModel::findById($post->getId()); + }); + } + + public function testFileJpeg() + { + $this->prepare(); + $this->grantAccess('editPostContent'); + $post = $this->uploadFromFile('image.jpg'); + $this->assert->areEqual('image/jpeg', $post->mimeType); + $this->assert->areEqual(PostType::Image, $post->getType()->toInteger()); + $this->assert->areEqual(320, $post->imageWidth); + $this->assert->areEqual(240, $post->imageHeight); + $this->assert->doesNotThrow(function() use ($post) + { + $post->generateThumb(); + }); + } + + public function testFilePng() + { + $this->prepare(); + $this->grantAccess('editPostContent'); + $post = $this->uploadFromFile('image.png'); + $this->assert->areEqual('image/png', $post->mimeType); + $this->assert->areEqual(PostType::Image, $post->getType()->toInteger()); + $this->assert->areEqual(320, $post->imageWidth); + $this->assert->areEqual(240, $post->imageHeight); + $this->assert->doesNotThrow(function() use ($post) + { + $post->generateThumb(); + }); + } + + public function testFileGif() + { + $this->prepare(); + $this->grantAccess('editPostContent'); + $post = $this->uploadFromFile('image.gif'); + $this->assert->areEqual('image/gif', $post->mimeType); + $this->assert->areEqual(PostType::Image, $post->getType()->toInteger()); + $this->assert->areEqual(320, $post->imageWidth); + $this->assert->areEqual(240, $post->imageHeight); + $this->assert->doesNotThrow(function() use ($post) + { + $post->generateThumb(); + }); + } + + public function testFileInvalid() + { + $this->prepare(); + $this->grantAccess('editPostContent'); + $this->assert->throws(function() + { + $this->uploadFromFile('text.txt'); + }, 'Invalid file type'); + } + + public function testUrl() + { + $this->prepare(); + $this->grantAccess('editPostContent'); + $post = $this->uploadFromUrl('image.jpg'); + $this->assert->doesNotThrow(function() use ($post) + { + PostModel::findById($post->getId()); + }); + } + + public function testUrlYoutube() + { + $this->prepare(); + $this->grantAccess('editPostContent'); + + $post = $this->mockPost(Auth::getCurrentUser()); + $post = Api::run( + new EditPostContentJob(), + [ + EditPostContentJob::POST_ID => $post->getId(), + EditPostContentJob::POST_CONTENT_URL => 'http://www.youtube.com/watch?v=qWq_jydCUw4', 'test.jpg', + ]); + $this->assert->areEqual(PostType::Youtube, $post->getType()->toInteger()); + $this->assert->areEqual('qWq_jydCUw4', $post->fileHash); + $this->assert->doesNotThrow(function() use ($post) + { + $post->generateThumb(); + }); + + $this->assert->doesNotThrow(function() use ($post) + { + PostModel::findById($post->getId()); + }); + } + + public function testNoAuth() + { + $this->prepare(); + $this->grantAccess('editPostContent'); + Auth::setCurrentUser(null); + + $this->assert->doesNotThrow(function() + { + $this->uploadFromFile('image.jpg'); + }); + } + + public function testOwnAccessDenial() + { + $this->prepare(); + + $this->assert->throws(function() + { + $this->uploadFromFile('image.jpg'); + }, 'Insufficient privileges'); + } + + public function testOtherAccessGrant() + { + $this->prepare(); + $this->grantAccess('editPostContent.all'); + + $post = $this->mockPost(Auth::getCurrentUser()); + + //login as someone else + $this->login($this->mockUser()); + + $this->assert->doesNotThrow(function() use ($post) + { + $this->uploadFromFile('image.jpg', $post); + }); + } + + public function testOtherAccessDenial() + { + $this->prepare(); + $this->grantAccess('editPostContent.own'); + + $post = $this->mockPost(Auth::getCurrentUser()); + + //login as someone else + $this->login($this->mockUser()); + + $this->assert->throws(function() use ($post) + { + $this->uploadFromFile('image.jpg', $post); + }, 'Insufficient privileges'); + } + + + public function testWrongPostId() + { + $this->assert->throws(function() + { + Api::run( + new EditPostContentJob(), + [ + EditPostContentJob::POST_ID => 100, + EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'), + ]); + }, 'Invalid post ID'); + } + + + protected function prepare() + { + $this->login($this->mockUser()); + } + + protected function uploadFromUrl($fileName, $post = null) + { + if ($post === null) + $post = $this->mockPost(Auth::getCurrentUser()); + + $url = 'http://example.com/mock_' . $fileName; + TransferHelper::mockForDownload($url, $this->getPath($fileName)); + + $post = Api::run( + new EditPostContentJob(), + [ + EditPostContentJob::POST_ID => $post->getId(), + EditPostContentJob::POST_CONTENT_URL => $url, + ]); + + $this->assert->areEqual( + file_get_contents($this->getPath($fileName)), + file_get_contents(getConfig()->main->filesPath . DS . $post->getName())); + + return $post; + } + + protected function uploadFromFile($fileName, $post = null) + { + if ($post === null) + $post = $this->mockPost(Auth::getCurrentUser()); + + $post = Api::run( + new EditPostContentJob(), + [ + EditPostContentJob::POST_ID => $post->getId(), + EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath($fileName), 'test.jpg'), + ]); + + $this->assert->areEqual( + file_get_contents($this->getPath($fileName)), + file_get_contents(getConfig()->main->filesPath . DS . $post->getName())); + + return $post; + } + + protected function getPath($name) + { + return getConfig()->rootDir . DS . 'tests' . DS . 'TestFiles' . DS . $name; + } +} diff --git a/tests/TestFiles/image.gif b/tests/TestFiles/image.gif new file mode 100644 index 0000000000000000000000000000000000000000..ca3bfe7e450012307f8ee95f7d22e6d4e58f270f GIT binary patch literal 426 zcmV;b0agA-Nk%w1VL$=!0Pp|+|Ns90001HR1ONa4001li0000$0q_6-0{(=LsmtvT zqnxzbi?iOm`wxcVNS5Y_rs~SJ?hD8AOxN~}=lag~{tpZahs2`sh)gP%%%<}RjY_A~ zs`ZM^YPa03_X`e-$K-YS={|^`_I7nD%c!-#&xX9S( z_y`#(IZ0V*d5M{+xyjk-`3V{-I!an`U)E>J4;(@dyAW^yUW|_`wJW_ zJWO0{e2ko|yv*F}{0to}JxyJ0eT|*1z0KY2{S6*2K2Bb4evY25zRuq6{th26KTlt8 ze~+K9zt7+A{|_*rz<~q{8a#+Fp~8g>8#;UlF`~qY6f0W1h%uwaf{h$Idi)47q{xvZ zOPV~1GNsCuEL*yK2{We5nKWzKyoocX&Ye7a`uqtrsL-KAiyA$OG^x_1Oq)7=3N@kh3pcLZxpeE=y^A-m-o1SL`uz(y Uu;9Uj3mZO+II-fzs0si8J7^l(lmGw# literal 0 HcmV?d00001 diff --git a/tests/TestFiles/image.jpg b/tests/TestFiles/image.jpg new file mode 100644 index 0000000000000000000000000000000000000000..028c68944f7541a1c85d0042587eb9f7133ce667 GIT binary patch literal 734 zcmeH@%MHRX42FM`QpHKyIxQ@~B1J-~fGL=dDL60$j|(HPL2$U0o;V`$MY57lend`} z^nmP#-2fuORu9r08<<%pN23&Fk+EetFWEZBmf3|SIA43_DpqwP62*nKYo*h>^#ugiAJhN< literal 0 HcmV?d00001 diff --git a/tests/TestFiles/image.png b/tests/TestFiles/image.png new file mode 100644 index 0000000000000000000000000000000000000000..8ea1e0dcd00f886b7ec11c5fddab204f7e509238 GIT binary patch literal 317 zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKQICbhRCpnV?c@}-O<;Pfnj4m_n$;oAfK@~ z$lZxy-8q?;K#oGPN02WALzNl>LqiJ#!!Mvv!wUw6QUeBtR|yOZRx=nF#0%!^3bX-A zFeQ1ryD%``?Gj!B2>S4={E+nQaGTn0dN5hGg7(d&&^#0v;xX-_x&g{bo0R z8R!xy;)(+uJzv;r9J6MNS;#%0392Qo5hW>!C8<`)MX5lF!N|bSMAyJn*T5{q(A3Jv s#LCn}+rY@mz@X<)<7N~Mx%nxXX_dG&{GD&f57fZm>FVdQ&MBb@07FPpaR2}S literal 0 HcmV?d00001 diff --git a/tests/TestFiles/text.txt b/tests/TestFiles/text.txt new file mode 100644 index 00000000..84102df4 --- /dev/null +++ b/tests/TestFiles/text.txt @@ -0,0 +1 @@ +The quick brown fox jumps over the lazy dog