Merge "Fix slow SG api calls when limiting fields" into stable/queens

neutron/db/securitygroups_db.py

@@ -161,7 +161,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
             sorts=sorts, limit=limit, marker=marker, page_reverse=page_reverse)
 
         sg_objs = sg_obj.SecurityGroup.get_objects(
-            context, _pager=pager, validate_filters=False, **filters)
+            context, _pager=pager, validate_filters=False,
+            fields=fields, **filters)
 
         return [self._make_security_group_dict(obj, fields) for obj in sg_objs]
 
@@ -176,7 +177,6 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
         """Tenant id is given to handle the case when creating a security
         group rule on behalf of another use.
         """
-
         if tenant_id:
             tmp_context_tenant_id = context.tenant_id
             context.tenant_id = tenant_id
@@ -184,16 +184,22 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
         try:
             with db_api.context_manager.reader.using(context):
                 ret = self._make_security_group_dict(self._get_security_group(
-                                                     context, id), fields)
+                                                     context, id,
-                ret['security_group_rules'] = self.get_security_group_rules(
+                                                     fields=fields),
-                    context, {'security_group_id': [id]})
+                                                     fields)
+                if (fields is None or len(fields) == 0 or
+                   'security_group_rules' in fields):
+                    rules = self.get_security_group_rules(context,
+                        {'security_group_id': [id]})
+                    ret['security_group_rules'] = rules
+
         finally:
             if tenant_id:
                 context.tenant_id = tmp_context_tenant_id
         return ret
 
-    def _get_security_group(self, context, id):
+    def _get_security_group(self, context, id, fields=None):
-        sg = sg_obj.SecurityGroup.get_object(context, id=id)
+        sg = sg_obj.SecurityGroup.get_object(context, fields=fields, id=id)
         if sg is None:
             raise ext_sg.SecurityGroupNotFound(id=id)
         return sg
@@ -218,7 +224,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
             if ports:
                 raise ext_sg.SecurityGroupInUse(id=id)
             # confirm security group exists
-            sg = self._get_security_group(context, id)
+            sg = self._get_security_group(context, id, fields=['id', 'name'])
 
             if sg['name'] == 'default' and not context.is_admin:
                 raise ext_sg.SecurityGroupCannotRemoveDefault()
@@ -291,10 +297,11 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
                'name': security_group['name'],
                'tenant_id': security_group['tenant_id'],
                'description': security_group['description']}
-        res['security_group_rules'] = [
+        if security_group.rules:
-            self._make_security_group_rule_dict(r.db_obj)
+            res['security_group_rules'] = [
-            for r in security_group.rules
+                self._make_security_group_rule_dict(r.db_obj)
-        ]
+                for r in security_group.rules
+            ]
         resource_extend.apply_funcs(ext_sg.SECURITYGROUPS, res,
                                     security_group.db_obj)
         return db_utils.resource_fields(res, fields)

neutron/objects/base.py

