diff --git a/README.md b/README.md index 702541dc..2bd1e663 100644 --- a/README.md +++ b/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` diff --git a/irods/manager/metadata_manager.py b/irods/manager/metadata_manager.py index c09a6ab6..eeb70aa1 100644 --- a/irods/manager/metadata_manager.py +++ b/irods/manager/metadata_manager.py @@ -27,14 +27,19 @@ 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 diff --git a/irods/meta.py b/irods/meta.py index 8ca94ae1..2c736ce3 100644 --- a/irods/meta.py +++ b/irods/meta.py @@ -131,6 +131,50 @@ def __init__(self, operation, avu, **kw): class iRODSMetaCollection: + def __setattr__(self, name, value): + """Prevent the flag attributes such as 'admin', 'timestamps', etc., from + being changed in a way not sanctioned by the library. + """ + 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}=)""" + ) + raise AttributeError(msg) + + super().__setattr__(name, value) + + def __getattr__(self, name): + """Here we intervene for the purpose of casting such settable flags such + as 'admin', 'timestamps', etc. as readable attributes of the object. + + If the attribute name exists as a key in _MetadataManager_opts_initializer + (for the "_opts" lookup in the manager object), then the current value from + _opts is returned. Otherwise we throw an exception to revert to the default + __getattr__ behavior. + + For more specific coverage of such attributes and how they are used, please + consult the test module irods/test/meta_test.py and read the test for + issue #709, named: + + test_cascading_changes_of_metadata_manager_options__issue_709 + """ + + 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: diff --git a/irods/test/meta_test.py b/irods/test/meta_test.py index cd1c3abd..f67d1a13 100644 --- a/irods/test/meta_test.py +++ b/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,7 +862,20 @@ 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())) + data = self.sess.data_objects.create(data_path) + # Note the correct way to effect a change is via a call, generating a new metacoll object with + # the option transformed as requested, rather than modifying the attribute on the original object. + # Therefore + # metacoll.admin = True + # instead could be done as + # metacoll = metacoll(admin = True) + with self.assertRaises(AttributeError): + # This should cause an error. + data.metadata.admin = True + if __name__ == "__main__": # let the tests find the parent irods lib sys.path.insert(0, os.path.abspath("../..")) - unittest.main() +