Refactored post content edit jobs; added unit test

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

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);
 
+		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()));
 	}
 }

src/Api/Jobs/EditPostJob.php

@@ -14,7 +14,6 @@ class EditPostJob extends AbstractPostEditJob
 			new EditPostSourceJob(),
 			new EditPostRelationsJob(),
 			new EditPostContentJob(),
-			new EditPostUrlJob(),
 			new EditPostThumbJob(),
 		];
 

src/Api/Jobs/EditPostUrlJob.php

@@ -1,29 +0,0 @@
-<?php
-class EditPostUrlJob extends AbstractPostEditJob
-{
-	const POST_CONTENT_URL = 'post-content-url';
-
-	public function execute()
-	{
-		$post = $this->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()));
-	}
-}

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);

src/Helpers/TransferHelper.php

@@ -1,8 +1,17 @@
 <?php
 class TransferHelper
 {
+	protected static $mocks = [];
+
 	public static function download($srcUrl, $dstPath, $maxBytes = null)
 	{
+		if (isset(self::$mocks[$srcUrl]))
+		{
+			self::copy(self::$mocks[$srcUrl], $dstPath);
+			chmod($dstPath, 0644);
+			return;
+		}
+
 		set_time_limit(0);
 		$srcHandle = fopen($srcUrl, 'rb');
 		if (!$srcHandle)
@@ -42,6 +51,11 @@ class TransferHelper
 		}
 	}
 
+	public static function mockForDownload($url, $sourceFile)
+	{
+		self::$mocks[$url] = $sourceFile;
+	}
+
 	public static function moveUpload($srcPath, $dstPath)
 	{
 		if ($srcPath == $dstPath)
@@ -55,8 +69,8 @@ class TransferHelper
 		{
 			//problems with permissions on some systems?
 			#rename($srcPath, $dstPath);
-			copy($srcPath, $dstPath);
+			self::copy($srcPath, $dstPath);
-			unlink($srcPath);
+			self::remove($srcPath);
 		}
 	}
 
@@ -68,6 +82,12 @@ class TransferHelper
 		copy($srcPath, $dstPath);
 	}
 
+	public static function remove($srcPath)
+	{
+		if (file_exists($srcPath))
+			unlink($srcPath);
+	}
+
 	public static function createDirectory($dirPath)
 	{
 		if (file_exists($dirPath))

src/Models/Entities/PostEntity.php

@@ -258,7 +258,7 @@ class PostEntity extends AbstractEntity implements IValidatable
 
 		$dstPath = $this->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))
+			if (file_exists($tmpPath))
-				unlink($srcPath);
+				unlink($tmpPath);
 		}
 	}
 

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;

src/Views/post-edit.phtml

@@ -86,7 +86,7 @@
 	<?php endif ?>
 
 	<?php if (Access::check(new Privilege(
-		Privilege::EditPostFile,
+		Privilege::EditPostContent,
 		Access::getIdentity($this->context->transport->post->getUploader())))): ?>
 
 		<div class="form-row url">

tests/JobTests/EditPostContentJobTest.php

@@ -0,0 +1,225 @@
+<?php
+class EditPostContentJobTest extends AbstractTest
+{
+	public function testFile()
+	{
+		$this->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;
+	}
+}

tests/TestFiles/text.txt

@@ -0,0 +1 @@
+The quick brown fox jumps over the lazy dog