Merge pull request #1583 from wallabag/v2-fix-delete

Fix `findOneByUrl` side effect in tests

8 files changed, 33 insertions, 18 deletions

diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php
index aa70307b..9dd904f1 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 a16be9e0..82eb9474 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 3a775182..1d1620dc 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 ce3cabe8..c4905478 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 cdcec1e2..72b9047c 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 393089d6..6f8feaf3 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 043b2114..76225fe4 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 d5b41777..90483480 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