fix(submissions): return proper status code instead of 202 when lock is held DEV-349 (#5805)

### 📣 Summary
Avoid returning HTTP 202 when an advisory lock prevents a task from
running, and use a more appropriate status code to reflect the lock
state.

### 📖 Description
Previously, when an operation was skipped due to an advisory lock
already being held (e.g., to prevent duplicate task execution), the API
would return a 202 Accepted, which was misleading since the operation
wasn't actually queued or performed.

This PR changes the behavior to return a more accurate status code (423
Locked) when a lock is in place and prevents the task from running.

Author: noliveleger

kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_submission_api.py

@@ -12,22 +12,21 @@
 from django.core.files.uploadedfile import InMemoryUploadedFile
 from django.test.testcases import LiveServerTestCase
 from django_digest.test import DigestAuth
+from rest_framework import status
 from rest_framework.authtoken.models import Token
 
 from kobo.apps.kobo_auth.shortcuts import User
-from kobo.apps.openrosa.apps.main.models import UserProfile
-from kobo.apps.openrosa.libs.tests.mixins.request_mixin import RequestMixin
-from rest_framework import status
-
 from kobo.apps.openrosa.apps.api.tests.viewsets.test_abstract_viewset import (
     TestAbstractViewSet,
 )
 from kobo.apps.openrosa.apps.api.viewsets.xform_submission_api import XFormSubmissionApi
 from kobo.apps.openrosa.apps.logger.models import Attachment
 from kobo.apps.openrosa.apps.main import tests as main_tests
+from kobo.apps.openrosa.apps.main.models import UserProfile
 from kobo.apps.openrosa.libs.constants import CAN_ADD_SUBMISSIONS
-from kobo.apps.openrosa.libs.utils.guardian import assign_perm
+from kobo.apps.openrosa.libs.tests.mixins.request_mixin import RequestMixin
 from kobo.apps.openrosa.libs.utils import logger_tools
+from kobo.apps.openrosa.libs.utils.guardian import assign_perm
 from kobo.apps.openrosa.libs.utils.logger_tools import (
     OpenRosaResponseNotAllowed,
     OpenRosaTemporarilyUnavailable,
@@ -610,7 +609,7 @@ def test_post_concurrent_same_submissions(self):
                 results[result] += 1
 
         assert results[status.HTTP_201_CREATED] == 1
-        assert results[status.HTTP_202_ACCEPTED] == DUPLICATE_SUBMISSIONS_COUNT - 1
+        assert results[status.HTTP_423_LOCKED] == DUPLICATE_SUBMISSIONS_COUNT - 1
 
 
 def submit_data(identifier, survey_, username_, live_server_url, token_):

kobo/apps/openrosa/apps/logger/exceptions.py

@@ -1,3 +1,6 @@
+from django.utils.translation import gettext_lazy as t
+
+
 class AccountInactiveError(Exception):
     pass
 
@@ -15,17 +18,17 @@ class BuildDbQueriesNoConfirmationProvidedError(Exception):
 
 
 class ConflictingAttachmentBasenameError(Exception):
-    def __init__(self, message='Attachment with same name already exists'):
+    def __init__(self, message=t('Attachment with same name already exists')):
         super().__init__(message)
 
 
 class ConflictingSubmissionUUIDError(Exception):
-    def __init__(self, message='Submission with this instance ID already exists'):
+    def __init__(self, message=t('Submission with this instance ID already exists')):
         super().__init__(message)
 
 
 class DuplicateInstanceError(Exception):
-    def __init__(self, message='Duplicate Instance'):
+    def __init__(self, message=t('Duplicate Instance')):
         super().__init__(message)
 
 
@@ -38,17 +41,17 @@ class FormInactiveError(Exception):
 
 
 class InstanceEmptyError(Exception):
-    def __init__(self, message='Empty instance'):
+    def __init__(self, message=t('Empty instance')):
         super().__init__(message)
 
 
 class InstanceInvalidUserError(Exception):
-    def __init__(self, message='Could not determine the user'):
+    def __init__(self, message=t('Could not determine the user')):
         super().__init__(message)
 
 
 class InstanceIdMissingError(Exception):
-    def __init__(self, message='Could not determine the instance ID'):
+    def __init__(self, message=t('Could not determine the instance ID')):
         super().__init__(message)
 
 
@@ -57,7 +60,12 @@ class InstanceMultipleNodeError(Exception):
 
 
 class InstanceParseError(Exception):
-    def __init__(self, message='The instance could not be parsed'):
+    def __init__(self, message=t('The instance could not be parsed')):
+        super().__init__(message)
+
+
+class LockedSubmissionError(Exception):
+    def __init__(self, message=t('Submission is currently being processed.')):
         super().__init__(message)
 
 

kobo/apps/openrosa/apps/logger/models/instance.py

@@ -202,9 +202,8 @@ def _set_uuid(self):
 
     def _populate_root_uuid(self):
         if self.xml and not self.root_uuid:
-            assert (
-                root_uuid := get_root_uuid_from_xml(self.xml)
-            ), 'root_uuid should not be empty'
+            root_uuid, _ = get_root_uuid_from_xml(self.xml)
+            assert root_uuid, 'root_uuid should not be empty'
             self.root_uuid = root_uuid
 
     def _populate_xml_hash(self):

kobo/apps/openrosa/apps/logger/xform_instance_parser.py

@@ -81,13 +81,17 @@ def get_uuid_from_xml(xml):
     return None
 
 
-def get_root_uuid_from_xml(xml):
+def get_root_uuid_from_xml(xml) -> tuple[str, bool]:
+    """
+    Returns the value of <meta/rootUuid> from the given XML. If that node is missing,
+    falls back to <meta/instanceID>. The boolean indicates whether a fallback was used.
+    """
     root_uuid = get_meta_from_xml(xml, 'rootUuid')
     if root_uuid:
-        return remove_uuid_prefix(root_uuid)
+        return remove_uuid_prefix(root_uuid), False
 
     # If no rootUuid, fall back to instanceID
-    return get_uuid_from_xml(xml)
+    return get_uuid_from_xml(xml), True
 
 
 def get_submission_date_from_xml(xml) -> Optional[datetime]:

kobo/apps/openrosa/libs/utils/logger_tools.py

@@ -34,6 +34,8 @@
 from django.utils.encoding import DjangoUnicodeDecodeError, smart_str
 from django.utils.translation import gettext as t
 from modilabs.utils.subprocess_timeout import ProcessTimedOut
+from pyxform.errors import PyXFormError
+from pyxform.xform2json import create_survey_element_from_xml
 from rest_framework.exceptions import NotAuthenticated
 
 from kobo.apps.openrosa.apps.logger.exceptions import (
@@ -47,6 +49,7 @@
     InstanceIdMissingError,
     InstanceInvalidUserError,
     InstanceMultipleNodeError,
+    LockedSubmissionError,
     TemporarilyUnavailableError,
 )
 from kobo.apps.openrosa.apps.logger.models import Attachment, Instance, XForm
@@ -67,6 +70,7 @@
     clean_and_parse_xml,
     get_abbreviated_xpath,
     get_deprecated_uuid_from_xml,
+    get_root_uuid_from_xml,
     get_submission_date_from_xml,
     get_uuid_from_xml,
     get_xform_media_question_xpaths,
@@ -83,8 +87,6 @@
 from kpi.utils.hash import calculate_hash
 from kpi.utils.mongo_helper import MongoHelper
 from kpi.utils.object_permission import get_database_user
-from pyxform.errors import PyXFormError
-from pyxform.xform2json import create_survey_element_from_xml
 
 OPEN_ROSA_VERSION_HEADER = 'X-OpenRosa-Version'
 HTTP_OPEN_ROSA_VERSION_HEADER = 'HTTP_X_OPENROSA_VERSION'
@@ -182,11 +184,10 @@ def create_instance(
 
     Raises:
         InstanceIdMissingError: If no valid UUID is found in the XML submission.
-        DuplicateInstanceError: If an advisory lock is not acquired, or if there
-                                is a submission with the same XML hash and user
+        DuplicateInstanceError: If there is a submission with the same XML hash and user
                                 without any new attachments.
-        ConflictingSubmissionUUIDError: If the same UUID is submitted with
-                                        differing content.
+        ConflictingSubmissionUUIDError: If the same UUID already exists or being
+                                        processed.
         PermissionDenied: If the submission fails permission checks.
     """
 
@@ -198,15 +199,19 @@ def create_instance(
     xform = get_xform_from_submission(xml, username, uuid)
     check_submission_permissions(request, xform)
 
-    # get new and deprecated uuid's
-    new_uuid = get_uuid_from_xml(xml)
+    # get root uuid
+    root_uuid, fallback_on_uuid = get_root_uuid_from_xml(xml)
+    new_uuid = root_uuid if fallback_on_uuid else get_uuid_from_xml(xml)
     if not new_uuid:
         raise InstanceIdMissingError
 
     with kc_transaction_atomic():
-        with get_instance_lock(xml_hash, new_uuid, xform.id) as lock_acquired:
+        with get_instance_lock(root_uuid, xform.id) as lock_acquired:
             if not lock_acquired:
-                raise DuplicateInstanceError
+                raise LockedSubmissionError(
+                    f'Submission {root_uuid} is currently being processed. '
+                    f'Try again later.'
+                )
 
             # Check for an existing instance
             existing_instance = Instance.objects.filter(
@@ -256,21 +261,19 @@ def dict2xform(submission: dict, xform_id_string: str) -> str:
 
 
 @contextlib.contextmanager
-def get_instance_lock(xml_hash: str, submission_uuid: str, xform_id: int) -> bool:
+def get_instance_lock(submission_uuid: str, xform_id: int) -> bool:
     """
     Acquires a PostgreSQL advisory lock to prevent race conditions when
     processing form submissions. This ensures that submissions with the same
-    unique identifiers (xform_id, submission_uuid, and xml_hash) are handled
+    unique identifiers (xform_id, submission_uuid) are handled
     sequentially, preventing duplicate records in the database.
 
     A unique integer lock key (int_lock) is generated by creating a SHAKE-128
     hash of the unique identifiers, truncating it to 7 bytes, and converting it
     to an integer.
     """
     int_lock = int.from_bytes(
-        hashlib.shake_128(
-            f'{xform_id}!!{submission_uuid}!!{xml_hash}'.encode()
-        ).digest(7), 'little'
+        hashlib.shake_128(f'{xform_id}!!{submission_uuid}'.encode()).digest(7), 'little'
     )
     acquired = False
 
@@ -389,9 +392,13 @@ def status_code(self):
         response.status_code = 409
         response['Location'] = request.build_absolute_uri(request.path)
         result.http_error_response = response
-    except DuplicateInstanceError:
-        result.error = t('Duplicate submission')
-        response = OpenRosaResponse(result.error)
+    except LockedSubmissionError as e:
+        response = OpenRosaResponse(str(e))
+        response.status_code = 423
+        response['Location'] = request.build_absolute_uri(request.path)
+        result.http_error_response = response
+    except DuplicateInstanceError as e:
+        response = OpenRosaResponse(str(e))
         response.status_code = 202
         response['Location'] = request.build_absolute_uri(request.path)
         result.http_error_response = response