[#835] allow key-agnostic iRODSMeta construction for AVU assignments

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())
 [<iRODSMeta 13182 key1 value1 units1>, <iRODSMeta 13183 key1 value2 None>,
 <iRODSMeta 13186 key2 value5 units2>]
 ```
 
-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'))
 <iRODSMeta 13186 key2 value5 units2>
 ```
 
-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()

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_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 @@
         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):

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 @@
             # 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