[_795,SQUASH] allow options to be accessed as attrs of metadata obj

Co-authored-by: Kory Draughn <korydraughn@ymail.com>

Author: d-w-moore

README.md

@@ -936,7 +936,7 @@ Disabling AVU reloads from the iRODS server
 
 With the default setting of `reload = True`, an `iRODSMetaCollection` will
 proactively read all current AVUs back from the iRODS server after any
-metadata write done by the client.  This helps methods such as `items()`
+metadata write done by the client.  This helps methods such as `keys()` and `items()`
 to return an up-to-date result.   Setting `reload = False` can, however, greatly
 increase code efficiency if for example a lot of AVUs must be added or deleted
 at once without reading any back again.
@@ -952,6 +952,22 @@ current_metadata = obj.metadata().items()
 print(f"{current_metadata = }")
 ```
 
+By way of explanation, please note that calls of the form
+`obj.metadata([opt1=value1[,opt2=value2...]])` will always
+produce new `iRODSMetaCollection` objects - which nevertheless share the same
+session object as the original, as the copy is shallow in most respects.
+This avoids always mutating the current instance and thus prevents any need to
+implement context manager semantics when temporarily altering options such
+as `reload` and `admin`.
+
+Additionally note that the call `obj.metadata()` without option parameters
+always syncs the AVU list within the resulting `iRODSMetaCollection` object to
+what is currently in the catalog, because the original object is unmutated with
+respect to all options (meaning `obj.metadata.reload` is always `True`) -- that
+is, absent any low-level meddling within reserved fields by the application.
+Thus, `obj.metadata().items()` will always agree with the in-catalog AVU list
+whereas `obj.metadata.items()` might not.
+
 Subclassing `iRODSMeta`
 ---------------------
 The keyword option `iRODSMeta_type` can be used to set up any `iRODSMeta`

irods/manager/metadata_manager.py

@@ -27,14 +27,15 @@ class InvalidAtomicAVURequest(Exception):
     pass
 
 
+# This was necessarily made separate from the MetadataManager definition
+# in order to avoid infinite recursion in iRODSMetaCollection.__getattr__
+_MetadataManager_opts_initializer = {'admin': False, 'timestamps': False, 'iRODSMeta_type': iRODSMeta, 'reload': True}
+
+
 class MetadataManager(Manager):
 
     def __init__(self, *_):
-        self._opts = {
-            'admin':False,
-            'timestamps':False,
-            'iRODSMeta_type':iRODSMeta
-        }
+        self._opts = _MetadataManager_opts_initializer.copy()
         super().__init__(*_)
 
     @property

irods/meta.py

@@ -131,6 +131,64 @@ def __init__(self, operation, avu, **kw):
 
 
 class iRODSMetaCollection:
+    def __setattr__(self, name, value):
+        """
+        Override __setattr__.
+
+        Protect the virtual, read-only attributes such as 'admin', 'timestamps', etc.,
+        from being written or created as concrete attributes, which would interfere with
+        __getattr__'s intended operation for these cases.
+
+        Args:
+            name: the name of the attribute to be written.
+            value: the value to be written to the attribute.
+
+        Raises:
+            AttributeError: on any attempt to write to these special attributes.
+        """
+        from irods.manager.metadata_manager import _MetadataManager_opts_initializer
+
+        if name in _MetadataManager_opts_initializer:
+            msg = (
+                f"""The "{name}" attribute is a special one, settable only via a """
+                f"""call on the object.  For example: admin_view = data_obj.metadata({name}=<value>)"""
+            )
+            raise AttributeError(msg)
+
+        super().__setattr__(name, value)
+
+    def __getattr__(self, name):
+        """
+        Override __getattr__.
+
+        Expose certain settable flags (e.g. "admin", "timestamps") as virtual, read-only
+        "attributes."  The names of these special attributes appear as the keys of the
+        _MetadataManager_opts_initializer dictionary.
+
+        Args:
+            name: the name of the attribute to be fetched.
+
+        Returns:
+            the value of the named attribute.
+
+        Raises:
+            AttributeError: because this is the protocol for deferring to __getattr__'s
+            default behavior for the case in which none of the special attribute keys are
+            a match for 'name'.
+        """
+        from irods.manager.metadata_manager import _MetadataManager_opts_initializer
+
+        # Separating _MetadataManager_opts_initializer from the MetadataManager class
+        # prevents the possibility of arbitrary access by copy.copy() to parts of
+        # our object's state before they have been initialized, as it is known to do
+        # by calling hasattr on the "__setstate__" attribute. The result of such
+        # unfettered access is infinite recursion.  See:
+        # https://nedbatchelder.com/blog/201010/surprising_getattr_recursion
+
+        if name in _MetadataManager_opts_initializer:
+            return self._manager._opts[name]  # noqa: SLF001
+        raise AttributeError
+
     def __call__(self, **opts):
         """
         Optional parameters in **opts are:

irods/test/meta_test.py

@@ -21,6 +21,7 @@
     iRODSMeta,
 )
 from irods.models import Collection, CollectionMeta, DataObject, ModelBase, Resource
+from irods.path import iRODSPath
 from irods.session import iRODSSession
 from irods.test import helpers
 
@@ -820,24 +821,22 @@ def test_binary_avu_fields__issue_707(self):
     def test_cascading_changes_of_metadata_manager_options__issue_709(self):
         d = None
 
-        def get_option(metacoll, key):
-            return metacoll._manager._opts[key]
         try:
             d = self.sess.data_objects.create(f'{self.coll.path}/issue_709_test_1')
             m = d.metadata
-            self.assertEqual(get_option(m, 'admin'), False)
+            self.assertEqual(m.admin, False)
 
             m2 = m(admin=True)
-            self.assertEqual(get_option(m2, 'timestamps'), False)
-            self.assertEqual(get_option(m2, 'admin'), True)
+            self.assertEqual(m2.timestamps, False)
+            self.assertEqual(m2.admin, True)
 
             m3 = m2(timestamps=True)
-            self.assertEqual(get_option(m3, 'timestamps'), True)
-            self.assertEqual(get_option(m3, 'admin'), True)
+            self.assertEqual(m3.timestamps, True)
+            self.assertEqual(m3.admin, True)
             self.assertEqual(m3._manager.get_api_keywords().get(kw.ADMIN_KW), "")
 
             m4 = m3(admin=False)
-            self.assertEqual(get_option(m4, 'admin'), False)
+            self.assertEqual(m4.admin, False)
             self.assertEqual(m4._manager.get_api_keywords().get(kw.ADMIN_KW), None)
         finally:
             if d:
@@ -863,6 +862,15 @@ def test_reload_can_be_deactivated__issue_768(self):
         self.assertIn(item_1, items_reloaded)
         self.assertIn(item_2, items_reloaded)
 
+    def test_prevention_of_attribute_creation__issue_795(self):
+        data_path = iRODSPath(self.coll_path, helpers.unique_name(datetime.datetime.now()))  # noqa: DTZ005
+        data = self.sess.data_objects.create(data_path)
+        with self.assertRaises(AttributeError):
+            # This should cause an error since "admin" is considered as a read-only attribute; whereas
+            # data.metadata(admin = True) generates a cloned object but for the one change to "admin".
+            data.metadata.admin = True
+
+
 if __name__ == "__main__":
     # let the tests find the parent irods lib
     sys.path.insert(0, os.path.abspath("../.."))