launchpad-reviewers team mailing list archive

[Enrique Sánchez <mp+485398@xxxxxxxxxxxxxxxxxx>]

Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-distropackage-tags into launchpad:master.

Commit message:
Fix SVT global tags and package tags conflict

Modify tests to support both of them

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/485398
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-distropackage-tags into launchpad:master.

diff --git a/lib/lp/bugs/scripts/tests/test_uct.py b/lib/lp/bugs/scripts/tests/test_uct.py
index ad6385a..a3b35dd 100644
--- a/lib/lp/bugs/scripts/tests/test_uct.py
+++ b/lib/lp/bugs/scripts/tests/test_uct.py
@@ -29,6 +29,8 @@ from lp.services.propertycache import clear_property_cache
 from lp.testing import TestCase, TestCaseWithFactory
 from lp.testing.layers import ZopelessDatabaseLayer
 
+TAG_SEPARATOR = UCTImporter.TAG_SEPARATOR
+
 
 class TestUCTRecord(TestCase):
     maxDiff = None
@@ -810,10 +812,18 @@ class TestUCTImporterExporter(TestCaseWithFactory):
         self.assertEqual(len(cve.bug_urls), len(watches))
         self.assertEqual(sorted(cve.bug_urls), sorted(w.url for w in watches))
 
-        self.assertEqual(sorted(bug.tags), sorted(list(cve.global_tags)))
-
+        self.checkBugTags(bug, cve)
         self.checkBugAttachments(bug, cve)
 
+    def checkBugTags(self, bug: Bug, cve: CVE):
+        tags = cve.global_tags.copy()
+        for distro_package in cve.distro_packages:
+            for tag in distro_package.tags:
+                tags.add(
+                    f"{distro_package.package_name.name}{TAG_SEPARATOR}{tag}"
+                )
+        self.assertEqual(sorted(bug.tags), sorted(list(tags)))
+
     def checkBugTasks(self, bug: Bug, cve: CVE):
         bug_tasks: List[BugTask] = bug.bugtasks
 
@@ -851,7 +861,8 @@ class TestUCTImporterExporter(TestCaseWithFactory):
             self.assertEqual(expected_status, t.status)
             self.assertIsNone(t.status_explanation)
 
-        self.assertEqual(tags, set(bug.tags))
+        distro_package_tags = {tag for tag in bug.tags if TAG_SEPARATOR in tag}
+        self.assertEqual(tags, distro_package_tags)
 
         for series_package in cve.series_packages:
             self.assertIn(series_package.target, bug_tasks_by_target)
@@ -1330,13 +1341,14 @@ class TestUCTImporterExporter(TestCaseWithFactory):
         self.importer.update_bug(bug, cve, self.lp_cve)
         self.checkBug(bug, cve)
 
-    def test_update_distro_packages_tags(self):
+    def test_update_tags(self):
         bug = self.importer.create_bug(self.cve, self.lp_cve)
         cve = self.cve
 
         # Add new tags
-        cve.distro_packages[0].tags.add("test-tag")
-        cve.distro_packages[0].tags.add("another-test-tag")
+        cve.global_tags.add("global-test-tag")
+        cve.distro_packages[0].tags.add("package-test-tag")
+        cve.distro_packages[0].tags.add("another-package-test-tag")
 
         self.importer.update_bug(bug, cve, self.lp_cve)
         self.checkBug(bug, cve)
diff --git a/lib/lp/bugs/scripts/uct/uctexport.py b/lib/lp/bugs/scripts/uct/uctexport.py
index 84e58dd..fd7df39 100644
--- a/lib/lp/bugs/scripts/uct/uctexport.py
+++ b/lib/lp/bugs/scripts/uct/uctexport.py
@@ -17,6 +17,7 @@ from lp.bugs.model.bugtask import BugTask
 from lp.bugs.model.cve import Cve as CveModel
 from lp.bugs.model.vulnerability import Vulnerability
 from lp.bugs.scripts.uct.models import CVE, CVSS
