refactor: split metadata creation in fixture

This allows more clearly expressing that we're modeling the fixtures after how ingestion works.

Note how `make_drv` now gets "parsed data" including the metadata.
It adds the separate homepage and description rows, and then completes the derivation.

Author: fricklerhandwerk

nix/tests/default.nix

@@ -186,6 +186,8 @@ pkgs.testers.runNixOSTest {
                 NixEvaluation,
                 NixDerivation,
                 NixDerivationMeta,
+                NixDerivationHomepage,
+                NixDerivationDescription,
                 NixMaintainer,
                 NixLicense,
               )
@@ -195,6 +197,8 @@ pkgs.testers.runNixOSTest {
               for model, count in [
                 (NixDerivation, 3),
                 (NixDerivationMeta, 3),
+                (NixDerivationHomepage, 1),
+                (NixDerivationDescription, 1),
                 (NixMaintainer, 1),
                 (NixLicense, 1),
               ]:

src/shared/cache_suggestions.py

@@ -10,7 +10,7 @@
 from django.db.models import Prefetch, Q
 from pydantic import BaseModel, field_serializer
 
-from shared.models import NixDerivation, NixDerivationDescription, NixMaintainer
+from shared.models import NixDerivation, NixMaintainer
 from shared.models.cached import CachedSuggestions
 from shared.models.cve import AffectedProduct, Metric, Reference, Version
 from shared.models.linkage import (
@@ -20,7 +20,6 @@
     ReferenceUrlOverlay,
 )
 from shared.models.nix_evaluation import (
-    NixDerivationMeta,
     get_major_channel,
 )
 
@@ -304,23 +303,6 @@ def get_src_position(derivation: NixDerivation) -> str | None:
     return None
 
 
-def _get_description(metadata: NixDerivationMeta) -> str | None:
-    """
-    Return the description for a derivation metadata record.
-    Falls back to the legacy ``NixDerivationMeta.description`` column so that
-    rows created before the migration continue to work until ``--null-after``
-    has been run.
-    """
-
-    description: str | None = None
-    try:
-        description = metadata.description_obj.text
-    except NixDerivationDescription.DoesNotExist:
-        description = metadata.description
-
-    return description
-
-
 def channel_structure(
     version_constraints: list[Version], derivations: list[NixDerivation]
 ) -> dict[str, CachedSuggestion.Package]:
@@ -334,9 +316,9 @@ def channel_structure(
         if attribute_path not in packages:
             packages[attribute_path] = CachedSuggestion.Package()
             if derivation.metadata:
-                description = _get_description(derivation.metadata)
-                if description:
-                    packages[attribute_path].description = description
+                packages[
+                    attribute_path
+                ].description = derivation.metadata.get_description()
                 packages[attribute_path].maintainers = [
                     CachedSuggestion.Maintainer.model_validate(to_dict(m))
                     for m in derivation.metadata.prefetched_maintainers

src/shared/models/nix_evaluation.py

@@ -132,6 +132,22 @@ class Meta:  # type: ignore[override]
             )
         ]
 
+    def get_description(self) -> str | None:
+        """
+        Return the description for a derivation metadata record.
+        Falls back to the legacy ``NixDerivationMeta.description`` column so that
+        rows created before the migration continue to work until ``--null-after``
+        has been run.
+        """
+
+        description: str | None = None
+        try:
+            description = self.description_obj.text
+        except NixDerivationDescription.DoesNotExist:
+            description = self.description
+
+        return description
+
 
 class NixDerivationHomepage(models.Model):
     """

src/shared/tests/conftest.py

@@ -233,10 +233,36 @@ def maintainer(make_maintainer: Callable[..., NixMaintainer]) -> NixMaintainer:
     return make_maintainer()
 
 
+@pytest.fixture
+def make_metadata(maintainer: NixMaintainer) -> Callable[..., NixDerivationMeta]:
+    def wrapped(
+        maintainer: NixMaintainer = maintainer,
+    ) -> NixDerivationMeta:
+        meta = NixDerivationMeta.objects.create(
+            homepage="https://example.com",
+            description="My package",
+            insecure=False,
+            available=True,
+            broken=False,
+            unfree=False,
+            unsupported=False,
+        )
+        meta.maintainers.add(maintainer)
+        return meta
+
+    return wrapped
+
+
+@pytest.fixture
+def metadata(make_metadata: Callable[..., NixDerivationMeta]) -> NixDerivationMeta:
+    return make_metadata()
+
+
 @pytest.fixture
 def make_drv(
-    maintainer: NixMaintainer,
     evaluation: NixEvaluation,
+    maintainer: NixMaintainer,
+    make_metadata: Callable[..., NixDerivationMeta],
 ) -> Callable[..., NixDerivation]:
     def wrapped(
         # Like `pname` in `mkDerivation`
@@ -245,22 +271,21 @@ def wrapped(
         system: str = "x86_64-linux",
         attribute: str | None = None,
         evaluation: NixEvaluation = evaluation,
+        meta: NixDerivationMeta | None = None,
         maintainer: NixMaintainer = maintainer,
     ) -> NixDerivation:
-        description = "Dummy derivation"
-        homepage = "https://nix.example.com"
-        meta = NixDerivationMeta.objects.create(
-            description=description,
-            homepage=homepage,
-            insecure=False,
-            available=True,
-            broken=False,
-            unfree=False,
-            unsupported=False,
+
+        if meta is None:
+            meta = make_metadata(maintainer=maintainer)
+
+        # get_or_create so callers can safely pass an existing meta that already
+        # has description/homepage rows (e.g. from make_metadata or a previous call).
+        NixDerivationDescription.objects.get_or_create(
+            metadata=meta, defaults={"text": meta.description}
+        )
+        NixDerivationHomepage.objects.get_or_create(
+            metadata=meta, defaults={"url": meta.homepage}
         )
-        NixDerivationDescription.objects.create(metadata=meta, text=description)
-        NixDerivationHomepage.objects.create(metadata=meta, url=homepage)
-        meta.maintainers.add(maintainer)
 
         if attribute is None:
             attribute = pname
@@ -459,17 +484,3 @@ def wrapped(
         return create_package_subscription_notifications(suggestion)
 
     return wrapped
-
-
-@pytest.fixture
-def legacy_meta(db: None) -> NixDerivationMeta:
-    """Create a NixDerivationMeta with only the legacy columns set, no new-table rows."""
-    return NixDerivationMeta.objects.create(
-        homepage="https://example.com",
-        description="My package",
-        insecure=False,
-        available=True,
-        broken=False,
-        unfree=False,
-        unsupported=False,
-    )

src/shared/tests/test_auth.py

@@ -23,7 +23,9 @@ def test_ismaintainer_respects_channel_tip(
         channel=channel,
         commit_sha1=channel.head_sha1_commit if at_tip else None,  # otherwise random
     )
-    make_drv(evaluation=evaluation, maintainer=make_maintainer_from_user(user))
+    drv = make_drv(evaluation=evaluation)
+    assert drv.metadata
+    drv.metadata.maintainers.add(make_maintainer_from_user(user))
     assert ismaintainer(user) is expected
 
 

src/shared/tests/test_migrate_homepage_description.py

@@ -11,38 +11,40 @@
 
 
 def test_migrates_homepage_and_description_to_new_table(
-    legacy_meta: NixDerivationMeta,
+    metadata: NixDerivationMeta,
 ) -> None:
 
+    pre_migration_description = metadata.description
+    pre_migration_homepage = metadata.homepage
     call_command("migrate_homepage_description", stdout=StringIO())
 
     assert NixDerivationHomepage.objects.filter(
-        metadata=legacy_meta, url="https://example.com"
+        metadata=metadata, url=pre_migration_homepage
     ).exists()
     assert NixDerivationDescription.objects.filter(
-        metadata=legacy_meta, text="My package"
+        metadata=metadata, text=pre_migration_description
     ).exists()
 
-    legacy_meta.refresh_from_db()
-    assert legacy_meta.homepage == "https://example.com"
-    assert legacy_meta.description == "My package"
+    metadata.refresh_from_db()
+    assert metadata.homepage == pre_migration_homepage
+    assert metadata.description == pre_migration_description
 
 
 def test_null_after_clears_homepage_and_description_column(
-    legacy_meta: NixDerivationMeta,
+    metadata: NixDerivationMeta,
 ) -> None:
 
     call_command("migrate_homepage_description", "--null-after", stdout=StringIO())
 
-    legacy_meta.refresh_from_db()
-    assert legacy_meta.homepage is None
-    assert legacy_meta.description is None
+    metadata.refresh_from_db()
+    assert metadata.homepage is None
+    assert metadata.description is None
 
-    assert NixDerivationHomepage.objects.filter(metadata=legacy_meta).exists()
-    assert NixDerivationDescription.objects.filter(metadata=legacy_meta).exists()
+    assert NixDerivationHomepage.objects.filter(metadata=metadata).exists()
+    assert NixDerivationDescription.objects.filter(metadata=metadata).exists()
 
 
-def test_command_is_idempotent_homepage(legacy_meta: NixDerivationMeta) -> None:
+def test_command_is_idempotent(metadata: NixDerivationMeta) -> None:
 
     call_command("migrate_homepage_description", stdout=StringIO())
     call_command("migrate_homepage_description", stdout=StringIO())

src/shared/tests/test_notifications.py

@@ -2,18 +2,19 @@
 
 from django.contrib.auth.models import User
 
+from shared.models.linkage import CVEDerivationClusterProposal, ProvenanceFlags
 from shared.models.nix_evaluation import (
     NixDerivation,
     NixMaintainer,
 )
-from webview.models import Notification
+from shared.notify_users import create_package_subscription_notifications
 
 
 def test_github_username_changed(
     user: User,
-    make_maintainer_from_user: Callable[..., NixMaintainer],
     make_drv: Callable[..., NixDerivation],
-    make_package_notification: Callable[..., list[Notification]],
+    make_cached_suggestion: Callable[..., CVEDerivationClusterProposal],
+    make_maintainer_from_user: Callable[..., NixMaintainer],
 ) -> None:
     """
     Check that notifications get sent even if the user's GitHub handle changed after first login.
@@ -23,5 +24,7 @@ def test_github_username_changed(
     # GitHub username could have changed between evaluations.
     maintainer.github = maintainer.github + "-new"
     maintainer.save()
-    notifications = make_package_notification(drv)
+
+    suggestion = make_cached_suggestion(drvs={drv: ProvenanceFlags.PACKAGE_NAME_MATCH})
+    notifications = create_package_subscription_notifications(suggestion)
     assert notifications

src/shared/tests/test_suggestion_caching.py

@@ -86,52 +86,48 @@ def test_cache_stale_check(
     assert cached.is_stale is True
 
 
-def test_cache_description_from_new_table(cve: Container, drv: NixDerivation) -> None:
+def test_cache_description_from_new_table(
+    suggestion: CVEDerivationClusterProposal,
+) -> None:
     """
-    When a NixDerivationDescription row exists for the derivation's metadata,
+    When a NixDerivationDescription row exists for the derivatipon's metadata,
     the cached payload must use that value as the package description.
     """
+    drv = suggestion.derivations.first()
+
+    assert drv
+    assert drv.metadata
+
+    description_obj = drv.metadata.description_obj
+    description_obj.text = "New Package"
+    description_obj.save()
 
-    suggestion = CVEDerivationClusterProposal.objects.create(
-        status="pending",
-        cve=cve.cve,
-    )
-    DerivationClusterProposalLink.objects.create(
-        proposal=suggestion,
-        derivation=drv,
-        provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH,
-    )
     cache_new_suggestions(suggestion)
 
     cached = CachedSuggestions.objects.get(proposal=suggestion)
     pkg = cached.payload["packages"][drv.attribute]
-    assert pkg["description"] == "Dummy derivation"
+    assert pkg["description"] == description_obj.text
 
 
 def test_cache_description_falls_back_when_no_new_table_entry(
-    cve: Container,
-    drv: NixDerivation,
+    suggestion: CVEDerivationClusterProposal,
 ) -> None:
     """
     When no NixDerivationDescription row exists (unmigrated derivation),
     the cached payload must fall back to NixDerivationMeta.description.
     """
+
+    drv = suggestion.derivations.first()
+    assert drv
+    assert drv.metadata
+
     NixDerivationDescription.objects.filter(metadata=drv.metadata).delete()
     # Confirm the legacy column still holds the value
     assert drv.metadata is not None
-    assert drv.metadata.description == "Dummy derivation"
+    assert drv.metadata.description
 
-    suggestion = CVEDerivationClusterProposal.objects.create(
-        status="pending",
-        cve=cve.cve,
-    )
-    DerivationClusterProposalLink.objects.create(
-        proposal=suggestion,
-        derivation=drv,
-        provenance_flags=ProvenanceFlags.PACKAGE_NAME_MATCH,
-    )
     cache_new_suggestions(suggestion)
 
     cached = CachedSuggestions.objects.get(proposal=suggestion)
     pkg = cached.payload["packages"][drv.attribute]
-    assert pkg["description"] == "Dummy derivation"
+    assert pkg["description"] == drv.metadata.description