diff --git a/src/api/tests/test_matching_training_data.py b/src/api/tests/test_matching_training_data.py index 12b5d5ae8..48d7d65ad 100644 --- a/src/api/tests/test_matching_training_data.py +++ b/src/api/tests/test_matching_training_data.py @@ -11,6 +11,7 @@ CVEDerivationClusterProposal, PackageOverlay, ) +from shared.models.package import Package url = reverse("matching-training-data") @@ -52,6 +53,7 @@ def curated_proposals( PackageOverlay.objects.create( suggestion=accepted, package_attribute="foo.tests", + package=Package.objects.get_or_create(name="foo.tests")[0], type=PackageOverlay.Type.IGNORED, ) return { diff --git a/src/api/tests/test_missing_cpes.py b/src/api/tests/test_missing_cpes.py index 169bd8504..ec49b83d2 100644 --- a/src/api/tests/test_missing_cpes.py +++ b/src/api/tests/test_missing_cpes.py @@ -182,6 +182,7 @@ def test_ignored_package_not_listed( PackageOverlay.objects.create( suggestion=suggestion, package_attribute="bar", + package=Package.objects.get(name="bar"), type=PackageOverlay.Type.IGNORED, ) response = APIClient().get(reverse("missing-cpes")) diff --git a/src/feeds/tests/test_package_feed.py b/src/feeds/tests/test_package_feed.py index a18565d36..4fbc034bb 100644 --- a/src/feeds/tests/test_package_feed.py +++ b/src/feeds/tests/test_package_feed.py @@ -96,6 +96,7 @@ def test_feed_excludes_suggestion_with_ignored_package( PackageOverlay.objects.create( suggestion=cached_suggestion, package_attribute=drv.attribute, + package=drv.package_link.package, type=PackageOverlay.Type.IGNORED, ) diff --git a/src/shared/matching_training_data/serializers.py b/src/shared/matching_training_data/serializers.py index 7fb1a3d0f..baaaaaa6a 100644 --- a/src/shared/matching_training_data/serializers.py +++ b/src/shared/matching_training_data/serializers.py @@ -19,6 +19,7 @@ from shared import models from shared.evaluation import DerivationKey, derivation_as_key +from shared.package_clustering import cluster_packages SCHEMA_VERSION = 1 @@ -415,10 +416,20 @@ def create( if link_objs: models.DerivationClusterProposalLink.objects.bulk_create(link_objs) + # Overlays refer to packages, so the imported derivations need to be + # clustered into packages first, as it happens on evaluation ingestion. + cluster_packages( + models.NixDerivation.objects.filter( + pk__in=[drv.pk for drv in by_fingerprint.values()] + ) + ) for overlay in overlays_data: models.PackageOverlay.objects.create( suggestion=proposal, package_attribute=overlay["package_attribute"], + package=models.PackageAttrpath.objects.get( + attrpath=overlay["package_attribute"] + ).package, type=overlay["type"], ) diff --git a/src/shared/migrations/0107_packageoverlay_package.py b/src/shared/migrations/0107_packageoverlay_package.py new file mode 100644 index 000000000..770f6a96f --- /dev/null +++ b/src/shared/migrations/0107_packageoverlay_package.py @@ -0,0 +1,118 @@ +import django.db.models.deletion +from django.db import migrations, models, transaction +from django.db.models import OuterRef, Subquery + +BATCH_SIZE = 10000 + + +def link_overlays_to_packages(apps, schema_editor): + """Link existing package overlays to the package of their attribute.""" + PackageOverlay = apps.get_model("shared", "PackageOverlay") + PackageAttrpath = apps.get_model("shared", "PackageAttrpath") + PackageOverlay.objects.filter(package__isnull=True).update( + package_id=Subquery( + PackageAttrpath.objects.filter( + attrpath=OuterRef("package_attribute") + ).values("package_id")[:1] + ) + ) + # If there's overlays left over that don't have a link at this point, they're orphans from garbage-collected derivations on old untriaged suggestions. + PackageOverlay.objects.filter(package__isnull=True).delete() + + +def link_events_to_packages(apps, schema_editor): + """ + Link existing package overlay events to the package of their attribute. + Events whose attribute has no package are left unlinked. + """ + PackageOverlayEvent = apps.get_model("shared", "PackageOverlayEvent") + PackageAttrpath = apps.get_model("shared", "PackageAttrpath") + pending = PackageOverlayEvent.objects.filter(package__isnull=True).order_by( + "pgh_id" + ) + package_id = Subquery( + PackageAttrpath.objects.filter(attrpath=OuterRef("package_attribute")).values( + "package_id" + )[:1] + ) + + last_id = 0 + while True: + batch = list( + pending.filter(pgh_id__gt=last_id).values_list("pgh_id", flat=True)[ + :BATCH_SIZE + ] + ) + if not batch: + break + with transaction.atomic(using=schema_editor.connection.alias): + schema_editor.execute( + "ALTER TABLE shared_packageoverlayevent DISABLE TRIGGER ALL" + ) + PackageOverlayEvent.objects.filter(pgh_id__in=batch).update( + package_id=package_id + ) + schema_editor.execute( + "ALTER TABLE shared_packageoverlayevent ENABLE TRIGGER ALL" + ) + last_id = batch[-1] + + +def delete_orphan_events(apps, schema_editor): + """ + Delete package overlay events whose attribute has no package. + Their suggestion or its matched derivations were garbage-collected, so nothing refers to them any more. + """ + PackageOverlayEvent = apps.get_model("shared", "PackageOverlayEvent") + with transaction.atomic(using=schema_editor.connection.alias): + schema_editor.execute( + "ALTER TABLE shared_packageoverlayevent DISABLE TRIGGER ALL" + ) + PackageOverlayEvent.objects.filter(package__isnull=True).delete() + schema_editor.execute( + "ALTER TABLE shared_packageoverlayevent ENABLE TRIGGER ALL" + ) + + +class Migration(migrations.Migration): + # Event batches commit separately instead of in one long transaction. + atomic = False + + dependencies = [ + ('shared', '0106_delete_untracked_branch_links'), + ] + + operations = [ + migrations.AddField( + model_name='packageoverlay', + name='package', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='overlays', to='shared.package'), + ), + migrations.AddField( + model_name='packageoverlayevent', + name='package', + field=models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', related_query_name='+', to='shared.package'), + ), + migrations.RunPython( + code=link_overlays_to_packages, + reverse_code=migrations.RunPython.noop, + ), + migrations.AlterField( + model_name='packageoverlay', + name='package', + field=models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='overlays', to='shared.package'), + ), + migrations.RunPython( + code=link_events_to_packages, + reverse_code=migrations.RunPython.noop, + ), + migrations.RunPython( + code=delete_orphan_events, + reverse_code=migrations.RunPython.noop, + ), + migrations.AlterField( + model_name='packageoverlayevent', + name='package', + field=models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='+', related_query_name='+', to='shared.package'), + ), + ] diff --git a/src/shared/models/linkage.py b/src/shared/models/linkage.py index 752a420e6..f2b57f6c2 100644 --- a/src/shared/models/linkage.py +++ b/src/shared/models/linkage.py @@ -15,7 +15,7 @@ import shared.models.cached from shared.models.cve import CveRecord, Reference from shared.models.nix_evaluation import NixDerivation, NixMaintainer, TimeStampMixin -from shared.models.package import Package +from shared.models.package import Package, PackageAttrpath class Overlay(models.Model): @@ -215,7 +215,12 @@ def ignore_package(self, package_attribute: str) -> None: with transaction.atomic(): self.package_overlays.get_or_create( package_attribute=package_attribute, - defaults={"type": PackageOverlay.Type.IGNORED}, + defaults={ + "type": PackageOverlay.Type.IGNORED, + "package_id": PackageAttrpath.objects.get( + attrpath=package_attribute + ).package.id, + }, ) self._recompute_package_cache() @@ -603,6 +608,15 @@ class Meta(Overlay.Meta): @pghistory.track( pghistory.ManualEvent("package.restore"), pghistory.ManualEvent("package.ignore"), + attrs={ + "package": models.ForeignKey( + Package, + on_delete=models.RESTRICT, + # Like pghistory's own copies of tracked references, keep history out of the package's reverse relations. + related_name="+", + related_query_name="+", + ) + }, ) class PackageOverlay(Overlay): """ @@ -610,6 +624,12 @@ class PackageOverlay(Overlay): """ package_attribute = models.CharField(max_length=255) + + package = models.ForeignKey( + Package, + on_delete=models.RESTRICT, + related_name="overlays", + ) suggestion = models.ForeignKey( CVEDerivationClusterProposal, related_name="package_overlays", diff --git a/src/shared/tests/conftest.py b/src/shared/tests/conftest.py index 72ccab27f..2cb24458b 100644 --- a/src/shared/tests/conftest.py +++ b/src/shared/tests/conftest.py @@ -43,6 +43,7 @@ ) from shared.models.package import Package, PackageAttrpath, PackageDerivation from shared.notify_users import create_package_subscription_notifications +from shared.package_clustering import cluster_packages from webview.models import SuggestionNotification as Notification @@ -415,6 +416,7 @@ def wrapped( ) for package, flags in package_flags.items() ) + cluster_packages(NixDerivation.objects.filter(pk__in=[drv.pk for drv in drvs])) return suggestion diff --git a/src/shared/tests/test_backfill_proposal_package_links.py b/src/shared/tests/test_backfill_proposal_package_links.py index fdd5014ec..d22f5973b 100644 --- a/src/shared/tests/test_backfill_proposal_package_links.py +++ b/src/shared/tests/test_backfill_proposal_package_links.py @@ -3,8 +3,10 @@ from django.core.management import call_command +from shared.models.cve import Container from shared.models.linkage import ( CVEDerivationClusterProposal, + DerivationClusterProposalLink, PackageClusterProposalLink, ProvenanceFlags, ) @@ -32,11 +34,17 @@ def test_backfill_creates_package_link_for_clustered_drv( def test_backfill_clusters_drv_with_no_package_link( + cve: Container, drv: NixDerivation, - suggestion: CVEDerivationClusterProposal, ) -> None: """A proposal's derivation that has no PackageDerivation yet is clustered into a package (instead of being skipped), then linked.""" + suggestion = CVEDerivationClusterProposal.objects.create(cve=cve.cve) + DerivationClusterProposalLink.objects.create( + proposal=suggestion, + derivation=drv, + provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH, + ) assert not PackageDerivation.objects.filter(derivation=drv).exists() call_command("backfill_proposal_package_links", stdout=StringIO()) diff --git a/src/shared/tests/test_garbage_collect.py b/src/shared/tests/test_garbage_collect.py index dd9c5088b..c9a47ca3a 100644 --- a/src/shared/tests/test_garbage_collect.py +++ b/src/shared/tests/test_garbage_collect.py @@ -130,6 +130,7 @@ def test_preserves_proposal_with_maintainer_overlay( PackageOverlay.objects.create( type=PackageOverlay.Type.IGNORED, package_attribute="foo", + package=PackageAttrpath.objects.get(attrpath="foo").package, suggestion=suggestion, ) elif type == "reference": diff --git a/src/shared/tests/test_matching_training_data.py b/src/shared/tests/test_matching_training_data.py index 0543d2049..d58777000 100644 --- a/src/shared/tests/test_matching_training_data.py +++ b/src/shared/tests/test_matching_training_data.py @@ -19,6 +19,7 @@ ProvenanceFlags, ) from shared.models.nix_evaluation import NixChannel, NixDerivation, NixEvaluation +from shared.models.package import Package def test_serializer_export_validates_like_api_payload( @@ -41,6 +42,7 @@ def test_serializer_export_validates_like_api_payload( PackageOverlay.objects.create( suggestion=proposal, package_attribute="foo.tests", + package=Package.objects.get_or_create(name="foo.tests")[0], type=PackageOverlay.Type.IGNORED, ) @@ -146,6 +148,7 @@ def test_export_import_export_roundtrip( PackageOverlay.objects.create( suggestion=proposal, package_attribute="foobar.tests", + package=Package.objects.get_or_create(name="foobar.tests")[0], type=PackageOverlay.Type.IGNORED, ) original = dict(serializers.CVEDerivationClusterProposal(proposal).data) diff --git a/src/shared/tests/test_matching_training_data_benchmark.py b/src/shared/tests/test_matching_training_data_benchmark.py index 0bd09dd4c..6fb5dedc2 100644 --- a/src/shared/tests/test_matching_training_data_benchmark.py +++ b/src/shared/tests/test_matching_training_data_benchmark.py @@ -30,6 +30,7 @@ ProvenanceFlags, ) from shared.models.nix_evaluation import NixDerivation +from shared.models.package import Package def _proposal( @@ -55,6 +56,7 @@ def _proposal( PackageOverlay.objects.create( suggestion=proposal, package_attribute=attr, + package=Package.objects.get_or_create(name=attr)[0], type=PackageOverlay.Type.IGNORED, ) return proposal diff --git a/src/shared/tests/test_matching_training_data_commands.py b/src/shared/tests/test_matching_training_data_commands.py index c8c966c9d..abebc9dac 100644 --- a/src/shared/tests/test_matching_training_data_commands.py +++ b/src/shared/tests/test_matching_training_data_commands.py @@ -28,6 +28,7 @@ ProvenanceFlags, ) from shared.models.nix_evaluation import NixChannel, NixDerivation, NixEvaluation +from shared.models.package import Package def _export(proposal: CVEDerivationClusterProposal) -> dict[str, Any]: @@ -277,6 +278,7 @@ def test_import_roundtrip_from_page_files( PackageOverlay.objects.create( suggestion=proposal, package_attribute="foobar.tests", + package=Package.objects.get_or_create(name="foobar.tests")[0], type=PackageOverlay.Type.IGNORED, ) diff --git a/src/shared/tests/test_package_overlay_links.py b/src/shared/tests/test_package_overlay_links.py new file mode 100644 index 000000000..9a132b27c --- /dev/null +++ b/src/shared/tests/test_package_overlay_links.py @@ -0,0 +1,31 @@ +import pytest +from django.db.models import RestrictedError + +from shared.models.linkage import CVEDerivationClusterProposal +from shared.models.nix_evaluation import NixDerivation + + +def test_ignore_package_links_overlay_to_package( + drv: NixDerivation, + cached_suggestion: CVEDerivationClusterProposal, +) -> None: + """Ignoring an attribute with a known package links the overlay to it.""" + cached_suggestion.ignore_package(drv.attribute) + + overlay = cached_suggestion.package_overlays.get(package_attribute=drv.attribute) + assert overlay.package == drv.package_link.package + + +def test_deleting_package_with_overlay_is_restricted( + drv: NixDerivation, + cached_suggestion: CVEDerivationClusterProposal, +) -> None: + """A package that overlays refer to cannot be deleted.""" + pkg = drv.package_link.package + cached_suggestion.ignore_package(drv.attribute) + + with pytest.raises(RestrictedError): + pkg.delete() + + overlay = cached_suggestion.package_overlays.get(package_attribute=drv.attribute) + assert overlay.package == pkg diff --git a/src/shared/tests/test_suggestion_caching.py b/src/shared/tests/test_suggestion_caching.py index 8a50d48d1..8d8dba682 100644 --- a/src/shared/tests/test_suggestion_caching.py +++ b/src/shared/tests/test_suggestion_caching.py @@ -153,13 +153,18 @@ def test_cache_stale_check( def test_cache_description_from_package( - suggestion: CVEDerivationClusterProposal, + cve: Container, + drv: NixDerivation, ) -> None: """ The cached payload must surface the description after clustering. """ - drv = suggestion.derivations.first() - assert drv + suggestion = CVEDerivationClusterProposal.objects.create(cve=cve.cve) + DerivationClusterProposalLink.objects.create( + proposal=suggestion, + derivation=drv, + provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH, + ) assert drv.metadata expected_description = drv.metadata.description @@ -223,13 +228,18 @@ def test_cache_skipped_without_title_or_description( def test_cache_description_falls_back_to_meta( - suggestion: CVEDerivationClusterProposal, + cve: Container, + drv: NixDerivation, ) -> None: """ When no PackageDerivation exists, the cached payload falls back to NixDerivationMeta.description. """ - drv = suggestion.derivations.first() - assert drv + suggestion = CVEDerivationClusterProposal.objects.create(cve=cve.cve) + DerivationClusterProposalLink.objects.create( + proposal=suggestion, + derivation=drv, + provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH, + ) assert drv.metadata cache_new_suggestions(suggestion)