fix: encode submission xml as utf-8 to avoid default latin-1

- strings passed in to request(data=some_str) get encoded as latin-1 by default. Data passed in as dictionaries to `json` or `params` are encoded to utf-8 by default. So encode the XML. - forms.py: tidy up docstring wrap length to 90 chars - submissions.py: - add encoding parameter, default utf-8, for encoding the XML, for submission.create and submission.edit - move example submission edit XML from private ._put method to public .edit method so users can see it. - test_client.py: add integration tests for non-ascii form_ids and instance_ids, incl. check for retrieval after create/update.
getodk · May 11, 2023 · ffc4ae7 · ffc4ae7
1 parent c0cc71a
commit ffc4ae7
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 23 deletions.
diff --git a/pyodk/_endpoints/forms.py b/pyodk/_endpoints/forms.py
@@ -137,8 +137,8 @@ def update(
         * form attachments only
         * form attachments with `version_updater`
 
-        If a definition is provided, the new version name must be specified in the definition.
-        If no definition is provided, a default version will be set using
+        If a definition is provided, the new version name must be specified in the
+        definition. If no definition is provided, a default version will be set using
         the current datetime is ISO format.
 
         The default datetime version can be overridden by providing a `version_updater`
@@ -150,12 +150,12 @@ def update(
 
         :param form_id: The xmlFormId of the Form being referenced.
         :param project_id: The id of the project this form belongs to.
-        :param definition: The path to a form definition file to upload. The form definition
-          must include an updated version string.
+        :param definition: The path to a form definition file to upload. The form
+          definition must include an updated version string.
         :param attachments: The paths of the form attachment file(s) to upload.
         :param version_updater: A function that accepts a version name string and returns
-          a version name string, which is used for the new form version. Not allowed if a form
-          definition is specified.
+          a version name string, which is used for the new form version. Not allowed if a
+          form definition is specified.
         """
         if definition is None and attachments is None:
             raise PyODKError("Must specify a form definition and/or attachments.")

diff --git a/pyodk/_endpoints/submissions.py b/pyodk/_endpoints/submissions.py
@@ -196,6 +196,7 @@ def create(
         form_id: Optional[str] = None,
         project_id: Optional[int] = None,
         device_id: Optional[str] = None,
+        encoding: str = "utf-8",
     ) -> Submission:
         """
         Create a Submission.
@@ -216,6 +217,7 @@ def create(
         :param form_id: The xmlFormId of the Form being referenced.
         :param project_id: The id of the project this form belongs to.
         :param device_id: An optional deviceID associated with the submission.
+        :param encoding: The encoding of the submission XML, default "utf-8".
         """
         try:
             pid = pv.validate_project_id(project_id, self.default_project_id)
@@ -233,7 +235,7 @@ def create(
             logger=log,
             headers={"Content-Type": "application/xml"},
             params=params,
-            data=xml,
+            data=xml.encode(encoding=encoding),
         )
         data = response.json()
         return Submission(**data)
@@ -244,27 +246,16 @@ def _put(
         xml: str,
         form_id: Optional[str] = None,
         project_id: Optional[int] = None,
+        encoding: str = "utf-8",
     ) -> Submission:
         """
         Update Submission data.
 
-        Example submission XML structure:
-
-        ```
-        <data id="my_form" version="v1">
-          <meta>
-            <deprecatedID>uuid:85cb9aff-005e-4edd-9739-dc9c1a829c44</deprecatedID>
-            <instanceID>uuid:315c2f74-c8fc-4606-ae3f-22f8983e441e</instanceID>
-          </meta>
-          <name>Alice</name>
-          <age>36</age>
-        </data>
-        ```
-
         :param instance_id: The instanceId of the Submission being referenced.
         :param xml: The submission XML.
         :param form_id: The xmlFormId of the Form being referenced.
         :param project_id: The id of the project this form belongs to.
+        :param encoding: The encoding of the submission XML, default "utf-8".
         """
         try:
             pid = pv.validate_project_id(project_id, self.default_project_id)
@@ -281,7 +272,7 @@ def _put(
             ),
             logger=log,
             headers={"Content-Type": "application/xml"},
-            data=xml,
+            data=xml.encode(encoding=encoding),
         )
         data = response.json()
         return Submission(**data)
@@ -330,18 +321,37 @@ def edit(
         form_id: Optional[str] = None,
         project_id: Optional[int] = None,
         comment: Optional[str] = None,
+        encoding: str = "utf-8",
     ) -> None:
         """
         Edit a submission and optionally comment on it.
 
-        :param instance_id: The instanceId of the Submission being referenced.
+        Example edited submission XML structure:
+
+        ```
+        <data id="my_form" version="v1">
+          <meta>
+            <deprecatedID>uuid:85cb9aff-005e-4edd-9739-dc9c1a829c44</deprecatedID>
+            <instanceID>uuid:315c2f74-c8fc-4606-ae3f-22f8983e441e</instanceID>
+          </meta>
+          <name>Alice</name>
+          <age>36</age>
+        </data>
+        ```
+
+        :param instance_id: The instanceId of the Submission being referenced. The
+          `instance_id` each Submission is first submitted with will always represent
+          that Submission as a whole. Each version of the Submission, though, has its own
+          `instance_id`. So `instance_id` will not necessarily match the values in
+           the XML elements named `instanceID` and `deprecatedID`.
         :param xml: The submission XML.
         :param form_id: The xmlFormId of the Form being referenced.
         :param project_id: The id of the project this form belongs to.
         :param comment: The text of the comment.
+        :param encoding: The encoding of the submission XML, default "utf-8".
         """
         fp_ids = {"form_id": form_id, "project_id": project_id}
-        self._put(instance_id=instance_id, xml=xml, **fp_ids)
+        self._put(instance_id=instance_id, xml=xml, encoding=encoding, **fp_ids)
         if comment is not None:
             self.add_comment(instance_id=instance_id, comment=comment, **fp_ids)
 

diff --git a/tests/resources/forms/non_ascii_form_id.xlsx b/tests/resources/forms/non_ascii_form_id.xlsx
diff --git a/tests/resources/forms/✅.xlsx b/tests/resources/forms/✅.xlsx
diff --git a/tests/test_client.py b/tests/test_client.py
@@ -1,3 +1,4 @@
+from datetime import datetime
 from unittest import TestCase, skip
 
 from pyodk.client import Client
@@ -49,6 +50,26 @@ def test_form_update__new_definition_and_attachments(self):
                 attachments=[(RESOURCES / "forms" / "fruits.csv").as_posix()],
             )
 
+    def test_form_update__new_definition_and_attachments__non_ascii_dingbat(self):
+        """Should create a new version with new definition and attachment."""
+        with Client() as client:
+            client.forms.update(
+                form_id="✅",
+                definition=(RESOURCES / "forms" / "✅.xlsx").as_posix(),
+                attachments=[(RESOURCES / "forms" / "fruits.csv").as_posix()],
+            )
+            form = client.forms.get("✅")
+            self.assertEqual(form.xmlFormId, "✅")
+
+    def test_form_update__with_version_updater__non_ascii_specials(self):
+        """Should create a new version with new definition and attachment."""
+        with Client() as client:
+            client.forms.update(
+                form_id="'=+/*-451%/%",
+                attachments=[],
+                version_updater=lambda v: datetime.now().isoformat(),
+            )
+
     def test_form_update__attachments(self):
         """Should create a new version with new attachment."""
         with Client() as client:
@@ -78,3 +99,40 @@ def test_project_create_app_users__names_and_forms(self):
             display_names=["test_assign3", "test_assign_23"],
             forms=["range", "pull_data"],
         )
+
+    def test_submission_create__non_ascii(self):
+        """Should create an instance of the form, encoded to utf-8."""
+        client = Client()
+        xml = """
+        <data id="'=+/*-451%/%" version="1">
+          <meta>
+            <instanceID>~!@#$%^&*()_+=-✅✅</instanceID>
+          </meta>
+          <fruit>Banana</fruit>
+          <note_fruit/>
+        </data>
+        """
+        client.submissions.create(xml=xml, form_id="'=+/*-451%/%")
+        submission = client.submissions.get(
+            form_id="'=+/*-451%/%", instance_id="~!@#$%^&*()_+=-✅✅"
+        )
+        self.assertEqual("~!@#$%^&*()_+=-✅✅", submission.instanceId)
+
+    def test_submission_edit__non_ascii(self):
+        """Should edit an existing instance of the form, encoded to utf-8."""
+        client = Client()
+        # The "instance_id" remains the id of the first submission, not the
+        # instanceID/deprecatedID used in the XML.
+        xml = """
+        <data id="'=+/*-451%/%" version="1">
+          <meta>
+            <deprecatedID>~!@#$%^&*()_+=-✘✘</deprecatedID>
+            <instanceID>~!@#$%^&*()_+=-✘✘✘</instanceID>
+          </meta>
+          <fruit>Papaya</fruit>
+          <note_fruit/>
+        </data>
+        """
+        client.submissions.edit(
+            xml=xml, form_id="'=+/*-451%/%", instance_id="~!@#$%^&*()_+=-✅✅"
+        )