From 1d0c041ac859603357a580a1a16d382e4b6bdc06 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 1 Dec 2015 14:51:26 +0100 Subject: [PATCH 1/2] Add a method to get the list of tags from the TagNotFound Exception --- lib/private/systemtag/systemtagmanager.php | 20 +++++++++---- .../systemtag/systemtagobjectmapper.php | 4 ++- lib/public/systemtag/isystemtagmanager.php | 2 +- lib/public/systemtag/tagnotfoundexception.php | 28 ++++++++++++++++++- tests/lib/systemtag/systemtagmanagertest.php | 2 +- 5 files changed, 47 insertions(+), 9 deletions(-) diff --git a/lib/private/systemtag/systemtagmanager.php b/lib/private/systemtag/systemtagmanager.php index 3cc45fc952..8caf10d69d 100644 --- a/lib/private/systemtag/systemtagmanager.php +++ b/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() ); } } diff --git a/lib/private/systemtag/systemtagobjectmapper.php b/lib/private/systemtag/systemtagobjectmapper.php index d8ff069910..75f2631a01 100644 --- a/lib/private/systemtag/systemtagobjectmapper.php +++ b/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 + ); } } } diff --git a/lib/public/systemtag/isystemtagmanager.php b/lib/public/systemtag/isystemtagmanager.php index ffdaf03879..4e3b263e56 100644 --- a/lib/public/systemtag/isystemtagmanager.php +++ b/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 */ diff --git a/lib/public/systemtag/tagnotfoundexception.php b/lib/public/systemtag/tagnotfoundexception.php index 94bb07c8c1..d0a35eb59e 100644 --- a/lib/public/systemtag/tagnotfoundexception.php +++ b/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; + } +} diff --git a/tests/lib/systemtag/systemtagmanagertest.php b/tests/lib/systemtag/systemtagmanagertest.php index 0a192f01f4..ad06b16e05 100644 --- a/tests/lib/systemtag/systemtagmanagertest.php +++ b/tests/lib/systemtag/systemtagmanagertest.php @@ -40,7 +40,7 @@ class SystemTagManagerTest extends TestCase { $this->connection = \OC::$server->getDatabaseConnection(); $this->tagManager = new SystemTagManager($this->connection); - } + } public function tearDown() { $query = $this->connection->getQueryBuilder(); From 08d07cc4db58a7f11532d73108b63a2f50390bf6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 1 Dec 2015 14:53:12 +0100 Subject: [PATCH 2/2] Fix the unit test by running tearDown() and cleaning tags before the test --- tests/lib/systemtag/systemtagmanagertest.php | 6 ++++++ tests/lib/systemtag/systemtagobjectmappertest.php | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/lib/systemtag/systemtagmanagertest.php b/tests/lib/systemtag/systemtagmanagertest.php index ad06b16e05..8498b85519 100644 --- a/tests/lib/systemtag/systemtagmanagertest.php +++ b/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(); diff --git a/tests/lib/systemtag/systemtagobjectmappertest.php b/tests/lib/systemtag/systemtagobjectmappertest.php index a4312fa722..43d0b8c696 100644 --- a/tests/lib/systemtag/systemtagobjectmappertest.php +++ b/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();