Fix overwriting of existing tags while creating new tags

It was observed that md-tag-create-multiple
(/v2/metadefs/namespaces/{namespace_name}/tags) API overwrites
existing tags for specified namespace rather than creating new one
in addition to the existing tags.
This patch resolves the issue by introducing a header 'X-Openstack-Append'
which on being True will append the new tags to existing ones and
if False will continue to overwrite the tags.

Implements: blueprint append-tags
Closes-Bug: #1939169
Change-Id: I29448746b14c542e5fbf0283011968ae1516642e

api-ref/source/v2/metadefs-namespaces-tags.inc

@@ -191,6 +191,7 @@ Request
 
 .. rest_parameters:: metadefs-parameters.yaml
 
+   - X-Openstack-Append: append
    - namespace_name: namespace_name
    - tags: tags
 

api-ref/source/v2/metadefs-parameters.yaml

@@ -1,4 +1,11 @@
 # variables in header
+append:
+  description: |
+    If present and set to True, new metadefs tags are appended to the existing ones.
+    Otherwise, existing tags are overwritten.
+  in: header
+  required: false
+  type: string
 Content-Type-json:
   description: |
     The media type descriptor for the request body.  Use

glance/api/v2/metadef_tags.py

@@ -18,6 +18,7 @@ import http.client as http
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import encodeutils
+from oslo_utils import strutils
 import webob.exc
 from wsme.rest import json
 
@@ -120,11 +121,14 @@ class TagsController(object):
                 md_resource=namespace_obj,
                 enforcer=self.policy).add_metadef_tags()
 
+            can_append = strutils.bool_from_string(req.headers.get(
+                'X-Openstack-Append'))
+
             tag_list = []
             for metadata_tag in metadata_tags.tags:
                 tag_list.append(tag_factory.new_tag(
                     namespace=namespace, **metadata_tag.to_dict()))
-            tag_repo.add_tags(tag_list)
+            tag_repo.add_tags(tag_list, can_append)
             tag_list_out = [MetadefTag(**{'name': db_metatag.name})
                             for db_metatag in tag_list]
             metadef_tags = MetadefTags()

glance/db/__init__.py

@@ -846,7 +846,7 @@ class MetadefTagRepo(object):
             self._format_metadef_tag_to_db(metadata_tag)
         )
 
-    def add_tags(self, metadata_tags):
+    def add_tags(self, metadata_tags, can_append=False):
         tag_list = []
         namespace = None
         for metadata_tag in metadata_tags:
@@ -855,7 +855,7 @@ class MetadefTagRepo(object):
                 namespace = metadata_tag.namespace
 
         self.db_api.metadef_tag_create_tags(
-            self.context, namespace, tag_list)
+            self.context, namespace, tag_list, can_append)
 
     def get(self, namespace, name):
         try:

glance/db/simple/api.py

@@ -1910,7 +1910,8 @@ def metadef_tag_create(context, namespace_name, values):
 
 
 @log_call
-def metadef_tag_create_tags(context, namespace_name, tag_list):
+def metadef_tag_create_tags(context, namespace_name, tag_list,
+                            can_append=False):
     """Create a metadef tag"""
     global DATA
 
@@ -1921,6 +1922,12 @@ def metadef_tag_create_tags(context, namespace_name, tag_list):
     allowed_attributes = ['name']
     data_tag_list = []
     tag_name_list = []
+    if can_append:
+        # NOTE(mrjoshi): We need to fetch existing tags here for duplicate
+        # check while adding new one
+        tag_name_list = [tag['name']
+                         for tag in metadef_tag_get_all(context,
+                                                        namespace_name)]
     for tag_value in tag_list:
         tag_values = copy.deepcopy(tag_value)
         tag_name = tag_values['name']
@@ -1945,8 +1952,8 @@ def metadef_tag_create_tags(context, namespace_name, tag_list):
 
         tag_values['namespace_id'] = namespace['id']
         data_tag_list.append(_format_tag(tag_values))
