Eager load resource_properties_data in resource

Eager load resource_properties_data in resources in the typical
resource-loading scenarios where properties data will be
accessed. Thus, we can save an extra db query per resource when
loading all the resources in a stack, for instance. Fall back to lazy
loading properties data in other scenarios.

Also, the resource object doesn't need to store a copy of its
ResourcePropertiesData object in self.rsrc_prop_data, so don't.

Change-Id: Ib7684af3fe06f818628fd21f1216de5047872948
Closes-Bug: #1665503

8 files changed, 102 insertions, 83 deletions

diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py
index 273ca4fbb..837517359 100644
--- a/heat/db/sqlalchemy/api.py
+++ b/heat/db/sqlalchemy/api.py
@@ -174,9 +174,14 @@ def raw_template_files_get(context, files_id):
     return result
 
 
-def resource_get(context, resource_id, refresh=False, refresh_data=False):
-    result = context.session.query(models.Resource).get(resource_id)
+def resource_get(context, resource_id, refresh=False, refresh_data=False,
+                 eager=True):
+    query = context.session.query(models.Resource)
+    if eager:
+        query = query.options(orm.joinedload("data")).options(
+            orm.joinedload("rsrc_prop_data"))
 
+    result = query.get(resource_id)
     if not result:
         raise exception.NotFound(_("resource with id %s not found") %
                                  resource_id)
@@ -443,7 +448,7 @@ def resource_get_all_by_stack(context, stack_id, filters=None):
         models.Resource
     ).filter_by(
         stack_id=stack_id
-    ).options(orm.joinedload("data"))
+    ).options(orm.joinedload("data")).options(orm.joinedload("rsrc_prop_data"))
 
     query = db_filters.exact_filter(query, models.Resource, filters)
     results = query.all()
diff --git a/heat/engine/event.py b/heat/engine/event.py
index da717587c..14c54edd3 100644
--- a/heat/engine/event.py
+++ b/heat/engine/event.py
@@ -13,14 +13,14 @@
 
 from heat.common import identifier
 from heat.objects import event as event_object
