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..533acce9b0 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) diff --git a/kobo/apps/openrosa/apps/logger/xform_instance_parser.py b/kobo/apps/openrosa/apps/logger/xform_instance_parser.py index c1b83addca..ea25f70992 100644 --- a/kobo/apps/openrosa/apps/logger/xform_instance_parser.py +++ b/kobo/apps/openrosa/apps/logger/xform_instance_parser.py @@ -55,15 +55,12 @@ 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): + 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 +71,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..df770eeeb3 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..5b45468154 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/test_pandas_mongo_bridge.py b/kobo/apps/openrosa/apps/viewer/tests/test_pandas_mongo_bridge.py index 79086df69c..b711741911 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 @@ -207,6 +207,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 +247,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, diff --git a/kobo/apps/openrosa/libs/utils/logger_tools.py b/kobo/apps/openrosa/libs/utils/logger_tools.py index b6402aaae1..66457dce30 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