From 5885fec9e2ddd6520ff1b492892b9536d279d50d Mon Sep 17 00:00:00 2001 From: Mikko Nieminen Date: Wed, 21 Dec 2022 17:34:02 +0100 Subject: [PATCH] refactor and cleanup IrodsAPI (#1546) --- CHANGELOG.rst | 3 + .../management/commands/irodsorphans.py | 11 +-- irodsbackend/api.py | 38 ++++------ irodsbackend/views.py | 8 +-- landingzones/forms.py | 32 ++++----- samplesheets/forms.py | 26 ++++--- samplesheets/plugins.py | 70 +++++++++---------- samplesheets/tests/test_views_taskflow.py | 5 +- samplesheets/views.py | 49 +++++++------ 9 files changed, 122 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 36c67d86..d4f4f794 100644 --- 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 index 74f52048..76bf8e95 100644 --- 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 index 05dae3ad..ef57e5c3 100644 --- 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 index 718b4734..9296dd6c 100644 --- 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 index 55d45038..4f357955 100644 --- a/landingzones/forms.py +++ b/landingzones/forms.py @@ -56,7 +56,6 @@ 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() @@ -64,10 +63,8 @@ def __init__( 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 index dd1e0839..8cf11708 100644 --- 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 index 60bc51cf..b04a68d1 100644 --- 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 index f8c79617..1050e251 100644 --- 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 index a7914f77..96e94d44 100644 --- 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):