-from heat.objects import resource_properties_data as rpd_objects
 
 
 class Event(object):
     """Class representing a Resource state change."""
 
     def __init__(self, context, stack, action, status, reason,
-                 physical_resource_id, resource_properties, resource_name,
+                 physical_resource_id, resource_prop_data_id,
+                 resource_properties, resource_name,
                  resource_type, uuid=None, timestamp=None, id=None):
         """Initialise from a context, stack, and event information.
 
@@ -35,17 +35,10 @@ class Event(object):
         self.physical_resource_id = physical_resource_id
         self.resource_name = resource_name
         self.resource_type = resource_type
-        self.rsrc_prop_data = None
-        if isinstance(resource_properties,
-                      rpd_objects.ResourcePropertiesData):
-            self.rsrc_prop_data = resource_properties
-            self.resource_properties = self.rsrc_prop_data.data
-        elif resource_properties is None:
+        self.rsrc_prop_data_id = resource_prop_data_id
+        self.resource_properties = resource_properties
+        if self.resource_properties is None:
             self.resource_properties = {}
-        else:
-            raise AssertionError(
-                'resource_properties is unexpected type %s' %
-                type(resource_properties).__name__)
         self.uuid = uuid
         self.timestamp = timestamp
         self.id = id
@@ -68,8 +61,8 @@ class Event(object):
         if self.timestamp is not None:
             ev['created_at'] = self.timestamp
 
-        if self.rsrc_prop_data:
-            ev['rsrc_prop_data_id'] = self.rsrc_prop_data.id
+        if self.rsrc_prop_data_id is not None:
+            ev['rsrc_prop_data_id'] = self.rsrc_prop_data_id
 
         new_ev = event_object.Event.create(self.context, ev)
 
diff --git a/heat/engine/resource.py b/heat/engine/resource.py
index 6f255beca..2fa127f90 100644
--- a/heat/engine/resource.py
+++ b/heat/engine/resource.py
@@ -240,7 +240,7 @@ class Resource(status.ResourceStatus):
         self._data = None
         self._attr_data_id = None
         self._rsrc_metadata = None
-        self._rsrc_prop_data = None
+        self._rsrc_prop_data_id = None
         self._stored_properties_data = None
         self.created_time = stack.created_time
         self.updated_time = stack.updated_time
@@ -289,7 +289,7 @@ class Resource(status.ResourceStatus):
         self._attr_data_id = resource.attr_data_id
         self._rsrc_metadata = resource.rsrc_metadata
         self._stored_properties_data = resource.properties_data
-        self._rsrc_prop_data = resource.rsrc_prop_data
+        self._rsrc_prop_data_id = resource.rsrc_prop_data_id
         self.created_time = resource.created_at
         self.updated_time = resource.updated_at
         self.needed_by = resource.needed_by
@@ -944,7 +944,7 @@ class Resource(status.ResourceStatus):
         old_props = self._stored_properties_data
         self._stored_properties_data = function.resolve(self.properties.data)
         if self._stored_properties_data != old_props:
-            self._rsrc_prop_data = None
+            self._rsrc_prop_data_id = None
             self.attributes.reset_resolved_values()
 
     def referenced_attrs(self, stk_defn=None,
@@ -2022,8 +2022,8 @@ class Resource(status.ResourceStatus):
         """Add a state change event to the database."""
         physical_res_id = self.resource_id or self.physical_resource_name()
         ev = event.Event(self.context, self.stack, action, status, reason,
-                         physical_res_id, self._rsrc_prop_data,
-                         self.name, self.type())
+                         physical_res_id, self._rsrc_prop_data_id,
+                         self._stored_properties_data, self.name, self.type())
 
         ev.store()
         self.stack.dispatch_event(ev)
@@ -2487,16 +2487,16 @@ class Resource(status.ResourceStatus):
             return True
 
     def _create_or_replace_rsrc_prop_data(self):
-        if self._rsrc_prop_data is not None:
-            return self._rsrc_prop_data.id
+        if self._rsrc_prop_data_id is not None:
+            return self._rsrc_prop_data_id
 
         if not self._stored_properties_data:
             return None
 
-        self._rsrc_prop_data = \
+        self._rsrc_prop_data_id = \
             rpd_objects.ResourcePropertiesData(self.context).create(
-                self.context, self._stored_properties_data)
-        return self._rsrc_prop_data.id
+                self.context, self._stored_properties_data).id
+        return self._rsrc_prop_data_id
 
     def is_using_neutron(self):
         try:
diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index d28617adb..01c68211b 100644
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -922,7 +922,7 @@ class Stack(collections.Mapping):
     def _add_event(self, action, status, reason):
         """Add a state change event to the database."""
         ev = event.Event(self.context, self, action, status, reason,
-                         self.id, None,
+                         self.id, None, None,
                          self.name, 'OS::Heat::Stack')
 
         ev.store()
diff --git a/heat/objects/resource.py b/heat/objects/resource.py
index 102aa45c7..49c21ac7b 100644
--- a/heat/objects/resource.py
+++ b/heat/objects/resource.py
@@ -85,8 +85,6 @@ class Resource(
             resource_data.ResourceData,
             nullable=True
         ),
-        'rsrc_prop_data': fields.ObjectField(
-            rpd.ResourcePropertiesData, nullable=True),
         'rsrc_prop_data_id': fields.ObjectField(
             fields.IntegerField(nullable=True)),
         'engine_id': fields.StringField(nullable=True),
@@ -114,12 +112,21 @@ class Resource(
             elif field != 'attr_data':
                 resource[field] = db_resource[field]
 
-        if db_resource['rsrc_prop_data'] is not None:
-            resource['rsrc_prop_data'] = \
-                rpd.ResourcePropertiesData._from_db_object(
-                    rpd.ResourcePropertiesData(context), context,
-                    db_resource['rsrc_prop_data'])
-            resource._properties_data = resource['rsrc_prop_data'].data
+        if db_resource['rsrc_prop_data_id'] is not None:
+            if hasattr(db_resource, '__dict__'):
+                rpd_obj = db_resource.__dict__.get('rsrc_prop_data')
+            else:
+                rpd_obj = None
+            if rpd_obj is not None:
+                # Object is already eager loaded
+                rpd_obj = (
+                    rpd.ResourcePropertiesData._from_db_object(
+                        rpd.ResourcePropertiesData(),
+                        context,
+                        rpd_obj))
+                resource._properties_data = rpd_obj.data
+            else:
+                resource._properties_data = {}
             if db_resource['properties_data']:
                 LOG.error(
                     'Unexpected condition where resource.rsrc_prop_data '
@@ -155,6 +162,12 @@ class Resource(
 
     @property
     def properties_data(self):
+        if (not self._properties_data and
+                self.rsrc_prop_data_id is not None):
+            LOG.info('rsrp_prop_data lazy load')
+            rpd_obj = rpd.ResourcePropertiesData.get_by_id(
+                self._context, self.rsrc_prop_data_id)
+            self._properties_data = rpd_obj.data or {}
         return self._properties_data
 
     @classmethod
diff --git a/heat/tests/test_engine_api_utils.py b/heat/tests/test_engine_api_utils.py
index 948d8fd32..f21687da1 100644
--- a/heat/tests/test_engine_api_utils.py
+++ b/heat/tests/test_engine_api_utils.py
@@ -63,7 +63,9 @@ class FormatTest(common.HeatTestCase):
         ev = event.Event(self.context, self.stack, 'CREATE',
                          'COMPLETE', 'state changed',
                          'z3455xyc-9f88-404d-a85b-5315293e67de',
-                         res_properties, resource.name, resource.type(),
+                         resource._rsrc_prop_data_id,
+                         resource._stored_properties_data,
+                         resource.name, resource.type(),
                          uuid=ev_uuid)
         ev.store()
         return event_object.Event.get_all_by_stack(
@@ -322,7 +324,8 @@ class FormatTest(common.HeatTestCase):
         resource = self.stack['generic1']
         resource._update_stored_properties()
         resource.store()
-        event = self._dummy_event(res_properties=resource._rsrc_prop_data)
+        event = self._dummy_event(
+            res_properties=resource._stored_properties_data)
         formatted = api.format_event(event, self.stack.identifier(),
                                      include_rsrc_prop_data=True)
         self.assertEqual({'k1': 'v1'}, formatted[rpc_api.EVENT_RES_PROPERTIES])
diff --git a/heat/tests/test_event.py b/heat/tests/test_event.py
index 40f598ac3..2506d7093 100644
--- a/heat/tests/test_event.py
+++ b/heat/tests/test_event.py
@@ -85,14 +85,16 @@ class EventTest(EventCommon):
         self.resource.resource_id_set('resource_physical_id')
 
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'alabama', self.resource._rsrc_prop_data,
+                        'alabama', self.resource._rsrc_prop_data_id,
+                        self.resource._stored_properties_data,
                         self.resource.name, self.resource.type())
         e.store()
         self.assertEqual(1, len(event_object.Event.get_all_by_stack(
             self.ctx,
             self.stack.id)))
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'arizona', self.resource._rsrc_prop_data,
+                        'arizona', self.resource._rsrc_prop_data_id,
+                        self.resource._stored_properties_data,
                         self.resource.name, self.resource.type())
         e.store()
         events = event_object.Event.get_all_by_stack(self.ctx, self.stack.id)
@@ -105,7 +107,7 @@ class EventTest(EventCommon):
         self.resource.resource_id_set('resource_physical_id')
 
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'arkansas', self.resource._rsrc_prop_data,
+                        'arkansas', None, None,
                         self.resource.name, self.resource.type())
         e.store()
 
@@ -114,7 +116,7 @@ class EventTest(EventCommon):
             mock_random_uniform.return_value = 2.0 / 100 - .0001
             e = event.Event(self.ctx, self.stack, 'TEST',
                             'IN_PROGRESS', 'Testing',
-                            'alaska', self.resource._rsrc_prop_data,
+                            'alaska', None, None,
                             self.resource.name, self.resource.type())
             e.store()
         events = event_object.Event.get_all_by_stack(self.ctx, self.stack.id)
@@ -126,7 +128,7 @@ class EventTest(EventCommon):
             mock_random_uniform.return_value = 2.0 / 100 + .0001
             e = event.Event(self.ctx, self.stack, 'TEST',
                             'IN_PROGRESS', 'Testing',
-                            'aardvark', self.resource._rsrc_prop_data,
+                            'aardvark', None, None,
                             self.resource.name, self.resource.type())
             e.store()
         events = event_object.Event.get_all_by_stack(self.ctx, self.stack.id)
@@ -138,16 +140,17 @@ class EventTest(EventCommon):
         self.resource.resource_id_set('resource_physical_id')
 
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'alabama', self.resource._rsrc_prop_data,
+                        'alabama', self.resource._rsrc_prop_data_id,
+                        self.resource._stored_properties_data,
                         self.resource.name, self.resource.type())
         e.store()
-        rpd1_id = self.resource._rsrc_prop_data.id
+        rpd1_id = self.resource._rsrc_prop_data_id
 
         rpd2 = rpd_object.ResourcePropertiesData.create(
             self.ctx, {'encrypted': False, 'data': {'foo': 'bar'}})
         rpd2_id = rpd2.id
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'arizona', rpd2,
+                        'arizona', rpd2_id, rpd2.data,
                         self.resource.name, self.resource.type())
         e.store()
 
@@ -155,7 +158,7 @@ class EventTest(EventCommon):
             self.ctx, {'encrypted': False, 'data': {'foo': 'bar'}})
         rpd3_id = rpd3.id
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'arkansas', rpd3,
+                        'arkansas', rpd3_id, rpd3.data,
                         self.resource.name, self.resource.type())
         e.store()
 
@@ -163,7 +166,7 @@ class EventTest(EventCommon):
             self.ctx, {'encrypted': False, 'data': {'foo': 'bar'}})
         rpd4_id = rpd4.id
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'arkansas', rpd4,
+                        'arkansas', rpd4_id, rpd4.data,
                         self.resource.name, self.resource.type())
         e.store()
 
@@ -187,7 +190,8 @@ class EventTest(EventCommon):
     def test_identifier(self):
         event_uuid = 'abc123yc-9f88-404d-a85b-531529456xyz'
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'wibble', self.resource._rsrc_prop_data,
+                        'wibble', self.resource._rsrc_prop_data_id,
+                        self.resource._stored_properties_data,
                         self.resource.name, self.resource.type(),
                         uuid=event_uuid)
 
@@ -202,7 +206,7 @@ class EventTest(EventCommon):
 
     def test_identifier_is_none(self):
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'wibble', self.resource._rsrc_prop_data,
+                        'wibble', None, None,
                         self.resource.name, self.resource.type())
 
         self.assertIsNone(e.identifier())
@@ -211,7 +215,8 @@ class EventTest(EventCommon):
 
     def test_as_dict(self):
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'wibble', self.resource._rsrc_prop_data,
+                        'wibble', self.resource._rsrc_prop_data_id,
+                        self.resource._stored_properties_data,
                         self.resource.name, self.resource.type())
 
         e.store()
@@ -233,7 +238,8 @@ class EventTest(EventCommon):
 
     def test_load_deprecated_prop_data(self):
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'wibble', self.resource._rsrc_prop_data,
+                        'wibble', self.resource._rsrc_prop_data_id,
+                        self.resource._stored_properties_data,
                         self.resource.name, self.resource.type())
         e.store()
 
@@ -273,7 +279,8 @@ class EventEncryptedTest(EventCommon):
 
     def test_props_encrypted(self):
         e = event.Event(self.ctx, self.stack, 'TEST', 'IN_PROGRESS', 'Testing',
-                        'wibble', self.resource._rsrc_prop_data,
+                        'wibble', self.resource._rsrc_prop_data_id,
+                        self.resource._stored_properties_data,
                         self.resource.name, self.resource.type())
         e.store()
 
diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py
index bfd39ccd9..96dc8c888 100644
--- a/heat/tests/test_resource.py
+++ b/heat/tests/test_resource.py
@@ -680,13 +680,13 @@ class ResourceTest(common.HeatTestCase):
             ctx2, res_obj2.id)
 
         # verify the resource_properties_data association
-        # can be set by id or object
-        self.assertEqual(rpd_db_obj.id, res_obj1.rsrc_prop_data.id)
-        self.assertEqual(res_obj1.rsrc_prop_data.id,
-                         res_obj2.rsrc_prop_data.id)
+        # can be set by id
+        self.assertEqual(rpd_db_obj.id, res_obj1.rsrc_prop_data_id)
+        self.assertEqual(res_obj1.rsrc_prop_data_id,
+                         res_obj2.rsrc_prop_data_id)
         # properties data appears unencrypted to resource object
-        self.assertEqual(data, res_obj1.rsrc_prop_data.data)
-        self.assertEqual(data, res_obj2.rsrc_prop_data.data)
+        self.assertEqual(data, res_obj1.properties_data)
+        self.assertEqual(data, res_obj2.properties_data)
 
     def test_make_replacement(self):
         tmpl = rsrc_defn.ResourceDefinition('test_resource', 'Foo')
@@ -921,7 +921,10 @@ class ResourceTest(common.HeatTestCase):
         res.state_set(res.CREATE, res.IN_PROGRESS, 'test_rpd')
 
         # Modernity, the data is where it belongs
-        self.assertEqual(res._rsrc_prop_data.data, {'Foo': 'lucky'})
+        rsrc_prop_data_db_obj = db_api.resource_prop_data_get(
+            self.stack.context, res._rsrc_prop_data_id)
+        self.assertEqual(rsrc_prop_data_db_obj['data'], {'Foo': 'lucky'})
+        # legacy locations aren't being used anymore
         self.assertFalse(hasattr(res, 'properties_data'))
         self.assertFalse(hasattr(res, 'properties_data_encrypted'))
 
@@ -962,11 +965,14 @@ class ResourceTest(common.HeatTestCase):
         # Modernity, the data is where it belongs
         # The db object data is encrypted
         rsrc_prop_data_db_obj = db_api.resource_prop_data_get(
-            self.stack.context, res._rsrc_prop_data.id)
+            self.stack.context, res._rsrc_prop_data_id)
         self.assertNotEqual(rsrc_prop_data_db_obj['data'], {'Foo': 'lucky'})
-        self.assertEqual(rsrc_prop_data_db_obj.encrypted, True)
         # But the objects/ rsrc_prop_data.data is always unencrypted
-        self.assertEqual(res._rsrc_prop_data.data, {'Foo': 'lucky'})
+        rsrc_prop_data_obj = rpd_object.ResourcePropertiesData._from_db_object(
+            rpd_object.ResourcePropertiesData(), self.stack.context,
+            rsrc_prop_data_db_obj)
+        self.assertEqual(rsrc_prop_data_obj.data, {'Foo': 'lucky'})
+        # legacy locations aren't being used anymore
         self.assertFalse(hasattr(res, 'properties_data'))
         self.assertFalse(hasattr(res, 'properties_data_encrypted'))
 
@@ -1996,20 +2002,20 @@ class ResourceTest(common.HeatTestCase):
         # The properties data should be decrypted when the object is
         # loaded using get_obj
         res_obj = resource_objects.Resource.get_obj(res.context, res.id)
-        self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
+        self.assertEqual('string', res_obj.properties_data['prop1'])
 
         # _stored_properties_data should be decrypted when the object is
         # loaded using get_all_by_stack
         res_objs = resource_objects.Resource.get_all_by_stack(res.context,
                                                               self.stack.id)
         res_obj = res_objs['test_res_enc']
-        self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
+        self.assertEqual('string', res_obj.properties_data['prop1'])
 
         # The properties data should be decrypted when the object is
         # refreshed
         res_obj = resource_objects.Resource.get_obj(res.context, res.id)
         res_obj.refresh()
-        self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
+        self.assertEqual('string', res_obj.properties_data['prop1'])
 
     def test_properties_data_no_encryption(self):
         cfg.CONF.set_override('encrypt_parameters_and_properties', False)
@@ -2040,16 +2046,16 @@ class ResourceTest(common.HeatTestCase):
         # is loaded using get_obj
         prev_rsrc_prop_data_id = db_res.rsrc_prop_data.id
         res_obj = resource_objects.Resource.get_obj(res.context, res.id)
-        self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
-        self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data.id)
+        self.assertEqual('string', res_obj.properties_data['prop1'])
+        self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data_id)
 
         # The properties data should not be modified when the object
         # is loaded using get_all_by_stack
         res_objs = resource_objects.Resource.get_all_by_stack(res.context,
                                                               self.stack.id)
         res_obj = res_objs['test_res_enc']
-        self.assertEqual('string', res_obj.rsrc_prop_data.data['prop1'])
-        self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data.id)
+        self.assertEqual('string', res_obj.properties_data['prop1'])
+        self.assertEqual(prev_rsrc_prop_data_id, res_obj.rsrc_prop_data_id)
 
     def _assert_resource_lock(self, res_id, engine_id, atomic_key):
         rs = resource_objects.Resource.get_obj(self.stack.context, res_id)
@@ -4555,19 +4561,11 @@ class TestResourcePropDataUpdate(common.HeatTestCase):
     def test_create_or_replace_rsrc_prop_data(self):
         res = self.res
         res._stored_properties_data = self.old_rpd
-        if self.replaced:
-            res._rsrc_prop_data = None
         res.store()
-        if res._rsrc_prop_data is None:
-            old_rpd_id = -1
-        else:
-            old_rpd_id = res._rsrc_prop_data.id
-        res._stored_properties_data = self.new_rpd
-        if self.replaced:
-            res._rsrc_prop_data = None
+        old_rpd_id = res._rsrc_prop_data_id
+        with mock.patch("heat.engine.function.resolve") as mock_fr:
+            mock_fr.return_value = self.new_rpd
+            res._update_stored_properties()
         res.store()
-        if res._rsrc_prop_data is None:
-            new_rpd_id = -1
-        else:
-            new_rpd_id = res._rsrc_prop_data.id
+        new_rpd_id = res._rsrc_prop_data_id
         self.assertEqual(self.replaced, old_rpd_id != new_rpd_id)