Fixed/refactored tag validation

data/config.ini

@@ -39,6 +39,11 @@ showDislikedPostsDefault=1
 maxSearchTokens=4
 maxRelatedPosts=50
 
+[tags]
+minLength = 1
+maxLength = 64
+regex = "/^[()\[\]a-zA-Z0-9_.-]+$/i"
+
 [posts]
 maxSourceLength = 200
 

src/Api/Jobs/EditPostTagsJob.php

@@ -9,10 +9,15 @@ class EditPostTagsJob extends AbstractPostJob
 	public function execute()
 	{
 		$post = $this->post;
-		$tags = $this->getArgument(self::TAG_NAMES);
+		$tagNames = $this->getArgument(self::TAG_NAMES);
+
+		if (!is_array($tagNames))
+			throw new SimpleException('Expected array');
+
+		$tags = TagModel::spawnFromNames($tagNames);
 
 		$oldTags = array_map(function($tag) { return $tag->getName(); }, $post->getTags());
-		$post->setTagsFromText($tags);
+		$post->setTags($tags);
 		$newTags = array_map(function($tag) { return $tag->getName(); }, $post->getTags());
 
 		if ($this->getContext() == self::CONTEXT_NORMAL)

src/Controllers/PostController.php

@@ -90,7 +90,7 @@ class PostController
 		[
 			AddPostJob::ANONYMOUS => InputHelper::get('anonymous'),
 			EditPostSafetyJob::SAFETY => InputHelper::get('safety'),
-			EditPostTagsJob::TAG_NAMES => InputHelper::get('tags'),
+			EditPostTagsJob::TAG_NAMES => $this->splitTags(InputHelper::get('tags')),
 			EditPostSourceJob::SOURCE => InputHelper::get('source'),
 		];
 
@@ -131,7 +131,7 @@ class PostController
 		[
 			EditPostJob::POST_ID => $id,
 			EditPostSafetyJob::SAFETY => InputHelper::get('safety'),
-			EditPostTagsJob::TAG_NAMES => InputHelper::get('tags'),
+			EditPostTagsJob::TAG_NAMES => $this->splitTags(InputHelper::get('tags')),
 			EditPostSourceJob::SOURCE => InputHelper::get('source'),
 			EditPostRelationsJob::RELATED_POST_IDS => InputHelper::get('relations'),
 		];
@@ -283,4 +283,14 @@ class PostController
 		$context->transport->lastModified = $ret->lastModified;
 		$context->layoutName = 'layout-file';
 	}
+
+
+	protected function splitTags($string)
+	{
+		$tags = trim($string);
+		$tags = preg_split('/[,;\s]+/', $tags);
+		$tags = array_filter($tags, function($x) { return $x != ''; });
+		$tags = array_unique($tags);
+		return $tags;
+	}
 }

src/Models/Entities/PostEntity.php

@@ -27,6 +27,9 @@ class PostEntity extends AbstractEntity implements IValidatable
 		if (empty($this->getType()))
 			throw new SimpleException('No post type detected');
 
+		if (empty($this->getTags()))
+			throw new SimpleException('No tags set');
+
 		$this->getType()->validate();
 
 		$this->getSafety()->validate();
@@ -163,24 +166,6 @@ class PostEntity extends AbstractEntity implements IValidatable
 		$this->setCache('tags', $tags);
 	}
 
-	public function setTagsFromText($tagsText)
-	{
-		$tagNames = TagModel::validateTags($tagsText);
-		$tags = [];
-		foreach ($tagNames as $tagName)
-		{
-			$tag = TagModel::findByName($tagName, false);
-			if (!$tag)
-			{
-				$tag = TagModel::spawn();
-				$tag->setName($tagName);
-				TagModel::save($tag);
-			}
-			$tags []= $tag;
-		}
-		$this->setTags($tags);
-	}
-
 	public function isTaggedWith($tagName)
 	{
 		$tagName = trim(strtolower($tagName));

src/Models/Entities/TagEntity.php

@@ -8,12 +8,27 @@ class TagEntity extends AbstractEntity implements IValidatable
 
 	public function validate()
 	{
-		//todo
+		$minLength = getConfig()->tags->minLength;
+		$maxLength = getConfig()->tags->maxLength;
+		$regex = getConfig()->tags->regex;
+
+		$name = $this->getName();
+
+		if (strlen($name) < $minLength)
+			throw new SimpleException('Tag must have at least %d characters', $minLength);
+		if (strlen($name) > $maxLength)
+			throw new SimpleException('Tag must have at most %d characters', $maxLength);
+
+		if (!preg_match($regex, $name))
+			throw new SimpleException('Invalid tag "%s"', $name);
+
+		if (preg_match('/^\.\.?$/', $name))
+			throw new SimpleException('Invalid tag "%s"', $name);
 	}
 
 	public function setName($name)
 	{
-		$this->name = $name;
+		$this->name = trim($name);
 	}
 
 	public function getName()

src/Models/TagModel.php

@@ -161,41 +161,30 @@ class TagModel extends AbstractCrudModel
 		Database::exec($stmt);
 	}
 
-
+	public static function spawnFromNames(array $tagNames)
-
-	public static function validateTag($tag)
 	{
-		$tag = trim($tag);
+		$tags = [];
-
+		foreach ($tagNames as $tagName)
-		$minLength = 1;
+		{
-		$maxLength = 64;
+			$tag = TagModel::findByName($tagName, false);
-		if (strlen($tag) < $minLength)
+			if (!$tag)
-			throw new SimpleException('Tag must have at least %d characters', $minLength);
+			{
-		if (strlen($tag) > $maxLength)
+				$tag = TagModel::spawn();
-			throw new SimpleException('Tag must have at most %d characters', $maxLength);
+				$tag->setName($tagName);
-
+				TagModel::save($tag);
-		if (!preg_match('/^[()\[\]a-zA-Z0-9_.-]+$/i', $tag))
+			}
-			throw new SimpleException('Invalid tag "%s"', $tag);
+			$tags []= $tag;
-
+		}
-		if (preg_match('/^\.\.?$/', $tag))
+		return $tags;
-			throw new SimpleException('Invalid tag "%s"', $tag);
-
-		return $tag;
 	}
 
+
+
 	public static function validateTags($tags)
 	{
-		$tags = trim($tags);
-		$tags = preg_split('/[,;\s]+/', $tags);
-		$tags = array_filter($tags, function($x) { return $x != ''; });
-		$tags = array_unique($tags);
-
 		foreach ($tags as $key => $tag)
 			$tags[$key] = self::validateTag($tag);
 
-		if (empty($tags))
-			throw new SimpleException('No tags set');
-
 		return $tags;
 	}
 }

tests/AbstractTest.php

@@ -25,11 +25,19 @@ class AbstractTest
 		return UserModel::save($user);
 	}
 
+	protected function mockTag()
+	{
+		$tag = TagModel::spawn();
+		$tag->setName(uniqid());
+		return TagModel::save($tag);
+	}
+
 	protected function mockPost($owner)
 	{
 		$post = PostModel::spawn();
 		$post->setUploader($owner);
 		$post->setType(new PostType(PostType::Image));
+		$post->setTags([$this->mockTag(), $this->mockTag()]);
 		return PostModel::save($post);
 	}
 

tests/JobTests/AddPostJobTest.php

@@ -21,6 +21,7 @@ class AddPostJobTest extends AbstractTest
 					EditPostSafetyJob::SAFETY => PostSafety::Safe,
 					EditPostSourceJob::SOURCE => '',
 					EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'),
+					EditPostTagsJob::TAG_NAMES => ['kamen', 'raider'],
 				]);
 		});
 
@@ -47,6 +48,7 @@ class AddPostJobTest extends AbstractTest
 				[
 					AddPostJob::ANONYMOUS => true,
 					EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'),
+					EditPostTagsJob::TAG_NAMES => ['kamen', 'raider'],
 				]);
 		});
 
@@ -57,7 +59,7 @@ class AddPostJobTest extends AbstractTest
 		$this->assert->areEqual(null, $post->getUploaderId());
 	}
 
