From 7036d91fe7332a797bf5cbccec8790bcef8437d4 Mon Sep 17 00:00:00 2001 From: Kevin Decherf Date: Sun, 21 May 2017 17:01:59 +0200 Subject: [PATCH 1/4] Tag: render tags case-insensitive by storing them in lowercase Fixes #2502 Signed-off-by: Kevin Decherf --- src/Wallabag/CoreBundle/Entity/Tag.php | 2 +- src/Wallabag/CoreBundle/Helper/TagsAssigner.php | 2 +- tests/Wallabag/CoreBundle/Controller/TagControllerTest.php | 6 ++++-- .../ImportBundle/Controller/WallabagV1ControllerTest.php | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Wallabag/CoreBundle/Entity/Tag.php b/src/Wallabag/CoreBundle/Entity/Tag.php index c19023af5..a6dc8c509 100644 --- a/src/Wallabag/CoreBundle/Entity/Tag.php +++ b/src/Wallabag/CoreBundle/Entity/Tag.php @@ -78,7 +78,7 @@ class Tag */ public function setLabel($label) { - $this->label = $label; + $this->label = mb_convert_case($label, MB_CASE_LOWER); return $this; } diff --git a/src/Wallabag/CoreBundle/Helper/TagsAssigner.php b/src/Wallabag/CoreBundle/Helper/TagsAssigner.php index a2fb0b9a9..0bfe5c572 100644 --- a/src/Wallabag/CoreBundle/Helper/TagsAssigner.php +++ b/src/Wallabag/CoreBundle/Helper/TagsAssigner.php @@ -45,7 +45,7 @@ class TagsAssigner } foreach ($tags as $label) { - $label = trim($label); + $label = trim(mb_convert_case($label, MB_CASE_LOWER)); // avoid empty tag if (0 === strlen($label)) { diff --git a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php index be25a8b5e..5a973a7e0 100644 --- a/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/TagControllerTest.php @@ -9,6 +9,7 @@ use Wallabag\CoreBundle\Entity\Tag; class TagControllerTest extends WallabagCoreTestCase { public $tagName = 'opensource'; + public $caseTagName = 'OpenSource'; public function testList() { @@ -36,7 +37,7 @@ class TagControllerTest extends WallabagCoreTestCase $form = $crawler->filter('form[name=tag]')->form(); $data = [ - 'tag[label]' => $this->tagName, + 'tag[label]' => $this->caseTagName, ]; $client->submit($form, $data); @@ -45,6 +46,7 @@ class TagControllerTest extends WallabagCoreTestCase // be sure to reload the entry $entry = $this->getEntityManager()->getRepository(Entry::class)->find($entry->getId()); $this->assertCount(1, $entry->getTags()); + $this->assertContains($this->tagName, $entry->getTags()); // tag already exists and already assigned $client->submit($form, $data); @@ -80,7 +82,7 @@ class TagControllerTest extends WallabagCoreTestCase $form = $crawler->filter('form[name=tag]')->form(); $data = [ - 'tag[label]' => 'foo2, bar2', + 'tag[label]' => 'foo2, Bar2', ]; $client->submit($form, $data); diff --git a/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php b/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php index 25625c35e..4bc982e0a 100644 --- a/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php +++ b/tests/Wallabag/ImportBundle/Controller/WallabagV1ControllerTest.php @@ -125,7 +125,7 @@ class WallabagV1ControllerTest extends WallabagCoreTestCase $tags = $content->getTags(); $this->assertContains('foot', $tags, 'It includes the "foot" tag'); - $this->assertContains('Framabag', $tags, 'It includes the "Framabag" tag'); + $this->assertContains('framabag', $tags, 'It includes the "framabag" tag'); $this->assertSame(2, count($tags)); $this->assertInstanceOf(\DateTime::class, $content->getCreatedAt()); From bd164a75c42accdc1601a69d101e759d4326e018 Mon Sep 17 00:00:00 2001 From: Kevin Decherf Date: Thu, 20 Jul 2017 22:05:44 +0200 Subject: [PATCH 2/4] Add migration to change case of tags This migration does not support SQLite as long as this engine does not support Unicode in LOWER(). This migration starts by retrieving the list of lowercase tags which need to be migrated. Then it retrieves the list of tags for each tags from the previous step in order to migrate entries. It handles deletion of empty tags. At the end the migration makes a full scan to update the label of all remaining tags. WARNING: THIS MIGRATION IS IRREVERSIBLE. Signed-off-by: Kevin Decherf --- .../Version20170719231144.php | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 app/DoctrineMigrations/Version20170719231144.php diff --git a/app/DoctrineMigrations/Version20170719231144.php b/app/DoctrineMigrations/Version20170719231144.php new file mode 100644 index 000000000..691eae515 --- /dev/null +++ b/app/DoctrineMigrations/Version20170719231144.php @@ -0,0 +1,103 @@ +container = $container; + } + + private function getTable($tableName) + { + return $this->container->getParameter('database_table_prefix').$tableName; + } + + /** + * @param Schema $schema + */ + public function up(Schema $schema) + { + $this->skipIf($this->connection->getDatabasePlatform()->getName() == 'sqlite', 'Migration can only be executed safely on \'mysql\' or \'postgresql\'.'); + + // Find tags which need to be merged + $dupTags = $this->connection->query(" + SELECT LOWER(label) + FROM ".$this->getTable('tag')." + GROUP BY LOWER(label) + HAVING COUNT(*) > 1" + ); + $dupTags->execute(); + + foreach ($dupTags->fetchAll() as $duplicates) { + $label = $duplicates['LOWER(label)']; + + // Retrieve all duplicate tags for a given tag + $tags = $this->connection->query(" + SELECT id + FROM ".$this->getTable('tag')." + WHERE LOWER(label) = '".$label."' + ORDER BY id ASC" + ); + $tags->execute(); + + $first = true; + $newId = null; + $ids = []; + + foreach ($tags->fetchAll() as $tag) { + // Ignore the first tag as we use it as the new reference tag + if ($first) { + $first = false; + $newId = $tag['id']; + } else { + $ids[] = $tag['id']; + } + } + + // Just in case... + if (count($ids) > 0) { + // Merge tags + $this->addSql(" + UPDATE ".$this->getTable('entry_tag')." + SET tag_id = ".$newId." + WHERE tag_id IN (".implode(',', $ids).")" + ); + + // Delete unused tags + $this->addSql(" + DELETE FROM ".$this->getTable('tag')." + WHERE id IN (".implode(',', $ids).")" + ); + } + } + + // Iterate over all tags to lowercase them + $this->addSql(" + UPDATE ".$this->getTable('tag')." + SET label = LOWER(label)" + ); + } + + /** + * @param Schema $schema + */ + public function down(Schema $schema) + { + throw new SkipMigrationException('Too complex ...'); + } +} From e437ad810bc0b40b6501b69d1bf81978d9c29032 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Sat, 1 Jul 2017 08:41:55 +0200 Subject: [PATCH 3/4] Fix tests Signed-off-by: Kevin Decherf --- .../Wallabag/ImportBundle/Controller/PinboardControllerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Wallabag/ImportBundle/Controller/PinboardControllerTest.php b/tests/Wallabag/ImportBundle/Controller/PinboardControllerTest.php index c307f96c9..9bb597664 100644 --- a/tests/Wallabag/ImportBundle/Controller/PinboardControllerTest.php +++ b/tests/Wallabag/ImportBundle/Controller/PinboardControllerTest.php @@ -125,7 +125,7 @@ class PinboardControllerTest extends WallabagCoreTestCase $tags = $content->getTags(); $this->assertContains('foot', $tags, 'It includes the "foot" tag'); $this->assertContains('varnish', $tags, 'It includes the "varnish" tag'); - $this->assertContains('PHP', $tags, 'It includes the "PHP" tag'); + $this->assertContains('php', $tags, 'It includes the "php" tag'); $this->assertSame(3, count($tags)); $this->assertInstanceOf(\DateTime::class, $content->getCreatedAt()); From 7b4f66881d694c948aca9372da6362b8873de6bb Mon Sep 17 00:00:00 2001 From: Kevin Decherf Date: Sun, 27 Aug 2017 16:59:02 +0200 Subject: [PATCH 4/4] php-cs-fixer on DoctrineMigrations/Version20170719231144 Signed-off-by: Kevin Decherf --- .../Version20170719231144.php | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/app/DoctrineMigrations/Version20170719231144.php b/app/DoctrineMigrations/Version20170719231144.php index 691eae515..0f5fa75ae 100644 --- a/app/DoctrineMigrations/Version20170719231144.php +++ b/app/DoctrineMigrations/Version20170719231144.php @@ -8,7 +8,7 @@ use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** - * Changed tags to lowercase + * Changed tags to lowercase. */ class Version20170719231144 extends AbstractMigration implements ContainerAwareInterface { @@ -22,24 +22,19 @@ class Version20170719231144 extends AbstractMigration implements ContainerAwareI $this->container = $container; } - private function getTable($tableName) - { - return $this->container->getParameter('database_table_prefix').$tableName; - } - /** * @param Schema $schema */ public function up(Schema $schema) { - $this->skipIf($this->connection->getDatabasePlatform()->getName() == 'sqlite', 'Migration can only be executed safely on \'mysql\' or \'postgresql\'.'); - + $this->skipIf($this->connection->getDatabasePlatform()->getName() === 'sqlite', 'Migration can only be executed safely on \'mysql\' or \'postgresql\'.'); + // Find tags which need to be merged - $dupTags = $this->connection->query(" + $dupTags = $this->connection->query(' SELECT LOWER(label) - FROM ".$this->getTable('tag')." + FROM ' . $this->getTable('tag') . ' GROUP BY LOWER(label) - HAVING COUNT(*) > 1" + HAVING COUNT(*) > 1' ); $dupTags->execute(); @@ -47,10 +42,10 @@ class Version20170719231144 extends AbstractMigration implements ContainerAwareI $label = $duplicates['LOWER(label)']; // Retrieve all duplicate tags for a given tag - $tags = $this->connection->query(" + $tags = $this->connection->query(' SELECT id - FROM ".$this->getTable('tag')." - WHERE LOWER(label) = '".$label."' + FROM ' . $this->getTable('tag') . " + WHERE LOWER(label) = '" . $label . "' ORDER BY id ASC" ); $tags->execute(); @@ -72,24 +67,24 @@ class Version20170719231144 extends AbstractMigration implements ContainerAwareI // Just in case... if (count($ids) > 0) { // Merge tags - $this->addSql(" - UPDATE ".$this->getTable('entry_tag')." - SET tag_id = ".$newId." - WHERE tag_id IN (".implode(',', $ids).")" + $this->addSql(' + UPDATE ' . $this->getTable('entry_tag') . ' + SET tag_id = ' . $newId . ' + WHERE tag_id IN (' . implode(',', $ids) . ')' ); // Delete unused tags - $this->addSql(" - DELETE FROM ".$this->getTable('tag')." - WHERE id IN (".implode(',', $ids).")" + $this->addSql(' + DELETE FROM ' . $this->getTable('tag') . ' + WHERE id IN (' . implode(',', $ids) . ')' ); } } // Iterate over all tags to lowercase them - $this->addSql(" - UPDATE ".$this->getTable('tag')." - SET label = LOWER(label)" + $this->addSql(' + UPDATE ' . $this->getTable('tag') . ' + SET label = LOWER(label)' ); } @@ -100,4 +95,9 @@ class Version20170719231144 extends AbstractMigration implements ContainerAwareI { throw new SkipMigrationException('Too complex ...'); } + + private function getTable($tableName) + { + return $this->container->getParameter('database_table_prefix') . $tableName; + } }