Refining the resource refresh

Introduces a public method named ``invalidate()`` in ResourceBase
and facilitates the re-fetching of the resource attributes by
invoking ``refresh()`` method only when the resource is marked as
stale. Nested resource/s are invalidated in case of resource refresh
and are lazily reloaded only when they are accessed the next time,
provided those are already initialized in contrast to recreation as
earlier.

``force`` argument introduced in ``refresh()`` method which will
force refresh the resource and its sub-resources, if set to True.
So if the resource needs to be reloaded post its initialization,
it has to be invalidated first and then refreshed:

    resource.invalidate()
    resource.refresh()

or if the resource needs to be reloaded forcefully post its
initialization, it can be achieved by:

    resource.refresh(force=True)

this will also invariably reload (greedy-refresh) its sub-resources
as well.

Co-Authored-By: Dmitry Tantsur <divius.inside@gmail.com>
Closes-Bug: 1709039

Change-Id: I89fc69ef0569c5a56abe7631d1fa6a0e5da17b80

releasenotes/notes/fix-refine-resource-refresh-86c21ce230967251.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The library now supports reloading of the attributes by invoking
+    ``refresh()`` method for nested resources in contrast to recreation.
+    Resources can now be marked stale by invoking ``invalidate()``.

sushy/resources/base.py

@@ -197,6 +197,11 @@ class ResourceBase(object):
         self._path = path
         self._json = None
         self.redfish_version = redfish_version
+        # Note(deray): Indicates if the resource holds stale data or not.
+        # Starting off with True and eventually gets set to False when
+        # attribute values are fetched.
+        self._is_stale = True
+
         self.refresh()
 
     def _parse_attributes(self):
@@ -205,20 +210,66 @@ class ResourceBase(object):
             # Hide the Field object behind the real value
             setattr(self, attr, field._load(self.json, self))
 
-    def refresh(self):
+    def refresh(self, force=False):
         """Refresh the resource
 
         Freshly retrieves/fetches the resource attributes and invokes
         ``_parse_attributes()`` method on successful retrieval.
+        Advised not to override this method in concrete ResourceBase classes.
+        Resource classes can place their refresh specific operations in
+        ``_do_refresh()`` method, if needed. This method represents the
+        template method in the paradigm of Template design pattern.
+
+        :param force: will force refresh the resource and its sub-resources,
+            if set to True.
         :raises: ResourceNotFoundError
         :raises: ConnectionError
         :raises: HTTPError
         """
+        # Note(deray): Don't re-fetch / invalidate the sub-resources if the
+        # resource is "_not_ stale" (i.e. fresh) OR _not_ forced.
+        if not self._is_stale and not force:
+            return
+
         self._json = self._conn.get(path=self._path).json()
         LOG.debug('Received representation of %(type)s %(path)s: %(json)s',
                   {'type': self.__class__.__name__,
                    'path': self._path, 'json': self._json})
         self._parse_attributes()
+        self._do_refresh(force)
+
+        # Mark it fresh
+        self._is_stale = False
+
+    def _do_refresh(self, force):
+        """Primitive method to be overridden by refresh related activities.
+
+        Derived classes are supposed to override this method with the
+        resource specific refresh operations to be performed. This is a
+        primitive method in the paradigm of Template design pattern.
+
+        :param force: should force refresh the resource and its sub-resources,
+            if set to True.
+        :raises: ResourceNotFoundError
+        :raises: ConnectionError
+        :raises: HTTPError
+        """
+
+    def invalidate(self, force_refresh=False):
+        """Mark the resource as stale, prompting refresh() before getting used.
+
+        If ``force_refresh`` is set to True, then it invokes ``refresh()``
+        on the resource.
+
+        :param force_refresh: will invoke refresh on the resource,
+            if set to True.
+        :raises: ResourceNotFoundError
+        :raises: ConnectionError
+        :raises: HTTPError
+        """
+        self._is_stale = True
+        if force_refresh:
+            self.refresh()
 
     @property
     def json(self):

