refactor access lookup retrieval (#1832)

bihealth · Nov 16, 2023 · d640f79 · d640f79
1 parent 3ee616e
commit d640f79
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 24 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -13,6 +13,7 @@ Added
 
 - **Irodsbackend**
     - ``get_version()`` helper (#1592, #1817, #1831)
+    - ``get_access_lookup()`` helper (#1832)
 - **Samplesheets**
     - Custom validation for ``sheet_sync_url`` and ``sheet_sync_token`` (#1310, #1384)
     - ``hpo.jax.org`` in ``SHEETS_ONTOLOGY_URL_SKIP`` (#1821)
@@ -32,7 +33,7 @@ Changed
     - Change default IGV genome to ``b37_1kg`` (#1812)
     - Update existing ``b37`` IGV genome settings with a migration (#1812)
 - **Taskflowbackend**
-    - iRODS v4.3 support (#1592, #1817)
+    - iRODS v4.3 support (#1592, #1817, #1832)
 
 Fixed
 -----

diff --git a/irodsbackend/api.py b/irodsbackend/api.py
@@ -10,6 +10,7 @@
 import uuid
 
 from contextlib import contextmanager
+from packaging import version
 
 from irods.api_number import api_number
 from irods.collection import iRODSCollection
@@ -502,6 +503,26 @@ def get_version(cls, irods):
         """
         return '.'.join(str(x) for x in irods.server_version)
 
+    @classmethod
+    def get_access_lookup(cls, irods):
+        """
+        Return an ACL lookup dict compatible with the currently used iRODS
+        server version (4.2 and 4.3 supported).
+
+        :param irods: iRODSSession object
+        :return: Dict
+        """
+        v = version.parse(cls.get_version(irods))
+        d = '_' if v >= version.parse('4.3') else ' '
+        return {
+            'read': 'read{}object'.format(d),
+            'read{}object'.format(d): 'read',
+            'write': 'modify{}object'.format(d),
+            'modify{}object'.format(d): 'write',
+            'null': 'null',
+            'own': 'own',
+        }
+
     def get_object_stats(self, irods, path):
         """
         Return file count and total file size for all files within a path.

diff --git a/taskflowbackend/flows/landing_zone_create.py b/taskflowbackend/flows/landing_zone_create.py
@@ -25,6 +25,7 @@ def build(self, force_fail=False):
         user_path = os.path.join(zone_root, zone.user.username)
         zone_path = self.irods_backend.get_path(zone)
         root_access = 'read' if self.flow_data['restrict_colls'] else 'own'
+        access_lookup = self.irods_backend.get_access_lookup(self.irods)
 
         self.add_task(
             lz_tasks.RevertLandingZoneFailTask(
@@ -54,6 +55,7 @@ def build(self, force_fail=False):
                     'access_name': 'read',
                     'path': zone_root,
                     'user_name': project_group,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                     'recursive': False,
                 },
@@ -85,6 +87,7 @@ def build(self, force_fail=False):
                     'access_name': 'read',
                     'path': user_path,
                     'user_name': zone.user.username,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                     'recursive': False,
                 },
@@ -116,6 +119,7 @@ def build(self, force_fail=False):
                     'access_name': root_access,
                     'path': zone_path,
                     'user_name': zone.user.username,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
             )
@@ -132,6 +136,7 @@ def build(self, force_fail=False):
                         'access_name': 'write',
                         'path': zone_path,
                         'user_name': self.flow_data['script_user'],
+                        'access_lookup': access_lookup,
                         'irods_backend': self.irods_backend,
                     },
                 )
@@ -170,6 +175,7 @@ def build(self, force_fail=False):
                             'access_name': 'own',
                             'path': coll_path,
                             'user_name': zone.user.username,
+                            'access_lookup': access_lookup,
                             'irods_backend': self.irods_backend,
                         },
                     )

diff --git a/taskflowbackend/flows/landing_zone_move.py b/taskflowbackend/flows/landing_zone_move.py
@@ -41,6 +41,7 @@ def build(self, force_fail=False):
         sample_path = self.irods_backend.get_path(zone.assay)
         zone_path = self.irods_backend.get_path(zone)
         admin_name = self.irods.username
+        access_lookup = self.irods_backend.get_access_lookup(self.irods)
 
         # HACK: Set zone status in the Django site
         zone.set_status(
@@ -196,6 +197,7 @@ def build(self, force_fail=False):
                     'access_name': 'own',
                     'path': zone_path,
                     'user_name': admin_name,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
             )
@@ -210,6 +212,7 @@ def build(self, force_fail=False):
                     'access_name': 'read',
                     'path': zone_path,
                     'user_name': zone.user.username,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
             )
@@ -226,6 +229,7 @@ def build(self, force_fail=False):
                         'access_name': 'read',
                         'path': zone_path,
                         'user_name': self.flow_data['script_user'],
+                        'access_lookup': access_lookup,
                         'irods_backend': self.irods_backend,
                     },
                 )
@@ -293,6 +297,7 @@ def build(self, force_fail=False):
                     'src_paths': zone_objects,
                     'access_name': 'read',
                     'user_name': project_group,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
             )
@@ -307,6 +312,7 @@ def build(self, force_fail=False):
                     'access_name': 'null',
                     'path': sample_path,
                     'user_name': zone.user.username,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
             )
@@ -322,6 +328,7 @@ def build(self, force_fail=False):
                         'access_name': 'null',
                         'path': sample_path,
                         'user_name': self.flow_data['script_user'],
+                        'access_lookup': access_lookup,
                         'irods_backend': self.irods_backend,
                     },
                 )

