diff --git a/README.md b/README.md index d136657b6..9f96e9f12 100644 --- a/README.md +++ b/README.md @@ -836,22 +836,60 @@ of an "imeta set \...", e.g. overwriting all AVUs with a name field of "key2" in a single update: ```python ->>> new_meta = iRODSMeta('key2','value5','units2') ->>> obj.metadata\[new_meta.name\] = new_meta +>>> obj.metadata['key2'] = iRODSMeta('key2','value5','units2') >>> print(obj.metadata.items()) [, , ] ``` -With only one AVU on the object with a name of "key2", *get_one* -is assured of not throwing an exception: +Alternatively, in the indexed AVU assignment, the following construction can +be used in lieu of the direct call to the iRODSMeta constructor, thus avoiding redundant use of the +key string on both left- and right-hand sides of the assignment: + +``` +>>> obj.metadata['key2'] = iRODSMeta.builder(value='value5', units='units2') +``` + +Lest there should be a misunderstanding, this form of assignment will also clear +any other pre-existing AVUs with a name field of 'key2' before the requested assignment is +actually made. + +Be aware that use of the indexing form on obj.metadata to retrieve AVUs +with code such as + +``` +x = obj.metadata['key1'] +``` + +can also act in ways unexpected by the unwary developer. If multiple AVUs +exist under the given name field, one will be chosen and returned at random. + +For these reasons, the indexed assignment may be better considered an artifact of convenience +rather than reliable, straightforward coding practice. For clear and unambiguous intent, the +canonical iRODS API endpoints should be used, with calls such as + +```python +obj.metadata.set('mykey1','myvalue1') +obj.metadata.set('mykey2','myvalue2','myunits2') +obj.metadata.set(*iRODSMeta('mykey3','myvalue3')) +``` + +being preferred. (All of the same usages apply for the `add` API endpoint as well.) + +Enforcing a singleton AVU +------------------------- +*get_one()* is a way of retrieving an AVU by its name field if we want to +assert that exactly one such AVU should exist (fewer or more than 1 will raise +a `KeyError`). Here, it can be used to retrieve the "key2" AVU (since the indexed +assignment from the last section has removed all but the one): ```python >>> print(obj.metadata.get_one('key2')) ``` -However, the same is not true of "key1": +But for our present example, the same is not true in the case of "key1", since +we have now left several AVUs under that name. ```python >>> print(obj.metadata.get_one('key1')) @@ -862,6 +900,9 @@ Traceback (most recent call last): KeyError ``` +Metadata removal and clean disposal +----------------------------------- + Finally, to remove a specific AVU from an object: ```python @@ -922,20 +963,21 @@ Since v1.1.4, `set()` can be used instead: >>> album.metadata.set( meta ) ``` -In versions of iRODS 4.2.12 and later, we can also do: +In iRODS 4.2.12 and after, a rodsadmin can apply the ADMIN_KW +thus allowing modification of AVUs owned by other users: ```python ->>> album.metadata.set( meta, \*\*{kw.ADMIN_KW: ''} ) +>>> album.metadata.set(meta, **{kw.ADMIN_KW: ''}) ``` -or even: +Equivalently, but with increased overhead, this does the same thing: ```python ->>> album.metadata(admin = True)\[meta.name\] = meta +>>> album.metadata(admin=True)[meta.name] = meta ``` -Since v1.1.5, the "timestamps" keyword is provided to enable the loading -of create and modify timestamps for every AVU returned from the server: +A "timestamps" keyword is also provided to enable loading of the +`create_time` and `modify_time` attributes for every AVU returned from the server: ```python >>> avus = album.metadata(timestamps = True).items() diff --git a/irods/meta.py b/irods/meta.py index 5829e68d1..f87ffc1c7 100644 --- a/irods/meta.py +++ b/irods/meta.py @@ -1,8 +1,27 @@ import base64 +import collections import copy +import functools +class avubuilder: + def __init__(self, value, units=None, *, name=None): + self.avu_builder = _AVU_builder(name=name, value=value, units=units) + + def __call__(self): + return iRODSMeta(*self.avu_builder) + +_AVU_builder = functools.partial( + _AVU_type:=collections.namedtuple( + '_AVU_type', + ['name','value','units'] + ), + name=None, units=None +) class iRODSMeta: + + builder = avubuilder + def _to_column_triple(self): return (self.name, self.forward_translate(self.value)) + ( ('',) if not self.units else (self.forward_translate(self.units),) @@ -235,7 +254,7 @@ def get_one(self, key): def _get_meta(self, *args): if not len(args): raise ValueError("Must specify an iRODSMeta object or key, value, units)") - return args[0] if len(args) == 1 else self._manager._opts['iRODSMeta_type'](*args) + return self._manager._opts['iRODSMeta_type'](*(args[0] if len(args)==1 else args)) def apply_atomic_operations(self, *avu_ops): self._manager.apply_atomic_operations(self._model_cls, self._path, *avu_ops) @@ -296,7 +315,11 @@ def __setitem__(self, key, meta): the key with a single iRODSMeta tuple """ self._delete_all_values(key) - self.add(meta) + if isinstance(meta, iRODSMeta.builder): + meta = meta() + if meta.name is None: + meta.name = key + self.add(*meta) def _delete_all_values(self, key): for meta in self.get_all(key): diff --git a/irods/test/meta_test.py b/irods/test/meta_test.py index 880bf1fe5..eff3a4ac7 100644 --- a/irods/test/meta_test.py +++ b/irods/test/meta_test.py @@ -1,6 +1,7 @@ #! /usr/bin/env python # -*- coding: utf-8 -*- +import collections import datetime import os import re @@ -798,6 +799,72 @@ def test_prevention_of_attribute_creation__issue_795(self): # data.metadata(admin = True) generates a cloned object but for the one change to "admin". data.metadata.admin = True + def test_iRODSMeta_builder__issue_835(self): + data_path = iRODSPath(self.coll_path, helpers.unique_name(datetime.datetime.now())) # noqa: DTZ005 + data_obj = None + try: + # Create a test object on which to set metadata. + data_obj = self.sess.data_objects.create(data_path) + + # Set a number of metadata AVUs with the iRODSMeta builder invocation. (Each of + # these AVU values follow a predictable relation defined by the test_mapping function.) + def test_mapping(name): return str(ord(name)) + avu_names = [chr(_) for _ in range(ord('a'), ord('z')+1)] + for ch in avu_names: + data_obj.metadata[ch] = iRODSMeta.builder(value = test_mapping(ch)) + + # Assert there are as many AVUs as expected + self.assertEqual( + len(myitems := data_obj.metadata.items()), + len(avu_names) + ) + + # Assert that each AVU conforms to the expected name->value mapping. + for avu in myitems: + self.assertEqual(avu.value, test_mapping(avu.name)) + + # Define constants. + KM_PER_MILE = '1.609344' + CM_PER_FOOT = '30.48' + + # Use both forms of the iRODSMeta builder invocation, testing that both attempts + # resulted in the AVU we expected. (For sets, s1 < s2 iff s1 is a proper subset of s2.) + data_obj.metadata['mile'] = iRODSMeta.builder(value=KM_PER_MILE, units='km') + data_obj.metadata['foot'] = iRODSMeta.builder(CM_PER_FOOT, 'cm') + self.assertLess( + { + iRODSMeta('mile', KM_PER_MILE, 'km'), + iRODSMeta('foot', CM_PER_FOOT, 'cm'), + }, + set(data_obj.metadata.items()) + ) + finally: + # Delete the test object. + if data_obj: + data_obj.unlink(force=True) + + def test_indexed_assignments_are_iRODSMeta_subclass_compatible__issue_835(self): + data_path = iRODSPath(self.coll_path, helpers.unique_name(datetime.datetime.now())) # noqa: DTZ005 + data_obj = None + try: + data_obj = self.sess.data_objects.create(data_path) + + # Test use of the iRODSMeta builder with a custom iRODSMeta-derived getter/setter (which + # in this case automatically performs user defined conversions to and from byte strings). + dm = data_obj.metadata(iRODSMeta_type=iRODSBinOrStringMeta) + + # Assign a new AVU. + dm['test_key'] = iRODSMeta.builder(**( + avu_without_name := collections.OrderedDict(value=b'\3', units=b'd\0ef') + )) + + # Test that AVU storage happened with supported by the proper conversions (within the client) + # to and from 'str' type required for the 'value' and 'units' fields of an AVU. + test_avu = iRODSMeta('test_key',*list(avu_without_name.values())) + self.assertEqual(test_avu, dm['test_key']) + finally: + if data_obj: + data_obj.unlink(force=True) if __name__ == "__main__": # let the tests find the parent irods lib