sushy/resources/system/ethernet_interface.py

@@ -79,6 +79,11 @@ class EthernetInterfaceCollection(base.ResourceCollectionBase):
             self._summary = mac_dict
         return self._summary
 
-    def refresh(self):
-        super(EthernetInterfaceCollection, self).refresh()
+    def _do_refresh(self, force=False):
+        """Do custom resource specific refresh activities
+
+        On refresh, all sub-resources are marked as stale, i.e.
+        greedy-refresh not done for them unless forced by ``force``
+        argument.
+        """
         self._summary = None

sushy/resources/system/processor.py

@@ -120,8 +120,12 @@ class ProcessorCollection(base.ResourceCollectionBase):
         super(ProcessorCollection, self).__init__(connector, path,
                                                   redfish_version)
 
-    def refresh(self):
-        """Refresh the resource"""
-        super(ProcessorCollection, self).refresh()
+    def _do_refresh(self, force=False):
+        """Do custom resource specific refresh activities
+
+        On refresh, all sub-resources are marked as stale, i.e.
+        greedy-refresh not done for them unless forced by ``force``
+        argument.
+        """
         # Reset summary attribute
         self._summary = None

sushy/resources/system/system.py

@@ -248,25 +248,28 @@ class System(base.ResourceBase):
 
     @property
     def processors(self):
-        """Property to provide reference to `ProcessorCollection` instance
+        """Property to reference `ProcessorCollection` instance
 