@@ -137,6 +137,7 @@ class NeutronObject(obj_base.VersionedObject,
 
     def __init__(self, context=None, **kwargs):
         super(NeutronObject, self).__init__(context, **kwargs)
+        self._load_synthetic_fields = True
         self.obj_set_defaults()
 
     def _synthetic_fields_items(self):
@@ -223,7 +224,7 @@ class NeutronObject(obj_base.VersionedObject,
         return obj
 
     @classmethod
-    def get_object(cls, context, **kwargs):
+    def get_object(cls, context, fields=None, **kwargs):
         raise NotImplementedError()
 
     @classmethod
@@ -252,7 +253,7 @@ class NeutronObject(obj_base.VersionedObject,
     @classmethod
     @abc.abstractmethod
     def get_objects(cls, context, _pager=None, validate_filters=True,
-                    **kwargs):
+                    fields=None, **kwargs):
         raise NotImplementedError()
 
     @classmethod
@@ -443,7 +444,8 @@ class NeutronDbObject(NeutronObject):
         for field in self.fields:
             if field in fields and not self.is_synthetic(field):
                 setattr(self, field, fields[field])
-        self.load_synthetic_db_fields(db_obj)
+        if self._load_synthetic_fields:
+            self.load_synthetic_db_fields(db_obj)
         self._captured_db_model = db_obj
         self.obj_reset_changes()
 
@@ -497,8 +499,13 @@ class NeutronDbObject(NeutronObject):
         return result
 
     @classmethod
-    def _load_object(cls, context, db_obj):
+    def _load_object(cls, context, db_obj, fields=None):
         obj = cls(context)
+
+        if fields is not None and len(fields) != 0:
+            if len(set(fields).intersection(set(cls.synthetic_fields))) == 0:
+                obj._load_synthetic_fields = False
+
         obj.from_db_object(db_obj)
         # detach the model so that consequent fetches don't reuse it
         context.session.expunge(obj.db_obj)
@@ -533,12 +540,18 @@ class NeutronDbObject(NeutronObject):
         return db_api.autonested_transaction(context.session)
 
     @classmethod
-    def get_object(cls, context, **kwargs):
+    def get_object(cls, context, fields=None, **kwargs):
-        """
+        """Fetch a single object
+
         Return the first result of given context or None if the result doesn't
         contain any row. Next, convert it to a versioned object.
 
         :param context:
+        :param fields: indicate which fields the caller is interested in
+                       using. Note that currently this is limited to
+                       avoid loading synthetic fields when possible, and
+                       does not affect db queries. Default is None, which
+                       is the same as []. Example: ['id', 'name']
         :param kwargs: multiple keys defined by key=value pairs
         :return: single object of NeutronDbObject class or None
         """
@@ -553,12 +566,13 @@ class NeutronDbObject(NeutronObject):
             db_obj = obj_db_api.get_object(
                 cls, context, **cls.modify_fields_to_db(kwargs))
             if db_obj:
-                return cls._load_object(context, db_obj)
+                return cls._load_object(context, db_obj, fields=fields)
 
     @classmethod
     def get_objects(cls, context, _pager=None, validate_filters=True,
-                    **kwargs):
+                    fields=None, **kwargs):
-        """
+        """Fetch a list of objects
+
         Fetch all results from DB and convert them to versioned objects.
 
         :param context:
@@ -566,6 +580,11 @@ class NeutronDbObject(NeutronObject):
                        criteria
         :param validate_filters: Raises an error in case of passing an unknown
                                  filter
+        :param fields: indicate which fields the caller is interested in
+                       using. Note that currently this is limited to
+                       avoid loading synthetic fields when possible, and
+                       does not affect db queries. Default is None, which
+                       is the same as []. Example: ['id', 'name']
         :param kwargs: multiple keys defined by key=value pairs
         :return: list of objects of NeutronDbObject class or empty list
         """
@@ -574,7 +593,9 @@ class NeutronDbObject(NeutronObject):
         with cls.db_context_reader(context):
             db_objs = obj_db_api.get_objects(
                 cls, context, _pager=_pager, **cls.modify_fields_to_db(kwargs))
-            return [cls._load_object(context, db_obj) for db_obj in db_objs]
+
+            return [cls._load_object(context, db_obj, fields=fields)
+                    for db_obj in db_objs]
 
     @classmethod
     def update_object(cls, context, values, validate_filters=True, **kwargs):

neutron/objects/securitygroup.py

@@ -59,8 +59,10 @@ class SecurityGroup(base.NeutronDbObject):
 
     def from_db_object(self, db_obj):
         super(SecurityGroup, self).from_db_object(db_obj)
-        setattr(self, 'is_default', bool(db_obj.get('default_security_group')))
+        if self._load_synthetic_fields:
-        self.obj_reset_changes(['is_default'])
+            setattr(self, 'is_default',
+                    bool(db_obj.get('default_security_group')))
+            self.obj_reset_changes(['is_default'])
 
 
 @base.NeutronObjectRegistry.register

neutron/tests/unit/objects/test_securitygroup.py

@@ -97,6 +97,38 @@ class SecurityGroupDbObjTestCase(test_base.BaseDbObjectTestCase,
         # follow-up patch.
         pass
 
+    def test_get_object_no_synth(self):
+        fields = self.obj_fields[0].copy()
+        sg_obj = self._make_object(fields)
+        sg_obj.is_default = True
+        sg_obj.create()
+
+        listed_obj = securitygroup.SecurityGroup.get_object(
+            self.context,
+            fields=['id', 'name'],
+            id=sg_obj.id,
+            project_id=sg_obj.project_id
+        )
+        self.assertIsNotNone(listed_obj)
+        self.assertEqual(len(sg_obj.rules), 0)
+        self.assertIsNone(listed_obj.rules)
+
+    def test_get_objects_no_synth(self):
+        fields = self.obj_fields[0].copy()
+        sg_obj = self._make_object(fields)
+        sg_obj.is_default = True
+        sg_obj.create()
+
+        listed_objs = securitygroup.SecurityGroup.get_objects(
+            self.context,
+            fields=['id', 'name'],
+            id=sg_obj.id,
+            project_id=sg_obj.project_id
+        )
+        self.assertEqual(len(listed_objs), 1)
+        self.assertEqual(len(sg_obj.rules), 0)
+        self.assertIsNone(listed_objs[0].rules)
+
 
 class DefaultSecurityGroupIfaceObjTestCase(test_base.BaseObjectIfaceTestCase):