-
+    if not can_append:
-    DATA['metadef_tags'] = []
+        DATA['metadef_tags'] = []
     for tag in data_tag_list:
         DATA['metadef_tags'].append(tag)
 

glance/db/sqlalchemy/api.py

@@ -2180,11 +2180,11 @@ def metadef_tag_create(context, namespace_name, tag_dict,
 
 
 def metadef_tag_create_tags(context, namespace_name, tag_list,
-                            session=None):
+                            can_append=False, session=None):
     """Create a metadata-schema tag or raise if it already exists."""
     session = get_session()
     return metadef_tag_api.create_tags(
-        context, namespace_name, tag_list, session)
+        context, namespace_name, tag_list, can_append, session)
 
 
 @utils.no_4byte_params

glance/db/sqlalchemy/metadef_api/tag.py

@@ -111,7 +111,7 @@ def create(context, namespace_name, values, session):
     return metadef_tag.to_dict()
 
 
-def create_tags(context, namespace_name, tag_list, session):
+def create_tags(context, namespace_name, tag_list, can_append, session):
 
     metadef_tags_list = []
     if tag_list:
@@ -119,10 +119,10 @@ def create_tags(context, namespace_name, tag_list, session):
 
         try:
             with session.begin():
-                query = (session.query(models.MetadefTag).filter_by(
+                if not can_append:
-                    namespace_id=namespace['id']))
+                    query = (session.query(models.MetadefTag).filter_by(
-                query.delete(synchronize_session='fetch')
+                             namespace_id=namespace['id']))
-
+                    query.delete(synchronize_session='fetch')
                 for value in tag_list:
                     value.update({'namespace_id': namespace['id']})
                     metadef_utils.drop_protected_attrs(

glance/domain/proxy.py

@@ -547,11 +547,11 @@ class MetadefTagRepo(object):
     def add(self, meta_tag):
         self.base.add(self.tag_proxy_helper.unproxy(meta_tag))
 
-    def add_tags(self, meta_tags):
+    def add_tags(self, meta_tags, can_append=False):
         tags_list = []
         for meta_tag in meta_tags:
             tags_list.append(self.tag_proxy_helper.unproxy(meta_tag))
-        self.base.add_tags(tags_list)
+        self.base.add_tags(tags_list, can_append)
 
     def list(self, *args, **kwargs):
         tags = self.base.list(*args, **kwargs)

glance/notifier.py

@@ -913,8 +913,9 @@ class MetadefTagRepoProxy(NotificationRepoProxy, domain_proxy.MetadefTagRepo):
         self.send_notification('metadef_tag.create', metadef_tag)
         return result
 
-    def add_tags(self, metadef_tags):
+    def add_tags(self, metadef_tags, can_append=False):
-        result = super(MetadefTagRepoProxy, self).add_tags(metadef_tags)
+        result = super(MetadefTagRepoProxy, self).add_tags(metadef_tags,
+                                                           can_append)
         for metadef_tag in metadef_tags:
             self.send_notification('metadef_tag.create', metadef_tag)
 

glance/tests/functional/db/base_metadef.py

@@ -590,6 +590,34 @@ class MetadefTagTests(object):
         expected = set(['Tag1', 'Tag2', 'Tag3'])
         self.assertEqual(expected, actual)
 
+    def test_tag_create_tags_with_append(self):
+        fixture = build_namespace_fixture()
+        created_ns = self.db_api.metadef_namespace_create(self.context,
+                                                          fixture)
+        self.assertIsNotNone(created_ns)
+        self._assert_saved_fields(fixture, created_ns)
+
+        tags = build_tags_fixture(['Tag1', 'Tag2', 'Tag3'])
+        created_tags = self.db_api.metadef_tag_create_tags(
+            self.context, created_ns['namespace'], tags)
+        actual = set([tag['name'] for tag in created_tags])
+        expected = set(['Tag1', 'Tag2', 'Tag3'])
+        self.assertEqual(expected, actual)
+
+        new_tags = build_tags_fixture(['Tag4', 'Tag5', 'Tag6'])
+        new_created_tags = self.db_api.metadef_tag_create_tags(
+            self.context, created_ns['namespace'], new_tags, can_append=True)
+        actual = set([tag['name'] for tag in new_created_tags])
+        expected = set(['Tag4', 'Tag5', 'Tag6'])
+        self.assertEqual(expected, actual)
+
+        tags = self.db_api.metadef_tag_get_all(self.context,
+                                               created_ns['namespace'],
+                                               sort_key='created_at')
+        actual = set([tag['name'] for tag in tags])
+        expected = set(['Tag1', 'Tag2', 'Tag3', 'Tag4', 'Tag5', 'Tag6'])
+        self.assertEqual(expected, actual)
+
     def test_tag_create_duplicate_tags_1(self):
         fixture = build_namespace_fixture()
         created_ns = self.db_api.metadef_namespace_create(self.context,
@@ -619,6 +647,22 @@ class MetadefTagTests(object):
                           self.db_api.metadef_tag_create,
                           self.context, created_ns['namespace'], dup_tag)
 
+    def test_tag_create_duplicate_tags_3(self):
+        fixture = build_namespace_fixture()
+        created_ns = self.db_api.metadef_namespace_create(self.context,
+                                                          fixture)
+        self.assertIsNotNone(created_ns)
+        self._assert_saved_fields(fixture, created_ns)
+
+        tags = build_tags_fixture(['Tag1', 'Tag2', 'Tag3'])
+        self.db_api.metadef_tag_create_tags(self.context,
+                                            created_ns['namespace'], tags)
+        dup_tags = build_tags_fixture(['Tag3', 'Tag4', 'Tag5'])
+        self.assertRaises(exception.Duplicate,
+                          self.db_api.metadef_tag_create_tags,
+                          self.context, created_ns['namespace'],
+                          dup_tags, can_append=True)
+
     def test_tag_get(self):
         fixture_ns = build_namespace_fixture()
         created_ns = self.db_api.metadef_namespace_create(self.context,

glance/tests/functional/v2/test_metadef_tags.py

@@ -160,6 +160,36 @@ class TestMetadefTags(metadef_base.MetadefFunctionalTestBase):
         tags = jsonutils.loads(response.text)['tags']
         self.assertEqual(3, len(tags))
 
+        # Create new tags and append to existing tags.
+        path = self._url('/v2/metadefs/namespaces/%s/tags' %
+                         (namespace_name))
+        headers = self._headers({'content-type': 'application/json',
+                                 'X-Openstack-Append': 'True'})
+        data = jsonutils.dumps(
+            {"tags": [{"name": "tag4"}, {"name": "tag5"}, {"name": "tag6"}]}
+        )
+        response = requests.post(path, headers=headers, data=data)
+        self.assertEqual(http.CREATED, response.status_code)
+
+        # List out all six tags.
+        response = requests.get(path, headers=self._headers())
+        self.assertEqual(http.OK, response.status_code)
+        tags = jsonutils.loads(response.text)['tags']
+        self.assertEqual(6, len(tags))
+
+        # Attempt to create duplicate existing tag6
+        data = jsonutils.dumps(
+            {"tags": [{"name": "tag6"}, {"name": "tag7"}, {"name": "tag8"}]}
+        )
+        response = requests.post(path, headers=headers, data=data)
+        self.assertEqual(http.CONFLICT, response.status_code)
+
+        # Verify the previous 6 still exist
+        response = requests.get(path, headers=self._headers())
+        self.assertEqual(http.OK, response.status_code)
+        tags = jsonutils.loads(response.text)['tags']
+        self.assertEqual(6, len(tags))
+
     def _create_tags(self, namespaces):
         tags = []
         for namespace in namespaces:

glance/tests/unit/test_db_metadef.py

@@ -515,6 +515,18 @@ class TestMetadefRepo(test_utils.BaseTestCase):
         tag_names = set([t.name for t in tags])
         self.assertEqual(set([TAG3, TAG4, TAG5]), tag_names)
 
+    def test_add_tags_with_append_true(self):
+        tags = self.tag_repo.list(filters={'namespace': NAMESPACE1})
+        tag_names = set([t.name for t in tags])
+        self.assertEqual(set([TAG1, TAG2, TAG3]), tag_names)
+
+        tags = _db_tags_fixture([TAG4, TAG5])
+        self.db.metadef_tag_create_tags(self.context, NAMESPACE1, tags,
+                                        can_append=True)
+        tags = self.tag_repo.list(filters={'namespace': NAMESPACE1})
+        tag_names = set([t.name for t in tags])
+        self.assertEqual(set([TAG1, TAG2, TAG3, TAG4, TAG5]), tag_names)
+
     def test_add_duplicate_tags_with_pre_existing_tags(self):
         tags = self.tag_repo.list(filters={'namespace': NAMESPACE1})
         tag_names = set([t.name for t in tags])

glance/tests/unit/test_policy.py

@@ -239,7 +239,7 @@ class MdTagRepoStub(object):
     def add(self, tag):
         return 'mdtag_add'
 
-    def add_tags(self, tags):
+    def add_tags(self, tags, can_append=False):
         return ['mdtag_add_tags']
 
     def get(self, ns, tag_name):

glance/tests/unit/utils.py

@@ -64,7 +64,7 @@ def sort_url_by_qs_keys(url):
 
 
 def get_fake_request(path='', method='POST', is_admin=False, user=USER1,
-                     roles=None, tenant=TENANT1):
+                     roles=None, headers=None, tenant=TENANT1):
     if roles is None:
         roles = ['member', 'reader']
 
@@ -72,6 +72,9 @@ def get_fake_request(path='', method='POST', is_admin=False, user=USER1,
     req.method = method
     req.headers = {'x-openstack-request-id': 'my-req'}
 
+    if headers is not None:
+        req.headers.update(headers)
+
     kwargs = {
         'user': user,
         'tenant': tenant,

glance/tests/unit/v2/test_metadef_resources.py

@@ -2002,6 +2002,94 @@ class TestMetadefsControllers(base.IsolatedUnitTest):
             ]
         )
 
+    def test_tag_create_tags_with_append_true(self):
+        request = unit_test_utils.get_fake_request(
+            headers={'X-Openstack-Append': 'True'}, roles=['admin'])
+
+        metadef_tags = tags.MetadefTags()
+        # As TAG1 is already created in setup, just creating other two tags.
+        metadef_tags.tags = _db_tags_fixture([TAG2, TAG3])
+        output = self.tag_controller.create_tags(
+            request, metadef_tags, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(2, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG2, TAG3])
+        self.assertEqual(expected, actual)
+        self.assertNotificationLog(
+            "metadef_tag.create", [
+                {'name': TAG2, 'namespace': NAMESPACE1},
+                {'name': TAG3, 'namespace': NAMESPACE1},
+            ]
+        )
+
+        metadef_tags = tags.MetadefTags()
+        metadef_tags.tags = _db_tags_fixture([TAG4, TAG5])
+        output = self.tag_controller.create_tags(
+            request, metadef_tags, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(2, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG4, TAG5])
+        self.assertEqual(expected, actual)
+        self.assertNotificationLog(
+            "metadef_tag.create", [
+                {'name': TAG4, 'namespace': NAMESPACE1},
+                {'name': TAG5, 'namespace': NAMESPACE1},
+            ]
+        )
+
+        output = self.tag_controller.index(request, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(5, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG1, TAG2, TAG3, TAG4, TAG5])
+        self.assertEqual(expected, actual)
+
+    def test_tag_create_tags_with_append_false(self):
+        request = unit_test_utils.get_fake_request(
+            headers={'X-Openstack-Append': 'False'}, roles=['admin'])
+
+        metadef_tags = tags.MetadefTags()
+        # As TAG1 is already created in setup, just creating other two tags.
+        metadef_tags.tags = _db_tags_fixture([TAG2, TAG3])
+        output = self.tag_controller.create_tags(
+            request, metadef_tags, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(2, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG2, TAG3])
+        self.assertEqual(expected, actual)
+        self.assertNotificationLog(
+            "metadef_tag.create", [
+                {'name': TAG2, 'namespace': NAMESPACE1},
+                {'name': TAG3, 'namespace': NAMESPACE1},
+            ]
+        )
+
+        metadef_tags = tags.MetadefTags()
+        metadef_tags.tags = _db_tags_fixture([TAG4, TAG5])
+        output = self.tag_controller.create_tags(
+            request, metadef_tags, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(2, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG4, TAG5])
+        self.assertEqual(expected, actual)
+        self.assertNotificationLog(
+            "metadef_tag.create", [
+                {'name': TAG4, 'namespace': NAMESPACE1},
+                {'name': TAG5, 'namespace': NAMESPACE1},
+            ]
+        )
+
+        output = self.tag_controller.index(request, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(2, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG4, TAG5])
+        self.assertEqual(expected, actual)
+
     def test_tag_create_duplicate_tags(self):
         request = unit_test_utils.get_fake_request(roles=['admin'])
 
@@ -2048,6 +2136,42 @@ class TestMetadefsControllers(base.IsolatedUnitTest):
         expected = set([TAG1, TAG2, TAG3])
         self.assertEqual(expected, actual)
 
+    def test_tag_create_duplicate_with_pre_existing_tags_with_append(self):
+        request = unit_test_utils.get_fake_request(
+            headers={'X-Openstack-Append': 'True'}, roles=['admin'])
+
+        metadef_tags = tags.MetadefTags()
+        # As TAG1 is already created in setup, just creating other two tags.
+        metadef_tags.tags = _db_tags_fixture([TAG2, TAG3])
+        output = self.tag_controller.create_tags(
+            request, metadef_tags, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(2, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG2, TAG3])
+        self.assertEqual(expected, actual)
+        self.assertNotificationLog(
+            "metadef_tag.create", [
+                {'name': TAG2, 'namespace': NAMESPACE1},
+                {'name': TAG3, 'namespace': NAMESPACE1},
+            ]
+        )
+
+        metadef_tags = tags.MetadefTags()
+        metadef_tags.tags = _db_tags_fixture([TAG4, TAG5, TAG4])
+        self.assertRaises(
+            webob.exc.HTTPConflict,
+            self.tag_controller.create_tags,
+            request, metadef_tags, NAMESPACE1)
+        self.assertNotificationsLog([])
+
+        output = self.tag_controller.index(request, NAMESPACE1)
+        output = output.to_dict()
+        self.assertEqual(3, len(output['tags']))
+        actual = set([tag.name for tag in output['tags']])
+        expected = set([TAG1, TAG2, TAG3])
+        self.assertEqual(expected, actual)
+
     def test_tag_create_conflict(self):
         request = unit_test_utils.get_fake_request(roles=['admin'])
         self.assertRaises(webob.exc.HTTPConflict,

releasenotes/notes/fix-md-tag-create-multiple-c04756cf5155983d.yaml

@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    A new optional header ``X-Openstack-Append`` has been added to append the
+    new metadef tags to the existing tags. If the header is present it will
+    append the new tags to the existing one, if not then it will default to the
+    old behaviour i.e. overwriting the existing tags with the new one.
+Fixes:
+  - |
+    * Bug 1939169_: glance md-tag-create-multiple overwrites existing tags
+
+     .. _1939169: https://bugs.launchpad.net/glance/+bug/1939169
+