Only call session.refresh from within db_api functions

This change removes the refresh method from the parent db object, and
adds a refresh boolean argument to db_api.resource_get function (which is
the only known use of refresh). Constraining refresh calls to be
inside db_api methods helps with bug #1479723 where the refresh can
happen under a context manager session.

Change-Id: I6fc5c03e8572eee90f89455e0925d503514040b3
Related-Bug: #1479723

heat/db/api.py

@@ -100,8 +100,8 @@ def stack_tags_get(context, stack_id):
     return IMPL.stack_tags_get(context, stack_id)
 
 
-def resource_get(context, resource_id):
-    return IMPL.resource_get(context, resource_id)
+def resource_get(context, resource_id, refresh=False):
+    return IMPL.resource_get(context, resource_id, refresh=refresh)
 
 
 def resource_get_all(context):

heat/db/sqlalchemy/api.py

@@ -154,12 +154,17 @@ def raw_template_files_get(context, files_id):
     return result
 
 
-def resource_get(context, resource_id):
+def resource_get(context, resource_id, refresh=False):
     result = context.session.query(models.Resource).get(resource_id)
 
     if not result:
         raise exception.NotFound(_("resource with id %s not found") %
                                  resource_id)
+    if refresh:
+        context.session.refresh(result)
+        # ensure data is loaded (lazy or otherwise)
+        result.data
+
     return result
 
 

heat/db/sqlalchemy/models.py

@@ -46,14 +46,6 @@ class HeatBase(models.ModelBase, models.TimestampMixin):
                 session = get_session()
         session.expire(self, attrs)
 
-    def refresh(self, session=None, attrs=None):
-        """Refresh this object."""
-        if not session:
-            session = orm_session.Session.object_session(self)
-            if not session:
-                session = get_session()
-        session.refresh(self, attrs)
-
     def delete(self, session=None):
         """Delete this object."""
         if not session:

heat/engine/resource.py

@@ -386,8 +386,8 @@ class Resource(object):
             return self.t.metadata()
         if self._rsrc_metadata is not None:
             return self._rsrc_metadata
-        rs = resource_objects.Resource.get_obj(self.stack.context, self.id)
-        rs.refresh(attrs=['rsrc_metadata'])
+        rs = resource_objects.Resource.get_obj(self.stack.context, self.id,
+                                               refresh=True)
         self._rsrc_metadata = rs.rsrc_metadata
         return rs.rsrc_metadata
 
@@ -408,9 +408,10 @@ class Resource(object):
         if self.id is None or self.action == self.INIT:
             raise exception.ResourceNotAvailable(resource_name=self.name)
         LOG.debug('Setting metadata for %s', six.text_type(self))
-        db_res = resource_objects.Resource.get_obj(self.stack.context, self.id)
-        if merge_metadata is not None:
-            db_res = db_res.refresh(attrs=['rsrc_metadata'])
+        refresh = merge_metadata is not None
+        db_res = resource_objects.Resource.get_obj(self.stack.context, self.id,
+                                                   refresh=refresh)
+        if refresh:
             metadata = merge_metadata(metadata, db_res.rsrc_metadata)
         db_res.update_metadata(metadata)
         self._rsrc_metadata = metadata
@@ -925,7 +926,7 @@ class Resource(object):
             'type': self.type(),
             'action': self.action,
             'status': self.status,
-            'metadata': self.metadata_get(refresh=True),
+            'metadata': self.metadata_get(),
             'resource_data': self.data()
         }
 

heat/objects/resource.py

@@ -101,8 +101,9 @@ class Resource(
         return resource
 
     @classmethod
-    def get_obj(cls, context, resource_id):
-        resource_db = db_api.resource_get(context, resource_id)
+    def get_obj(cls, context, resource_id, refresh=False):
+        resource_db = db_api.resource_get(context, resource_id,
+                                          refresh=refresh)
         return cls._from_db_object(cls(context), context, resource_db)
 
     @classmethod
@@ -206,9 +207,8 @@ class Resource(
                                       atomic_key=atomic_key,
                                       expected_engine_id=expected_engine_id)
 
-    def refresh(self, attrs=None):
-        resource_db = db_api.resource_get(self._context, self.id)
-        resource_db.refresh(attrs=attrs)
+    def refresh(self):
+        resource_db = db_api.resource_get(self._context, self.id, refresh=True)
         return self.__class__._from_db_object(
             self,
             self._context,