fix finished zone status updating (#1909)

CHANGELOG.rst

@@ -59,6 +59,7 @@ Fixed
 - **Landingzones**
     - Stats badge displayed to superusers for ``DELETED`` zones (#1866)
     - Zone status updating not working in project details card (#1902)
+    - Modifying finished lock status allowed in ``SetLandingZoneStatusTask`` (#1909)
 - **Samplesheets**
     - Invalid WebDAV URLs generated in ``IrodsDataRequestListView`` (#1860)
     - Superuser not allowed to edit iRODS request from other users in UI (#1863)
@@ -71,6 +72,7 @@ Fixed
     - Hardcoded iRODS path length in ``landing_zone_move`` (#1888)
     - Uncaught exceptions in ``SetAccessTask`` (#1906)
     - Crash in ``landing_zone_create`` with large amount of collections (#1905)
+    - Finished landing zone status modified by lock exception (#1909)
 
 Removed
 -------

landingzones/tasks_taskflow.py

@@ -19,6 +19,7 @@
 
 from landingzones.constants import (
     STATUS_BUSY,
+    STATUS_FINISHED,
     ZONE_STATUS_FAILED,
     ZONE_STATUS_NOT_CREATED,
     ZONE_STATUS_MOVED,
@@ -381,10 +382,13 @@ def execute(
         *args,
         **kwargs
     ):
-        self.set_status(
-            landing_zone, flow_name, status, status_info, extra_data
-        )
-        self.data_modified = True
+        # Prevent setting status if already finished (see #1909)
+        landing_zone.refresh_from_db()
+        if landing_zone.status not in STATUS_FINISHED:
+            self.set_status(
+                landing_zone, flow_name, status, status_info, extra_data
+            )
+            self.data_modified = True
         super().execute(*args, **kwargs)
 
     def revert(

taskflowbackend/api.py

@@ -8,6 +8,7 @@
     ZONE_STATUS_NOT_CREATED,
     ZONE_STATUS_CREATING,
     ZONE_STATUS_FAILED,
+    STATUS_FINISHED,
 )
 from landingzones.models import LandingZone
 
@@ -62,6 +63,29 @@ def _raise_flow_exception(cls, ex_msg, tl_event=None, zone=None):
         # TODO: Create app alert for failure if async (see #1499)
         raise cls.FlowSubmitException(ex_msg)
 
+    @classmethod
+    def _raise_lock_exception(cls, ex_msg, tl_event=None, zone=None):
+        """
+        Raise exception specifically for project lock errors. Updates the status
+        of the landing zone only if zone has not yet been finished by a
+        concurrent taskflow.
+
+        :param ex_msg: Exception message (string)
+        :param tl_event: Timeline event or None
+        :param zone: LandingZone object or None
+        :raise: FlowSubmitException
+        """
+        lock_zone = None
+        if zone:
+            zone.refresh_from_db()
+            if zone.status not in STATUS_FINISHED:
+                lock_zone = zone  # Exclude zone if already finished (see #1909)
+        cls._raise_flow_exception(
+            LOCK_FAIL_MSG + ': ' + str(ex_msg),
+            tl_event,
+            lock_zone,
+        )
+
     @classmethod
     def get_flow(
         cls,
@@ -137,22 +161,16 @@ def run_flow(
             # Acquire lock
             coordinator = lock_api.get_coordinator()
             if not coordinator:
-                cls._raise_flow_exception(
-                    LOCK_FAIL_MSG + ': Failed to retrieve lock coordinator',
-                    tl_event,
-                    zone,
+                cls._raise_lock_exception(
+                    'Failed to retrieve lock coordinator', tl_event, zone
                 )
             else:
                 lock_id = str(project.sodar_uuid)
                 lock = coordinator.get_lock(lock_id)
                 try:
                     lock_api.acquire(lock)
                 except Exception as ex:
-                    cls._raise_flow_exception(
-                        LOCK_FAIL_MSG + ': {}'.format(ex),
-                        tl_event,
-                        zone,
-                    )
+                    cls._raise_lock_exception(str(ex), tl_event, zone)
         else:
             logger.info('Lock not required (flow.require_lock=False)')
 

taskflowbackend/tests/test_flows.py

@@ -81,6 +81,7 @@
 UPDATED_DESC = 'updated description'
 SCRIPT_USER_NAME = 'script_user'
 IRODS_ROOT_PATH = 'sodar/root'
+INVALID_REDIS_URL = 'redis://127.0.0.1:6666/0'
 
 
 class TaskflowbackendFlowTestBase(TaskflowViewTestBase):
@@ -454,7 +455,7 @@ def setUp(self):
         self.make_irods_colls(self.investigation)
 
     def test_delete(self):
-        """Test landing_zone_delete with an empty landing zone"""
+        """Test landing_zone_delete with empty landing zone"""
         zone = self.make_landing_zone(
             title=ZONE_TITLE,
             project=self.project,
@@ -561,6 +562,54 @@ def test_delete_files_restrict(self):
         self.assertEqual(self.irods.data_objects.exists(obj_path), False)
         self.assertEqual(self.irods.collections.exists(zone_path), False)
 
+    def test_delete_finished(self):
+        """Test landing_zone_delete with finished zone"""
+        # NOTE: This may happen with concurrent requests. See #1909, #1910
+        zone = self.make_landing_zone(
+            title=ZONE_TITLE,
+            project=self.project,
+            user=self.user,
+            assay=self.assay,
+            description=ZONE_DESC,
+            status=ZONE_STATUS_DELETED,
+        )
+        # Do not create in taskflow
+        self.assertEqual(zone.status, ZONE_STATUS_DELETED)
+        flow_data = {'zone_uuid': str(zone.sodar_uuid)}
+        flow = self.taskflow.get_flow(
+            irods_backend=self.irods_backend,
+            project=self.project,
+            flow_name='landing_zone_delete',
+            flow_data=flow_data,
+        )
+        self._build_and_run(flow)
+        zone.refresh_from_db()
+        # This should still be deleted, not failed
+        self.assertEqual(zone.status, ZONE_STATUS_DELETED)
+
+    @override_settings(REDIS_URL=INVALID_REDIS_URL)
+    def test_delete_finished_lock_failure(self):
+        """Test landing_zone_delete with finished zone and lock failure"""
+        zone = self.make_landing_zone(
+            title=ZONE_TITLE,
+            project=self.project,
+            user=self.user,
+            assay=self.assay,
+            description=ZONE_DESC,
+            status=ZONE_STATUS_DELETED,
+        )
+        self.assertEqual(zone.status, ZONE_STATUS_DELETED)
+        flow_data = {'zone_uuid': str(zone.sodar_uuid)}
+        flow = self.taskflow.get_flow(
+            irods_backend=self.irods_backend,
+            project=self.project,
+            flow_name='landing_zone_delete',
+            flow_data=flow_data,
+        )
+        self._build_and_run(flow)
+        zone.refresh_from_db()
+        self.assertEqual(zone.status, ZONE_STATUS_DELETED)
+
 
 class TestLandingZoneMove(
     LandingZoneMixin,