diff --git a/taskflowbackend/flows/project_create.py b/taskflowbackend/flows/project_create.py
@@ -15,6 +15,7 @@ def validate(self):
     def build(self, force_fail=False):
         project_path = self.irods_backend.get_path(self.project)
         project_group = self.irods_backend.get_user_group_name(self.project)
+        access_lookup = self.irods_backend.get_access_lookup(self.irods)
 
         self.add_task(
             irods_tasks.CreateCollectionTask(
@@ -80,6 +81,7 @@ def build(self, force_fail=False):
                     'access_name': 'read',
                     'path': project_path,
                     'user_name': project_group,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
             )

diff --git a/taskflowbackend/flows/public_access_update.py b/taskflowbackend/flows/public_access_update.py
@@ -18,6 +18,7 @@ def build(self, force_fail=False):
         # TODO: Use project.public_guest_access instead of flow_data['access']?
         access_name = 'read' if self.flow_data['access'] else 'null'
         ticket_str = self.flow_data.get('ticket_str')
+        access_lookup = self.irods_backend.get_access_lookup(self.irods)
 
         self.add_task(
             irods_tasks.SetAccessTask(
@@ -27,6 +28,7 @@ def build(self, force_fail=False):
                     'access_name': access_name,
                     'path': self.flow_data['path'],
                     'user_name': PUBLIC_GROUP,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
                 force_fail=force_fail if not ticket_str else False,

diff --git a/taskflowbackend/flows/sheet_colls_create.py b/taskflowbackend/flows/sheet_colls_create.py
@@ -22,6 +22,7 @@ def validate(self):
     def build(self, force_fail=False):
         sample_path = self.irods_backend.get_sample_path(self.project)
         project_group = self.irods_backend.get_user_group_name(self.project)
+        access_lookup = self.irods_backend.get_access_lookup(self.irods)
 
         self.add_task(
             irods_tasks.CreateCollectionTask(
@@ -48,6 +49,7 @@ def build(self, force_fail=False):
                     'access_name': 'read',
                     'path': sample_path,
                     'user_name': project_group,
+                    'access_lookup': access_lookup,
                     'irods_backend': self.irods_backend,
                 },
             )
@@ -71,6 +73,7 @@ def build(self, force_fail=False):
                         'access_name': 'read',
                         'path': sample_path,
                         'user_name': PUBLIC_GROUP,
+                        'access_lookup': access_lookup,
                         'irods_backend': self.irods_backend,
                     },
                 )

diff --git a/taskflowbackend/tasks/irods_tasks.py b/taskflowbackend/tasks/irods_tasks.py
@@ -14,7 +14,6 @@
     CAT_SUCCESS_BUT_WITH_NO_INFO,
 )
 from irods.models import Collection
-from packaging import version
 
 from taskflowbackend.tasks.base_task import BaseTask
 
@@ -51,22 +50,6 @@ def _raise_irods_exception(self, ex, info=None):
         logger.error(desc)
         raise Exception(desc)
 
-    def _get_access_conversion(self, irods_backend):
-        """
-        Return the access conversion dict compatible with the currently used
-        iRODS server version (4.2 and 4.3 supported).
-        """
-        v = version.parse(irods_backend.get_version(self.irods))
-        d = '_' if v >= version.parse('4.3') else ' '
-        return {
-            'read': 'read{}object'.format(d),
-            'read{}object'.format(d): 'read',
-            'write': 'modify{}object'.format(d),
-            'modify{}object'.format(d): 'write',
-            'null': 'null',
-            'own': 'own',
-        }
-
 
 class CreateCollectionTask(IrodsBaseTask):
     """
@@ -302,13 +285,13 @@ def execute(
         access_name,
         path,
         user_name,
+        access_lookup,
         irods_backend,
         obj_target=False,
         recursive=True,
         *args,
         **kwargs
     ):
-        ac = self._get_access_conversion(irods_backend)
         if obj_target:
             target = self.irods.data_objects.get(path)
             recursive = False
@@ -320,8 +303,13 @@ def execute(
         user_access = next(
             (x for x in target_access if x.user_name == user_name), None
         )
-        if user_access and user_access.access_name != ac[access_name]:
-            self.execute_data['access_name'] = ac[user_access.access_name]
+        if (
+            user_access
+            and user_access.access_name != access_lookup[access_name]
+        ):
+            self.execute_data['access_name'] = access_lookup[
+                user_access.access_name
+            ]
             modifying_data = True
         elif not user_access:
             self.execute_data['access_name'] = 'null'
@@ -349,6 +337,7 @@ def revert(
         access_name,
         path,
         user_name,
+        access_lookup,
         irods_backend,
         obj_target=False,
         recursive=True,
@@ -634,11 +623,11 @@ def execute(
         src_paths,
         access_name,
         user_name,
+        access_lookup,
         irods_backend,
         *args,
         **kwargs
     ):
-        ac = self._get_access_conversion(irods_backend)
         self.execute_data['moved_objects'] = []
 
         for src_path in src_paths:
@@ -680,8 +669,11 @@ def execute(
                 (x for x in target_access if x.user_name == user_name), None
             )
             prev_access = None
-            if user_access and user_access.access_name != ac[access_name]:
-                prev_access = ac[user_access.access_name]
+            if (
+                user_access
+                and user_access.access_name != access_lookup[access_name]
+            ):
+                prev_access = access_lookup[user_access.access_name]
                 modifying_access = True
             elif not user_access:
                 prev_access = 'null'
@@ -714,6 +706,7 @@ def revert(
         dest_root,
         access_name,
         user_name,
+        access_lookup,
         irods_backend,
         *args,
         **kwargs