From 78833672469f7beb0c4a195aa0a76f7ca4133057 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 15 Jan 2016 15:28:22 +0100 Subject: [PATCH] Fix `findOneByUrl` side effect in tests Fix #1566 --- .../CoreBundle/Controller/EntryController.php | 3 +-- .../CoreBundle/Repository/EntryRepository.php | 3 +-- .../Tests/Controller/EntryControllerTest.php | 20 +++++++++---------- .../CoreBundle/Tests/WallabagCoreTestCase.php | 17 ++++++++++++++++ .../ImportBundle/Import/PocketImport.php | 2 +- .../ImportBundle/Import/WallabagV1Import.php | 2 +- .../Tests/Import/PocketImportTest.php | 2 +- .../Tests/Import/WallabagV1ImportTest.php | 2 +- 8 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php index 3d22c7bca..1949bdf8f 100644 --- a/src/Wallabag/CoreBundle/Controller/EntryController.php +++ b/src/Wallabag/CoreBundle/Controller/EntryController.php @@ -49,8 +49,7 @@ class EntryController extends Controller if ($form->isValid()) { // check for existing entry, if it exists, redirect to it with a message - $existingEntry = $this->get('wallabag_core.entry_repository') - ->existByUrlAndUserId($entry->getUrl(), $this->getUser()->getId()); + $existingEntry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId($entry->getUrl(), $this->getUser()->getId()); if (false !== $existingEntry) { $this->get('session')->getFlashBag()->add( diff --git a/src/Wallabag/CoreBundle/Repository/EntryRepository.php b/src/Wallabag/CoreBundle/Repository/EntryRepository.php index a16be9e01..82eb94746 100644 --- a/src/Wallabag/CoreBundle/Repository/EntryRepository.php +++ b/src/Wallabag/CoreBundle/Repository/EntryRepository.php @@ -235,10 +235,9 @@ class EntryRepository extends EntityRepository * * @return array|bool */ - public function existByUrlAndUserId($url, $userId) + public function findByUrlAndUserId($url, $userId) { $res = $this->createQueryBuilder('e') - ->select('e.id, e.createdAt') ->where('e.url = :url')->setParameter('url', $url) ->andWhere('e.user = :user_id')->setParameter('user_id', $userId) ->getQuery() diff --git a/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php b/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php index 3a7751824..1d1620dc2 100644 --- a/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php +++ b/src/Wallabag/CoreBundle/Tests/Controller/EntryControllerTest.php @@ -38,7 +38,7 @@ class EntryControllerTest extends WallabagCoreTestCase $form = $crawler->filter('button[type=submit]')->form(); $data = array( - 'entry[url]' => 'https://www.wallabag.org/blog/2016/01/08/wallabag-alpha1-v2', + 'entry[url]' => $this->url, ); $client->submit($form, $data); @@ -82,7 +82,7 @@ class EntryControllerTest extends WallabagCoreTestCase ->get('doctrine.orm.entity_manager'); $entry = $em ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $em->remove($entry); $em->flush(); } @@ -202,7 +202,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $client->request('GET', '/view/'.$content->getId()); @@ -223,7 +223,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); // empty content $content->setContent(''); @@ -237,7 +237,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $this->assertNotEmpty($content->getContent()); } @@ -250,7 +250,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $crawler = $client->request('GET', '/edit/'.$content->getId()); @@ -268,7 +268,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $crawler = $client->request('GET', '/edit/'.$content->getId()); @@ -298,7 +298,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $client->request('GET', '/archive/'.$content->getId()); @@ -320,7 +320,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $client->request('GET', '/star/'.$content->getId()); @@ -342,7 +342,7 @@ class EntryControllerTest extends WallabagCoreTestCase $content = $client->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUrl($this->url); + ->findByUrlAndUserId($this->url, $this->getLoggedInUserId()); $client->request('GET', '/delete/'.$content->getId()); diff --git a/src/Wallabag/CoreBundle/Tests/WallabagCoreTestCase.php b/src/Wallabag/CoreBundle/Tests/WallabagCoreTestCase.php index ce3cabe8d..c4905478b 100644 --- a/src/Wallabag/CoreBundle/Tests/WallabagCoreTestCase.php +++ b/src/Wallabag/CoreBundle/Tests/WallabagCoreTestCase.php @@ -31,4 +31,21 @@ abstract class WallabagCoreTestCase extends WebTestCase $this->client->submit($form, $data); } + + /** + * Return the user id of the logged in user. + * You should be sure that you called `logInAs` before. + * + * @return int + */ + public function getLoggedInUserId() + { + $token = static::$kernel->getContainer()->get('security.token_storage')->getToken(); + + if (null !== $token) { + return $token->getUser()->getId(); + } + + throw new \RuntimeException('No logged in User.'); + } } diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php index cdcec1e28..72b9047c8 100644 --- a/src/Wallabag/ImportBundle/Import/PocketImport.php +++ b/src/Wallabag/ImportBundle/Import/PocketImport.php @@ -214,7 +214,7 @@ class PocketImport implements ImportInterface $existingEntry = $this->em ->getRepository('WallabagCoreBundle:Entry') - ->existByUrlAndUserId($url, $this->user->getId()); + ->findByUrlAndUserId($url, $this->user->getId()); if (false !== $existingEntry) { ++$this->skippedEntries; diff --git a/src/Wallabag/ImportBundle/Import/WallabagV1Import.php b/src/Wallabag/ImportBundle/Import/WallabagV1Import.php index 393089d63..6f8feaf36 100644 --- a/src/Wallabag/ImportBundle/Import/WallabagV1Import.php +++ b/src/Wallabag/ImportBundle/Import/WallabagV1Import.php @@ -127,7 +127,7 @@ class WallabagV1Import implements ImportInterface foreach ($entries as $importedEntry) { $existingEntry = $this->em ->getRepository('WallabagCoreBundle:Entry') - ->existByUrlAndUserId($importedEntry['url'], $this->user->getId()); + ->findByUrlAndUserId($importedEntry['url'], $this->user->getId()); if (false !== $existingEntry) { ++$this->skippedEntries; diff --git a/src/Wallabag/ImportBundle/Tests/Import/PocketImportTest.php b/src/Wallabag/ImportBundle/Tests/Import/PocketImportTest.php index 043b2114e..76225fe4f 100644 --- a/src/Wallabag/ImportBundle/Tests/Import/PocketImportTest.php +++ b/src/Wallabag/ImportBundle/Tests/Import/PocketImportTest.php @@ -248,7 +248,7 @@ class PocketImportTest extends \PHPUnit_Framework_TestCase ->getMock(); $entryRepo->expects($this->exactly(2)) - ->method('existByUrlAndUserId') + ->method('findByUrlAndUserId') ->will($this->onConsecutiveCalls(false, true)); $tag = $this->getMockBuilder('Wallabag\CoreBundle\Entity\Tag') diff --git a/src/Wallabag/ImportBundle/Tests/Import/WallabagV1ImportTest.php b/src/Wallabag/ImportBundle/Tests/Import/WallabagV1ImportTest.php index d5b41777c..904834808 100644 --- a/src/Wallabag/ImportBundle/Tests/Import/WallabagV1ImportTest.php +++ b/src/Wallabag/ImportBundle/Tests/Import/WallabagV1ImportTest.php @@ -53,7 +53,7 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase ->getMock(); $entryRepo->expects($this->exactly(3)) - ->method('existByUrlAndUserId') + ->method('findByUrlAndUserId') ->will($this->onConsecutiveCalls(false, true, false)); $this->em