-        It is calculated once when the first time it is queried. On refresh,
-        this property gets reset.
+        It is set once when the first time it is queried. On refresh,
+        this property is marked as stale (greedy-refresh not done).
+        Here the actual refresh of the sub-resource happens, if stale.
         """
         if self._processors is None:
             self._processors = processor.ProcessorCollection(
                 self._conn, self._get_processor_collection_path(),
                 redfish_version=self.redfish_version)
 
+        self._processors.refresh(force=False)
         return self._processors
 
-    def refresh(self):
-        super(System, self).refresh()
-        self._processors = None
-        self._ethernet_interfaces = None
-
     @property
     def ethernet_interfaces(self):
+        """Property to reference `EthernetInterfaceCollection` instance
+
+        It is set once when the first time it is queried. On refresh,
+        this property is marked as stale (greedy-refresh not done).
+        Here the actual refresh of the sub-resource happens, if stale.
+        """
         if self._ethernet_interfaces is None:
             self._ethernet_interfaces = (
                 ethernet_interface.EthernetInterfaceCollection(
@@ -274,8 +277,21 @@ class System(base.ResourceBase):
                     utils.get_sub_resource_path_by(self, "EthernetInterfaces"),
                     redfish_version=self.redfish_version))
 
+        self._ethernet_interfaces.refresh(force=False)
         return self._ethernet_interfaces
 
+    def _do_refresh(self, force=False):
+        """Do custom resource specific refresh activities
+
+        On refresh, all sub-resources are marked as stale, i.e.
+        greedy-refresh not done for them unless forced by ``force``
+        argument.
+        """
+        if self._processors is not None:
+            self._processors.invalidate(force)
+        if self._ethernet_interfaces is not None:
+            self._ethernet_interfaces.invalidate(force)
+
 
 class SystemCollection(base.ResourceCollectionBase):
 

sushy/tests/unit/resources/system/test_processor.py

@@ -140,7 +140,7 @@ class ProcessorCollectionTestCase(base.TestCase):
         with open('sushy/tests/unit/json_samples/'
                   'processor_collection.json', 'r') as f:
             self.conn.get.return_value.json.return_value = json.loads(f.read())
-        self.sys_processor_col.refresh()
+        self.sys_processor_col.refresh(force=True)
 
         # | WHEN & THEN |
         self.assertIsNone(self.sys_processor_col._summary)

sushy/tests/unit/resources/system/test_system.py

@@ -290,10 +290,13 @@ class SystemTestCase(base.TestCase):
         # On refreshing the system instance...
         with open('sushy/tests/unit/json_samples/system.json', 'r') as f:
             self.conn.get.return_value.json.return_value = json.loads(f.read())
+
+        self.sys_inst.invalidate()
         self.sys_inst.refresh()
 
         # | WHEN & THEN |
-        self.assertIsNone(self.sys_inst._processors)
+        self.assertIsNotNone(self.sys_inst._processors)
+        self.assertTrue(self.sys_inst._processors._is_stale)
 
         # | GIVEN |
         with open('sushy/tests/unit/json_samples/processor_collection.json',
@@ -302,6 +305,7 @@ class SystemTestCase(base.TestCase):
         # | WHEN & THEN |
         self.assertIsInstance(self.sys_inst.processors,
                               processor.ProcessorCollection)
+        self.assertFalse(self.sys_inst._processors._is_stale)
 
     def _setUp_processor_summary(self):
         self.conn.get.return_value.json.reset_mock()

sushy/tests/unit/resources/test_base.py

@@ -36,11 +36,27 @@ class ResourceBaseTestCase(base.TestCase):
         self.conn = mock.Mock()
         self.base_resource = BaseResource(connector=self.conn, path='/Foo',
                                           redfish_version='1.0.2')
+        self.assertFalse(self.base_resource._is_stale)
         # refresh() is called in the constructor
         self.conn.reset_mock()
 
     def test_refresh(self):
         self.base_resource.refresh()
+        self.conn.get.assert_not_called()
+
+    def test_refresh_force(self):
+        self.base_resource.refresh(force=True)
+        self.conn.get.assert_called_once_with(path='/Foo')
+
+    def test_invalidate(self):
+        self.base_resource.invalidate()
+        self.conn.get.assert_not_called()
+
+        self.base_resource.refresh()
+        self.conn.get.assert_called_once_with(path='/Foo')
+
+    def test_invalidate_force_refresh(self):
+        self.base_resource.invalidate(force_refresh=True)
         self.conn.get.assert_called_once_with(path='/Foo')
 
 
@@ -187,37 +203,39 @@ class FieldTestCase(base.TestCase):
 
     def test_missing_required(self):
         del self.json['String']
-        self.assertRaisesRegex(exceptions.MissingAttributeError,
-                               'String', self.test_resource.refresh)
+        self.assertRaisesRegex(
+            exceptions.MissingAttributeError,
+            'String', self.test_resource.refresh, force=True)
 
     def test_missing_nested_required(self):
         del self.json['Nested']['String']
-        self.assertRaisesRegex(exceptions.MissingAttributeError,
-                               'Nested/String', self.test_resource.refresh)
+        self.assertRaisesRegex(
+            exceptions.MissingAttributeError,
+            'Nested/String', self.test_resource.refresh, force=True)
 
     def test_missing_nested_required2(self):
         del self.json['Nested']['Object']['Field']
         self.assertRaisesRegex(exceptions.MissingAttributeError,
                                'Nested/Object/Field',
-                               self.test_resource.refresh)
+                               self.test_resource.refresh, force=True)
 
     def test_malformed_int(self):
         self.json['Integer'] = 'banana'
         self.assertRaisesRegex(
             exceptions.MalformedAttributeError,
             'attribute Integer is malformed.*invalid literal for int',
-            self.test_resource.refresh)
+            self.test_resource.refresh, force=True)
 
     def test_malformed_nested_int(self):
         self.json['Nested']['Integer'] = 'banana'
         self.assertRaisesRegex(
             exceptions.MalformedAttributeError,
             'attribute Nested/Integer is malformed.*invalid literal for int',
-            self.test_resource.refresh)
+            self.test_resource.refresh, force=True)
 
     def test_mapping_missing(self):
         self.json['Nested']['Mapped'] = 'banana'
-        self.test_resource.refresh()
+        self.test_resource.refresh(force=True)
 
         self.assertIsNone(self.test_resource.nested.mapped)