Merge pull request #3271 from wallabag/store-resolved-url

Add `given_url` in Entry table to check if a redirected url has already added

5 files changed, 164 insertions, 11 deletions

diff --git a/app/DoctrineMigrations/Version20190601125843.php b/app/DoctrineMigrations/Version20190601125843.php
new file mode 100644
index 00000000..0e97606e
--- /dev/null
+++ b/app/DoctrineMigrations/Version20190601125843.php
@@ -0,0 +1,54 @@
+<?php
+
+namespace Application\Migrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Wallabag\CoreBundle\Doctrine\WallabagMigration;
+
+/**
+ * Added `given_url` & `hashed_given_url` field in entry table.
+ */
+class Version20190601125843 extends WallabagMigration
+{
+    /**
+     * @param Schema $schema
+     */
+    public function up(Schema $schema)
+    {
+        $entryTable = $schema->getTable($this->getTable('entry'));
+
+        if (!$entryTable->hasColumn('given_url')) {
+            $entryTable->addColumn('given_url', 'text', [
+                'notnull' => false,
+            ]);
+        }
+
+        if (!$entryTable->hasColumn('hashed_given_url')) {
+            $entryTable->addColumn('hashed_given_url', 'text', [
+                'length' => 40,
+                'notnull' => false,
+            ]);
+        }
+
+        // 40 = length of sha1 field hashed_given_url
+        $entryTable->addIndex(['user_id', 'hashed_given_url'], 'hashed_given_url_user_id', [], ['lengths' => [null, 40]]);
+    }
+
+    /**
+     * @param Schema $schema
+     */
+    public function down(Schema $schema)
+    {
+        $entryTable = $schema->getTable($this->getTable('entry'));
+
+        if ($entryTable->hasColumn('given_url')) {
+            $entryTable->dropColumn('given_url');
+        }
+
+        if ($entryTable->hasColumn('hashed_given_url')) {
+            $entryTable->dropColumn('hashed_given_url');
+        }
+
+        $entryTable->dropIndex('hashed_given_url_user_id');
+    }
+}
diff --git a/src/Wallabag/CoreBundle/Entity/Entry.php b/src/Wallabag/CoreBundle/Entity/Entry.php
index 8637c62f..4d5e6fc9 100644
--- a/src/Wallabag/CoreBundle/Entity/Entry.php
+++ b/src/Wallabag/CoreBundle/Entity/Entry.php
@@ -27,7 +27,8 @@ use Wallabag\UserBundle\Entity\User;
  *     indexes={
  *         @ORM\Index(name="created_at", columns={"created_at"}),
  *         @ORM\Index(name="uid", columns={"uid"}),
- *         @ORM\Index(name="hashed_url_user_id", columns={"user_id", "hashed_url"}, options={"lengths"={null, 40}})
+ *         @ORM\Index(name="hashed_url_user_id", columns={"user_id", "hashed_url"}, options={"lengths"={null, 40}}),
+ *         @ORM\Index(name="hashed_given_url_user_id", columns={"user_id", "hashed_given_url"}, options={"lengths"={null, 40}})
  *     }
  * )
  * @ORM\HasLifecycleCallbacks()
@@ -68,6 +69,8 @@ class Entry
     private $title;
 
     /**
+     * Define the url fetched by wallabag (the final url after potential redirections).
+     *
      * @var string
      *
      * @Assert\NotBlank()
@@ -85,6 +88,35 @@ class Entry
     private $hashedUrl;
 
     /**
+     * From where user retrieved/found the url (an other article, a twitter, or the given_url if non are provided).
+     *
+     * @var string
+     *
+     * @ORM\Column(name="origin_url", type="text", nullable=true)
+     *
+     * @Groups({"entries_for_user", "export_all"})
+     */
+    private $originUrl;
+
+    /**
+     * Define the url entered by the user (without redirections).
+     *
+     * @var string
+     *
+     * @ORM\Column(name="given_url", type="text", nullable=true)
+     *
+     * @Groups({"entries_for_user", "export_all"})
+     */
+    private $givenUrl;
+
+    /**
+     * @var string
+     *
+     * @ORM\Column(name="hashed_given_url", type="string", length=40, nullable=true)
+     */
+    private $hashedGivenUrl;
+
+    /**
      * @var bool
      *
      * @Exclude
@@ -263,15 +295,6 @@ class Entry
      */
     private $tags;
 
-    /**
-     * @var string
-     *
-     * @ORM\Column(name="origin_url", type="text", nullable=true)
-     *
-     * @Groups({"entries_for_user", "export_all"})
-     */
-    private $originUrl;
-
     /*
      * @param User     $user
      */
@@ -923,6 +946,31 @@ class Entry
     }
 
     /**
+     * Set given url.
+     *
+     * @param string $givenUrl
+     *
+     * @return Entry
+     */
+    public function setGivenUrl($givenUrl)
+    {
+        $this->givenUrl = $givenUrl;
+        $this->hashedGivenUrl = UrlHasher::hashUrl($givenUrl);
+
+        return $this;
+    }
+
+    /**
+     * Get given url.
+     *
+     * @return string
+     */
+    public function getGivenUrl()
+    {
+        return $this->givenUrl;
+    }
+
+    /**
      * @return string
      */
     public function getHashedUrl()
diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
index c6fa0d98..5901df8b 100644
--- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
@@ -78,6 +78,8 @@ class ContentProxy
             $entry->setUrl($url);
         }
 
+        $entry->setGivenUrl($url);
+
         $this->stockEntry($entry, $content);
     }
 
diff --git a/src/Wallabag/CoreBundle/Repository/EntryRepository.php b/src/Wallabag/CoreBundle/Repository/EntryRepository.php
index f9cf5233..16c44885 100644
--- a/src/Wallabag/CoreBundle/Repository/EntryRepository.php
+++ b/src/Wallabag/CoreBundle/Repository/EntryRepository.php
@@ -366,6 +366,7 @@ class EntryRepository extends EntityRepository
      */
     public function findByHashedUrlAndUserId($hashedUrl, $userId)
     {
+        // try first using hashed_url (to use the database index)
         $res = $this->createQueryBuilder('e')
             ->where('e.hashedUrl = :hashed_url')->setParameter('hashed_url', $hashedUrl)
             ->andWhere('e.user = :user_id')->setParameter('user_id', $userId)
@@ -376,6 +377,17 @@ class EntryRepository extends EntityRepository
             return current($res);
         }
 
+        // then try using hashed_given_url (to use the database index)
+        $res = $this->createQueryBuilder('e')
+            ->where('e.hashedGivenUrl = :hashed_given_url')->setParameter('hashed_given_url', $hashedUrl)
+            ->andWhere('e.user = :user_id')->setParameter('user_id', $userId)
+            ->getQuery()
+            ->getResult();
+
+        if (\count($res)) {
+            return current($res);
+        }
+
         return false;
     }
 
diff --git a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
index 9dee9891..e9c12c49 100644
--- a/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
+++ b/tests/Wallabag/CoreBundle/Controller/EntryControllerTest.php
@@ -166,7 +166,6 @@ class EntryControllerTest extends WallabagCoreTestCase
         $this->assertSame($this->url, $content->getUrl());
         $this->assertContains('Google', $content->getTitle());
         $this->assertSame('fr', $content->getLanguage());
-        $this->assertSame('2015-03-28 11:43:19', $content->getPublishedAt()->format('Y-m-d H:i:s'));
         $this->assertArrayHasKey('x-frame-options', $content->getHeaders());
         $client->getContainer()->get('craue_config')->set('store_article_headers', 0);
     }
@@ -269,6 +268,44 @@ class EntryControllerTest extends WallabagCoreTestCase
     /**
      * This test will require an internet connection.
      */
+    public function testPostNewOkUrlExistWithRedirection()
+    {
+        $this->logInAs('admin');
+        $client = $this->getClient();
+
+        $url = 'https://wllbg.org/test-redirect/c51c';
+
+        $crawler = $client->request('GET', '/new');
+
+        $this->assertSame(200, $client->getResponse()->getStatusCode());
+
+        $form = $crawler->filter('form[name=entry]')->form();
+
+        $data = [
+            'entry[url]' => $url,
+        ];
+
+        $client->submit($form, $data);
+
+        $crawler = $client->request('GET', '/new');
+
+        $this->assertSame(200, $client->getResponse()->getStatusCode());
+
+        $form = $crawler->filter('form[name=entry]')->form();
+
+        $data = [
+            'entry[url]' => $url,
+        ];
+
+        $client->submit($form, $data);
+
+        $this->assertSame(302, $client->getResponse()->getStatusCode());
+        $this->assertContains('/view/', $client->getResponse()->getTargetUrl());
+    }
+
+    /**
+     * This test will require an internet connection.
+     */
     public function testPostNewThatWillBeTagged()
     {
         $this->logInAs('admin');