Merge pull request #20872 from owncloud/systemtags-better-not-found-exception

Systemtags better not found exception

lib/private/systemtag/systemtagmanager.php

@@ -94,7 +94,9 @@ class SystemTagManager implements ISystemTagManager {
 		$result->closeCursor();
 
 		if (count($tags) !== count($tagIds)) {
-			throw new TagNotFoundException(json_encode(array_diff($tagIds, array_keys($tags))));
+			throw new TagNotFoundException(
+				'Tag id(s) not found', 0, null, array_diff($tagIds, array_keys($tags))
+			);
 		}
 
 		return $tags;
@@ -218,7 +220,7 @@ class SystemTagManager implements ISystemTagManager {
 		try {
 			if ($query->execute() === 0) {
 				throw new TagNotFoundException(
-					'Tag ("' . $tagName . '", '. $userVisible . ', ' . $userAssignable . ') does not exist'
+					'Tag does not exist', 0, null, [$tagId]
 				);
 			}
 		} catch (UniqueConstraintViolationException $e) {
@@ -238,6 +240,13 @@ class SystemTagManager implements ISystemTagManager {
 			$tagIds = [$tagIds];
 		}
 
+		$tagNotFoundException = null;
+		try {
+			$this->getTagsById($tagIds);
+		} catch (TagNotFoundException $e) {
+			$tagNotFoundException = $e;
+		}
+
 		// delete relationships first
 		$query = $this->connection->getQueryBuilder();
 		$query->delete(SystemTagObjectMapper::RELATION_TABLE)
@@ -248,11 +257,12 @@ class SystemTagManager implements ISystemTagManager {
 		$query = $this->connection->getQueryBuilder();
 		$query->delete(self::TAG_TABLE)
 			->where($query->expr()->in('id', $query->createParameter('tagids')))
-			->setParameter('tagids', $tagIds, Connection::PARAM_INT_ARRAY);
+			->setParameter('tagids', $tagIds, Connection::PARAM_INT_ARRAY)
+			->execute();
 
-		if ($query->execute() === 0) {
+		if ($tagNotFoundException !== null) {
 			throw new TagNotFoundException(
-				'Tag does not exist'
+				'Tag id(s) not found', 0, $tagNotFoundException, $tagNotFoundException->getMissingTags()
 			);
 		}
 	}

lib/private/systemtag/systemtagobjectmapper.php

@@ -219,7 +219,9 @@ class SystemTagObjectMapper implements ISystemTagObjectMapper {
 				$tags
 			);
 			$missingTagIds = array_diff($tagIds, $foundTagIds);
-			throw new TagNotFoundException('Tags ' . json_encode($missingTagIds) . ' do not exist');
+			throw new TagNotFoundException(
+				'Tags not found', 0, null, $missingTagIds
+			);
 		}
 	}
 }

lib/public/systemtag/isystemtagmanager.php

@@ -106,7 +106,7 @@ interface ISystemTagManager {
 	 *
 	 * @param string|array $tagIds array of tag ids
 	 *
-	 * @throws \OCP\SystemTag\TagNotFoundException if tag did not exist
+	 * @throws \OCP\SystemTag\TagNotFoundException if at least one tag did not exist
 	 *
 	 * @since 9.0.0
 	 */

lib/public/systemtag/tagnotfoundexception.php

@@ -26,4 +26,30 @@ namespace OCP\SystemTag;
  *
  * @since 9.0.0
  */
-class TagNotFoundException extends \RuntimeException {}
+class TagNotFoundException extends \RuntimeException {
+
+	/** @var string[] */
+	protected $tags;
+
+	/**
+	 * TagNotFoundException constructor.
+	 *
+	 * @param string $message
+	 * @param int $code
+	 * @param \Exception $previous
+	 * @param string[] $tags
+	 * @since 9.0.0
+	 */
+	public function __construct($message = '', $code = 0, \Exception $previous = null, array $tags = []) {
+		parent::__construct($message, $code, $previous);
+		$this->tags = $tags;
+	}
+
+	/**
+	 * @return string[]
+	 * @since 9.0.0
+	 */
+	public function getMissingTags() {
+		return $this->tags;
+	}
+}

tests/lib/systemtag/systemtagmanagertest.php

@@ -40,9 +40,15 @@ class SystemTagManagerTest extends TestCase {
 
 		$this->connection = \OC::$server->getDatabaseConnection();
 		$this->tagManager = new SystemTagManager($this->connection);
-	} 
+		$this->pruneTagsTables();
+	}
 
 	public function tearDown() {
+		$this->pruneTagsTables();
+		parent::tearDown();
+	}
+
+	protected function pruneTagsTables() {
 		$query = $this->connection->getQueryBuilder();
 		$query->delete(SystemTagObjectMapper::RELATION_TABLE)->execute();
 		$query->delete(SystemTagManager::TAG_TABLE)->execute();

tests/lib/systemtag/systemtagobjectmappertest.php

@@ -62,6 +62,7 @@ class SystemTagObjectMapperTest extends TestCase {
 		parent::setUp();
 
 		$this->connection = \OC::$server->getDatabaseConnection();
+		$this->pruneTagsTables();
 
 		$this->tagManager = $this->getMockBuilder('OCP\SystemTag\ISystemTagManager')
 			->getMock();
@@ -92,9 +93,14 @@ class SystemTagObjectMapperTest extends TestCase {
 		$this->tagMapper->assignTags(1, 'testtype', $this->tag2->getId());
 		$this->tagMapper->assignTags(2, 'testtype', $this->tag1->getId());
 		$this->tagMapper->assignTags(3, 'anothertype', $this->tag1->getId());
-	} 
+	}
 
 	public function tearDown() {
+		$this->pruneTagsTables();
+		parent::tearDown();
+	}
+
+	protected function pruneTagsTables() {
 		$query = $this->connection->getQueryBuilder();
 		$query->delete(SystemTagObjectMapper::RELATION_TABLE)->execute();
 		$query->delete(SystemTagManager::TAG_TABLE)->execute();