rest: remove convert_metric()

This function logic is now moved inside the indexer itself, so it can
create the resource and metric in only one pass and one transaction that
can be easily rolled-back on errors.

Change-Id: I0b57adf44246bb8e84ff0e567a30667fae75f3f6
Closes-Bug: #1483634

gnocchi/indexer/sqlalchemy.py

@@ -17,6 +17,7 @@ from __future__ import absolute_import
 import itertools
 import operator
 import os.path
+import uuid
 
 from oslo_db import exception
 from oslo_db.sqlalchemy import models
@@ -340,17 +341,39 @@ class SQLAlchemyIndexer(indexer.IndexerDriver):
 
     @staticmethod
     def _set_metrics_for_resource(session, r, metrics):
-        for name, metric_id in six.iteritems(metrics):
-            try:
-                update = session.query(Metric).filter(
-                    Metric.id == metric_id,
-                    Metric.created_by_user_id == r.created_by_user_id,
-                    Metric.created_by_project_id == r.created_by_project_id,
-                ).update({"resource_id": r.id, "name": name})
-            except exception.DBDuplicateEntry:
-                raise indexer.NamedMetricAlreadyExists(name)
-            if update == 0:
-                raise indexer.NoSuchMetric(metric_id)
+        for name, value in six.iteritems(metrics):
+            if isinstance(value, uuid.UUID):
+                try:
+                    update = session.query(Metric).filter(
+                        Metric.id == value,
+                        (Metric.created_by_user_id
+                         == r.created_by_user_id),
+                        (Metric.created_by_project_id
+                         == r.created_by_project_id),
+                    ).update({"resource_id": r.id, "name": name})
+                except exception.DBDuplicateEntry:
+                    raise indexer.NamedMetricAlreadyExists(name)
+                if update == 0:
+                    raise indexer.NoSuchMetric(value)
+            else:
+                ap_name = value['archive_policy_name']
+                m = Metric(id=uuid.uuid4(),
+                           created_by_user_id=r.created_by_user_id,
+                           created_by_project_id=r.created_by_project_id,
+                           archive_policy_name=ap_name,
+                           name=name,
+                           resource_id=r.id)
+                session.add(m)
+                try:
+                    session.flush()
+                except exception.DBDuplicateEntry:
+                    raise indexer.NamedMetricAlreadyExists(name)
+                except exception.DBReferenceError as e:
+                    if (e.constraint ==
+                       'fk_metric_archive_policy_name_archive_policy_name'):
+                        raise indexer.NoSuchArchivePolicy(ap_name)
+                    raise
+
         session.expire(r, ['metrics'])
 
     def delete_resource(self, resource_id, delete_metrics=None):

gnocchi/rest/__init__.py

@@ -149,20 +149,6 @@ def Timestamp(v):
     return utils.to_timestamp(v)
 
 
-def convert_metric_list(metrics, created_by_user_id, created_by_project_id):
-    # Replace an archive policy as value for an metric by a brand
-    # a new metric
-    new_metrics = {}
-    for k, v in six.iteritems(metrics):
-        if isinstance(v, uuid.UUID):
-            new_metrics[k] = v
-        else:
-            new_metrics[k] = str(MetricsController.create_metric(
-                created_by_user_id, created_by_project_id,
-                v['archive_policy_name']).id)
-    return new_metrics
-
-
 def PositiveOrNullInt(value):
     value = int(value)
     if value < 0:
@@ -669,13 +655,14 @@ class NamedMetricController(rest.RestController):
         if not resource:
             abort(404)
         enforce("update resource", resource)
-        user, project = get_user_and_project()
-        metrics = convert_metric_list(deserialize(Metrics), user, project)
+        metrics = deserialize(Metrics)
         try:
             pecan.request.indexer.update_resource(
                 self.resource_type, self.resource_id, metrics=metrics,
                 append_metrics=True)
-        except (indexer.NoSuchMetric, ValueError) as e:
+        except (indexer.NoSuchMetric,
+                indexer.NoSuchArchivePolicy,
+                ValueError) as e:
             abort(400, e)
         except indexer.NamedMetricAlreadyExists as e:
             abort(409, e)
@@ -783,12 +770,12 @@ class GenericResourceController(rest.RestController):
         try:
             if 'metrics' in body:
                 user, project = get_user_and_project()
-                body['metrics'] = convert_metric_list(
-                    body['metrics'], user, project)
             resource = pecan.request.indexer.update_resource(
                 self._resource_type,
                 self.id, **body)
-        except (indexer.NoSuchMetric, ValueError) as e:
+        except (indexer.NoSuchMetric,
+                indexer.NoSuchArchivePolicy,
+                ValueError) as e:
             abort(400, e)
         except indexer.NoSuchResource as e:
             abort(404, e)
@@ -906,15 +893,15 @@ class GenericResourcesController(rest.RestController):
         target.update(body)
         enforce("create resource", target)
         user, project = get_user_and_project()
-        body['metrics'] = convert_metric_list(
-            body.get('metrics', {}), user, project)
         rid = body['id']
         del body['id']
         try:
             resource = pecan.request.indexer.create_resource(
                 self._resource_type, rid, user, project,
                 **body)
-        except (ValueError, indexer.NoSuchMetric) as e:
+        except (ValueError,
+                indexer.NoSuchMetric,
+                indexer.NoSuchArchivePolicy) as e:
             abort(400, e)
         except indexer.ResourceAlreadyExists as e:
             abort(409, e)

gnocchi/tests/gabbi/gabbits/resource.yaml

@@ -263,7 +263,7 @@ tests:
                   archive_policy_name: noexist
       status: 400
       response_strings:
-          - Unknown archive policy noexist
+          - Archive policy noexist does not exist
 
     - name: get patched resource
       desc: confirm the patched resource is properly patched
@@ -603,10 +603,10 @@ tests:
           content-type: application/json
       status: 400
       data:
-          electron.spin:
+          electron.charge:
               archive_policy_name: high
       response_strings:
-          - Unknown archive policy high
+          - Archive policy high does not exist
 
 # Check bad timestamps
 

gnocchi/tests/test_indexer.py

@@ -125,6 +125,17 @@ class TestIndexerDriver(tests_base.TestCase):
                           self.index.create_resource,
                           'generic', r1, user, project)
 
+    def test_create_resource_with_new_metrics(self):
+        r1 = uuid.uuid4()
+        user = uuid.uuid4()
+        project = uuid.uuid4()
+        rc = self.index.create_resource(
+            'generic', r1, user, project,
+            metrics={"foobar": {"archive_policy_name": "low"}})
+        self.assertEqual(1, len(rc.metrics))
+        m = self.index.get_metrics([rc.metrics[0].id])
+        self.assertEqual(m[0], rc.metrics[0])
+
     def _do_test_create_instance(self, server_group=None):
         r1 = uuid.uuid4()
         user = uuid.uuid4()