From aeedfca54652cab2efb5f6f7de4d242325d22261 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 1 Nov 2023 10:15:42 +0100 Subject: [PATCH] add: support for specifying a custom camera app for image questions - Added support for the app parameter in image questions - Added tests to check that adding package name to draw/signature/selfie image questions is ignored - Ignore adding package name to draw/signature/selfie image questions - Include the row number in the error message - Moved max-pixels tests to TestImageAppParameter class - Test using max-pixels and app parameters together - Improved the error message displayed when there are not allowed characters --- .../pyxform/android_package_name.py | 33 ++++ pyxform/xls2json.py | 23 ++- tests/test_image_app_parameter.py | 183 ++++++++++++++++++ tests/test_max_pixels.py | 67 ------- .../pyxform/test_android_package_name.py | 63 ++++++ 5 files changed, 301 insertions(+), 68 deletions(-) create mode 100644 pyxform/validators/pyxform/android_package_name.py create mode 100644 tests/test_image_app_parameter.py delete mode 100644 tests/test_max_pixels.py create mode 100644 tests/validators/pyxform/test_android_package_name.py diff --git a/pyxform/validators/pyxform/android_package_name.py b/pyxform/validators/pyxform/android_package_name.py new file mode 100644 index 00000000..b1c0cd03 --- /dev/null +++ b/pyxform/validators/pyxform/android_package_name.py @@ -0,0 +1,33 @@ +import re +from typing import Optional + +PACKAGE_NAME_REGEX = re.compile(r"[^a-zA-Z0-9._]") + + +def validate_android_package_name(name: str) -> Optional[str]: + prefix = "Parameter 'app' has an invalid Android package name - " + + if not name.strip(): + return f"{prefix}package name is missing." + + if "." not in name: + return f"{prefix}the package name must have at least one '.' separator." + + if name[-1] == ".": + return f"{prefix}the package name cannot end in a '.' separator." + + segments = name.split(".") + if any(segment == "" for segment in segments): + return f"{prefix}package segments must be of non-zero length." + + if any(segment.startswith("_") for segment in segments): + return f"{prefix}the character '_' cannot be the first character in a package name segment." + + if any(segment[0].isdigit() for segment in segments): + return f"{prefix}a digit cannot be the first character in a package name segment." + + for segment in segments: + if PACKAGE_NAME_REGEX.search(segment): + return f"{prefix}the package name can only include letters (a-z, A-Z), numbers (0-9), dots (.), and underscores (_)." + + return None diff --git a/pyxform/xls2json.py b/pyxform/xls2json.py index 56197e90..a3a848a0 100644 --- a/pyxform/xls2json.py +++ b/pyxform/xls2json.py @@ -24,6 +24,7 @@ from pyxform.errors import PyXFormError from pyxform.utils import PYXFORM_REFERENCE_REGEX, default_is_dynamic from pyxform.validators.pyxform import parameters_generic, select_from_file_params +from pyxform.validators.pyxform.android_package_name import validate_android_package_name from pyxform.validators.pyxform.translations_checks import SheetTranslations from pyxform.xls2json_backends import csv_to_dict, xls_to_dict, xlsx_to_dict from pyxform.xlsparseutils import find_sheet_misspellings, is_valid_xml_tag @@ -1310,7 +1311,13 @@ def workbook_to_json( if row.get("default"): new_dict["default"] = process_image_default(row["default"]) - parameters_generic.validate(parameters=parameters, allowed=("max-pixels",)) + parameters_generic.validate( + parameters=parameters, + allowed=( + "max-pixels", + "app", + ), + ) if "max-pixels" in parameters.keys(): try: int(parameters["max-pixels"]) @@ -1324,6 +1331,20 @@ def workbook_to_json( (ROW_FORMAT_STRING % row_number) + " Use the max-pixels parameter to speed up submission sending and save storage space. Learn more: https://xlsform.org/#image" ) + + if "app" in parameters.keys(): + appearance = row.get("control", {}).get("appearance") + if appearance is None or appearance == "annotate": + app_package_name = str(parameters["app"]) + validation_result = validate_android_package_name(app_package_name) + if validation_result is None: + new_dict["control"] = new_dict.get("control", {}) + new_dict["control"].update({"intent": app_package_name}) + else: + raise PyXFormError( + (ROW_FORMAT_STRING % row_number) + " " + validation_result + ) + parent_children_array.append(new_dict) continue diff --git a/tests/test_image_app_parameter.py b/tests/test_image_app_parameter.py new file mode 100644 index 00000000..6f6324f5 --- /dev/null +++ b/tests/test_image_app_parameter.py @@ -0,0 +1,183 @@ +# -*- coding: utf-8 -*- +""" +Test image max-pixels and app parameters. +""" +from tests.pyxform_test_case import PyxformTestCase + + +class TestImageParameters(PyxformTestCase): + def test_adding_valid_android_package_name_in_image_with_supported_appearances(self): + appearances = ("", "annotate") + md = """ + | survey | | | | | | + | | type | name | label | parameters | appearance | + | | image | my_image | Image | app=com.jeyluta.timestampcamerafree | {case} | + """ + for case in appearances: + with self.subTest(msg=case): + self.assertPyxformXform( + name="data", + md=md.format(case=case), + xml__xpath_match=[ + "/h:html/h:body/x:upload[@intent='com.jeyluta.timestampcamerafree' and @mediatype='image/*' and @ref='/data/my_image']" + ], + ) + + def test_throwing_error_when_invalid_android_package_name_is_used_with_supported_appearances( + self, + ): + appearances = ("", "annotate") + parameters = ("app=something", "app=_") + md = """ + | survey | | | | | | + | | type | name | label | parameters | appearance | + | | image | my_image | Image | {parameter} | {appearance} | + """ + for appearance in appearances: + for parameter in parameters: + with self.subTest(msg=f"{appearance} - {parameter}"): + self.assertPyxformXform( + name="data", + errored=True, + error__contains=[ + "[row : 2] Parameter 'app' has an invalid Android package name - the package name must have at least one '.' separator." + ], + md=md.format(parameter=parameter, appearance=appearance), + xml__xpath_match=[ + "/h:html/h:body/x:upload[not(@intent) and @mediatype='image/*' and @ref='/data/my_image']" + ], + ) + + def test_throwing_error_when_blank_android_package_name_is_used_with_supported_appearances( + self, + ): + appearances = ("", "annotate") + parameters = ("app=", "app= ") + md = """ + | survey | | | | | | + | | type | name | label | parameters | appearance | + | | image | my_image | Image | {parameter} | {appearance} | + """ + for appearance in appearances: + for parameter in parameters: + with self.subTest(msg=f"{appearance} - {parameter}"): + self.assertPyxformXform( + name="data", + errored=True, + error__contains=[ + "[row : 2] Parameter 'app' has an invalid Android package name - package name is missing." + ], + md=md.format(parameter=parameter, appearance=appearance), + xml__xpath_match=[ + "/h:html/h:body/x:upload[not(@intent) and @mediatype='image/*' and @ref='/data/my_image']" + ], + ) + + def test_ignoring_invalid_android_package_name_with_not_supported_appearances( + self, + ): + appearances = ("signature", "draw", "new-front") + md = """ + | survey | | | | | | + | | type | name | label | parameters | appearance | + | | image | my_image | Image | app=something | {case} | + """ + for case in appearances: + with self.subTest(msg=case): + self.assertPyxformXform( + name="data", + md=md.format(case=case), + xml__xpath_match=[ + "/h:html/h:body/x:upload[not(@intent) and @mediatype='image/*' and @ref='/data/my_image']" + ], + ) + + def test_ignoring_android_package_name_in_image_with_not_supported_appearances(self): + appearances = ("signature", "draw", "new-front") + md = """ + | survey | | | | | | + | | type | name | label | parameters | appearance | + | | image | my_image | Image | app=com.jeyluta.timestampcamerafree | {case} | + """ + for case in appearances: + with self.subTest(msg=case): + self.assertPyxformXform( + name="data", + md=md.format(case=case), + xml__xpath_match=[ + "/h:html/h:body/x:upload[not(@intent) and @mediatype='image/*' and @ref='/data/my_image']" + ], + ) + + def test_integer_max_pixels(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | parameters | + | | image | my_image | Image | max-pixels=640 | + """, + xml__contains=[ + 'xmlns:orx="http://openrosa.org/xforms"', + '', + ], + ) + + def test_string_max_pixels(self): + self.assertPyxformXform( + name="data", + errored=True, + md=""" + | survey | | | | | + | | type | name | label | parameters | + | | image | my_image | Image | max-pixels=foo | + """, + error__contains=["Parameter max-pixels must have an integer value."], + ) + + def test_string_extra_params(self): + self.assertPyxformXform( + name="data", + errored=True, + md=""" + | survey | | | | | + | | type | name | label | parameters | + | | image | my_image | Image | max-pixels=640 foo=bar | + """, + error__contains=[ + "Accepted parameters are 'app, max-pixels'. The following are invalid parameter(s): 'foo'." + ], + ) + + def test_image_with_no_max_pixels_should_warn(self): + warnings = [] + + self.md_to_pyxform_survey( + """ + | survey | | | | + | | type | name | label | + | | image | my_image | Image | + | | image | my_image_1 | Image 1 | + """, + warnings=warnings, + ) + + self.assertTrue(len(warnings) == 2) + self.assertTrue("max-pixels" in warnings[0] and "max-pixels" in warnings[1]) + + def test_max_pixels_and_app(self): + self.assertPyxformXform( + name="data", + md=""" + | survey | | | | | + | | type | name | label | parameters | + | | image | my_image | Image | max-pixels=640 app=com.jeyluta.timestampcamerafree | + """, + xml__contains=[ + 'xmlns:orx="http://openrosa.org/xforms"', + '', + ], + xml__xpath_match=[ + "/h:html/h:body/x:upload[@intent='com.jeyluta.timestampcamerafree' and @mediatype='image/*' and @ref='/data/my_image']" + ], + ) diff --git a/tests/test_max_pixels.py b/tests/test_max_pixels.py deleted file mode 100644 index 9a37067f..00000000 --- a/tests/test_max_pixels.py +++ /dev/null @@ -1,67 +0,0 @@ -# -*- coding: utf-8 -*- -""" -Test image max-pixel parameters. -""" -from tests.pyxform_test_case import PyxformTestCase - - -class MaxPixelsTest(PyxformTestCase): - """ - Test image max-pixel parameters. - """ - - def test_integer_max_pixels(self): - self.assertPyxformXform( - name="data", - md=""" - | survey | | | | | - | | type | name | label | parameters | - | | image | my_image | Image | max-pixels=640 | - """, - xml__contains=[ - 'xmlns:orx="http://openrosa.org/xforms"', - '', - ], - ) - - def test_string_max_pixels(self): - self.assertPyxformXform( - name="data", - errored=True, - md=""" - | survey | | | | | - | | type | name | label | parameters | - | | image | my_image | Image | max-pixels=foo | - """, - error__contains=["Parameter max-pixels must have an integer value."], - ) - - def test_string_extra_params(self): - self.assertPyxformXform( - name="data", - errored=True, - md=""" - | survey | | | | | - | | type | name | label | parameters | - | | image | my_image | Image | max-pixels=640 foo=bar | - """, - error__contains=[ - "Accepted parameters are 'max-pixels'. The following are invalid parameter(s): 'foo'." - ], - ) - - def test_image_with_no_max_pixels_should_warn(self): - warnings = [] - - self.md_to_pyxform_survey( - """ - | survey | | | | - | | type | name | label | - | | image | my_image | Image | - | | image | my_image_1 | Image 1 | - """, - warnings=warnings, - ) - - self.assertTrue(len(warnings) == 2) - self.assertTrue("max-pixels" in warnings[0] and "max-pixels" in warnings[1]) diff --git a/tests/validators/pyxform/test_android_package_name.py b/tests/validators/pyxform/test_android_package_name.py new file mode 100644 index 00000000..e1f7f290 --- /dev/null +++ b/tests/validators/pyxform/test_android_package_name.py @@ -0,0 +1,63 @@ +from pyxform.validators.pyxform.android_package_name import validate_android_package_name +from tests.pyxform_test_case import PyxformTestCase + + +class TestAndroidPackageNameValidator(PyxformTestCase): + def test_empty_package_name(self): + result = validate_android_package_name("") + self.assertEqual( + result, + "Parameter 'app' has an invalid Android package name - package name is missing.", + ) + + def test_blank_package_name(self): + result = validate_android_package_name(" ") + self.assertEqual( + result, + "Parameter 'app' has an invalid Android package name - package name is missing.", + ) + + def test_missing_separator(self): + result = validate_android_package_name("comexampleapp") + self.assertEqual( + result, + "Parameter 'app' has an invalid Android package name - the package name must have at least one '.' separator.", + ) + + def test_invalid_start_with_underscore(self): + result = validate_android_package_name("_com.example.app") + expected_error = "Parameter 'app' has an invalid Android package name - the character '_' cannot be the first character in a package name segment." + self.assertEqual(result, expected_error) + + def test_invalid_start_with_digit(self): + result = validate_android_package_name("1com.example.app") + expected_error = "Parameter 'app' has an invalid Android package name - a digit cannot be the first character in a package name segment." + self.assertEqual(result, expected_error) + + def test_invalid_character(self): + result = validate_android_package_name("com.example.app$") + expected_error = "Parameter 'app' has an invalid Android package name - the package name can only include letters (a-z, A-Z), numbers (0-9), dots (.), and underscores (_)." + self.assertEqual(result, expected_error) + + def test_package_name_segment_with_zero_length(self): + result = validate_android_package_name("com..app") + expected_error = "Parameter 'app' has an invalid Android package name - package segments must be of non-zero length." + self.assertEqual(result, expected_error) + + def test_separator_as_last_char_in_package_name(self): + result = validate_android_package_name("com.example.app.") + expected_error = "Parameter 'app' has an invalid Android package name - the package name cannot end in a '.' separator." + self.assertEqual(result, expected_error) + + def test_valid_package_name(self): + package_names = ( + "com.zenstudios.zenpinball", + "com.outfit7.talkingtom", + "com.zeptolab.ctr2.f2p.google", + "com.ea.game.pvzfree_row", + "com.rovio.angrybirdsspace.premium", + ) + + for case in package_names: + result = validate_android_package_name(case) + self.assertIsNone(result)