Refine the implementation and add unit tests

Signed-off-by: tdruez <[email protected]>

Author: tdruez

component_catalog/models.py

@@ -2630,24 +2630,26 @@ def update_from_purldb(self, user):
         )
         return updated_fields
 
-    def update_from_scan(self, user, update_product_relations=False):
-        scancodeio = ScanCodeIO(self.dataspace)
+    def update_from_scan(self, user, update_products=False):
+        package = self
+        dataspace = self.dataspace
+        scancodeio = ScanCodeIO(dataspace)
+
         can_update_from_scan = all(
             [
-                self.dataspace.enable_package_scanning,
-                self.dataspace.update_packages_from_scan,
+                dataspace.enable_package_scanning,
+                dataspace.update_packages_from_scan,
                 scancodeio.is_configured(),
             ]
         )
-
         if not can_update_from_scan:
             return
 
-        updated_fields = scancodeio.update_from_scan(package=self, user=user)
+        updated_fields = scancodeio.update_from_scan(package=package, user=user)
 
-        if update_product_relations and "declared_license_expression" in updated_fields:
-            for product_package in self.productpackages.license_unknown():
-                product_package.update_license_unknown()
+        if update_products:
+            if "declared_license_expression" in updated_fields:
+                package.productpackages.update_license_unknown()
 
         return updated_fields
 

component_catalog/tests/test_models.py

@@ -1769,22 +1769,30 @@ def test_package_model_create_from_url_enable_purldb_access(
 
     @mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.is_configured")
     @mock.patch("dejacode_toolkit.scancodeio.ScanCodeIO.update_from_scan")
-    def test_package_model_update_from_scan(self, mock_update_from_scan, mock_is_configured):
+    def test_package_model_update_from_scan(self, mock_scio_update_from_scan, mock_is_configured):
         mock_is_configured.return_value = True
-        package1 = make_package(self.dataspace)
+        package1 = make_package(self.dataspace, declared_license_expression="mit")
+        product1 = make_product(self.dataspace, inventory=[package1])
+
+        pp1 = product1.productpackages.get()
+        self.assertEqual("", pp1.license_expression)
+        pp1.update(license_expression="unknown")
 
         results = package1.update_from_scan(user=self.user)
-        mock_update_from_scan.assert_not_called()
+        mock_scio_update_from_scan.assert_not_called()
         self.assertIsNone(results)
 
         self.dataspace.enable_package_scanning = True
         self.dataspace.update_packages_from_scan = True
         self.dataspace.save()
 
-        mock_update_from_scan.return_value = ["updated_field"]
-        results = package1.update_from_scan(user=self.user)
-        mock_update_from_scan.assert_called()
-        self.assertEqual(["updated_field"], results)
+        mock_scio_update_from_scan.return_value = ["declared_license_expression"]
+        results = package1.update_from_scan(user=self.user, update_products=True)
+        mock_scio_update_from_scan.assert_called()
+        self.assertEqual(["declared_license_expression"], results)
+
+        pp1.refresh_from_db()
+        self.assertEqual("mit", pp1.license_expression)
 
     def test_package_model_get_url_methods(self):
         package = Package(

component_catalog/views.py

@@ -1761,7 +1761,7 @@ def send_scan_notification(request, key):
     run = json_data.get("run")
     scan_status = run.get("status")
     if scan_status.lower() == "success":
-        updated_fields = package.update_from_scan(user, update_product_relations=True)
+        updated_fields = package.update_from_scan(user, update_products=True)
 
     if updated_fields:
         description = (

product_portfolio/models.py

@@ -579,11 +579,8 @@ def improve_packages_from_purldb(self, user):
         # Update the Product Package relationship `license_expression` if the
         # Package.declared_license_expression was updated from "unknwon" value using
         # PurlDB data.
-        productpackages_qs = self.productpackages.filter(
-            package__in=updated_packages
-        ).license_unknown()
-        for product_package in productpackages_qs:
-            product_package.update_license_unknown()
+        productpackages_qs = self.productpackages.filter(package__in=updated_packages)
+        productpackages_qs.update_license_unknown()
 
         return updated_packages
 
@@ -708,6 +705,10 @@ def vulnerable(self):
     def license_unknown(self):
         return self.filter(license_expression="unknown")
 
+    def update_license_unknown(self):
+        for product_package in self.license_unknown():
+            product_package.update_license_unknown()
+
     def annotate_weighted_risk_score(self):
         """Annotate the Queeryset with the weighted_risk_score computed value."""
         purpose = ProductItemPurpose.objects.filter(productpackage=OuterRef("pk"))

product_portfolio/tests/test_models.py

@@ -780,6 +780,19 @@ def test_productcomponent_model_is_custom_component(self):
         pc1.save()
         self.assertFalse(pc1.is_custom_component)
 
+    def test_productpackage_model_update_license_unknown(self):
+        package1 = make_package(self.dataspace, declared_license_expression="mit")
+        pp1 = make_product_package(self.product1, package=package1)
+
+        pp1.update_license_unknown()
+        pp1.refresh_from_db()
+        self.assertEqual("", pp1.license_expression)
+
+        pp1.update(license_expression="unknown")
+        pp1.update_license_unknown()
+        pp1.refresh_from_db()
+        self.assertEqual("mit", pp1.license_expression)
+
     def test_product_relationship_queryset_vulnerable(self):
         pp1 = make_product_package(self.product1)
         product_package_qs = ProductPackage.objects.vulnerable()
@@ -833,6 +846,39 @@ def test_product_relationship_queryset_update_weighted_risk_score(self):
         pp1.refresh_from_db()
         self.assertIsNone(pp1.weighted_risk_score)
 
+    def test_productpackage_queryset_license_unknown(self):
+        package1 = make_package(self.dataspace, declared_license_expression="unknown")
+        package2 = make_package(self.dataspace, declared_license_expression="mit")
+        pp1 = make_product_package(self.product1, package=package1)
+        pp2 = make_product_package(self.product1, package=package2)
+
+        qs = ProductPackage.objects.license_unknown()
+        self.assertQuerySetEqual([], qs)
+
+        pp1.update(license_expression="unknown")
+        pp2.update(license_expression="mit")
+        qs = ProductPackage.objects.license_unknown()
+        self.assertQuerySetEqual(qs, [pp1])
+
+    def test_productpackage_queryset_update_license_unknown(self):
+        package1 = make_package(self.dataspace, declared_license_expression="mit")
+        package2 = make_package(self.dataspace, declared_license_expression="mit")
+        pp1 = make_product_package(self.product1, package=package1)
+        pp2 = make_product_package(self.product1, package=package2)
+
+        ProductPackage.objects.update_license_unknown()
+        pp1.refresh_from_db()
+        pp2.refresh_from_db()
+        self.assertEqual("", pp1.license_expression)
+        self.assertEqual("", pp2.license_expression)
+
+        pp1.update(license_expression="unknown")
+        ProductPackage.objects.update_license_unknown()
+        pp1.refresh_from_db()
+        pp2.refresh_from_db()
+        self.assertEqual("mit", pp1.license_expression)
+        self.assertEqual("", pp2.license_expression)
+
     def test_productrelation_model_compute_weighted_risk_score(self):
         purpose1 = make_product_item_purpose(self.dataspace)