cleaning up instance metadata api code

5 files changed, 75 insertions, 55 deletions

diff --git a/nova/api/openstack/server_metadata.py b/nova/api/openstack/server_metadata.py
index b0b014f86..969769729 100644
--- a/nova/api/openstack/server_metadata.py
+++ b/nova/api/openstack/server_metadata.py
@@ -57,16 +57,7 @@ class Controller(object):
 
         context = req.environ['nova.context']
 
-        try:
-            self.compute_api.update_or_create_instance_metadata(context,
-                                                                server_id,
-                                                                metadata)
-        except exception.InstanceNotFound:
-            msg = _('Server does not exist')
-            raise exc.HTTPNotFound(explanation=msg)
-
-        except quota.QuotaError as error:
-            self._handle_quota_error(error)
+        self._update_instance_metadata(context, server_id, metadata, False)
 
         return body
 
@@ -88,7 +79,7 @@ class Controller(object):
             raise exc.HTTPBadRequest(explanation=expl)
 
         context = req.environ['nova.context']
-        self._set_instance_metadata(context, server_id, meta_item)
+        self._update_instance_metadata(context, server_id, meta_item, False)
 
         return {'meta': {id: meta_value}}
 
@@ -100,20 +91,23 @@ class Controller(object):
             raise exc.HTTPBadRequest(explanation=expl)
 
         context = req.environ['nova.context']
-        self._set_instance_metadata(context, server_id, metadata)
+        self._update_instance_metadata(context, server_id, metadata, True)
 
         return {'metadata': metadata}
 
-    def _set_instance_metadata(self, context, server_id, metadata):
+    def _update_instance_metadata(self, context, server_id, metadata,
+                                  delete=False):
         try:
-            self.compute_api.update_or_create_instance_metadata(context,
-                                                                server_id,
-                                                                metadata)
+            self.compute_api.update_instance_metadata(context,
+                                                      server_id,
+                                                      metadata,
+                                                      delete)
+
         except exception.InstanceNotFound:
             msg = _('Server does not exist')
             raise exc.HTTPNotFound(explanation=msg)
 
-        except ValueError:
+        except (ValueError, AttributeError):
             msg = _("Malformed request body")
             raise exc.HTTPBadRequest(explanation=msg)
 
diff --git a/nova/compute/api.py b/nova/compute/api.py
index d2c08678b..646290e57 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1175,11 +1175,20 @@ class API(base.Base):
         """Delete the given metadata item from an instance."""
         self.db.instance_metadata_delete(context, instance_id, key)
 
-    def update_or_create_instance_metadata(self, context, instance_id,
-                                            metadata):
-        """Updates or creates instance metadata."""
-        combined_metadata = self.get_instance_metadata(context, instance_id)
-        combined_metadata.update(metadata)
-        self._check_metadata_properties_quota(context, combined_metadata)
-        self.db.instance_metadata_update_or_create(context, instance_id,
-                                                    metadata)
+    def update_instance_metadata(self, context, instance_id,
+                                 metadata, delete=False):
+        """Updates or creates instance metadata.
+
+        If delete is True, metadata items that are not specified in the
+        `metadata` argument will be deleted.
+
+        """
+        if not delete:
+            _metadata = self.get_instance_metadata(context, instance_id)
+            _metadata.update(metadata)
+        else:
+            _metadata = metadata
+
+        self._check_metadata_properties_quota(context, _metadata)
+
+        self.db.instance_metadata_update(context, instance_id, _metadata, True)
diff --git a/nova/db/api.py b/nova/db/api.py
index 47308bdba..90003cf86 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1381,9 +1381,9 @@ def instance_metadata_delete(context, instance_id, key):
     IMPL.instance_metadata_delete(context, instance_id, key)
 
 
-def instance_metadata_update_or_create(context, instance_id, metadata):
-    """Create or update instance metadata."""
-    IMPL.instance_metadata_update_or_create(context, instance_id, metadata)
+def instance_metadata_update(context, instance_id, metadata, delete):
+    """Update metadata if it exists, otherwise create it."""
+    IMPL.instance_metadata_update(context, instance_id, metadata)
 
 
 ####################
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 003eb8bcb..5ae14261c 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -1345,9 +1345,10 @@ def instance_update(context, instance_id, values):
     session = get_session()
     metadata = values.get('metadata')
     if metadata is not None:
