fixup: refactors

Author: pedro-psb

docs/admin/reference/settings.md

@@ -295,10 +295,8 @@ Defaults to `False`.
 
 ### DISTRIBUTED\_PUBLICATION\_RETENTION\_PERIOD
 
-When a distribution switches to a newer publication, the previously served (superseded) publication
-continues to be available for this many seconds.
-This grace period prevents 404 errors for clients that began downloading from the old publication
-before the switch occurred.
+When a distribution switches to a newer publication, the previously served (superseded) publication continues to be available for this many seconds.
+This grace period prevents 404 errors for clients that began downloading from the old publication before the switch occurred.
 
 Set to `0` to disable the grace period and serve only the latest publication immediately.
 

pulpcore/app/models/publication.py

@@ -756,9 +756,8 @@ def get_fallback_ca(self, path):
         if not retain_distributed_pub_enabled():
             return None
         recent_dp = (
-            self.core_distributedpublications.filter(
-                models.Q(expires_at__gte=timezone.now()) | models.Q(expires_at__isnull=True)
-            )
+            DistributedPublication.get_non_expired()
+            .filter(distribution=self)
             .order_by("-pulp_created")
             .select_related("publication__repository_version")
         )
@@ -881,11 +880,8 @@ class DistributedPublication(BaseModel):
     """
     Records the history of publications served by each Distribution.
 
-    Keeps superseded publications alive and serveable for a configurable grace period
-    so clients mid-download don't receive 404 errors when a distribution switches publications.
-
-    - ``expires_at=null``       — currently active
-    - ``expires_at=<datetime>`` — superseded; still served until that time
+    Keeps superseded publications alive and serveable for a configurable grace period.
+    Null `expired_at` means the publication was not superseded yet.
     """
 
     distribution = models.ForeignKey(
@@ -897,21 +893,21 @@ class DistributedPublication(BaseModel):
     expires_at = models.DateTimeField(null=True)
 
     @classmethod
-    def get_active(cls, distribution):
-        """Return records that are still being served (current or within grace period)."""
-        return cls.objects.filter(distribution=distribution).filter(
-            models.Q(expires_at__isnull=True) | models.Q(expires_at__gte=timezone.now())
-        )
+    def get_non_expired(cls, include_current=True):
+        if include_current:
+            return cls.objects.filter(
+                models.Q(expires_at__isnull=True) | models.Q(expires_at__gte=timezone.now())
+            )
+        return cls.objects.filter(expires_at__gte=timezone.now())
 
     @classmethod
-    def get_expired(cls, distribution):
-        """Return records whose grace period has elapsed for a distribution."""
-        return cls.objects.filter(distribution=distribution, expires_at__lt=timezone.now())
+    def get_expired(cls):
+        return cls.objects.filter(expires_at__lt=timezone.now())
 
     @hook(AFTER_CREATE)
     def cleanup(self):
         """Expire older active DistributedPublications; delete already-expired ones."""
-        DistributedPublication.objects.filter(expires_at__lt=timezone.now()).delete()
+        DistributedPublication.get_expired().delete()
         superseded = DistributedPublication.objects.exclude(pk=self.pk).filter(
             distribution=self.distribution, expires_at__isnull=True
         )

pulpcore/tests/unit/models/test_publication_retention.py

@@ -90,15 +90,22 @@ class TestDistributedPublication:
     def test_created_when_publication_added_to_distribution(self, db):
         pub = pub_factory()
         dist = dist_factory(pub=pub)
-        active = DistributedPublication.get_active(dist)
-        assert active.count() == 1
-        assert active.first().publication_id == pub.pk
-        assert active.first().expires_at is None
+        non_expired = DistributedPublication.get_non_expired(include_current=True).filter(
+            distribution=dist
+        )
+        assert non_expired.count() == 1
+        assert non_expired.first().publication_id == pub.pk
+        assert non_expired.first().expires_at is None
 
     def test_first_distributed_publication_is_active(self, db):
         dist = dist_factory(pub=pub_factory())
-        assert DistributedPublication.get_active(dist).count() == 1
-        assert DistributedPublication.get_expired(dist).count() == 0
+        assert (
+            DistributedPublication.get_non_expired(include_current=True)
+            .filter(distribution=dist)
+            .count()
+            == 1
+        )
+        assert DistributedPublication.get_expired().filter(distribution=dist).count() == 0
 
     def test_switching_publication_expires_old_and_activates_new(self, db):
         pub1 = pub_factory()
@@ -107,10 +114,12 @@ def test_switching_publication_expires_old_and_activates_new(self, db):
         pub2 = pub_factory()
         update_dist(dist, pub=pub2)
 
-        active = DistributedPublication.get_active(dist)
-        assert active.count() == 2
-        assert active.filter(expires_at__isnull=True).first().publication_id == pub2.pk
-        assert active.filter(expires_at__isnull=False).first().publication_id == pub1.pk
+        non_expired = DistributedPublication.get_non_expired(include_current=True).filter(
+            distribution=dist
+        )
+        assert non_expired.count() == 2
+        assert non_expired.filter(expires_at__isnull=True).first().publication_id == pub2.pk
+        assert non_expired.filter(expires_at__isnull=False).first().publication_id == pub1.pk
 
 
 @pytest.mark.django_db