diff --git a/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_submission_api.py b/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_submission_api.py index 9681717c64..8e884ada52 100644 --- a/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_submission_api.py +++ b/kobo/apps/openrosa/apps/api/tests/viewsets/test_xform_submission_api.py @@ -233,7 +233,7 @@ def test_post_submission_authenticated_bad_json(self): self.assertTrue('error' in rendered_response.data) self.assertTrue( rendered_response.data['error'].startswith( - 'Received empty submission' + 'Instance ID is required' ) ) self.assertTrue(rendered_response.status_code == 400) @@ -513,7 +513,7 @@ def test_post_concurrent_same_submissions(self): results[result] += 1 assert results[status.HTTP_201_CREATED] == 1 - assert results[status.HTTP_409_CONFLICT] == DUPLICATE_SUBMISSIONS_COUNT - 1 + assert results[status.HTTP_202_ACCEPTED] == DUPLICATE_SUBMISSIONS_COUNT - 1 def submit_data(identifier, survey_, username_, live_server_url, token_): diff --git a/kobo/apps/openrosa/apps/logger/exceptions.py b/kobo/apps/openrosa/apps/logger/exceptions.py index a2944ffd5e..62fd58117a 100644 --- a/kobo/apps/openrosa/apps/logger/exceptions.py +++ b/kobo/apps/openrosa/apps/logger/exceptions.py @@ -10,10 +10,6 @@ class BuildDbQueriesNoConfirmationProvidedError(Exception): pass -class ConflictingXMLHashInstanceError(Exception): - pass - - class DuplicateInstanceError(Exception): def __init__(self, message='Duplicate Instance'): super().__init__(message) @@ -37,6 +33,11 @@ def __init__(self, message='Could not determine the user'): super().__init__(message) +class InstanceIdMissingError(Exception): + def __init__(self, message='Could not determine the instance ID'): + super().__init__(message) + + class InstanceMultipleNodeError(Exception): pass diff --git a/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/multiple_nodes_error.xml b/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/multiple_nodes_error.xml index 9b7b9a3cde..f0569b1166 100644 --- a/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/multiple_nodes_error.xml +++ b/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/multiple_nodes_error.xml @@ -14,4 +14,7 @@ -1.2627557 36.7926442 0.0 30.0 chrome ie + + uuid:364f173c688e482486a48661700466gg + diff --git a/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/new_repeats_2012-07-05-14-33-53.xml b/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/new_repeats_2012-07-05-14-33-53.xml index f0ee825508..eaab7f531e 100644 --- a/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/new_repeats_2012-07-05-14-33-53.xml +++ b/kobo/apps/openrosa/apps/logger/fixtures/new_repeats/instances/new_repeats_2012-07-05-14-33-53.xml @@ -13,4 +13,7 @@ -1.2627557 36.7926442 0.0 30.0 chrome ie + + uuid:364f173c688e482486a48661700466gg + diff --git a/kobo/apps/openrosa/apps/logger/fixtures/test_forms/survey_names/instances/survey_names_2012-08-17_11-24-53.xml b/kobo/apps/openrosa/apps/logger/fixtures/test_forms/survey_names/instances/survey_names_2012-08-17_11-24-53.xml index 4a4550a9f6..6e07597a68 100644 --- a/kobo/apps/openrosa/apps/logger/fixtures/test_forms/survey_names/instances/survey_names_2012-08-17_11-24-53.xml +++ b/kobo/apps/openrosa/apps/logger/fixtures/test_forms/survey_names/instances/survey_names_2012-08-17_11-24-53.xml @@ -1 +1 @@ -2012-08-17T11:24:53.254+03HD2012-08-17T11:24:57.897+03 +2012-08-17T11:24:53.254+03HD2012-08-17T11:24:57.897+03uuid:729f173c688e482486a48661700455ff diff --git a/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_xform_uuid.xml b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_xform_uuid.xml index 78426e9db8..1054169b4c 100644 --- a/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_xform_uuid.xml +++ b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_xform_uuid.xml @@ -7,4 +7,7 @@ 0 -1.2836198 36.8795437 0.0 1044.0 firefox chrome safari + + uuid:364f173c688e482486a48661700466gg + diff --git a/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_unicode_submission.xml b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_unicode_submission.xml index aad35e71a4..92c434acac 100644 --- a/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_unicode_submission.xml +++ b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_unicode_submission.xml @@ -6,4 +6,7 @@ 0 -1.2836198 36.8795437 0.0 1044.0 firefox chrome safari + + uuid:639f173c688e482486a48661700456ty + diff --git a/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_with_attachment/1335783522563.jpg b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_with_attachment/1335783522563.jpg new file mode 100755 index 0000000000..e8d953e387 Binary files /dev/null and b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_with_attachment/1335783522563.jpg differ diff --git a/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_with_attachment/tutorial_2012-06-27_11-27-53_w_attachment.xml b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_with_attachment/tutorial_2012-06-27_11-27-53_w_attachment.xml new file mode 100644 index 0000000000..718993c919 --- /dev/null +++ b/kobo/apps/openrosa/apps/logger/fixtures/tutorial/instances/tutorial_with_attachment/tutorial_2012-06-27_11-27-53_w_attachment.xml @@ -0,0 +1,14 @@ + + + Larry + Again + + 23 + 1335783522563.jpg + 0 + -1.2836198 36.8795437 0.0 1044.0 + firefox chrome safari + + uuid:729f173c688e482486a48661700455ff + + diff --git a/kobo/apps/openrosa/apps/logger/tests/Water_2011_03_17_2011-03-17_16-29-59/Water_2011_03_17_2011-03-17_16-29-59.xml b/kobo/apps/openrosa/apps/logger/tests/Water_2011_03_17_2011-03-17_16-29-59/Water_2011_03_17_2011-03-17_16-29-59.xml index f4e00ed363..a8dfcd00eb 100644 --- a/kobo/apps/openrosa/apps/logger/tests/Water_2011_03_17_2011-03-17_16-29-59/Water_2011_03_17_2011-03-17_16-29-59.xml +++ b/kobo/apps/openrosa/apps/logger/tests/Water_2011_03_17_2011-03-17_16-29-59/Water_2011_03_17_2011-03-17_16-29-59.xml @@ -1 +1 @@ -Andrew1300375832136.jpg9.045921564102173 7.526530623435974 572.0 48.0northwestkebbibunzaRYRR4TRn/aboreholesingle_pointdieselfederal_governmentfederal_government state_governmentTYnonono_diesel missing_stolen_partswell_maintainedyesyear_roundnobody +Andrew1300375832136.jpg9.045921564102173 7.526530623435974 572.0 48.0northwestkebbibunzaRYRR4TRn/aboreholesingle_pointdieselfederal_governmentfederal_government state_governmentTYnonono_diesel missing_stolen_partswell_maintainedyesyear_roundnobodyuuid:435f173c688e482486a48661700467gh diff --git a/kobo/apps/openrosa/apps/logger/tests/data_from_sdcard/bulk_submission.zip b/kobo/apps/openrosa/apps/logger/tests/data_from_sdcard/bulk_submission.zip index 2da0f47884..5d92a67af0 100644 Binary files a/kobo/apps/openrosa/apps/logger/tests/data_from_sdcard/bulk_submission.zip and b/kobo/apps/openrosa/apps/logger/tests/data_from_sdcard/bulk_submission.zip differ diff --git a/kobo/apps/openrosa/apps/logger/tests/test_backup_tools.py b/kobo/apps/openrosa/apps/logger/tests/test_backup_tools.py index 90378b698a..276eaee338 100644 --- a/kobo/apps/openrosa/apps/logger/tests/test_backup_tools.py +++ b/kobo/apps/openrosa/apps/logger/tests/test_backup_tools.py @@ -33,7 +33,7 @@ def test_date_created_override(self): """ xml_file_path = os.path.join( settings.OPENROSA_APP_DIR, "apps", "logger", "fixtures", - "tutorial", "instances", "tutorial_2012-06-27_11-27-53.xml") + "tutorial", "instances", "tutorial_2012-06-27_11-27-53_w_uuid.xml") xml_file = django_file( xml_file_path, field_name="xml_file", content_type="text/xml") media_files = [] diff --git a/kobo/apps/openrosa/apps/logger/tests/test_form_submission.py b/kobo/apps/openrosa/apps/logger/tests/test_form_submission.py index 33bce7684b..796fa4b298 100644 --- a/kobo/apps/openrosa/apps/logger/tests/test_form_submission.py +++ b/kobo/apps/openrosa/apps/logger/tests/test_form_submission.py @@ -10,7 +10,7 @@ from kobo.apps.openrosa.apps.main.models.user_profile import UserProfile from kobo.apps.openrosa.apps.main.tests.test_base import TestBase -from kobo.apps.openrosa.apps.logger.models import Instance +from kobo.apps.openrosa.apps.logger.models import Instance, Attachment from kobo.apps.openrosa.apps.logger.models.instance import InstanceHistory from kobo.apps.openrosa.apps.logger.xform_instance_parser import clean_and_parse_xml from kobo.apps.openrosa.apps.viewer.models.parsed_instance import ParsedInstance @@ -36,7 +36,7 @@ def test_form_post(self): """ xml_submission_file_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), - "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53.xml" + "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_uuid.xml" ) self._make_submission(xml_submission_file_path) @@ -114,7 +114,7 @@ def test_submission_to_not_required_auth_as_anonymous_user(self): xml_submission_file_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), - "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53.xml" + "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_uuid.xml" ) # Anonymous should be able to submit data @@ -154,7 +154,7 @@ def test_submission_to_require_auth_with_perm(self): xml_submission_file_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), - "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53.xml" + "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_uuid.xml" ) self._make_submission(xml_submission_file_path, auth=auth) self.assertEqual(self.response.status_code, 201) @@ -252,6 +252,38 @@ def test_duplicate_submission_with_different_content(self): another_inst = Instance.objects.order_by('pk').last() self.assertEqual(inst.xml, another_inst.xml) + def test_duplicate_submission_with_same_content_but_with_attachment(self): + """ + Test that submitting the same XML content twice, + first without and then with an attachment, + results in a single instance with the attachment added. + """ + xml_submission_file_path = os.path.join( + os.path.dirname(__file__), + "../fixtures/tutorial/instances/tutorial_with_attachment", + "tutorial_2012-06-27_11-27-53_w_attachment.xml" + ) + media_file_path = os.path.join( + os.path.dirname(__file__), + "../fixtures/tutorial/instances/tutorial_with_attachment", + "1335783522563.jpg" + ) + initial_instance_count = Instance.objects.count() + + # Test submission with XML file + self._make_submission(xml_submission_file_path) + initial_instance = Instance.objects.last() + self.assertEqual(self.response.status_code, 201) + self.assertEqual(Instance.objects.count(), initial_instance_count + 1) + + # Test duplicate submission with attachment + with open(media_file_path, 'rb') as media_file: + self._make_submission(xml_submission_file_path, media_file=media_file) + self.assertEqual(self.response.status_code, 201) + self.assertEqual(Instance.objects.count(), initial_instance_count + 1) + self.assertEqual(Attachment.objects.filter(instance=initial_instance).count(), 1) + + def test_owner_can_edit_submissions(self): xml_submission_file_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), @@ -393,7 +425,7 @@ def test_submission_when_requires_auth(self): xml_submission_file_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), - "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53.xml" + "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_uuid.xml" ) auth = DigestAuth('alice', 'alice') self._make_submission( @@ -410,7 +442,7 @@ def test_submission_linked_to_reporter(self): xml_submission_file_path = os.path.join( os.path.dirname(os.path.abspath(__file__)), - "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53.xml" + "../fixtures/tutorial/instances/tutorial_2012-06-27_11-27-53_w_uuid.xml" ) auth = DigestAuth('alice', 'alice') self._make_submission( diff --git a/kobo/apps/openrosa/apps/logger/tests/test_parsing.py b/kobo/apps/openrosa/apps/logger/tests/test_parsing.py index a80c001373..f9fe094571 100644 --- a/kobo/apps/openrosa/apps/logger/tests/test_parsing.py +++ b/kobo/apps/openrosa/apps/logger/tests/test_parsing.py @@ -65,7 +65,10 @@ def test_parse_xform_nested_repeats(self): 'has_kids': '1' }, 'web_browsers': 'chrome ie', - 'gps': '-1.2627557 36.7926442 0.0 30.0' + 'gps': '-1.2627557 36.7926442 0.0 30.0', + 'meta': { + 'instanceID': 'uuid:364f173c688e482486a48661700466gg' + } } } self.assertEqual(dict_, expected_dict) @@ -83,7 +86,8 @@ def test_parse_xform_nested_repeats(self): 'kids/has_kids': '1', 'info/age': '80', 'web_browsers': 'chrome ie', - 'info/name': 'Adam' + 'info/name': 'Adam', + 'meta/instanceID': 'uuid:364f173c688e482486a48661700466gg' } self.assertEqual(flat_dict, expected_flat_dict) @@ -140,6 +144,15 @@ def test_get_uuid_from_xml(self): instanceID = get_uuid_from_xml(xml_str) self.assertEqual(instanceID, "729f173c688e482486a48661700455ff") + # Additional test case for a custom prefixed UUID + submission = """ + + uuid:getodk.org:123456789 + + """ + custom_instance_id = get_uuid_from_xml(submission) + self.assertEqual(custom_instance_id, "getodk.org:123456789") + def test_get_deprecated_uuid_from_xml(self): with open( os.path.join( diff --git a/kobo/apps/openrosa/apps/logger/tests/test_simple_submission.py b/kobo/apps/openrosa/apps/logger/tests/test_simple_submission.py index 574ba38cfa..0bfe486220 100644 --- a/kobo/apps/openrosa/apps/logger/tests/test_simple_submission.py +++ b/kobo/apps/openrosa/apps/logger/tests/test_simple_submission.py @@ -1,4 +1,6 @@ # coding: utf-8 +import uuid + from django.test import TestCase, RequestFactory from pyxform import SurveyElementBuilder @@ -35,9 +37,12 @@ def _get_xml_for_form(self, xform): xform.save() def _submit_at_hour(self, hour): - st_xml = '2012-01-11T%d:00:00.000+00' % hour + st_xml = ( + f'' + f'2012-01-11T{hour}:00:00.000+00' + f'uuid:918a1889-389f-4427-b48a-0ba16b7c9b{hour}' + f'' + ) try: create_instance(self.user.username, TempFileProxy(st_xml), []) except DuplicateInstanceError: @@ -45,8 +50,10 @@ def _submit_at_hour(self, hour): def _submit_simple_yes(self): create_instance(self.user.username, TempFileProxy( - 'Yes<' - '/yesno>'), []) + f'Yes<' + f'/yesno>' + f'uuid:{str(uuid.uuid4())}' + f''), []) def setUp(self): self.user = User.objects.create( diff --git a/kobo/apps/openrosa/apps/logger/xform_instance_parser.py b/kobo/apps/openrosa/apps/logger/xform_instance_parser.py index c1b83addca..c4c55b0550 100644 --- a/kobo/apps/openrosa/apps/logger/xform_instance_parser.py +++ b/kobo/apps/openrosa/apps/logger/xform_instance_parser.py @@ -55,15 +55,19 @@ def get_meta_from_xml(xml_str: str, meta_name: str) -> str: def get_uuid_from_xml(xml): - def _uuid_only(uuid, regex): - matches = regex.match(uuid) - if matches and len(matches.groups()) > 0: - return matches.groups()[0] - return None + def _uuid_only(uuid): + """ + Strips the 'uuid:' prefix from the provided identifier if it exists. + This preserves any custom ID schemes (e.g., 'getodk.org:123456789') + while ensuring only the 'uuid:' prefix is removed. This approach + adheres to the OpenRosa spec, allowing custom prefixes to be stored + intact in the database to prevent potential ID collisions. + """ + return re.sub(r'^uuid:', '', uuid) + uuid = get_meta_from_xml(xml, "instanceID") - regex = re.compile(r"uuid:(.*)") if uuid: - return _uuid_only(uuid, regex) + return _uuid_only(uuid) # check in survey_node attributes xml = clean_and_parse_xml(xml) children = xml.childNodes @@ -74,12 +78,12 @@ def _uuid_only(uuid, regex): survey_node = children[0] uuid = survey_node.getAttribute('instanceID') if uuid != '': - return _uuid_only(uuid, regex) + return _uuid_only(uuid) return None def get_root_uuid_from_xml(xml): - root_uuid = get_meta_from_xml(xml, "rootUuid") + root_uuid = get_meta_from_xml(xml, 'rootUuid') if root_uuid: return root_uuid diff --git a/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_20-52-08.xml b/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_20-52-08.xml index d5c336bef7..d8276b02c4 100644 --- a/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_20-52-08.xml +++ b/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_20-52-08.xml @@ -1 +1 @@ -40.81101715564728 -73.96446704864502 -152.0 16.0 +40.81101715564728 -73.96446704864502 -152.0 16.0uuid:729f173c688e482486a48661700455ff diff --git a/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_21-21-33.xml b/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_21-21-33.xml index fe1f4db02e..57bbed33cb 100644 --- a/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_21-21-33.xml +++ b/kobo/apps/openrosa/apps/main/tests/fixtures/gps/instances/gps_1980-01-23_21-21-33.xml @@ -1 +1 @@ -40.811086893081665 -73.96449387073517 -113.0 16.0 +40.811086893081665 -73.96449387073517 -113.0 16.0uuid:435f173c688e482486a48661700467gh diff --git a/kobo/apps/openrosa/apps/viewer/tests/fixtures/grouped_gps/instances/grouped_gps_01.xml b/kobo/apps/openrosa/apps/viewer/tests/fixtures/grouped_gps/instances/grouped_gps_01.xml index 2cb36d609a..5bfed2204b 100644 --- a/kobo/apps/openrosa/apps/viewer/tests/fixtures/grouped_gps/instances/grouped_gps_01.xml +++ b/kobo/apps/openrosa/apps/viewer/tests/fixtures/grouped_gps/instances/grouped_gps_01.xml @@ -4,4 +4,7 @@ -1.2627107 36.7925771 0.0 37.5 chrome safari + + uuid:435f173c688e482486a486617004534df + diff --git a/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_01.xml b/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_01.xml index 0e5f835ffc..5cd3b97dec 100644 --- a/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_01.xml +++ b/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_01.xml @@ -17,4 +17,7 @@ -1.2627557 36.7926442 0.0 30.0 chrome ie + + uuid:435f173c688e482463a486617004534df + diff --git a/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_02.xml b/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_02.xml index 2d2659147b..5b92f562ad 100644 --- a/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_02.xml +++ b/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_02.xml @@ -13,4 +13,7 @@ -1.2627557 36.7926442 0.0 30.0 chrome ie firefox + + uuid:32sd3c688e482486a48661700455ff + diff --git a/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_03.xml b/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_03.xml new file mode 100644 index 0000000000..5aaf43f931 --- /dev/null +++ b/kobo/apps/openrosa/apps/viewer/tests/fixtures/new_repeats/instances/new_repeats_03.xml @@ -0,0 +1,20 @@ + + + + Adam + 80 + + + 1 + + Liam + 40 + + + Emma + 70 + + +-1.2627557 36.7926442 0.0 30.0 +chrome + diff --git a/kobo/apps/openrosa/apps/viewer/tests/test_pandas_mongo_bridge.py b/kobo/apps/openrosa/apps/viewer/tests/test_pandas_mongo_bridge.py index 79086df69c..9a400c56ff 100644 --- a/kobo/apps/openrosa/apps/viewer/tests/test_pandas_mongo_bridge.py +++ b/kobo/apps/openrosa/apps/viewer/tests/test_pandas_mongo_bridge.py @@ -51,13 +51,16 @@ def _publish_xls_fixture_set_xform(self, fixture): self.xform = XForm.objects.all().reverse()[0] def _submit_fixture_instance( - self, fixture, instance, submission_time=None): + self, fixture, instance, submission_time=None, add_submission_uuid=None): """ Submit an instance at tests/fixtures/[fixture]/instances/[fixture]_[instance].xml """ xml_submission_file_path = xml_inst_filepath_from_fixture_name( fixture, instance) + + if add_submission_uuid: + xml_submission_file_path = self._add_submission_uuid_to_submission_xml(xml_submission_file_path, self.xform) self._make_submission( xml_submission_file_path, forced_submission_time=submission_time) self.assertEqual(self.response.status_code, 201) @@ -207,6 +210,7 @@ def test_csv_columns_for_gps_within_groups(self): 'gps_group/_gps_longitude', 'gps_group/_gps_altitude', 'gps_group/_gps_precision', + 'meta/instanceID', 'web_browsers/firefox', 'web_browsers/chrome', 'web_browsers/ie', @@ -246,6 +250,7 @@ def test_format_mongo_data_for_csv(self): 'kids/kids_details[1]/kids_age': '50', 'kids/kids_details[2]/kids_name': 'Cain', 'kids/kids_details[2]/kids_age': '76', + 'meta/instanceID': 'uuid:435f173c688e482463a486617004534df', 'web_browsers/chrome': True, 'web_browsers/ie': True, 'web_browsers/safari': False, @@ -494,7 +499,7 @@ def test_query_mongo(self): self._publish_single_level_repeat_form() # submit 3 instances for i in range(3): - self._submit_fixture_instance("new_repeats", "01") + self._submit_fixture_instance('new_repeats', '03', None, True) df_builder = XLSDataFrameBuilder(self.user.username, self.xform.id_string) record_count = df_builder._query_mongo(count=True) @@ -526,10 +531,10 @@ def test_csv_export_with_df_size_limit(self): self._publish_single_level_repeat_form() # submit 7 instances for i in range(4): - self._submit_fixture_instance("new_repeats", "01") + self._submit_fixture_instance('new_repeats', '03', None, True) self._submit_fixture_instance("new_repeats", "02") for i in range(2): - self._submit_fixture_instance("new_repeats", "01") + self._submit_fixture_instance('new_repeats', '03', None, True) csv_df_builder = CSVDataFrameBuilder(self.user.username, self.xform.id_string) record_count = csv_df_builder._query_mongo(count=True) diff --git a/kobo/apps/openrosa/libs/tests/mixins/make_submission_mixin.py b/kobo/apps/openrosa/libs/tests/mixins/make_submission_mixin.py index 99c0089b9a..999561c46f 100644 --- a/kobo/apps/openrosa/libs/tests/mixins/make_submission_mixin.py +++ b/kobo/apps/openrosa/libs/tests/mixins/make_submission_mixin.py @@ -1,6 +1,7 @@ # coding: utf-8 import os import re +import uuid from tempfile import NamedTemporaryFile from typing import Union @@ -39,6 +40,33 @@ def _add_uuid_to_submission_xml(self, path, xform): return path + def _add_submission_uuid_to_submission_xml(self, path, xform): + with open(path, 'rb') as _file: + xml_content = _file.read().decode() + + # Find the closing tag of the root element (e.g., ) + closing_tag_index = xml_content.rfind('') + + if closing_tag_index == -1: + raise ValueError("Root element closing tag not found") + + # Construct the meta element with a new UUID + meta_element = f'uuid:{str(uuid.uuid4())}' + + # Insert the meta element before the closing tag of the root element + xml_content = ( + xml_content[:closing_tag_index] + + meta_element + + xml_content[closing_tag_index:] + ) + + # Write the updated XML content to a temporary file and return the path + with NamedTemporaryFile(delete=False, mode='w') as tmp_file: + tmp_file.write(xml_content) + path = tmp_file.name + + return path + def _make_submission( self, path: str, diff --git a/kobo/apps/openrosa/libs/utils/logger_tools.py b/kobo/apps/openrosa/libs/utils/logger_tools.py index b6402aaae1..e1082a6045 100644 --- a/kobo/apps/openrosa/libs/utils/logger_tools.py +++ b/kobo/apps/openrosa/libs/utils/logger_tools.py @@ -42,9 +42,9 @@ from wsgiref.util import FileWrapper from kobo.apps.openrosa.apps.logger.exceptions import ( - ConflictingXMLHashInstanceError, DuplicateUUIDError, FormInactiveError, + InstanceIdMissingError, TemporarilyUnavailableError, ) from kobo.apps.openrosa.apps.logger.models import Attachment, Instance, XForm @@ -168,10 +168,12 @@ def create_instance( # get new and deprecated uuid's new_uuid = get_uuid_from_xml(xml) + if not new_uuid: + raise InstanceIdMissingError() with get_instance_lock(xml_hash, new_uuid, xform.id) as lock_acquired: if not lock_acquired: - raise ConflictingXMLHashInstanceError() + raise DuplicateInstanceError() # Dorey's rule from 2012 (commit 890a67aa): # Ignore submission as a duplicate IFF # * a submission's XForm collects start time @@ -184,13 +186,13 @@ def create_instance( # and still exactly matches an existing submission, it's certainly a # duplicate (https://docs.opendatakit.org/openrosa-metadata/#fields). - if xform.has_start_time or new_uuid is not None: + if xform.has_start_time or new_uuid: # XML matches are identified by identical content hash OR, when a # content hash is not present, by string comparison of the full # content, which is slow! Use the management command # `populate_xml_hashes_for_instances` to hash existing submissions existing_instance = Instance.objects.filter( - Q(xml_hash=xml_hash) | Q(xml_hash=Instance.DEFAULT_XML_HASH, xml=xml), + xml_hash=xml_hash, xform__user=xform.user, ).first() else: @@ -591,6 +593,10 @@ def safe_create_instance( error = OpenRosaResponseBadRequest( t("Received empty submission. No instance was created") ) + except InstanceIdMissingError: + error = OpenRosaResponseBadRequest( + t('Instance ID is required') + ) except FormInactiveError: error = OpenRosaResponseNotAllowed(t("Form is not active")) except TemporarilyUnavailableError: @@ -601,11 +607,6 @@ def safe_create_instance( ) except ExpatError as e: error = OpenRosaResponseBadRequest(t("Improperly formatted XML.")) - except ConflictingXMLHashInstanceError: - response = OpenRosaResponse(t('Conflict with already existing instance')) - response.status_code = 409 - response['Location'] = request.build_absolute_uri(request.path) - error = response except DuplicateInstanceError: response = OpenRosaResponse(t('Duplicate instance')) response.status_code = 202 diff --git a/kobo/apps/subsequences/tests/test_submission_stream.py b/kobo/apps/subsequences/tests/test_submission_stream.py index a8e19b19a2..0af053d518 100644 --- a/kobo/apps/subsequences/tests/test_submission_stream.py +++ b/kobo/apps/subsequences/tests/test_submission_stream.py @@ -257,7 +257,8 @@ def test_stream_with_extras_handles_duplicated_submission_uuids(self): '_uuid': '1c05898e-b43c-491d-814c-79595eb84e81', }, ] - self.asset.deployment.mock_submissions(submissions) + with self.assertRaises(Exception) as ex: + self.asset.deployment.mock_submissions(submissions) # Process submissions with extras output = list( diff --git a/kpi/deployment_backends/mock_backend.py b/kpi/deployment_backends/mock_backend.py index 4dfa0b8e28..ec8d5b7f4d 100644 --- a/kpi/deployment_backends/mock_backend.py +++ b/kpi/deployment_backends/mock_backend.py @@ -5,11 +5,13 @@ from typing import Optional from uuid import uuid4 +from defusedxml import ElementTree as DET from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.utils.dateparse import parse_datetime from kobo.apps.kobo_auth.shortcuts import User +from kobo.apps.openrosa.apps.logger.exceptions import InstanceIdMissingError from kobo.apps.openrosa.libs.utils.logger_tools import ( dict2xform, safe_create_instance, @@ -118,6 +120,14 @@ class FakeRequest: request=request, ) if error: + error_dict = error.__dict__ + if '_container' in error_dict and len(error_dict['_container']) > 0: + xml_response = error_dict['_container'][0] + message_text = DET.fromstring(xml_response).findtext( + './/{http://openrosa.org/http/response}message' + ) + if message_text == 'Instance ID is required': + raise InstanceIdMissingError() raise Exception(error) # Inject (or update) real PKs in submission… diff --git a/kpi/tests/api/v2/test_api_submissions.py b/kpi/tests/api/v2/test_api_submissions.py index d051790f8e..ba084a336a 100644 --- a/kpi/tests/api/v2/test_api_submissions.py +++ b/kpi/tests/api/v2/test_api_submissions.py @@ -23,6 +23,7 @@ from kobo.apps.kobo_auth.shortcuts import User from kobo.apps.openrosa.apps.main.models.user_profile import UserProfile from kobo.apps.openrosa.libs.utils.logger_tools import dict2xform +from kobo.apps.openrosa.apps.logger.exceptions import InstanceIdMissingError from kpi.constants import ( ASSET_TYPE_SURVEY, PERM_CHANGE_ASSET, @@ -1567,6 +1568,16 @@ def test_edit_submission_with_xml_missing_uuids(self): # The form UUID is already omitted by these tests, but fail if that # changes in the future assert 'formhub/uuid' not in submission.keys() + + # Attempt to mock the submission without UUIDs + with self.assertRaises(InstanceIdMissingError) as ex: + self.asset.deployment.mock_submissions([submission], create_uuids=False) + + # Rejecting the submission because it does not have an instance ID + self.assertEqual(str(ex.exception), 'Could not determine the instance ID') + + # Test the edit flow with a submission that has a UUID + submission['meta/instanceID'] = "uuid:9710c729-00a5-41f1-b740-8dd618bb4a49" self.asset.deployment.mock_submissions([submission], create_uuids=False) # Find and verify the new submission @@ -1584,8 +1595,6 @@ def test_edit_submission_with_xml_missing_uuids(self): submission_xml_root = fromstring_preserve_root_xmlns(submission_xml) assert submission_json['_id'] == submission['_id'] assert submission_xml_root.find('./find_this').text == 'hello!' - assert submission_xml_root.find('./meta/instanceID') is None - assert submission_xml_root.find('./formhub/uuid') is None # Get edit endpoint edit_url = reverse(