-	public function testPrivilegeFail()
+	public function testPartialPrivilegeFail()
 	{
 		$this->prepare();
 
@@ -66,19 +68,75 @@ class AddPostJobTest extends AbstractTest
 		$this->grantAccess('addPostTags');
 		$this->grantAccess('addPostContent');
 
-		$args =
+		$this->assert->throws(function()
-		[
-			EditPostSafetyJob::SAFETY => PostSafety::Safe,
-			EditPostSourceJob::SOURCE => '',
-			EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'),
-		];
-
-		$this->assert->throws(function() use ($args)
 		{
-			Api::run(new AddPostJob(), $args);
+			Api::run(
+				new AddPostJob(),
+				[
+					EditPostSafetyJob::SAFETY => PostSafety::Safe,
+					EditPostSourceJob::SOURCE => '',
+					EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'),
+				]);
 		}, 'Insufficient privilege');
 	}
 
+	public function testInvalidSafety()
+	{
+		$this->prepare();
+
+		$this->grantAccess('addPost');
+		$this->grantAccess('addPostTags');
+		$this->grantAccess('addPostContent');
+		$this->grantAccess('addPostSafety');
+
+		$this->assert->throws(function()
+		{
+			Api::run(
+				new AddPostJob(),
+				[
+					EditPostSafetyJob::SAFETY => 666,
+					EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'),
+					EditPostTagsJob::TAG_NAMES => ['kamen', 'raider'],
+				]);
+		}, 'Invalid safety type');
+	}
+
+	public function testNoContentFail()
+	{
+		$this->prepare();
+
+		$this->grantAccess('addPost');
+		$this->grantAccess('addPostTags');
+		$this->grantAccess('addPostContent');
+
+		$this->assert->throws(function()
+		{
+			Api::run(
+				new AddPostJob(),
+				[
+					EditPostTagsJob::TAG_NAMES => ['kamen', 'raider'],
+				]);
+		}, 'No post type detected');
+	}
+
+	public function testEmptyTagsFail()
+	{
+		$this->prepare();
+
+		$this->grantAccess('addPost');
+		$this->grantAccess('addPostTags');
+		$this->grantAccess('addPostContent');
+
+		$this->assert->throws(function()
+		{
+			Api::run(
+				new AddPostJob(),
+				[
+					EditPostContentJob::POST_CONTENT => new ApiFileInput($this->getPath('image.jpg'), 'test.jpg'),
+				]);
+		}, 'No tags set');
+	}
+
 	public function testLogBuffering()
 	{
 		$this->testSaving();

tests/JobTests/EditPostTagsJobTest.php

@@ -0,0 +1,113 @@
+<?php
+class EditPostTagsJobTest extends AbstractTest
+{
+	public function testEditing()
+	{
+		$post = $this->mockPost($this->mockUser());
+		$this->grantAccess('editPostTags');
+
+		$newTagNames = ['big', 'boss'];
+		$post = $this->assert->doesNotThrow(function() use ($post, $newTagNames)
+		{
+			return Api::run(
+				new EditPostTagsJob(),
+				[
+					EditPostTagsJob::POST_ID => $post->getId(),
+					EditPostTagsJob::TAG_NAMES => $newTagNames,
+				]);
+		});
+
+		$receivedTagNames = array_map(function($tag)
+		{
+			return $tag->getName();
+		}, $post->getTags());
+
+		natcasesort($receivedTagNames);
+		natcasesort($newTagNames);
+
+		$this->assert->areEquivalent($newTagNames, $receivedTagNames);
+	}
+
+	public function testFailOnEmptyTags()
+	{
+		$post = $this->mockPost($this->mockUser());
+		$this->grantAccess('editPostTags');
+
+		$this->assert->throws(function() use ($post)
+		{
+			Api::run(
+				new EditPostTagsJob(),
+				[
+					EditPostTagsJob::POST_ID => $post->getId(),
+					EditPostTagsJob::TAG_NAMES => [],
+				]);
+		}, 'No tags set');
+	}
+
+	public function testTooShortTag()
+	{
+		$post = $this->mockPost($this->mockUser());
+		$this->grantAccess('editPostTags');
+
+		$newTagNames = [str_repeat('u', getConfig()->tags->minLength - 1)];
+		$this->assert->throws(function() use ($post, $newTagNames)
+		{
+			Api::run(
+				new EditPostTagsJob(),
+				[
+					EditPostTagsJob::POST_ID => $post->getId(),
+					EditPostTagsJob::TAG_NAMES => $newTagNames,
+				]);
+		}, 'Tag must have at least');
+	}
+
+	public function testTooLongTag()
+	{
+		$post = $this->mockPost($this->mockUser());
+		$this->grantAccess('editPostTags');
+
+		$newTagNames = [str_repeat('u', getConfig()->tags->maxLength + 1)];
+		$this->assert->throws(function() use ($post, $newTagNames)
+		{
+			Api::run(
+				new EditPostTagsJob(),
+				[
+					EditPostTagsJob::POST_ID => $post->getId(),
+					EditPostTagsJob::TAG_NAMES => $newTagNames,
+				]);
+		}, 'Tag must have at most');
+	}
+
+	public function testInvalidTag()
+	{
+		$post = $this->mockPost($this->mockUser());
+		$this->grantAccess('editPostTags');
+
+		$newTagNames = ['bulma/goku'];
+		$this->assert->throws(function() use ($post, $newTagNames)
+		{
+			Api::run(
+				new EditPostTagsJob(),
+				[
+					EditPostTagsJob::POST_ID => $post->getId(),
+					EditPostTagsJob::TAG_NAMES => $newTagNames,
+				]);
+		}, 'Invalid tag');
+	}
+
+	public function testInvalidPost()
+	{
+		$this->grantAccess('editPostTags');
+
+		$newTagNames = ['lisa'];
+		$this->assert->throws(function() use ($newTagNames)
+		{
+			Api::run(
+				new EditPostTagsJob(),
+				[
+					EditPostTagsJob::POST_ID => 100,
+					EditPostTagsJob::TAG_NAMES => $newTagNames,
+				]);
+		}, 'Invalid post ID');
+	}
+}

tests/config.ini

@@ -35,6 +35,11 @@ showDislikedPostsDefault=1
 maxSearchTokens=4
 maxRelatedPosts=50
 
+[tags]
+minLength = 1
+maxLength = 64
+regex = "/^[()\[\]a-zA-Z0-9_.-]+$/i"
+
 [posts]
 maxSourceLength = 200