From b5ec68751547087eebc4a255614ea9d927534628 Mon Sep 17 00:00:00 2001 From: Fernando Bravo <39527354+fernando79513@users.noreply.github.com> Date: Fri, 12 Jan 2024 13:54:40 +0100 Subject: [PATCH] Unique validator for packaging metadata (BugFix) (#920) * Added validation check for same package Check if a packaging metadata unit contains the same package name during the validation step * Updated documentation * Increased test coverage --- checkbox-ng/plainbox/impl/unit/packaging.py | 44 ++++++++--- .../plainbox/impl/unit/test_packaging.py | 79 +++++++++++++++++++ checkbox-ng/plainbox/impl/unit/validators.py | 2 +- docs/reference/units/packaging-meta-data.rst | 3 + 4 files changed, 118 insertions(+), 10 deletions(-) diff --git a/checkbox-ng/plainbox/impl/unit/packaging.py b/checkbox-ng/plainbox/impl/unit/packaging.py index 3e40d5ef80..1bc9a74645 100644 --- a/checkbox-ng/plainbox/impl/unit/packaging.py +++ b/checkbox-ng/plainbox/impl/unit/packaging.py @@ -124,6 +124,7 @@ from plainbox.i18n import gettext as _ from plainbox.impl.symbol import SymbolDef from plainbox.impl.unit import concrete_validators +from plainbox.impl.unit.validators import UniqueValueValidator from plainbox.impl.unit.unit import Unit _logger = logging.getLogger("plainbox.unit.packaging") @@ -153,6 +154,22 @@ def os_version_id(self): """Version of the operating system.""" return self.get_record_value(self.Meta.fields.os_version_id) + @property + def Depends(self): + """Package dependencies""" + return self.get_record_value(self.Meta.fields.Depends) + + @property + def Recommends(self): + """Package recommendations""" + return self.get_record_value(self.Meta.fields.Recommends) + + @property + def Suggests(self): + """Package suggestions""" + return self.get_record_value(self.Meta.fields.Suggests) + + class Meta: name = 'packaging meta-data' @@ -163,6 +180,9 @@ class fields(SymbolDef): os_id = 'os-id' os_version_id = 'os-version-id' + Depends = 'Depends' + Recommends = 'Recommends' + Suggests = 'Suggests' field_validators = { fields.os_id: [ @@ -172,20 +192,26 @@ class fields(SymbolDef): fields.os_version_id: [ concrete_validators.untranslatable, ], + fields.Depends: [ + UniqueValueValidator(), + ], + fields.Recommends: [ + UniqueValueValidator(), + ], + fields.Suggests: [ + UniqueValueValidator(), + ], } def __str__(self): parts = [_("Operating System: {}").format(self.os_id)] if self.os_id == 'debian' or self.os_id == 'ubuntu': - Depends = self.get_record_value('Depends') - Recommends = self.get_record_value('Recommends') - Suggests = self.get_record_value('Suggests') - if Depends: - parts.append(_("Depends: {}").format(Depends)) - if Recommends: - parts.append(_("Recommends: {}").format(Recommends)) - if Suggests: - parts.append(_("Suggests: {}").format(Suggests)) + if self.Depends: + parts.append(_("Depends: {}").format(self.Depends)) + if self.Recommends: + parts.append(_("Recommends: {}").format(self.Recommends)) + if self.Suggests: + parts.append(_("Suggests: {}").format(self.Suggests)) else: parts.append("...") return ', '.join(parts) diff --git a/checkbox-ng/plainbox/impl/unit/test_packaging.py b/checkbox-ng/plainbox/impl/unit/test_packaging.py index e227bbc2d9..98e17065e4 100644 --- a/checkbox-ng/plainbox/impl/unit/test_packaging.py +++ b/checkbox-ng/plainbox/impl/unit/test_packaging.py @@ -24,6 +24,11 @@ from plainbox.impl.unit.packaging import DebianPackagingDriver from plainbox.impl.unit.packaging import PackagingDriverBase from plainbox.impl.unit.packaging import PackagingMetaDataUnit +from plainbox.impl.unit.test_unit import UnitFieldValidationTests +from plainbox.impl.unit.validators import UnitValidationContext +from plainbox.impl.validation import Problem +from plainbox.impl.validation import Severity + from plainbox.impl.secure.rfc822 import load_rfc822_records @@ -300,3 +305,77 @@ def test_malformed_versions(self): # Using wrong version format with self.assertRaises(SystemExit) as context: compare_versions("== ***", "1.0.0") + + def test_unit_to_string(self): + unit = PackagingMetaDataUnit( + { + "os-id": "ubuntu", + "os-version-id": "22.04", + "Depends": "dep_package", + "Recommends": "rec_package", + "Suggests": "sug_package", + } + ) + + unit_string = str(unit) + + self.assertIn("ubuntu", unit_string) + self.assertIn("Depends: dep_package", unit_string) + self.assertIn("Recommends: rec_package", unit_string) + self.assertIn("Suggests: sug_package", unit_string) + + +class PackagingUnitFieldValidationTests(UnitFieldValidationTests): + def test_validation_unique_package(self): + unit_1 = PackagingMetaDataUnit( + { + "os-id": "ubuntu", + "os-version-id": ">=20.04", + "Depends": "dep_package1", + "Recommends": "rec_package1", + "Suggests": "sug_package1", + }, + provider=self.provider, + ) + unit_2 = PackagingMetaDataUnit( + { + "os-id": "ubuntu", + "os-version-id": ">=20.04", + "Depends": "dep_package1", + "Recommends": "rec_package1", + "Suggests": "sug_package1", + }, + provider=self.provider, + ) + + self.provider.unit_list = [unit_1, unit_2] + self.provider.problem_list = [] + context = UnitValidationContext([self.provider]) + issue_list = unit_1.check(context=context) + + message_start = "field 'Depends', clashes with 1 other unit" + depends_issue = self.assertIssueFound( + issue_list, + PackagingMetaDataUnit.Meta.fields.Depends, + Problem.not_unique, + Severity.error, + ) + self.assertTrue(depends_issue.message.startswith(message_start)) + + message_start = "field 'Recommends', clashes with 1 other unit" + recommends_issue = self.assertIssueFound( + issue_list, + PackagingMetaDataUnit.Meta.fields.Recommends, + Problem.not_unique, + Severity.error, + ) + self.assertTrue(recommends_issue.message.startswith(message_start)) + + message_start = "field 'Suggests', clashes with 1 other unit" + suggests_issue = self.assertIssueFound( + issue_list, + PackagingMetaDataUnit.Meta.fields.Suggests, + Problem.not_unique, + Severity.error, + ) + self.assertTrue(suggests_issue.message.startswith(message_start)) diff --git a/checkbox-ng/plainbox/impl/unit/validators.py b/checkbox-ng/plainbox/impl/unit/validators.py index af027f8117..97022f236a 100644 --- a/checkbox-ng/plainbox/impl/unit/validators.py +++ b/checkbox-ng/plainbox/impl/unit/validators.py @@ -582,7 +582,7 @@ def check_in_context(self, parent, unit, field, context): value = getattr(unit, field2prop(field)) units_with_this_value = value_map[value] n = len(units_with_this_value) - if n > 1: + if n > 1 and value is not None: # come up with unit_list where this unit is always at the front unit_list = list(units_with_this_value) unit_list = sorted( diff --git a/docs/reference/units/packaging-meta-data.rst b/docs/reference/units/packaging-meta-data.rst index cf0c250adb..db2034de34 100644 --- a/docs/reference/units/packaging-meta-data.rst +++ b/docs/reference/units/packaging-meta-data.rst @@ -52,6 +52,9 @@ for **Debian** are: The syntax is the same as in normal Debian control files (including package version dependencies). This field can be split into multiple lines, for readability, as newlines are discarded. + In order to avoid unwanted dependency repetitions, this field is declared as + an unique value for validation. + .. _Packaging Meta Data Suggests field: