diff options
author | Jeremy Benoist <jeremy.benoist@gmail.com> | 2019-04-01 13:51:57 +0200 |
---|---|---|
committer | Jeremy Benoist <jeremy.benoist@gmail.com> | 2019-04-01 13:51:57 +0200 |
commit | 8a6456629814039cfc623cdb279bcba06dacff50 (patch) | |
tree | b3ae2459651b6ad085588fca0c624c6df554deb3 | |
parent | 9c2b2aae70b06411336e6eb6ac43b3ebd30dc38c (diff) | |
download | wallabag-8a6456629814039cfc623cdb279bcba06dacff50.tar.gz wallabag-8a6456629814039cfc623cdb279bcba06dacff50.tar.zst wallabag-8a6456629814039cfc623cdb279bcba06dacff50.zip |
Use a better index for hashed_url
It'll most often be used in addition to the `user_id`.
Also, automatically generate the hash when saving the url.
Switch from `md5` to `sha1`.
7 files changed, 21 insertions, 23 deletions
diff --git a/app/DoctrineMigrations/Version20190401105353.php b/app/DoctrineMigrations/Version20190401105353.php index 4afc8b15..c1c22053 100644 --- a/app/DoctrineMigrations/Version20190401105353.php +++ b/app/DoctrineMigrations/Version20190401105353.php | |||
@@ -20,7 +20,7 @@ class Version20190401105353 extends WallabagMigration | |||
20 | $this->skipIf($entryTable->hasColumn('hashed_url'), 'It seems that you already played this migration.'); | 20 | $this->skipIf($entryTable->hasColumn('hashed_url'), 'It seems that you already played this migration.'); |
21 | 21 | ||
22 | $entryTable->addColumn('hashed_url', 'text', [ | 22 | $entryTable->addColumn('hashed_url', 'text', [ |
23 | 'length' => 32, | 23 | 'length' => 40, |
24 | 'notnull' => false, | 24 | 'notnull' => false, |
25 | ]); | 25 | ]); |
26 | 26 | ||
@@ -28,6 +28,8 @@ class Version20190401105353 extends WallabagMigration | |||
28 | if ('sqlite' !== $this->connection->getDatabasePlatform()->getName()) { | 28 | if ('sqlite' !== $this->connection->getDatabasePlatform()->getName()) { |
29 | $this->addSql('UPDATE ' . $this->getTable('entry') . ' SET hashed_url = MD5(url)'); | 29 | $this->addSql('UPDATE ' . $this->getTable('entry') . ' SET hashed_url = MD5(url)'); |
30 | } | 30 | } |
31 | |||
32 | $entryTable->addIndex(['user_id', 'hashed_url'], 'hashed_url_user_id'); | ||
31 | } | 33 | } |
32 | 34 | ||
33 | /** | 35 | /** |
@@ -39,6 +41,7 @@ class Version20190401105353 extends WallabagMigration | |||
39 | 41 | ||
40 | $this->skipIf(!$entryTable->hasColumn('hashed_url'), 'It seems that you already played this migration.'); | 42 | $this->skipIf(!$entryTable->hasColumn('hashed_url'), 'It seems that you already played this migration.'); |
41 | 43 | ||
44 | $entryTable->dropIndex('hashed_url_user_id'); | ||
42 | $entryTable->dropColumn('hashed_url'); | 45 | $entryTable->dropColumn('hashed_url'); |
43 | } | 46 | } |
44 | } | 47 | } |
diff --git a/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php b/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php index fb598390..685e1672 100644 --- a/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php +++ b/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php | |||
@@ -69,7 +69,7 @@ class GenerateUrlHashesCommand extends ContainerAwareCommand | |||
69 | 69 | ||
70 | $i = 1; | 70 | $i = 1; |
71 | foreach ($entries as $entry) { | 71 | foreach ($entries as $entry) { |
72 | $entry->setHashedUrl(hash('md5', $entry->getUrl())); | 72 | $entry->setHashedUrl(hash('sha1', $entry->getUrl())); |
73 | $em->persist($entry); | 73 | $em->persist($entry); |
74 | 74 | ||
75 | if (0 === ($i % 20)) { | 75 | if (0 === ($i % 20)) { |
diff --git a/src/Wallabag/CoreBundle/DataFixtures/EntryFixtures.php b/src/Wallabag/CoreBundle/DataFixtures/EntryFixtures.php index 1b18cad6..024fcfdc 100644 --- a/src/Wallabag/CoreBundle/DataFixtures/EntryFixtures.php +++ b/src/Wallabag/CoreBundle/DataFixtures/EntryFixtures.php | |||
@@ -89,7 +89,6 @@ class EntryFixtures extends Fixture implements DependentFixtureInterface | |||
89 | foreach ($entries as $reference => $item) { | 89 | foreach ($entries as $reference => $item) { |
90 | $entry = new Entry($this->getReference($item['user'])); | 90 | $entry = new Entry($this->getReference($item['user'])); |
91 | $entry->setUrl($item['url']); | 91 | $entry->setUrl($item['url']); |
92 | $entry->setHashedUrl(hash('md5', $item['url'])); | ||
93 | $entry->setReadingTime($item['reading_time']); | 92 | $entry->setReadingTime($item['reading_time']); |
94 | $entry->setDomainName($item['domain']); | 93 | $entry->setDomainName($item['domain']); |
95 | $entry->setMimetype($item['mime']); | 94 | $entry->setMimetype($item['mime']); |
diff --git a/src/Wallabag/CoreBundle/Entity/Entry.php b/src/Wallabag/CoreBundle/Entity/Entry.php index a04f101f..faf4d259 100644 --- a/src/Wallabag/CoreBundle/Entity/Entry.php +++ b/src/Wallabag/CoreBundle/Entity/Entry.php | |||
@@ -26,7 +26,7 @@ use Wallabag\UserBundle\Entity\User; | |||
26 | * indexes={ | 26 | * indexes={ |
27 | * @ORM\Index(name="created_at", columns={"created_at"}), | 27 | * @ORM\Index(name="created_at", columns={"created_at"}), |
28 | * @ORM\Index(name="uid", columns={"uid"}), | 28 | * @ORM\Index(name="uid", columns={"uid"}), |
29 | * @ORM\Index(name="hashed_url", columns={"hashed_url"}) | 29 | * @ORM\Index(name="hashed_url_user_id", columns={"user_id", "hashed_url"}) |
30 | * } | 30 | * } |
31 | * ) | 31 | * ) |
32 | * @ORM\HasLifecycleCallbacks() | 32 | * @ORM\HasLifecycleCallbacks() |
@@ -79,7 +79,7 @@ class Entry | |||
79 | /** | 79 | /** |
80 | * @var string | 80 | * @var string |
81 | * | 81 | * |
82 | * @ORM\Column(name="hashed_url", type="string", length=32, nullable=true) | 82 | * @ORM\Column(name="hashed_url", type="string", length=40, nullable=true) |
83 | */ | 83 | */ |
84 | private $hashedUrl; | 84 | private $hashedUrl; |
85 | 85 | ||
@@ -324,6 +324,7 @@ class Entry | |||
324 | public function setUrl($url) | 324 | public function setUrl($url) |
325 | { | 325 | { |
326 | $this->url = $url; | 326 | $this->url = $url; |
327 | $this->hashedUrl = hash('sha1', $url); | ||
327 | 328 | ||
328 | return $this; | 329 | return $this; |
329 | } | 330 | } |
diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index 0534d27b..31953f12 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php | |||
@@ -248,8 +248,6 @@ class ContentProxy | |||
248 | { | 248 | { |
249 | $this->updateOriginUrl($entry, $content['url']); | 249 | $this->updateOriginUrl($entry, $content['url']); |
250 | 250 | ||
251 | $entry->setHashedUrl(hash('md5', $entry->getUrl())); | ||
252 | |||
253 | $this->setEntryDomainName($entry); | 251 | $this->setEntryDomainName($entry); |
254 | 252 | ||
255 | if (!empty($content['title'])) { | 253 | if (!empty($content['title'])) { |
diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php index fc4dc9d9..34de8ec8 100644 --- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php +++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php | |||
@@ -973,7 +973,7 @@ class EntryRestControllerTest extends WallabagApiTestCase | |||
973 | 973 | ||
974 | public function dataForEntriesExistWithUrl() | 974 | public function dataForEntriesExistWithUrl() |
975 | { | 975 | { |
976 | $url = hash('md5', 'http://0.0.0.0/entry2'); | 976 | $url = hash('sha1', 'http://0.0.0.0/entry2'); |
977 | 977 | ||
978 | return [ | 978 | return [ |
979 | 'with_id' => [ | 979 | 'with_id' => [ |
@@ -1047,37 +1047,37 @@ class EntryRestControllerTest extends WallabagApiTestCase | |||
1047 | { | 1047 | { |
1048 | $url1 = 'http://0.0.0.0/entry2'; | 1048 | $url1 = 'http://0.0.0.0/entry2'; |
1049 | $url2 = 'http://0.0.0.0/entry10'; | 1049 | $url2 = 'http://0.0.0.0/entry10'; |
1050 | $this->client->request('GET', '/api/entries/exists?hashed_urls[]=' . hash('md5', $url1) . '&hashed_urls[]=' . hash('md5', $url2) . '&return_id=1'); | 1050 | $this->client->request('GET', '/api/entries/exists?hashed_urls[]=' . hash('sha1', $url1) . '&hashed_urls[]=' . hash('sha1', $url2) . '&return_id=1'); |
1051 | 1051 | ||
1052 | $this->assertSame(200, $this->client->getResponse()->getStatusCode()); | 1052 | $this->assertSame(200, $this->client->getResponse()->getStatusCode()); |
1053 | 1053 | ||
1054 | $content = json_decode($this->client->getResponse()->getContent(), true); | 1054 | $content = json_decode($this->client->getResponse()->getContent(), true); |
1055 | 1055 | ||
1056 | $this->assertArrayHasKey(hash('md5', $url1), $content); | 1056 | $this->assertArrayHasKey(hash('sha1', $url1), $content); |
1057 | $this->assertArrayHasKey(hash('md5', $url2), $content); | 1057 | $this->assertArrayHasKey(hash('sha1', $url2), $content); |
1058 | $this->assertSame(2, $content[hash('md5', $url1)]); | 1058 | $this->assertSame(2, $content[hash('sha1', $url1)]); |
1059 | $this->assertNull($content[hash('md5', $url2)]); | 1059 | $this->assertNull($content[hash('sha1', $url2)]); |
1060 | } | 1060 | } |
1061 | 1061 | ||
1062 | public function testGetEntriesExistsWithManyUrlsHashedReturnBool() | 1062 | public function testGetEntriesExistsWithManyUrlsHashedReturnBool() |
1063 | { | 1063 | { |
1064 | $url1 = 'http://0.0.0.0/entry2'; | 1064 | $url1 = 'http://0.0.0.0/entry2'; |
1065 | $url2 = 'http://0.0.0.0/entry10'; | 1065 | $url2 = 'http://0.0.0.0/entry10'; |
1066 | $this->client->request('GET', '/api/entries/exists?hashed_urls[]=' . hash('md5', $url1) . '&hashed_urls[]=' . hash('md5', $url2)); | 1066 | $this->client->request('GET', '/api/entries/exists?hashed_urls[]=' . hash('sha1', $url1) . '&hashed_urls[]=' . hash('sha1', $url2)); |
1067 | 1067 | ||
1068 | $this->assertSame(200, $this->client->getResponse()->getStatusCode()); | 1068 | $this->assertSame(200, $this->client->getResponse()->getStatusCode()); |
1069 | 1069 | ||
1070 | $content = json_decode($this->client->getResponse()->getContent(), true); | 1070 | $content = json_decode($this->client->getResponse()->getContent(), true); |
1071 | 1071 | ||
1072 | $this->assertArrayHasKey(hash('md5', $url1), $content); | 1072 | $this->assertArrayHasKey(hash('sha1', $url1), $content); |
1073 | $this->assertArrayHasKey(hash('md5', $url2), $content); | 1073 | $this->assertArrayHasKey(hash('sha1', $url2), $content); |
1074 | $this->assertTrue($content[hash('md5', $url1)]); | 1074 | $this->assertTrue($content[hash('sha1', $url1)]); |
1075 | $this->assertFalse($content[hash('md5', $url2)]); | 1075 | $this->assertFalse($content[hash('sha1', $url2)]); |
1076 | } | 1076 | } |
1077 | 1077 | ||
1078 | public function testGetEntriesExistsWhichDoesNotExists() | 1078 | public function testGetEntriesExistsWhichDoesNotExists() |
1079 | { | 1079 | { |
1080 | $this->client->request('GET', '/api/entries/exists?hashed_url=' . hash('md5', 'http://google.com/entry2')); | 1080 | $this->client->request('GET', '/api/entries/exists?hashed_url=' . hash('sha1', 'http://google.com/entry2')); |
1081 | 1081 | ||
1082 | $this->assertSame(200, $this->client->getResponse()->getStatusCode()); | 1082 | $this->assertSame(200, $this->client->getResponse()->getStatusCode()); |
1083 | 1083 | ||
diff --git a/tests/Wallabag/CoreBundle/Command/GenerateUrlHashesCommandTest.php b/tests/Wallabag/CoreBundle/Command/GenerateUrlHashesCommandTest.php index cc1e3fbc..17eed210 100644 --- a/tests/Wallabag/CoreBundle/Command/GenerateUrlHashesCommandTest.php +++ b/tests/Wallabag/CoreBundle/Command/GenerateUrlHashesCommandTest.php | |||
@@ -72,11 +72,8 @@ class GenerateUrlHashesCommandTest extends WallabagCoreTestCase | |||
72 | $entry1->setUrl($url); | 72 | $entry1->setUrl($url); |
73 | 73 | ||
74 | $em->persist($entry1); | 74 | $em->persist($entry1); |
75 | |||
76 | $em->flush(); | 75 | $em->flush(); |
77 | 76 | ||
78 | $this->assertNull($entry1->getHashedUrl()); | ||
79 | |||
80 | $application = new Application($this->getClient()->getKernel()); | 77 | $application = new Application($this->getClient()->getKernel()); |
81 | $application->add(new GenerateUrlHashesCommand()); | 78 | $application->add(new GenerateUrlHashesCommand()); |
82 | 79 | ||
@@ -92,7 +89,7 @@ class GenerateUrlHashesCommandTest extends WallabagCoreTestCase | |||
92 | 89 | ||
93 | $entry = $em->getRepository('WallabagCoreBundle:Entry')->findOneByUrl($url); | 90 | $entry = $em->getRepository('WallabagCoreBundle:Entry')->findOneByUrl($url); |
94 | 91 | ||
95 | $this->assertSame($entry->getHashedUrl(), hash('md5', $url)); | 92 | $this->assertSame($entry->getHashedUrl(), hash('sha1', $url)); |
96 | 93 | ||
97 | $query = $em->createQuery('DELETE FROM Wallabag\CoreBundle\Entity\Entry e WHERE e.url = :url'); | 94 | $query = $em->createQuery('DELETE FROM Wallabag\CoreBundle\Entity\Entry e WHERE e.url = :url'); |
98 | $query->setParameter('url', $url); | 95 | $query->setParameter('url', $url); |