+from lp.bugs.scripts.uct.uctimport import UCTImporter
 from lp.registry.model.distributionsourcepackage import (
     DistributionSourcePackage,
 )
@@ -28,7 +29,7 @@ __all__ = [
     "UCTExporter",
 ]
 
-
+TAG_SEPARATOR = UCTImporter.TAG_SEPARATOR
 logger = logging.getLogger(__name__)
 
 
@@ -110,8 +111,14 @@ class UCTExporter:
         cve_importance = vulnerability.importance
 
         tags_by_pkg = defaultdict(set)
+        global_tags = set()
         for tag in bug.tags:
-            tags_by_pkg[tag.split(".")[0]].add(tag.split(".")[1])
+            if TAG_SEPARATOR in tag:
+                tags_by_pkg[tag.split(TAG_SEPARATOR)[0]].add(
+                    tag.split(TAG_SEPARATOR)[1]
+                )
+            else:
+                global_tags.add(tag)
 
         # When exporting, we shouldn't output the importance value if it
         # hasn't been specified in the original UCT file.
@@ -259,7 +266,7 @@ class UCTExporter:
                 for authority in lp_cve.cvss
                 for vector_string in lp_cve.cvss[authority]
             ],
-            global_tags=set(bug.tags),
+            global_tags=global_tags,
             patch_urls=patch_urls,
         )
 
diff --git a/lib/lp/bugs/scripts/uct/uctimport.py b/lib/lp/bugs/scripts/uct/uctimport.py
index 7e1a3a6..a4abb5e 100644
--- a/lib/lp/bugs/scripts/uct/uctimport.py
+++ b/lib/lp/bugs/scripts/uct/uctimport.py
@@ -28,7 +28,7 @@ import logging
 from datetime import timezone
 from itertools import chain
 from pathlib import Path
-from typing import Dict, List, Optional
+from typing import Dict, List, Optional, Set
 
 import transaction
 from zope.component import getUtility
@@ -69,6 +69,8 @@ class UCTImporter:
     `UCTImporter` is used to import UCT CVE files to Launchpad database.
     """
 
+    TAG_SEPARATOR = "."
+
     def __init__(self, dry_run=False):
         self.dry_run = dry_run
         self.bug_importer = getUtility(ILaunchpadCelebrities).bug_importer
@@ -171,13 +173,12 @@ class UCTImporter:
                 target=distro_package.target,
                 importance=distro_package.importance,
                 cve=lp_cve,
-                tags=cve.global_tags,
             )
         )
 
         self._update_external_bug_urls(bug, cve.bug_urls)
         self._update_patches(bug, cve.patch_urls)
-        self._update_distro_packages_tags(bug, cve.distro_packages)
+        self._update_tags(bug, cve.global_tags, cve.distro_packages)
 
         self._create_bug_tasks(
             bug,
@@ -235,9 +236,7 @@ class UCTImporter:
         self._assign_bug_tasks(bug, cve.assignee)
         self._update_external_bug_urls(bug, cve.bug_urls)
         self._update_patches(bug, cve.patch_urls)
-        self._update_distro_packages_tags(bug, cve.distro_packages)
-
-        bug.tags = cve.global_tags
+        self._update_tags(bug, cve.global_tags, cve.distro_packages)
 
         # Update or add new Vulnerabilities
         vulnerabilities_by_distro = {
@@ -250,13 +249,16 @@ class UCTImporter:
             else:
                 self._update_vulnerability(vulnerability, cve)
 
-    def _update_distro_packages_tags(
-        self, bug: BugModel, distro_packages: List
+    def _update_tags(
+        self, bug: BugModel, global_tags: Set, distro_packages: List
     ):
-        tags = set()
+        tags = global_tags.copy()
         for distro_package in distro_packages:
             for tag in distro_package.tags:
-                tags.add(f"{distro_package.package_name.name}.{tag}")
+                tags.add(
+                    f"{distro_package.package_name.name}"
+                    f"{self.TAG_SEPARATOR}{tag}"
+                )
 
         bug.tags = tags