Merge pull request #25 from burzum/bug/duplicates

Bug/duplicates

config/Migrations/20151013144821_unique_tags.php

@@ -0,0 +1,46 @@
+<?php
+
+use Phinx\Migration\AbstractMigration;
+
+class UniqueTags extends AbstractMigration
+{
+    /**
+     * Change Method.
+     *
+     * Write your reversible migrations using this method.
+     *
+     * More information on writing migrations is available here:
+     * http://docs.phinx.org/en/latest/migrations.html#the-abstractmigration-class
+     *
+     * The following commands can be used in this method and Phinx will
+     * automatically reverse them when rolling back:
+     *
+     *    createTable
+     *    renameTable
+     *    addColumn
+     *    renameColumn
+     *    addIndex
+     *    addForeignKey
+     *
+     * Remember to call "create()" or "update()" and NOT "save()" when working
+     * with the Table class.
+     */
+    public function change()
+    {
+        $table = $this->table('tags_tags');
+
+        $table->addColumn('tag_key', 'string', [
+            'default' => null,
+            'limit' => 255,
+            'null' => false,
+        ]);
+
+        $table->addIndex(['tag_key', 'label', 'namespace'], ['unique' => true]);
+        $table->update();
+
+        $table = $this->table('tags_tagged');
+
+        $table->addIndex(['tag_id', 'fk_id', 'fk_table'], ['unique' => true]);
+        $table->update();
+    }
+}

src/Model/Behavior/TagBehavior.php

@@ -7,6 +7,7 @@
 use Cake\ORM\Entity;
 use Cake\ORM\Table;
 use Cake\ORM\TableRegistry;
+use Cake\Utility\Inflector;
 use RuntimeException;
 
 class TagBehavior extends Behavior
@@ -222,13 +223,55 @@ public function normalizeTags($tags)
             if (empty($tag)) {
                 continue;
             }
+            $tagKey = $this->_getTagKey($tag);
+            $existingTag = $this->_tagExists($tagKey);
+            if (!empty($existingTag)) {
+                $result[] = $common + ['id' => $existingTag];
+                continue;
+            }
             list($id, $label) = $this->_normalizeTag($tag);
-            $result[] = $common + compact(empty($id) ? $df : $pk);
+            $result[] = $common + compact(empty($id) ? $df : $pk) + [
+                'tag_key' => $tagKey
+            ];
         }
 
         return $result;
     }
 
+    /**
+     * Generates the unique tag key.
+     *
+     * @param string $tag Tag label.
+     * @return string
+     */
+    protected function _getTagKey($tag)
+    {
+        return strtolower(Inflector::slug($tag));
+    }
+
+    /**
+     * Checks if a tag already exists and returns the id if yes.
+     *
+     * @param string $tag Tag key.
+     * @return null|int
+     */
+    protected function _tagExists($tag)
+    {
+        $tagsTable = $this->_table->{$this->config('tagsAlias')}->target();
+        $result = $tagsTable->find()
+            ->where([
+                $tagsTable->aliasField('tag_key') => $tag,
+            ])
+            ->select([
+                $tagsTable->aliasField($tagsTable->primaryKey())
+            ])
+            ->first();
+        if (!empty($result)) {
+            return $result->id;
+        }
+        return null;
+    }
+
     /**
      * Normalizes a tag string by trimming unnecessary whitespace and extracting the tag identifier
      * from a tag in case it exists.
@@ -238,15 +281,15 @@ public function normalizeTags($tags)
      */
     protected function _normalizeTag($tag)
     {
-        $id = null;
+        $namespace = null;
         $label = $tag;
         $separator = $this->config('separator');
         if (strpos($tag, $separator) !== false) {
-            list($id, $label) = explode($separator, $tag);
+            list($namespace, $label) = explode($separator, $tag);
         }
 
         return [
-            trim($id),
+            trim($namespace),
             trim($label)
         ];
     }

tests/Fixture/TagsFixture.php

@@ -10,6 +10,7 @@ class TagsFixture extends TestFixture
     public $fields = [
         'id' => ['type' => 'integer'],
         'namespace' => ['type' => 'string', 'length' => 255, 'null' => true],
+        'tag_key' => ['type' => 'string', 'length' => 255],
         'slug' => ['type' => 'string', 'length' => 255],
         'label' => ['type' => 'string', 'length' => 255],
         'counter' => ['type' => 'integer', 'unsigned' => true, 'default' => 0, 'null' => true],
@@ -23,12 +24,14 @@ class TagsFixture extends TestFixture
     public $records = [
         [
             'namespace' => null,
+            'tag_key' => 'color',
             'slug' => 'color',
             'label' => 'Color',
             'counter' => 3,
         ],
         [
             'namespace' => null,
+            'tag_key' => 'dark-color',
             'slug' => 'dark-color',
             'label' => 'Dark Color',
             'counter' => 2,

tests/TestCase/Model/Behavior/TagBehaviorTest.php

@@ -32,6 +32,20 @@ public function tearDown()
         unset($this->Behavior);
     }
 
+    public function testSavingDuplicates()
+    {
+        $entity = $this->Table->newEntity([
+            'name' => 'Duplicate Tags?',
+            'tags' => 'Color, Dark Color'
+        ]);
+        $this->Table->save($entity);
+        $Tags = $this->Table->Tagged->Tags;
+        $count = $Tags->find()->where(['label' => 'Color'])->count();
+        $this->assertEquals(1, $count);
+        $count = $Tags->find()->where(['label' => 'Dark Color'])->count();
+        $this->assertEquals(1, $count);
+    }
+
     public function testDefaultInitialize()
     {
         $belongsToMany = $this->Table->association('Tags');
@@ -60,51 +74,58 @@ public function testNormalizeTags()
     {
         $result = $this->Behavior->normalizeTags('foo, 3:foobar, bar');
         $expected = [
-            [
-                'label' => 'foo',
+            0 => [
                 '_joinData' => [
-                    'fk_table' => 'tags_muffins',
+                    'fk_table' => 'tags_muffins'
                 ],
+                'label' => 'foo',
+                'tag_key' => 'foo'
             ],
-            [
-                'id' => 3,
+            1 => [
                 '_joinData' => [
-                    'fk_table' => 'tags_muffins',
+                    'fk_table' => 'tags_muffins'
                 ],
+                'id' => '3',
+                'tag_key' => '3-foobar'
             ],
-            [
-                'label' => 'bar',
+            2 => [
                 '_joinData' => [
-                    'fk_table' => 'tags_muffins',
+                    'fk_table' => 'tags_muffins'
                 ],
-            ],
+                'label' => 'bar',
+                'tag_key' => 'bar'
+            ]
         ];
         $this->assertEquals($expected, $result);
 
         $result = $this->Behavior->normalizeTags(['foo', 'bar']);
         $expected = [
-            [
-                'label' => 'foo',
+            0 => [
                 '_joinData' => [
-                    'fk_table' => 'tags_muffins',
+                    'fk_table' => 'tags_muffins'
                 ],
+                'label' => 'foo',
+                'tag_key' => 'foo'
             ],
-            [
-                'label' => 'bar',
+            1 => [
                 '_joinData' => [
-                    'fk_table' => 'tags_muffins',
+                    'fk_table' => 'tags_muffins'
                 ],
-            ],
+                'label' => 'bar',
+                'tag_key' => 'bar'
+            ]
         ];
+
         $this->assertEquals($expected, $result);
 
         $result = $this->Behavior->normalizeTags('first, ');
         $expected = [
             [
-                'label' => 'first',
                 '_joinData' => [
                     'fk_table' => 'tags_muffins',
                 ],
+                'label' => 'first',
+                'tag_key' => 'first',
             ],
         ];
         $this->assertEquals($expected, $result);