From 530ae47ba39f43b2b37ca14f078a50f2aca9339b Mon Sep 17 00:00:00 2001 From: David Wallace Date: Thu, 23 Jan 2025 15:15:27 +0100 Subject: [PATCH 1/3] feat(xml,import,export):#1205 add required field to export and optionally validate it at import Signed-off-by: David Wallace --- rdmo/conditions/renderers/__init__.py | 1 + rdmo/core/renderers.py | 5 +++ rdmo/core/settings.py | 2 ++ rdmo/core/tests/test_renderers.py | 7 ++++ rdmo/core/xml.py | 47 +++++++++++++++++++-------- rdmo/domain/renderers/__init__.py | 1 + rdmo/options/renderers/__init__.py | 2 ++ rdmo/projects/renderers.py | 1 + rdmo/questions/renderers/__init__.py | 5 +++ rdmo/tasks/renderers/__init__.py | 1 + rdmo/views/renderers/__init__.py | 1 + 11 files changed, 59 insertions(+), 14 deletions(-) diff --git a/rdmo/conditions/renderers/__init__.py b/rdmo/conditions/renderers/__init__.py index e597e65fb9..3e2dfd4e7e 100644 --- a/rdmo/conditions/renderers/__init__.py +++ b/rdmo/conditions/renderers/__init__.py @@ -11,6 +11,7 @@ def render_document(self, xml, conditions): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for condition in conditions: diff --git a/rdmo/core/renderers.py b/rdmo/core/renderers.py index 91e7e8417b..9d64d1175c 100644 --- a/rdmo/core/renderers.py +++ b/rdmo/core/renderers.py @@ -1,6 +1,7 @@ import re from io import StringIO +from django.conf import settings from django.utils.encoding import smart_str from django.utils.timezone import get_current_timezone, now from django.utils.xmlutils import SimplerXMLGenerator @@ -51,6 +52,10 @@ def render_document(self, xml, data): def version(self): return __version__ + @property + def required(self): + return settings.EXPORT_MIN_REQUIRED_VERSION + @property def created(self): return now().astimezone(get_current_timezone()).isoformat() diff --git a/rdmo/core/settings.py b/rdmo/core/settings.py index c6610be4de..29f701ee46 100644 --- a/rdmo/core/settings.py +++ b/rdmo/core/settings.py @@ -295,6 +295,8 @@ EXPORT_CONTENT_DISPOSITION = 'attachment' +EXPORT_MIN_REQUIRED_VERSION = '2.1.0' + PROJECT_TABLE_PAGE_SIZE = 20 PROJECT_VISIBILITY = True diff --git a/rdmo/core/tests/test_renderers.py b/rdmo/core/tests/test_renderers.py index a57b3417ea..8782a7b3a4 100644 --- a/rdmo/core/tests/test_renderers.py +++ b/rdmo/core/tests/test_renderers.py @@ -1,3 +1,7 @@ +from django.conf import settings + +from rdmo import __version__ + from ..renderers import BaseXMLRenderer @@ -7,6 +11,7 @@ def render_document(self, xml, data): xml.startElement('rdmo', { 'xmlns:dc': "http://purl.org/dc/elements/1.1/", 'version': self.version, + 'required': self.required, 'created': self.created }) self.render_text_element(xml, 'text', {}, data['text']) @@ -17,6 +22,8 @@ def test_render(): renderer = TestRenderer() xml = renderer.render({'text': 'test'}) assert 'test' in xml + assert f'version="{__version__}"' in xml + assert f'required="{settings.EXPORT_MIN_REQUIRED_VERSION}"' in xml def test_render_ascii_code(): diff --git a/rdmo/core/xml.py b/rdmo/core/xml.py index 264977352e..22875dcbf9 100644 --- a/rdmo/core/xml.py +++ b/rdmo/core/xml.py @@ -45,16 +45,38 @@ def validate_root(root: Optional[xmlElement]) -> Tuple[bool, Optional[str]]: def validate_and_get_xml_version_from_root(root: xmlElement) -> Tuple[Optional[Version], list]: - unparsed_root_version = root.attrib.get('version') or LEGACY_RDMO_XML_VERSION - root_version, rdmo_version = parse(unparsed_root_version), parse(RDMO_INSTANCE_VERSION) - if root_version > rdmo_version: - logger.info('Import failed version validation (%s > %s)', root_version, rdmo_version) - errors = [ - _('This RDMO XML file does not have a valid version number.'), - f'XML Version ({root_version}) is greater than RDMO instance version {rdmo_version}' - ] + rdmo_version = parse(RDMO_INSTANCE_VERSION) + + # Extract version attributes from the XML root + unparsed_required_version = root.attrib.get('required') # New required version field + unparsed_root_version = root.attrib.get('version') or LEGACY_RDMO_XML_VERSION # Fallback to legacy default + + # Validate the 'required' attribute if it exists + if unparsed_required_version: + try: + required_version = parse(unparsed_required_version) + except ValueError: + logger.info('Import failed: Invalid "required" format in XML (%s)', unparsed_required_version) + errors = [_('The "required" attribute in this RDMO XML file is not a valid version.')] + return None, errors + + if required_version > rdmo_version: + logger.info('Import failed: Required version (%s) > RDMO instance version (%s)', required_version, + rdmo_version) + errors = [ + _('This RDMO XML file requires a newer RDMO version to be imported.'), + f'Required version: {required_version}, Current version: {rdmo_version}.' + ] + return None, errors + + # Fallback to validate the legacy 'version' field + try: + xml_version = parse(unparsed_root_version) + return xml_version, [] + except ValueError: + logger.info('Import failed: Invalid "version" format in XML (%s)', unparsed_root_version) + errors = [_('The "version" attribute in this RDMO XML file is not a valid version.')] return None, errors - return root_version, [] def validate_legacy_elements(elements: dict, root_version: Version) -> list: @@ -117,13 +139,13 @@ def parse_xml_to_elements(xml_file=None) -> Tuple[OrderedDict, list]: return OrderedDict(), errors # step 3.1.1: validate the legacy elements - legacy_errors = validate_legacy_elements(elements, parse(root.attrib.get('version', LEGACY_RDMO_XML_VERSION))) + legacy_errors = validate_legacy_elements(elements, root_version) if legacy_errors: errors.extend(legacy_errors) return OrderedDict(), errors # step 4: convert elements from previous versions - elements = convert_elements(elements, parse(root.attrib.get('version', LEGACY_RDMO_XML_VERSION))) + elements = convert_elements(elements, root_version) # step 5: order the elements and return # ordering of elements is done in the import_elements function @@ -232,9 +254,6 @@ def strip_ns(tag, ns_map): def convert_elements(elements, version: Version): - if not isinstance(version, Version): - raise TypeError('Version should be of parsed version type.') - if version < parse('2.0.0'): validate_pre_conversion_for_missing_key_in_legacy_elements(elements, version) elements = convert_legacy_elements(elements) diff --git a/rdmo/domain/renderers/__init__.py b/rdmo/domain/renderers/__init__.py index fce1febd34..91580f4d90 100644 --- a/rdmo/domain/renderers/__init__.py +++ b/rdmo/domain/renderers/__init__.py @@ -9,6 +9,7 @@ def render_document(self, xml, attributes): xml.startElement('rdmo', { 'xmlns:dc': "http://purl.org/dc/elements/1.1/", 'version': self.version, + 'required': self.required, 'created': self.created }) for attribute in attributes: diff --git a/rdmo/options/renderers/__init__.py b/rdmo/options/renderers/__init__.py index 41e18c2e63..f7342577bd 100644 --- a/rdmo/options/renderers/__init__.py +++ b/rdmo/options/renderers/__init__.py @@ -12,6 +12,7 @@ def render_document(self, xml, optionsets): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for optionset in optionsets: @@ -25,6 +26,7 @@ def render_document(self, xml, options): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for option in options: diff --git a/rdmo/projects/renderers.py b/rdmo/projects/renderers.py index 2a3e543418..75d18a1442 100644 --- a/rdmo/projects/renderers.py +++ b/rdmo/projects/renderers.py @@ -7,6 +7,7 @@ def render_document(self, xml, project): xml.startElement('project', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) self.render_text_element(xml, 'title', {}, project['title']) diff --git a/rdmo/questions/renderers/__init__.py b/rdmo/questions/renderers/__init__.py index 112ee856be..81e6753a0f 100644 --- a/rdmo/questions/renderers/__init__.py +++ b/rdmo/questions/renderers/__init__.py @@ -20,6 +20,7 @@ def render_document(self, xml, catalogs): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for catalog in catalogs: @@ -35,6 +36,7 @@ def render_document(self, xml, sections): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for section in sections: @@ -49,6 +51,7 @@ def render_document(self, xml, pages): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for page in pages: @@ -63,6 +66,7 @@ def render_document(self, xml, questionsets): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for questionset in questionsets: @@ -77,6 +81,7 @@ def render_document(self, xml, questions): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for question in questions: diff --git a/rdmo/tasks/renderers/__init__.py b/rdmo/tasks/renderers/__init__.py index 4257b7e3f2..7244d57f57 100644 --- a/rdmo/tasks/renderers/__init__.py +++ b/rdmo/tasks/renderers/__init__.py @@ -13,6 +13,7 @@ def render_document(self, xml, tasks): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for task in tasks: diff --git a/rdmo/views/renderers/__init__.py b/rdmo/views/renderers/__init__.py index 02ee2c3ebe..fb7cebec49 100644 --- a/rdmo/views/renderers/__init__.py +++ b/rdmo/views/renderers/__init__.py @@ -9,6 +9,7 @@ def render_document(self, xml, views): xml.startElement('rdmo', { 'xmlns:dc': 'http://purl.org/dc/elements/1.1/', 'version': self.version, + 'required': self.required, 'created': self.created }) for view in views: From 4e83f0bbe4386740f9de78ab359e486db41e1981 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Fri, 24 Jan 2025 11:23:31 +0100 Subject: [PATCH 2/3] tests(management,imports): add test for required xml field Signed-off-by: David Wallace --- rdmo/management/tests/helpers_xml.py | 11 ++++++++--- rdmo/management/tests/test_commands.py | 2 ++ testing/xml/error-version-required.xml | 3 +++ testing/xml/error-version.xml | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 testing/xml/error-version-required.xml diff --git a/rdmo/management/tests/helpers_xml.py b/rdmo/management/tests/helpers_xml.py index d5fd64a420..97b719baf0 100644 --- a/rdmo/management/tests/helpers_xml.py +++ b/rdmo/management/tests/helpers_xml.py @@ -1,25 +1,30 @@ +from packaging.version import parse +from rdmo import __version__ from rdmo.core.xml import parse_xml_to_elements, read_xml, resolve_file +current_version = parse(__version__) + xml_test_files = { "xml/elements/catalogs.xml": None, "xml/elements/updated-and-changed/optionsets-1.xml": None, 'file-does-not-exist.xml': - 'This file does not exists', + 'This field may not be blank.', # or 'This file does not exists' "xml/error.xml": "The content of the XML file does not consist of well-formed data or markup. XML Parsing Error: syntax error: line 1, column 0", # noqa: E501 "xml/project.xml": "This XML does not contain RDMO content.", 'xml/error-version.xml': - 'This RDMO XML file does not have a valid version number. XML Version (99.9.9) is greater', + 'The "version" attribute in this RDMO XML file is not a valid version.', + 'xml/error-version-required.xml': + f'This RDMO XML file requires a newer RDMO version to be imported. Required version: 99.9.9, Current version: {current_version}', # noqa: E501 'xml/elements/legacy/catalog-error-key.xml': 'XML Parsing Error: Missing legacy elements', } xml_error_files = {k: v for k,v in xml_test_files.items() if v is not None} -xml_error_files['file-does-not-exist.xml'] = 'This field may not be blank.' def read_xml_and_parse_to_root_and_elements(file): errors = [] diff --git a/rdmo/management/tests/test_commands.py b/rdmo/management/tests/test_commands.py index e3263b1925..661ac47571 100644 --- a/rdmo/management/tests/test_commands.py +++ b/rdmo/management/tests/test_commands.py @@ -23,4 +23,6 @@ def test_import(db, settings, xml_file_path, error_message): with pytest.raises(CommandError) as e: call_command('import', xml_file, stdout=stdout, stderr=stderr) + if error_message == 'This field may not be blank.': + error_message = 'This file does not exists' # overwrite error message for cli import assert str(e.value).startswith(error_message) diff --git a/testing/xml/error-version-required.xml b/testing/xml/error-version-required.xml new file mode 100644 index 0000000000..7c4347d3d2 --- /dev/null +++ b/testing/xml/error-version-required.xml @@ -0,0 +1,3 @@ + + + diff --git a/testing/xml/error-version.xml b/testing/xml/error-version.xml index efaaccaf70..2bdff9578c 100644 --- a/testing/xml/error-version.xml +++ b/testing/xml/error-version.xml @@ -1,3 +1,3 @@ - + From f30842f11cac90c05ae81ec9dc37417c20ee04ae Mon Sep 17 00:00:00 2001 From: David Wallace Date: Fri, 24 Jan 2025 11:24:42 +0100 Subject: [PATCH 3/3] feat(xml,import): add error for backwards incompatibility Signed-off-by: David Wallace --- rdmo/core/xml.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/rdmo/core/xml.py b/rdmo/core/xml.py index 22875dcbf9..5d1cd10894 100644 --- a/rdmo/core/xml.py +++ b/rdmo/core/xml.py @@ -72,12 +72,22 @@ def validate_and_get_xml_version_from_root(root: xmlElement) -> Tuple[Optional[V # Fallback to validate the legacy 'version' field try: xml_version = parse(unparsed_root_version) - return xml_version, [] + except ValueError: logger.info('Import failed: Invalid "version" format in XML (%s)', unparsed_root_version) errors = [_('The "version" attribute in this RDMO XML file is not a valid version.')] return None, errors + # RDMO 1.x can not import XMLs from RDMO 2.x + if rdmo_version < parse('2.0.0') and xml_version >= parse('2.0.0'): + logger.info('Import failed: Backwards compatibility is not supported for RDMO versions < 2.0.0') + errors = [ + _('This RDMO XML file was created with a newer version of RDMO and cannot be imported.'), + _('Backwards compatibility is not supported for RDMO versions lower than 2.0.0.') + ] + return None, errors + + return xml_version, [] def validate_legacy_elements(elements: dict, root_version: Version) -> list: