refactor and cleanup IrodsAPI (#1546)

bihealth · Dec 21, 2022 · 5885fec · 5885fec
1 parent 1b993c8
commit 5885fec
Show file tree

Hide file tree

Showing 9 changed files with 122 additions and 120 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -32,6 +32,7 @@ Changed
     - Upgrade to fastobo v0.12.2 (#1561)
 - **Irodsbackend**
     - Update backend iRODS connection handling (#909, #1542)
+    - Rename ``IrodsAPI.get_child_colls()``
 - **Samplesheets**
     - Upgrade ``loader-utils`` in Vue app (#1527)
     - Remove redundant node UUIDs from render tables (#708)
@@ -55,6 +56,8 @@ Removed
 
 - **Irodsbackend**
     - Backend API ``conn`` argument (#909)
+    - ``IrodsAPI.collection_exists()`` helper (#1546)
+    - ``IrodsAPI.get_coll_by_path()`` helper
 - **Samplesheets**
     - Unused ``config_set`` and ``num_col`` header parameters (#1551)
     - ``get_sample_libraries()`` helper (#1554)

diff --git a/irodsadmin/management/commands/irodsorphans.py b/irodsadmin/management/commands/irodsorphans.py
@@ -146,11 +146,12 @@ def get_orphans(irods, irods_backend, expected, assays):
     for assay in assays:
         if not assay.get_plugin():
             continue
-        for collection in irods_backend.get_child_colls_by_path(
-            irods_backend.get_path(assay)
-        ):
-            if collection.path not in expected:
-                orphans.append(collection.path)
+        with irods_backend.get_session() as irods:
+            for collection in irods_backend.get_child_colls(
+                irods, irods_backend.get_path(assay)
+            ):
+                if collection.path not in expected:
+                    orphans.append(collection.path)
     return orphans
 
 

diff --git a/irodsbackend/api.py b/irodsbackend/api.py
@@ -524,17 +524,6 @@ def get_object_stats(self, irods, path):
             query.remove()
         return ret
 
-    # TODO: Remove this, use session.collections.exists() instead
-    def collection_exists(self, path):
-        """
-        Return True/False depending if the collection defined in path exists
-
-        :param path: Full path to iRODS collection
-        :return: Boolean
-        """
-        with self.get_session() as irods:
-            return irods.collections.exists(self._sanitize_coll_path(path))
-
     @classmethod
     def get_colls_recursively(cls, coll):
         """
@@ -713,20 +702,21 @@ def get_objects(
             )
         return ret
 
-    # TODO: Remove this, refactor usages
-    def get_coll_by_path(self, path):
-        with self.get_session() as irods:
-            try:
-                return irods.collections.get(path)
-            except CollectionDoesNotExist:
-                return None
-
-    # TODO: Remove this, refactor usages
-    def get_child_colls_by_path(self, path):
-        coll = self.get_coll_by_path(path)
-        if coll:
+    @classmethod
+    def get_child_colls(cls, irods, path):
+        """
+        Return child collections for a collection by path. Does not return
+        children recursively.
+
+        :param irods: iRODSSession object
+        :param path: Full path to iRODS collection
+        :return: List
+        """
+        try:
+            coll = irods.collections.get(cls._sanitize_coll_path(path))
             return coll.subcollections
-        return []
+        except CollectionDoesNotExist:
+            return []
 
     def get_query(self, irods, sql, columns=None, register=True):
         """

diff --git a/irodsbackend/views.py b/irodsbackend/views.py
@@ -116,11 +116,11 @@ def dispatch(self, request, *args, **kwargs):
                 return JsonResponse(
                     self._get_detail(ERROR_NOT_IN_PROJECT), status=400
                 )
-            if not self.irods_backend.collection_exists(path):
-                return JsonResponse(
-                    self._get_detail(ERROR_NOT_FOUND), status=404
-                )
             with self.irods_backend.get_session() as irods:
+                if not irods.collections.exists(path):
+                    return JsonResponse(
+                        self._get_detail(ERROR_NOT_FOUND), status=404
+                    )
                 if (
                     not request.user.is_superuser
                     and not self._check_collection_perm(

diff --git a/landingzones/forms.py b/landingzones/forms.py
@@ -56,18 +56,15 @@ def __init__(
         from landingzones.plugins import LandingZoneConfigPluginPoint
 
         config_plugins = LandingZoneConfigPluginPoint.get_plugins()
-
         self.current_user = current_user
         if project:
             self.project = Project.objects.filter(sodar_uuid=project).first()
         if assay:
             self.assay = Assay.objects.filter(sodar_uuid=assay).first()
 
         # Form modifications
-
         # Modify ModelChoiceFields to use sodar_uuid
         self.fields['assay'].to_field_name = 'sodar_uuid'
-
         # Set suffix
         self.fields['title_suffix'].label = 'Title suffix'
         self.fields[
@@ -78,7 +75,6 @@ def __init__(
         # Get options for configuration
         self.fields['configuration'].widget = forms.Select()
         self.fields['configuration'].widget.choices = [(None, '--------------')]
-
         for plugin in config_plugins:
             self.fields['configuration'].widget.choices.append(
                 (plugin.config_name, plugin.config_display_name)
@@ -88,23 +84,21 @@ def __init__(
         if not self.instance.pk:
             self.fields['assay'].choices = []
             # Only show choices for assays which are in iRODS
-            for assay in Assay.objects.filter(
-                study__investigation__project=self.project,
-                study__investigation__active=True,
-            ):
-                if not irods_backend or irods_backend.collection_exists(
-                    irods_backend.get_path(assay)
+            with irods_backend.get_session() as irods:
+                for assay in Assay.objects.filter(
+                    study__investigation__project=self.project,
+                    study__investigation__active=True,
                 ):
-                    self.fields['assay'].choices.append(
-                        (
-                            assay.sodar_uuid,
-                            '{} / {}'.format(
-                                assay.study.get_display_name(),
-                                assay.get_display_name(),
-                            ),
+                    if irods.collections.exists(irods_backend.get_path(assay)):
+                        self.fields['assay'].choices.append(
+                            (
+                                assay.sodar_uuid,
+                                '{} / {}'.format(
+                                    assay.study.get_display_name(),
+                                    assay.get_display_name(),
+                                ),
+                            )
                         )
-                    )
-
         # Updating
         else:
             # Don't allow modifying certain fields

diff --git a/samplesheets/forms.py b/samplesheets/forms.py
@@ -279,16 +279,22 @@ def __init__(self, *args, **kwargs):
             study__investigation__project=kwargs['initial']['project']
         )
         if irods_backend:
-            choices = [
-                (
-                    track_hub.path,
-                    "{} / {}".format(assay.get_display_name(), track_hub.name),
-                )
-                for assay in assays
-                for track_hub in irods_backend.get_child_colls_by_path(
-                    irods_backend.get_path(assay) + '/' + TRACK_HUBS_COLL
-                )
-            ]
+            with irods_backend.get_session() as irods:
+                choices = [
+                    (
+                        track_hub.path,
+                        "{} / {}".format(
+                            assay.get_display_name(), track_hub.name
+                        ),
+                    )
+                    for assay in assays
+                    for track_hub in irods_backend.get_child_colls(
+                        irods,
+                        os.path.join(
+                            irods_backend.get_path(assay), TRACK_HUBS_COLL
+                        ),
+                    )
+                ]
         else:
             choices = []
 

diff --git a/samplesheets/plugins.py b/samplesheets/plugins.py
@@ -657,44 +657,42 @@ def update_cache(self, name=None, project=None, user=None):
             study__investigation__irods_status=True,
         )
 
-        for assay in assays:
-            item_name = 'irods/shortcuts/assay/{}'.format(assay.sodar_uuid)
-            assay_path = irods_backend.get_path(assay)
-            assay_plugin = assay.get_plugin()
-
-            # Default assay shortcuts
-            cache_data = {
-                'shortcuts': {
-                    'results_reports': irods_backend.collection_exists(
-                        assay_path + '/' + RESULTS_COLL
-                    ),
-                    'misc_files': irods_backend.collection_exists(
-                        assay_path + '/' + MISC_FILES_COLL
-                    ),
+        with irods_backend.get_session() as irods:
+            for assay in assays:
+                item_name = 'irods/shortcuts/assay/{}'.format(assay.sodar_uuid)
+                assay_path = irods_backend.get_path(assay)
+                assay_plugin = assay.get_plugin()
+                # Default assay shortcuts
+                cache_data = {
+                    'shortcuts': {
+                        'results_reports': irods.collections.exists(
+                            assay_path + '/' + RESULTS_COLL
+                        ),
+                        'misc_files': irods.collections.exists(
+                            assay_path + '/' + MISC_FILES_COLL
+                        ),
+                    }
                 }
-            }
-
-            # Plugin assay shortcuts
-            if assay_plugin:
-                plugin_shortcuts = assay_plugin.get_shortcuts(assay) or []
-                for sc in plugin_shortcuts:
-                    cache_data['shortcuts'][
-                        sc['id']
-                    ] = irods_backend.collection_exists(sc['path'])
-
-            cache_data['shortcuts']['track_hubs'] = [
-                track_hub_coll.path
-                for track_hub_coll in irods_backend.get_child_colls_by_path(
-                    assay_path + '/' + TRACK_HUBS_COLL
+                # Plugin assay shortcuts
+                if assay_plugin:
+                    plugin_shortcuts = assay_plugin.get_shortcuts(assay) or []
+                    for sc in plugin_shortcuts:
+                        cache_data['shortcuts'][
+                            sc['id']
+                        ] = irods.collections.exists(sc['path'])
+                cache_data['shortcuts']['track_hubs'] = [
+                    c.path
+                    for c in irods_backend.get_child_colls(
+                        irods, os.path.join(assay_path, TRACK_HUBS_COLL)
+                    )
+                ]
+                cache_backend.set_cache_item(
+                    name=item_name,
+                    app_name='samplesheets',
+                    user=user,
+                    data=cache_data,
+                    project=assay.get_project(),
                 )
-            ]
-            cache_backend.set_cache_item(
-                name=item_name,
-                app_name='samplesheets',
-                user=user,
-                data=cache_data,
-                project=assay.get_project(),
-            )
 
         # Assay sub-app plugins
         for assay_plugin in SampleSheetAssayPluginPoint.get_plugins():

diff --git a/samplesheets/tests/test_views_taskflow.py b/samplesheets/tests/test_views_taskflow.py
@@ -501,10 +501,11 @@ def test_render(self):
                 "{} / {}".format(self.assay.get_display_name(), track_hub.name),
             )
             for track_hub in (
-                self.irods_backend.get_child_colls_by_path(
+                self.irods_backend.get_child_colls(
+                    self.irods,
                     self.irods_backend.get_path(self.assay)
                     + '/'
-                    + TRACK_HUBS_COLL
+                    + TRACK_HUBS_COLL,
                 )
             )
         ]

diff --git a/samplesheets/views.py b/samplesheets/views.py
@@ -5,12 +5,13 @@
 import json
 import logging
 import os
+import pytz
 import requests
 import zipfile
-from packaging import version
 
-import pytz
 from cubi_tk.isa_tpl import _TEMPLATES as TK_TEMPLATES
+from irods.exception import CollectionDoesNotExist
+from packaging import version
 
 from django.conf import settings
 from django.contrib import messages
@@ -2004,15 +2005,19 @@ def get_context_data(self, *args, **kwargs):
         assays = Assay.objects.filter(
             study__investigation__project__sodar_uuid=self.kwargs['project']
         )
-        context['track_hubs_available'] = bool(
-            [
-                track_hub
-                for assay in assays
-                for track_hub in irods_backend.get_child_colls_by_path(
-                    irods_backend.get_path(assay) + '/' + TRACK_HUBS_COLL
-                )
-            ]
-        )
+        with irods_backend.get_session() as irods:
+            context['track_hubs_available'] = bool(
+                [
+                    track_hub
+                    for assay in assays
+                    for track_hub in irods_backend.get_child_colls(
+                        irods,
+                        os.path.join(
+                            irods_backend.get_path(assay), TRACK_HUBS_COLL
+                        ),
+                    )
+                ]
+            )
         return context
 
 
@@ -2281,16 +2286,20 @@ def get_context_data(self, *args, **kwargs):
         context_data['affected_collections'] = []
         context_data['is_collection'] = obj.is_collection()
         if context_data['is_collection']:
-            coll = irods_backend.get_coll_by_path(
-                context_data['irods_request'].path
-            )
             with irods_backend.get_session() as irods:
-                context_data[
-                    'affected_objects'
-                ] += irods_backend.get_objs_recursively(irods, coll)
-                context_data[
-                    'affected_collections'
-                ] += irods_backend.get_colls_recursively(irods, coll)
+                try:
+                    coll = irods.collections.get(
+                        context_data['irods_request'].path
+                    )
+                except CollectionDoesNotExist:
+                    coll = None
+                if coll:
+                    context_data[
+                        'affected_objects'
+                    ] += irods_backend.get_objs_recursively(irods, coll)
+                    context_data[
+                        'affected_collections'
+                    ] += irods_backend.get_colls_recursively(irods, coll)
         return context_data
 
     def form_valid(self, request, *args, **kwargs):