From ac8cf632bb3a225c1b69d16e714ff60a2e988c89 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 7 Oct 2016 23:31:53 +0200 Subject: [PATCH] Ensure orphan tag are remove in API MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When the association between a tag and an entry is removed, if the tag doesn’t have other entries, we can remove it. Also add more tests for that part and ensure TagControllerTest is isolated from the rest of the test suite (finally!) --- .../Controller/WallabagRestController.php | 28 ++++++++++++++++ .../CoreBundle/Controller/TagController.php | 6 ++-- .../CoreBundle/Resources/config/services.yml | 2 +- .../Controller/WallabagRestControllerTest.php | 9 +++++ .../Controller/EntryControllerTest.php | 27 ++++++++------- .../Controller/TagControllerTest.php | 33 +++++++++++++------ 6 files changed, 78 insertions(+), 27 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/WallabagRestController.php b/src/Wallabag/ApiBundle/Controller/WallabagRestController.php index a0d9d4f3..cc6923a0 100644 --- a/src/Wallabag/ApiBundle/Controller/WallabagRestController.php +++ b/src/Wallabag/ApiBundle/Controller/WallabagRestController.php @@ -400,6 +400,8 @@ class WallabagRestController extends FOSRestController ->getRepository('WallabagCoreBundle:Entry') ->removeTag($this->getUser()->getId(), $tag); + $this->cleanOrphanTag($tag); + $json = $this->get('serializer')->serialize($tag, 'json'); return (new JsonResponse())->setJson($json); @@ -440,6 +442,8 @@ class WallabagRestController extends FOSRestController ->getRepository('WallabagCoreBundle:Entry') ->removeTags($this->getUser()->getId(), $tags); + $this->cleanOrphanTag($tags); + $json = $this->get('serializer')->serialize($tags, 'json'); return (new JsonResponse())->setJson($json); @@ -464,6 +468,8 @@ class WallabagRestController extends FOSRestController ->getRepository('WallabagCoreBundle:Entry') ->removeTag($this->getUser()->getId(), $tag); + $this->cleanOrphanTag($tag); + $json = $this->get('serializer')->serialize($tag, 'json'); return (new JsonResponse())->setJson($json); @@ -485,6 +491,28 @@ class WallabagRestController extends FOSRestController return (new JsonResponse())->setJson($json); } + /** + * Remove orphan tag in case no entries are associated to it. + * + * @param Tag|array $tags + */ + private function cleanOrphanTag($tags) + { + if (!is_array($tags)) { + $tags = [$tags]; + } + + $em = $this->getDoctrine()->getManager(); + + foreach ($tags as $tag) { + if (count($tag->getEntries()) === 0) { + $em->remove($tag); + } + } + + $em->flush(); + } + /** * Validate that the first id is equal to the second one. * If not, throw exception. It means a user try to access information from an other user. diff --git a/src/Wallabag/CoreBundle/Controller/TagController.php b/src/Wallabag/CoreBundle/Controller/TagController.php index 623a6146..c5746734 100644 --- a/src/Wallabag/CoreBundle/Controller/TagController.php +++ b/src/Wallabag/CoreBundle/Controller/TagController.php @@ -63,10 +63,12 @@ class TagController extends Controller $entry->removeTag($tag); $em = $this->getDoctrine()->getManager(); $em->flush(); - if (count($tag->getEntries()) == 0) { + + // remove orphan tag in case no entries are associated to it + if (count($tag->getEntries()) === 0) { $em->remove($tag); + $em->flush(); } - $em->flush(); $redirectUrl = $this->get('wallabag_core.helper.redirect')->to($request->headers->get('referer')); diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml index fb97454e..a4b727f4 100644 --- a/src/Wallabag/CoreBundle/Resources/config/services.yml +++ b/src/Wallabag/CoreBundle/Resources/config/services.yml @@ -29,7 +29,7 @@ services: arguments: - "@doctrine" - wallabag_core.table_prefix_subscriber: + wallabag_core.subscriber.table_prefix: class: Wallabag\CoreBundle\Subscriber\TablePrefixSubscriber arguments: - "%database_table_prefix%" diff --git a/tests/Wallabag/ApiBundle/Controller/WallabagRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/WallabagRestControllerTest.php index 65b65290..c797daf7 100644 --- a/tests/Wallabag/ApiBundle/Controller/WallabagRestControllerTest.php +++ b/tests/Wallabag/ApiBundle/Controller/WallabagRestControllerTest.php @@ -561,6 +561,8 @@ class WallabagRestControllerTest extends WallabagApiTestCase */ public function testDeleteUserTag($tag) { + $tagName = $tag['label']; + $this->client->request('DELETE', '/api/tags/'.$tag['id'].'.json'); $this->assertEquals(200, $this->client->getResponse()->getStatusCode()); @@ -577,6 +579,13 @@ class WallabagRestControllerTest extends WallabagApiTestCase ->findAllByTagId($this->user->getId(), $tag['id']); $this->assertCount(0, $entries); + + $tag = $this->client->getContainer() + ->get('doctrine.orm.entity_manager') + ->getRepository('WallabagCoreBundle:Tag') + ->findOneByLabel($tagName); + + $this->assertNull($tag, $tagName.' was removed because it begun an orphan tag'); } public function testDeleteTagByLabel() diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php index d7bf03ba..9b03a519 100644 --- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php @@ -341,22 +341,23 @@ class EntryControllerTest extends WallabagCoreTestCase $this->logInAs('admin'); $client = $this->getClient(); - $content = $client->getContainer() - ->get('doctrine.orm.entity_manager') + $em = $client->getContainer() + ->get('doctrine.orm.entity_manager'); + + $content = $em ->getRepository('WallabagCoreBundle:Entry') ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); // empty content $content->setContent(''); - $client->getContainer()->get('doctrine.orm.entity_manager')->persist($content); - $client->getContainer()->get('doctrine.orm.entity_manager')->flush(); + $em->persist($content); + $em->flush(); $client->request('GET', '/reload/'.$content->getId()); $this->assertEquals(302, $client->getResponse()->getStatusCode()); - $content = $client->getContainer() - ->get('doctrine.orm.entity_manager') + $content = $em ->getRepository('WallabagCoreBundle:Entry') ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); @@ -486,9 +487,11 @@ class EntryControllerTest extends WallabagCoreTestCase $this->logInAs('admin'); $client = $this->getClient(); + $em = $client->getContainer() + ->get('doctrine.orm.entity_manager'); + // add a new content to be removed later - $user = $client->getContainer() - ->get('doctrine.orm.entity_manager') + $user = $em ->getRepository('WallabagUserBundle:User') ->findOneByUserName('admin'); @@ -502,12 +505,8 @@ class EntryControllerTest extends WallabagCoreTestCase $content->setArchived(true); $content->setLanguage('fr'); - $client->getContainer() - ->get('doctrine.orm.entity_manager') - ->persist($content); - $client->getContainer() - ->get('doctrine.orm.entity_manager') - ->flush(); + $em->persist($content); + $em->flush(); $client->request('GET', '/view/'.$content->getId()); $this->assertEquals(200, $client->getResponse()->getStatusCode()); diff --git a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php index 86a6cca2..769ce66e 100644 --- a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php @@ -3,6 +3,7 @@ namespace Tests\Wallabag\CoreBundle\Controller; use Tests\Wallabag\CoreBundle\WallabagCoreTestCase; +use Wallabag\CoreBundle\Entity\Tag; class TagControllerTest extends WallabagCoreTestCase { @@ -134,36 +135,48 @@ class TagControllerTest extends WallabagCoreTestCase $client->request('GET', '/remove-tag/'.$entry->getId().'/'.$tag->getId()); $this->assertEquals(404, $client->getResponse()->getStatusCode()); + + $tag = $client->getContainer() + ->get('doctrine.orm.entity_manager') + ->getRepository('WallabagCoreBundle:Tag') + ->findOneByLabel($this->tagName); + + $this->assertNull($tag, $this->tagName.' was removed because it begun an orphan tag'); } public function testShowEntriesForTagAction() { $this->logInAs('admin'); $client = $this->getClient(); + $em = $client->getContainer() + ->get('doctrine.orm.entity_manager'); + + $tag = new Tag(); + $tag->setLabel($this->tagName); $entry = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') ->findByUrlAndUserId('http://0.0.0.0/entry4', $this->getLoggedInUserId()); - $tag = $client->getContainer() - ->get('doctrine.orm.entity_manager') - ->getRepository('WallabagCoreBundle:Tag') - ->findOneByEntryAndTagLabel($entry, 'foo'); - - $crawler = $client->request('GET', '/tag/list/'.$tag->getSlug()); + $tag->addEntry($entry); - $this->assertEquals(200, $client->getResponse()->getStatusCode()); - $this->assertCount(2, $crawler->filter('div[class=entry]')); + $em->persist($entry); + $em->persist($tag); + $em->flush(); $tag = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Tag') - ->findOneByLabel('baz'); + ->findOneByEntryAndTagLabel($entry, $this->tagName); $crawler = $client->request('GET', '/tag/list/'.$tag->getSlug()); $this->assertEquals(200, $client->getResponse()->getStatusCode()); - $this->assertCount(1, $crawler->filter('div[class=entry]')); + $this->assertCount(1, $crawler->filter('[id*="entry-"]')); + + $entry->removeTag($tag); + $em->remove($tag); + $em->flush(); } } -- 2.41.0