Unique validator for packaging metadata (BugFix) (canonical#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

@@ -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)

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))

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(

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: