Ignore duplicate tags in v2 API

* Creating/updating an image with duplicate tags should result in
  only a single instance of that tag being added
* Attempting to explicitly tag an image with a duplicate tag
  should result in no tags being added
* Fixes bug 1035961

Change-Id: Ie16091f046de98914aea1a1a6777773ddc95c615

glance/api/v2/image_tags.py

@@ -28,7 +28,9 @@ class Controller(object):
 
     @utils.mutating
     def update(self, req, image_id, tag_value):
-        self.db_api.image_tag_create(req.context, image_id, tag_value)
+        context = req.context
+        if tag_value not in self.db_api.image_tag_get_all(context, image_id):
+            self.db_api.image_tag_create(context, image_id, tag_value)
 
     @utils.mutating
     def delete(self, req, image_id, tag_value):

glance/api/v2/images.py

@@ -56,7 +56,9 @@ class ImagesController(object):
 
     def _extract_tags(self, image):
         try:
-            return image.pop('tags')
+            #NOTE(bcwaldon): cast to set to make the list unique, then
+            # cast back to list since that's a more sane response type
+            return list(set(image.pop('tags')))
         except KeyError:
             pass
 

glance/tests/functional/v2/test_images.py

@@ -276,10 +276,10 @@ class TestImages(functional.FunctionalTest):
         self.stop_servers()
 
     def test_tag_lifecycle(self):
-        # Create an image with a tag
+        # Create an image with a tag - duplicate should be ignored
         path = self._url('/v2/images')
         headers = self._headers({'Content-Type': 'application/json'})
-        data = json.dumps({'name': 'image-1', 'tags': ['sniff']})
+        data = json.dumps({'name': 'image-1', 'tags': ['sniff', 'sniff']})
         response = requests.post(path, headers=headers, data=data)
         self.assertEqual(200, response.status_code)
         image_id = json.loads(response.text)['id']
@@ -291,6 +291,27 @@ class TestImages(functional.FunctionalTest):
         tags = json.loads(response.text)['tags']
         self.assertEqual(['sniff'], tags)
 
+        # Update image with duplicate tag - it should be ignored
+        path = self._url('/v2/images/%s' % image_id)
+        headers = self._headers({'Content-Type': 'application/json'})
+        data = json.dumps({'tags': ['sniff', 'snozz', 'snozz']})
+        response = requests.put(path, headers=headers, data=data)
+        self.assertEqual(200, response.status_code)
+        tags = json.loads(response.text)['tags']
+        self.assertEqual(['snozz', 'sniff'], tags)
+
+        # Image should show the appropriate tags
+        path = self._url('/v2/images/%s' % image_id)
+        response = requests.get(path, headers=self._headers())
+        self.assertEqual(200, response.status_code)
+        tags = json.loads(response.text)['tags']
+        self.assertEqual(['sniff', 'snozz'], tags)
+
+        # Attempt to tag the image with a duplicate should be ignored
+        path = self._url('/v2/images/%s/tags/snozz' % image_id)
+        response = requests.put(path, headers=self._headers())
+        self.assertEqual(204, response.status_code)
+
         # Create another more complex tag
         path = self._url('/v2/images/%s/tags/gabe%%40example.com' % image_id)
         response = requests.put(path, headers=self._headers())
@@ -301,7 +322,7 @@ class TestImages(functional.FunctionalTest):
         response = requests.get(path, headers=self._headers())
         self.assertEqual(200, response.status_code)
         tags = json.loads(response.text)['tags']
-        self.assertEqual(['sniff', 'gabe@example.com'], tags)
+        self.assertEqual(['sniff', 'snozz', 'gabe@example.com'], tags)
 
         # The tag should be deletable
         path = self._url('/v2/images/%s/tags/gabe%%40example.com' % image_id)
@@ -313,7 +334,7 @@ class TestImages(functional.FunctionalTest):
         response = requests.get(path, headers=self._headers())
         self.assertEqual(200, response.status_code)
         tags = json.loads(response.text)['tags']
-        self.assertEqual(['sniff'], tags)
+        self.assertEqual(['sniff', 'snozz'], tags)
 
         # Deleting the same tag should return a 404
         path = self._url('/v2/images/%s/tags/gabe%%40example.com' % image_id)

glance/tests/unit/v2/test_image_tags_resource.py

@@ -32,6 +32,17 @@ class TestImageTagsController(test_utils.BaseTestCase):
     def test_create_tag(self):
         request = unit_test_utils.get_fake_request()
         self.controller.update(request, unit_test_utils.UUID1, 'dink')
+        context = request.context
+        tags = self.db.image_tag_get_all(context, unit_test_utils.UUID1)
+        self.assertEqual(1, len([tag for tag in tags if tag == 'dink']))
+
+    def test_create_duplicate_tag_ignored(self):
+        request = unit_test_utils.get_fake_request()
+        self.controller.update(request, unit_test_utils.UUID1, 'dink')
+        self.controller.update(request, unit_test_utils.UUID1, 'dink')
+        context = request.context
+        tags = self.db.image_tag_get_all(context, unit_test_utils.UUID1)
+        self.assertEqual(1, len([tag for tag in tags if tag == 'dink']))
 
     def test_delete_tag(self):
         request = unit_test_utils.get_fake_request()

glance/tests/unit/v2/test_images_resource.py

@@ -312,6 +312,12 @@ class TestImagesController(test_utils.BaseTestCase):
         output = self.controller.create(request, image)
         self.assertEqual(True, output['is_public'])
 
+    def test_create_duplicate_tags(self):
+        request = unit_test_utils.get_fake_request()
+        image = {'tags': ['ping', 'ping']}
+        output = self.controller.create(request, image)
+        self.assertEqual(['ping'], output['tags'])
+
     def test_update(self):
         request = unit_test_utils.get_fake_request()
         image = {'name': 'image-2'}
@@ -325,6 +331,12 @@ class TestImagesController(test_utils.BaseTestCase):
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.update,
                           request, utils.generate_uuid(), image)
 
+    def test_update_duplicate_tags(self):
+        request = unit_test_utils.get_fake_request()
+        image = {'tags': ['ping', 'ping']}
+        output = self.controller.update(request, UUID1, image)
+        self.assertEqual(['ping'], output['tags'])
+
     def test_index_with_invalid_marker(self):
         fake_uuid = utils.generate_uuid()
         request = unit_test_utils.get_fake_request()