Address review comments

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Author: TG1999

vulnerabilities/importer.py

@@ -73,7 +73,7 @@ def __lt__(self, other):
 
     # TODO: Add cache
     def _cmp_key(self):
-        return (str(self.system), self.value, self.scoring_elements, self.published_at)
+        return (self.system.identifier, self.value, self.scoring_elements, self.published_at)
 
     @classmethod
     def from_dict(cls, severity: dict):
@@ -190,7 +190,11 @@ def __lt__(self, other):
 
     # TODO: Add cache
     def _cmp_key(self):
-        return (str(self.package), str(self.affected_version_range), str(self.fixed_version))
+        return (
+            str(self.package),
+            str(self.affected_version_range or ""),
+            str(self.fixed_version or ""),
+        )
 
     @classmethod
     def merge(

vulnerabilities/migrations/0089_alter_advisory_unique_content_id.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.2.16 on 2025-02-06 14:27
+# Generated by Django 4.2.16 on 2025-02-12 13:41
 
 from django.db import migrations, models
 
@@ -13,6 +13,11 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name="advisory",
             name="unique_content_id",
-            field=models.CharField(blank=True, max_length=64),
+            field=models.CharField(
+                blank=True,
+                db_index=True,
+                help_text="A 64 character unique identifier for the content of the advisory since we use sha256 as hex",
+                max_length=64,
+            ),
         ),
     ]

vulnerabilities/models.py

@@ -1317,7 +1317,9 @@ class Advisory(models.Model):
 
     unique_content_id = models.CharField(
         max_length=64,
+        db_index=True,
         blank=True,
+        help_text="A 64 character unique identifier for the content of the advisory since we use sha256 as hex",
     )
     aliases = models.JSONField(blank=True, default=list, help_text="A list of alias strings")
     summary = models.TextField(
@@ -1372,7 +1374,7 @@ def save(self, *args, **kwargs):
 
     def save(self, *args, **kwargs):
         advisory_data = self.to_advisory_data()
-        self.unique_content_id = compute_content_id(advisory_data, include_metadata=False)[:31]
+        self.unique_content_id = compute_content_id(advisory_data, include_metadata=False)
         super().save(*args, **kwargs)
 
     def to_advisory_data(self) -> "AdvisoryData":

vulnerabilities/pipelines/remove_duplicate_advisories.py

@@ -9,8 +9,8 @@
 
 import logging
 from itertools import groupby
-from operator import attrgetter
 
+from aboutcode.pipeline import LoopProgress
 from django.db.models import Count
 from django.db.models import Q
 
@@ -26,54 +26,58 @@ class RemoveDuplicateAdvisoriesPipeline(VulnerableCodePipeline):
 
     @classmethod
     def steps(cls):
-        return (cls.remove_duplicates,)
+        return (
+            cls.recompute_content_ids,
+            cls.remove_duplicates,
+        )
 
     def remove_duplicates(self):
         """
         Find advisories with the same content and keep only the latest one.
         """
-        # Get all advisories that have duplicates based on content ID
-        duplicate_content_ids = (
-            Advisory.objects.values("unique_content_id")
-            .annotate(count=Count("id"))
-            .filter(count__gt=1)
-            .values_list("unique_content_id", flat=True)
-        )
 
-        self.log(
-            f"Found {len(duplicate_content_ids)} content IDs with duplicates", level=logging.INFO
+        duplicated_advisories = groupby(
+            Advisory.objects.order_by("unique_content_id").all().paginated(),
+            key=lambda x: x.unique_content_id,
         )
-
-        for content_id in duplicate_content_ids:
-            # Get all advisories with this content ID
-            advisories = Advisory.objects.filter(unique_content_id=content_id)
-
-            # Find the latest advisory
-            latest = advisories.latest("date_imported")
-
-            # Delete all except the latest
-            advisories.exclude(id=latest.id).delete()
-
-            if self.log:
-                self.log(
-                    f"Kept advisory {latest.id} and removed "
-                    f"{advisories.count() - 1} duplicates for content ID {content_id}",
-                    level=logging.INFO,
-                )
-
-    def update_content_ids(self):
+        progress = LoopProgress(total_iterations=Advisory.objects.count(), logger=self.log)
+        for _content_id, advisories in progress.iter(duplicated_advisories):
+            advisories = list(advisories)
+            self.log(
+                f"Removing duplicates for content ID {_content_id} {len(advisories)}",
+                level=logging.INFO,
+            )
+            oldest = min(advisories, key=lambda x: x.date_imported)
+            try:
+                advisory_ids = []
+                for adv in advisories:
+                    if adv.id != oldest.id:
+                        advisory_ids.append(adv.id)
+                Advisory.objects.filter(id__in=advisory_ids).delete()
+            except Exception as e:
+                self.log(f"Error deleting advisories: {e}", level=logging.ERROR)
+
+            self.log(
+                f"Kept advisory {oldest.id} and removed "
+                f"{len(list(advisories)) - 1} duplicates for content ID {_content_id}",
+                level=logging.INFO,
+            )
+
+    def recompute_content_ids(self):
         """
-        Update content IDs for all advisories that don't have one.
+        Recompute content IDs for all advisories.
         """
-        advisories = Advisory.objects.filter(
-            Q(unique_content_id="") | Q(unique_content_id__isnull=True)
-        )
 
-        self.log(f"Found {advisories.count()} advisories without content ID", level=logging.INFO)
+        advisories = []
+
+        progress = LoopProgress(
+            total_iterations=Advisory.objects.count(),
+            progress_step=1,
+            logger=self.log,
+        )
 
-        for advisory in advisories:
+        for advisory in progress.iter(Advisory.objects.all().paginated()):
             advisory.unique_content_id = compute_content_id(advisory)
-            advisory.save()
+            advisories.append(advisory)
 
-            if self.log:
-                self.log(f"Updated content ID for advisory {advisory.id}", level=logging.DEBUG)
+        Advisory.objects.bulk_update(advisories, ["unique_content_id"], batch_size=1000)