update rest api views to return 503 with locked project (#1847)

CHANGELOG.rst

@@ -30,6 +30,8 @@ Added
     - Missing assay plugin ``__init__.py`` files (#2014)
     - Study plugin override via ISA-Tab comments (#1885)
     - Token auth support in study plugin IGV XML serving views (#1999, #2021)
+- **Taskflowbackend**
+    - ``TaskflowAPI.raise_submit_api_exception()`` helper (#1847)
 
 Changed
 -------
@@ -52,16 +54,20 @@ Changed
 - **Landingzones**
     - Update REST API versioning (#1936)
     - Update REST API views for OpenAPI compatibility (#1951)
+    - Return ``503`` in ``ZoneSubmitMoveAPIView`` if project is locked (#1847)
 - **Samplesheets**
     - Update REST API versioning (#1936)
     - Update REST API views for OpenAPI compatibility (#1951)
     - Send iRODS delete request emails to all addresses of user (#2000)
     - Disable ontology term select box while querying (#1974)
     - Refactor ``SampleSheetAssayPluginPoint.get_assay_path()`` (#2016)
+    - Return ``503`` in ``IrodsCollsCreateAPIView`` if project is locked (#1847)
+    - Return ``503`` in ``IrodsDataRequestAcceptAPIView`` if project is locked (#1847)
 - **Taskflowbackend**
     - Refactor task tests (#2002)
     - Unify user name parameter naming in flows (#1653)
     - Refactor ``landing_zone_move`` flow (#1846)
+    - Move ``lock_project()`` into ``TaskflowTestMixin`` (#1847)
 
 Fixed
 -----

docs_manual/source/sodar_release_notes.rst

@@ -22,6 +22,7 @@ Release for SODAR Core v1.0 upgrade, iRODS v4.3 upgrade and feature updates.
 - Update minimum supported iRODS version to v4.3.3
 - Update REST API versioning
 - Update REST API views for OpenAPI support
+- Update lock requiring REST API views to return 503 if project is locked
 - Upgrade to Postgres v16
 - Upgrade to python-irodsclient v2.2.0
 - Upgrade to SODAR Core v1.0.2
@@ -63,6 +64,18 @@ REST API Versioning Changes
     upgrading to this version due to the changed database requirements. Also it
     is strongly recommended to go through all documentation before upgrading.
 
+REST API Updates
+----------------
+
+- Sample Sheets API
+    * ``IrodsCollsCreateAPIView``
+        + Return ``503`` if project is locked
+    * ``IrodsDataRequestAcceptAPIView``
+        + Return ``503`` if project is locked
+- Landing Zones API
+    * ``ZoneSubmitMoveAPIView``
+        + Return ``503`` if project is locked
+
 
 v0.15.1 (2024-09-12)
 ====================

landingzones/tests/test_views_api_taskflow.py

@@ -448,6 +448,15 @@ def test_post_validate(self):
             'Successfully validated 0 files',
         )
 
+    def test_post_validate_locked(self):
+        """Test POST for validation with locked project (should fail)"""
+        self.lock_project(self.project)
+        self.landing_zone.status = ZONE_STATUS_FAILED
+        self.landing_zone.save()
+        response = self.request_knox(self.url, method='POST')
+        # NOTE: This should be updated to not require lock, see #1850
+        self.assertEqual(response.status_code, 503)
+
     def test_post_validate_invalid_status(self):
         """Test POST for validation with invalid zone status (should fail)"""
         self.landing_zone.status = ZONE_STATUS_MOVED
@@ -486,6 +495,15 @@ def test_post_move_invalid_status(self):
             LandingZone.objects.first().status, ZONE_STATUS_DELETED
         )
 
+    def test_post_move_locked(self):
+        """Test POST for moving with locked project (should fail)"""
+        self.lock_project(self.project)
+        response = self.request_knox(self.url_move, method='POST')
+        self.assertEqual(response.status_code, 503)
+        self.assertEqual(LandingZone.objects.count(), 1)
+        zone = LandingZone.objects.first()
+        self.assert_zone_status(zone, ZONE_STATUS_FAILED)
+
     @override_settings(REDIS_URL=INVALID_REDIS_URL)
     def test_post_move_lock_failure(self):
         """Test POST for moving with project lock failure"""

landingzones/views_api.py

@@ -384,6 +384,8 @@ class ZoneSubmitMoveAPIView(ZoneMoveMixin, ZoneSubmitBaseAPIView):
     For validating data without moving it to the sample repository, this view
     should be called with ``submit/validate``.
 
+    Returns ``503`` if the project is currently locked by another operation.
+
     **URL for Validation:** ``/landingzones/api/submit/validate/{LandingZone.sodar_uuid}``
 
     **URL for Moving:** ``/landingzones/api/submit/move/{LandingZone.sodar_uuid}``
@@ -402,6 +404,7 @@ def get_operation_id_base(self, path, method, action):
 
     def post(self, request, *args, **kwargs):
         """POST request for initiating landing zone validation/moving"""
+        taskflow = get_backend_api('taskflow')
         zone = LandingZone.objects.filter(
             sodar_uuid=self.kwargs['landingzone']
         ).first()
@@ -423,9 +426,10 @@ def post(self, request, *args, **kwargs):
         try:
             self._submit_validate_move(zone, validate_only)
         except Exception as ex:
-            raise APIException(
-                'Initiating landing zone {} failed: {}'.format(action_msg, ex)
-            )
+            ex_msg = 'Initiating landing zone {} failed: '.format(action_msg)
+            if taskflow:
+                taskflow.raise_submit_api_exception(ex_msg, ex)
+            raise APIException('{}{}'.format(ex_msg, ex))
         return Response(
             {
                 'detail': 'Landing zone {} initiated'.format(action_msg),

samplesheets/tests/test_views_api_taskflow.py

@@ -311,14 +311,17 @@ def test_get(self):
 class TestIrodsCollsCreateAPIView(SampleSheetAPITaskflowTestBase):
     """Tests for IrodsCollsCreateAPIView"""
 
-    def test_post(self):
-        """Test IrodsCollsCreateAPIView POST"""
-        self.assertEqual(self.investigation.irods_status, False)
-        url = reverse(
+    def setUp(self):
+        super().setUp()
+        self.url = reverse(
             'samplesheets:api_irods_colls_create',
             kwargs={'project': self.project.sodar_uuid},
         )
-        response = self.request_knox(url, method='POST')
+
+    def test_post(self):
+        """Test IrodsCollsCreateAPIView POST"""
+        self.assertEqual(self.investigation.irods_status, False)
+        response = self.request_knox(self.url, method='POST')
         self.assertEqual(response.status_code, 200)
         self.investigation.refresh_from_db()
         self.assertEqual(self.investigation.irods_status, True)
@@ -331,13 +334,18 @@ def test_post_created(self):
         # Set up iRODS collections
         self.make_irods_colls(self.investigation)
         self.assertEqual(self.investigation.irods_status, True)
-        url = reverse(
-            'samplesheets:api_irods_colls_create',
-            kwargs={'project': self.project.sodar_uuid},
-        )
-        response = self.request_knox(url, method='POST')
+        response = self.request_knox(self.url, method='POST')
         self.assertEqual(response.status_code, 400)
 
+    def test_post_locked(self):
+        """Test POST with locked project (should fail)"""
+        self.lock_project(self.project)
+        self.assertEqual(self.investigation.irods_status, False)
+        response = self.request_knox(self.url, method='POST')
+        self.assertEqual(response.status_code, 503)
+        self.investigation.refresh_from_db()
+        self.assertEqual(self.investigation.irods_status, False)
+
 
 # NOTE: For TestIrodsAccessTicketListAPIView, see test_views_api
 # NOTE: For TestIrodsAccessTicketRetrieveAPIView, see test_views_api
@@ -1034,9 +1042,22 @@ def test_accept_contributor(self):
         self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0)
         self.assert_alert_count(ACCEPT_ALERT, self.user_contributor, 0)
 
+    def test_accept_locked(self):
+        """Test POST to accept request with locked project (should fail)"""
+        self.lock_project(self.project)
+        self.assert_irods_obj(self.obj_path)
+        response = self.request_knox(self.url, 'POST')
+        self.assertEqual(response.status_code, 503)
+        self.request.refresh_from_db()
+        self.assertEqual(self.request.status, IRODS_REQUEST_STATUS_FAILED)
+        self.assert_irods_obj(self.obj_path)
+        self.assert_alert_count(ACCEPT_ALERT, self.user, 0)
+        self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0)
+        self.assert_alert_count(ACCEPT_ALERT, self.user_contributor, 0)
+
     @override_settings(REDIS_URL=INVALID_REDIS_URL)
     def test_accept_lock_failure(self):
-        """Test POST toa ccept request with project lock failure"""
+        """Test POST to accept request with project lock failure"""
         self.assert_irods_obj(self.obj_path)
         response = self.request_knox(self.url, 'POST')
         self.assertEqual(response.status_code, 400)

samplesheets/views_api.py

@@ -153,6 +153,8 @@ class IrodsCollsCreateAPIView(
     """
     Create iRODS collections for a project.
 
+    Returns ``503`` if the project is currently locked by another operation.
+
     **URL:** ``/samplesheets/api/irods/collections/create/{Project.sodar_uuid}``
 
     **Methods:** ``POST``
@@ -168,6 +170,7 @@ class IrodsCollsCreateAPIView(
     def post(self, request, *args, **kwargs):
         """POST request for creating iRODS collections"""
         irods_backend = get_backend_api('omics_irods')
+        taskflow = get_backend_api('taskflow')
         ex_msg = 'Creating iRODS collections failed: '
         investigation = Investigation.objects.filter(
             project__sodar_uuid=self.kwargs.get('project'), active=True
@@ -183,6 +186,8 @@ def post(self, request, *args, **kwargs):
         try:
             self.create_colls(investigation, request)
         except Exception as ex:
+            if taskflow:
+                taskflow.raise_submit_api_exception(ex_msg, ex)
             raise APIException('{}{}'.format(ex_msg, ex))
         return Response(
             {
@@ -750,7 +755,9 @@ class IrodsDataRequestAcceptAPIView(
     Accept an iRODS data request for a project.
 
     Accepting will delete the iRODS collection or data object targeted by the
-    request. This action can not  be undone.
+    request. This action can not be undone.
+
+    Returns ``503`` if the project is currently locked by another operation.
 
     **URL:** ``/samplesheets/api/irods/request/accept/{IrodsDataRequest.sodar_uuid}``
 
@@ -780,9 +787,10 @@ def post(self, request, *args, **kwargs):
                 app_alerts=app_alerts,
             )
         except Exception as ex:
-            raise ValidationError(
-                '{} {}'.format('Accepting ' + IRODS_REQUEST_EX_MSG + ':', ex)
-            )
+            ex_msg = 'Accepting ' + IRODS_REQUEST_EX_MSG + ': '
+            if taskflow:
+                taskflow.raise_submit_api_exception(ex_msg, ex, ValidationError)
+            raise ValidationError('{}{}'.format(ex_msg, ex))
         return Response(
             {'detail': 'iRODS data request accepted'}, status=status.HTTP_200_OK
         )

taskflowbackend/api.py

@@ -3,6 +3,8 @@
 import json
 import logging
 
+from rest_framework.exceptions import APIException
+
 # Landingzones dependency
 from landingzones.constants import (
     ZONE_STATUS_NOT_CREATED,
@@ -17,7 +19,7 @@
 from projectroles.plugins import get_backend_api
 
 from taskflowbackend import flows
-from taskflowbackend.lock_api import ProjectLockAPI
+from taskflowbackend.lock_api import ProjectLockAPI, PROJECT_LOCKED_MSG
 from taskflowbackend.tasks_celery import submit_flow_task
 
 
@@ -39,6 +41,9 @@ class TaskflowAPI:
     class FlowSubmitException(Exception):
         """SODAR Taskflow submission exception"""
 
+    #: Exception message for locked project
+    project_locked_msg = PROJECT_LOCKED_MSG
+
     @classmethod
     def _raise_flow_exception(cls, ex_msg, tl_event=None, zone=None):
         """
@@ -87,6 +92,27 @@ def _raise_lock_exception(cls, ex_msg, tl_event=None, zone=None):
             lock_zone,
         )
 
+    # HACK for returning 503 if project is locked (see #1505, #1847)
+    @classmethod
+    def raise_submit_api_exception(
+        cls, msg_prefix, ex, default_class=APIException
+    ):
+        """
+        Raise zone submit API exception. Selects appropriate API response based
+        on exception type.
+
+        :param msg_prefix: API response prefix
+        :param ex: Exception object
+        :param default_class: Default API exception class to be returned
+        :raises: Exception of varying type
+        """
+        msg = '{}{}'.format(msg_prefix, ex)
+        if PROJECT_LOCKED_MSG in msg:
+            ex = APIException(msg)
+            ex.status_code = 503
+            raise ex
+        raise default_class(msg)
+
     @classmethod
     def get_flow(
         cls,

taskflowbackend/lock_api.py

@@ -12,6 +12,7 @@
 LOCK_ENABLED = settings.TASKFLOW_LOCK_ENABLED
 LOCK_RETRY_COUNT = settings.TASKFLOW_LOCK_RETRY_COUNT
 LOCK_RETRY_INTERVAL = settings.TASKFLOW_LOCK_RETRY_INTERVAL
+PROJECT_LOCKED_MSG = 'Project is locked by another operation'
 
 
 logger = logging.getLogger('__name__')
@@ -79,7 +80,7 @@ def acquire(
                     return True
                 time.sleep(retry_interval)
         cls._log_status(lock, unlock=False, failed=True)
-        raise LockAcquireException('Project is locked by another operation')
+        raise LockAcquireException(PROJECT_LOCKED_MSG)
 
     @classmethod
     def release(cls, lock):

taskflowbackend/tests/base.py

@@ -45,8 +45,11 @@
     PROJECTROLES_API_DEFAULT_VERSION,
 )
 
+from taskflowbackend.lock_api import ProjectLockAPI
+
 
 app_settings = AppSettingAPI()
+lock_api = ProjectLockAPI()
 logger = logging.getLogger(__name__)
 
 
@@ -73,11 +76,19 @@
 class TaskflowTestMixin(ProjectMixin, RoleMixin, RoleAssignmentMixin):
     """Setup/teardown methods and helpers for taskflow tests"""
 
+    #: Project lock coordinator
+    coordinator = None
     #: iRODS backend object
     irods_backend = None
     #: iRODS session object
     irods = None
 
+    def lock_project(self, project):
+        self.coordinator = lock_api.get_coordinator()
+        lock_id = str(project.sodar_uuid)
+        lock = self.coordinator.get_lock(lock_id)
+        lock_api.acquire(lock)
+
     def make_irods_object(
         self, coll, obj_name, content=None, content_length=1024, checksum=True
     ):

taskflowbackend/tests/test_flows.py

@@ -59,7 +59,6 @@
     PUBLIC_GROUP,
 )
 from taskflowbackend.flows.sheet_delete import Flow as SheetDeleteFlow
-from taskflowbackend.lock_api import ProjectLockAPI
 from taskflowbackend.tasks.irods_tasks import META_EMPTY_VALUE
 from taskflowbackend.tests.base import (
     TaskflowViewTestBase,
@@ -68,9 +67,6 @@
 )
 
 
-lock_api = ProjectLockAPI()
-
-
 # SODAR constants
 PROJECT_TYPE_CATEGORY = SODAR_CONSTANTS['PROJECT_TYPE_CATEGORY']
 PROJECT_TYPE_PROJECT = SODAR_CONSTANTS['PROJECT_TYPE_PROJECT']
@@ -96,12 +92,6 @@ def build_and_run(self, flow, force_fail=False):
         flow.build(force_fail)
         flow.run()
 
-    def lock_project(self, project):
-        self.coordinator = lock_api.get_coordinator()
-        lock_id = str(project.sodar_uuid)
-        lock = self.coordinator.get_lock(lock_id)
-        lock_api.acquire(lock)
-
 
 class TestDataDelete(TaskflowbackendFlowTestBase):
     """Tests for the data_delete flow"""