Refactor rest_utils

* Method rest_utils.get_all() was not composed efficiently enough.
  It applied 'resource_function' to potentially load additional
  fields even if only specific fields were requested by a user.
  This patch fixes this by splitting the whole logic of the method
  into cases when specific fields are requested and when they are
  not.
* Added the method from_tuples() to rest resources to optimize
  loading of db objects in case if only specific fields are
  requested. Method from_dict() and from_db_model() are now
  based on it.

Change-Id: I318c07f0c86e8aec404ae2357c3c10bf3ca6bed3

mistral/api/controllers/resource.py

@@ -36,27 +36,24 @@ class Resource(wtypes.Base):
         return d
 
     @classmethod
-    def from_dict(cls, d):
+    def from_tuples(cls, tuple_iterator):
         obj = cls()
 
-        for key, val in d.items():
-            if hasattr(obj, key):
-                # Convert all datetime values to strings.
-                setattr(obj, key, utils.datetime_to_str(val))
-
-        return obj
-
-    @classmethod
-    def from_db_model(cls, db_model):
-        obj = cls()
-
-        for col_name, col_val in db_model.iter_columns():
+        for col_name, col_val in tuple_iterator:
             if hasattr(obj, col_name):
                 # Convert all datetime values to strings.
                 setattr(obj, col_name, utils.datetime_to_str(col_val))
 
         return obj
 
+    @classmethod
+    def from_dict(cls, d):
+        return cls.from_tuples(d.items())
+
+    @classmethod
+    def from_db_model(cls, db_model):
+        return cls.from_tuples(db_model.iter_columns())
+
     def __str__(self):
         """WSME based implementation of __str__."""
 

mistral/utils/rest_utils.py

@@ -44,6 +44,7 @@ def wrap_wsme_controller_exception(func):
             pecan.response.translatable_error = e
 
             LOG.error('Error during API call: %s' % str(e))
+
             raise wsme_exc.ClientSideError(
                 msg=six.text_type(e),
                 status_code=e.http_code
@@ -64,6 +65,7 @@ def wrap_pecan_controller_exception(func):
             return func(*args, **kwargs)
         except (exc.MistralException, exc.MistralError) as e:
             LOG.error('Error during API call: %s' % str(e))
+
             return webob.Response(
                 status=e.http_code,
                 content_type='application/json',
@@ -129,8 +131,9 @@ def get_all(list_cls, cls, get_all_function, get_function,
             all_projects=False, **filters):
     """Return a list of cls.
 
-    :param list_cls: Collection class (e.g.: Actions, Workflows, ...).
-    :param cls: Class (e.g.: Action, Workflow, ...).
+    :param list_cls: REST Resource collection class (e.g.: Actions,
+        Workflows, ...)
+    :param cls: REST Resource class (e.g.: Action, Workflow, ...)
     :param get_all_function: Request function to get all elements with
                              filtering (limit, marker, sort_keys, sort_dirs,
                              fields)
@@ -171,40 +174,11 @@ def get_all(list_cls, cls, get_all_function, get_function,
     if marker:
         marker_obj = get_function(marker)
 
-    list_to_return = []
+    rest_resources = []
 
-    if resource_function:
-        with db_api.transaction():
-            # Do not filter fields yet, resource_function needs the ORM object.
-            db_list = get_all_function(
-                limit=limit,
-                marker=marker_obj,
-                sort_keys=sort_keys,
-                sort_dirs=sort_dirs,
-                insecure=insecure,
-                **filters
-            )
-
-            for data in db_list:
-                obj = resource_function(data)
-
-                # Filter fields using a loop instead of the ORM.
-                if fields:
-                    data = []
-
-                    for f in fields:
-                        if hasattr(obj, f):
-                            data.append(getattr(obj, f))
-
-                    # TODO(rakhmerov): This still can be expensive
-                    # due to creation of extra dictionary.
-                    list_to_return.append(
-                        cls.from_dict(dict(zip(fields, data)))
-                    )
-                else:
-                    list_to_return.append(obj)
-
-    else:
+    # If only certain fields are requested then we ignore "resource_function"
+    # parameter because it doesn't make sense anymore.
+    if fields:
         db_list = get_all_function(
             limit=limit,
             marker=marker_obj,
@@ -215,16 +189,33 @@ def get_all(list_cls, cls, get_all_function, get_function,
             **filters
         )
 
-        for data in db_list:
-            if fields:
-                list_to_return.append(
-                    cls.from_dict(dict(zip(fields, data)))
-                )
-            else:
-                list_to_return.append(cls.from_db_model(data))
+        for obj_values in db_list:
+            # Note: in case if only certain fields have been requested
+            # "db_list" contains tuples with values of db objects.
+            rest_resources.append(
+                cls.from_tuples(zip(fields, obj_values))
+            )
+    else:
+        with db_api.transaction():
+            db_models = get_all_function(
+                limit=limit,
+                marker=marker_obj,
+                sort_keys=sort_keys,
+                sort_dirs=sort_dirs,
+                insecure=insecure,
+                **filters
+            )
+
+            for db_model in db_models:
+                if resource_function:
+                    rest_resource = resource_function(db_model)
+                else:
+                    rest_resource = cls.from_db_model(db_model)
+
+                rest_resources.append(rest_resource)
 
     return list_cls.convert_with_links(
-        list_to_return,
+        rest_resources,
         limit,
         pecan.request.host_url,
         sort_keys=','.join(sort_keys),