-        instance_metadata_delete_all(context, instance_id)
-        instance_metadata_update_or_create(context, instance_id,
-                                           values.pop('metadata'))
+        instance_metadata_update(context,
+                                 instance_id,
+                                 values.pop('metadata'),
+                                 delete=True)
     with session.begin():
         if utils.is_uuid_like(instance_id):
             instance_ref = instance_get_by_uuid(context, instance_id,
@@ -3201,21 +3202,37 @@ def instance_metadata_get_item(context, instance_id, key, session=None):
 
 @require_context
 @require_instance_exists
-def instance_metadata_update_or_create(context, instance_id, metadata):
+def instance_metadata_update(context, instance_id, metadata, delete):
     session = get_session()
 
-    original_metadata = instance_metadata_get(context, instance_id)
+    # Set all metadata that isn't passed in if delete kwarg is True
+    if delete:
+        original_metadata = instance_metadata_get(context, instance_id)
+        for meta in original_metadata.iteritems():
+            if meta.key not in metadata:
+                meta.update({"deleted": True})
+                meta.save(session=session)
 
     meta_ref = None
-    for key, value in metadata.iteritems():
+
+    # Now update all existing items with new values, or create new meta objects
+    for meta_key, meta_value in metadata.iteritems():
+
+        # update the value whether it exists or not
+        item = {"value": meta_value}
+
+        # if the metadata item exists, make sure it is not delete
         try:
-            meta_ref = instance_metadata_get_item(context, instance_id, key,
-                                                        session)
+            meta_ref = instance_metadata_get_item(context, instance_id,
+                                                  meta_key, session)
+            item.update({"deleted": False})
+
+        # if the item doesn't exist, we also need to set key and instance_id
         except exception.InstanceMetadataNotFound, e:
             meta_ref = models.InstanceMetadata()
-        meta_ref.update({"key": key, "value": value,
-                            "instance_id": instance_id,
-                            "deleted": False})
+            item.update({"key": meta_key, "instance_id": instance_id})
+
+        meta_ref.update(item)
         meta_ref.save(session=session)
 
     return metadata
diff --git a/nova/tests/api/openstack/test_server_metadata.py b/nova/tests/api/openstack/test_server_metadata.py
index 08a6a062a..a90d572c8 100644
--- a/nova/tests/api/openstack/test_server_metadata.py
+++ b/nova/tests/api/openstack/test_server_metadata.py
@@ -29,11 +29,11 @@ import nova.wsgi
 FLAGS = flags.FLAGS
 
 
-def return_create_instance_metadata_max(context, server_id, metadata):
+def return_create_instance_metadata_max(context, server_id, metadata, delete):
     return stub_max_server_metadata()
 
 
-def return_create_instance_metadata(context, server_id, metadata):
+def return_create_instance_metadata(context, server_id, metadata, delete):
     return stub_server_metadata()
 
 
@@ -202,7 +202,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(404, res.status_int)
 
     def test_create(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata')
         req.method = 'POST'
@@ -216,7 +216,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(expected, res_dict)
 
     def test_create_xml(self):
-        self.stubs.Set(nova.db.api, "instance_metadata_update_or_create",
+        self.stubs.Set(nova.db.api, "instance_metadata_update",
                        return_create_instance_metadata)
         req = webob.Request.blank("/v1.1/servers/1/metadata")
         req.method = "POST"
@@ -240,7 +240,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(request_metadata.toxml(), actual_metadata.toxml())
 
     def test_create_empty_body(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata')
         req.method = 'POST'
@@ -258,7 +258,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(404, res.status_int)
 
     def test_update_all(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata')
         req.method = 'PUT'
@@ -276,7 +276,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(expected, res_dict)
 
     def test_update_all_empty_container(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata')
         req.method = 'PUT'
@@ -289,7 +289,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(expected, res_dict)
 
     def test_update_all_malformed_container(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata')
         req.method = 'PUT'
@@ -300,7 +300,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(400, res.status_int)
 
     def test_update_all_malformed_data(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata')
         req.method = 'PUT'
@@ -320,7 +320,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(404, res.status_int)
 
     def test_update_item(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata/key1')
         req.method = 'PUT'
@@ -334,7 +334,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(expected, res_dict)
 
     def test_update_item_xml(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata/key9')
         req.method = 'PUT'
@@ -361,7 +361,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(404, res.status_int)
 
     def test_update_item_empty_body(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata/key1')
         req.method = 'PUT'
@@ -370,7 +370,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(400, res.status_int)
 
     def test_update_item_too_many_keys(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata/key1')
         req.method = 'PUT'
@@ -380,7 +380,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(400, res.status_int)
 
     def test_update_item_body_uri_mismatch(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         req = webob.Request.blank('/v1.1/servers/1/metadata/bad')
         req.method = 'PUT'
@@ -390,7 +390,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(400, res.status_int)
 
     def test_too_many_metadata_items_on_create(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata)
         data = {"metadata": {}}
         for num in range(FLAGS.quota_metadata_items + 1):
@@ -404,7 +404,7 @@ class ServerMetaDataTest(test.TestCase):
         self.assertEqual(400, res.status_int)
 
     def test_to_many_metadata_items_on_update_item(self):
-        self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
+        self.stubs.Set(nova.db.api, 'instance_metadata_update',
                        return_create_instance_metadata_max)
         req = webob.Request.blank('/v1.1/servers/1/metadata/key1')
         req.method = 'PUT'