optimize CRL view query-wise

ca/django_ca/managers.py

@@ -142,8 +142,6 @@ class CertificateAuthorityManager(
 
         def acme(self) -> "CertificateAuthorityQuerySet": ...
 
-        def default(self) -> "CertificateAuthority": ...
-
         def disabled(self) -> "CertificateAuthorityQuerySet": ...
 
         def enabled(self) -> "CertificateAuthorityQuerySet": ...
@@ -228,6 +226,11 @@ def _handle_crl_distribution_point(
             ),
         )
 
+    # PYLINT NOTE: documented in queryset
+    def default(self) -> "CertificateAuthority":  # pylint: disable=missing-function-docstring
+        # Needs to be here because the async_to_sync version in the queryset does not get mirrored here.
+        return self.all().default()
+
     @deprecate_argument("expires", RemovedInDjangoCA230Warning, replacement="not_after")
     def init(  # noqa: PLR0912,PLR0913,PLR0915
         self,
@@ -729,7 +732,7 @@ def reasons(
 
         def scope(
             self,
-            ca: "CertificateAuthority",
+            serial: str,
             only_contains_ca_certs: bool = False,
             only_contains_user_certs: bool = False,
             only_contains_attribute_certs: bool = False,
@@ -876,7 +879,7 @@ def create_certificate_revocation_list(
         # Create subquery for the current CRL number with the given scope.
         number_subquery = (
             self.scope(
-                ca=ca,
+                serial=ca.serial,
                 only_contains_ca_certs=only_contains_ca_certs,
                 only_contains_user_certs=only_contains_user_certs,
                 only_contains_attribute_certs=only_contains_attribute_certs,
@@ -902,7 +905,8 @@ def create_certificate_revocation_list(
         # https://docs.djangoproject.com/en/5.1/ref/models/expressions/#f-assignments-persist-after-model-save
         if django.VERSION >= (5, 0):  # pragma: django>=5.1 branch
             # Assure that ``ca`` is loaded already
-            obj.refresh_from_db(from_queryset=self.model.objects.select_related("ca"))
+            fields = ("ca", "number")  # only fetch required fields to optimize query
+            obj.refresh_from_db(from_queryset=self.model.objects.select_related("ca"), fields=fields)
         else:  # pragma: django<5.1 branch
             # The `from_queryset` argument was added in Django 5.0.
             obj = self.model.objects.select_related("ca").get(pk=obj.pk)
@@ -917,7 +921,7 @@ def create_certificate_revocation_list(
 
         # Store CRL in the database
         obj.data = crl.public_bytes(Encoding.DER)
-        obj.save()
+        obj.save(update_fields=("data",))  # only update single field to optimize query
 
         return obj
 
@@ -981,7 +985,7 @@ class AcmeCertificateManager(AcmeCertificateManagerBase):
     if typing.TYPE_CHECKING:
         # See CertificateManagerMixin for description on this branch
         #
-        # pylint: disable=missing-function-docstring,unused-argument; just defining stubs here
+        # pylint: disable=missing-function-docstring; just defining stubs here
         def account(self) -> "AcmeCertificateQuerySet": ...
 
         def url(self) -> "AcmeCertificateQuerySet": ...

ca/django_ca/models.py

@@ -1252,7 +1252,7 @@ def pem(self) -> bytes:
         """The CRL encoded in PEM format."""
         return self.loaded.public_bytes(Encoding.PEM)
 
-    def _cache_data(self) -> Iterator[tuple[str, bytes, int]]:
+    def _cache_data(self, serial: Optional[str] = None) -> Iterator[tuple[str, bytes, int]]:
         if self.data is None:
             raise ValueError("CRL is not yet generated for this object.")
 
@@ -1262,10 +1262,13 @@ def _cache_data(self) -> Iterator[tuple[str, bytes, int]]:
         else:  # pragma: no cover  # we never generate CRLs without a next_update flag.
             expires_seconds = 86400
 
+        if serial is None:
+            serial = self.ca.serial
+
         for encoding in [Encoding.PEM, Encoding.DER]:
             cache_key = get_crl_cache_key(
-                self.ca.serial,
-                encoding,
+                serial=serial,
+                encoding=encoding,
                 only_contains_ca_certs=self.only_contains_ca_certs,
                 only_contains_user_certs=self.only_contains_user_certs,
                 only_contains_attribute_certs=self.only_contains_attribute_certs,
@@ -1284,14 +1287,22 @@ def _cache_data(self) -> Iterator[tuple[str, bytes, int]]:
 
             yield cache_key, encoded_crl, expires_seconds
 
-    def cache(self) -> None:
-        """Cache this instance."""
-        for cache_key, encoded_crl, expires_seconds in self._cache_data():
+    def cache(self, serial: Optional[str] = None) -> None:
+        """Cache this instance.
+
+        If `serial` is not given, `self.ca` will be accessed (possibly triggering a database query) to
+        generate the cache keys.
+        """
+        for cache_key, encoded_crl, expires_seconds in self._cache_data(serial):
             cache.set(cache_key, encoded_crl, expires_seconds)
 
-    async def acache(self) -> None:
-        """Cache this instance."""
-        for cache_key, encoded_crl, expires_seconds in self._cache_data():
+    async def acache(self, serial: Optional[str] = None) -> None:
+        """Cache this instance (async version).
+
+        If `serial` is not given, `self.ca` will be accessed (possibly triggering a database query) to
+        generate the cache keys.
+        """
+        for cache_key, encoded_crl, expires_seconds in self._cache_data(serial):
             await cache.aset(cache_key, encoded_crl, expires_seconds)
 
 

ca/django_ca/querysets.py

@@ -19,6 +19,8 @@
 from datetime import datetime, timedelta
 from typing import TYPE_CHECKING, Any, Generic, Optional, TypeVar
 
+from asgiref.sync import async_to_sync
+
 from cryptography import x509
 
 from django.core.exceptions import ImproperlyConfigured
@@ -177,42 +179,6 @@ def acme(self) -> "CertificateAuthorityQuerySet":
         """Return usable CAs that have support for the ACME protocol enabled."""
         return self.filter(acme_enabled=True)
 
-    def default(self) -> "CertificateAuthority":
-        """Return the default CA to use when no CA is selected.
-
-        This function honors the :ref:`CA_DEFAULT_CA <settings-ca-default-ca>`. If no usable CA can be
-        returned, raises :py:exc:`~django:django.core.exceptions.ImproperlyConfigured`.
-
-        Raises
-        ------
-        :py:exc:`~django:django.core.exceptions.ImproperlyConfigured`
-            When the CA named by :ref:`CA_DEFAULT_CA <settings-ca-default-ca>` is either not found, disabled
-            or not currently valid. Or, if the setting is not set, no CA is currently usable.
-        """
-        if (serial := model_settings.CA_DEFAULT_CA) is not None:
-            try:
-                # NOTE: Don't prefilter queryset so that we can provide more specialized error messages below.
-                ca = self.get(serial=serial)
-            except self.model.DoesNotExist as ex:
-                raise ImproperlyConfigured(f"CA_DEFAULT_CA: {serial}: CA not found.") from ex
-
-            if ca.enabled is False:
-                raise ImproperlyConfigured(f"CA_DEFAULT_CA: {serial} is disabled.")
-
-            now = timezone.now()
-            if ca.not_after < now:
-                raise ImproperlyConfigured(f"CA_DEFAULT_CA: {serial} is expired.")
-            if ca.not_before > now:  # OK, how could this ever happen? ;-)
-                raise ImproperlyConfigured(f"CA_DEFAULT_CA: {serial} is not yet valid.")
-            return ca
-
-        # NOTE: We add the serial to sorting make *sure* we have deterministic behavior. In many cases, users
-        # will just create several CAs that all actually expire on the same day.
-        first_ca = self.usable().order_by("-not_after", "serial").first()  # usable == enabled and valid
-        if first_ca is None:
-            raise ImproperlyConfigured("No CA is currently usable.")
-        return first_ca
-
     async def adefault(self) -> "CertificateAuthority":
         """Return the default CA to use when no CA is selected.
 
@@ -250,6 +216,11 @@ async def adefault(self) -> "CertificateAuthority":
             raise ImproperlyConfigured("No CA is currently usable.")
         return first_ca
 
+    @async_to_sync
+    async def default(self) -> "CertificateAuthority":
+        """Return the default CA to use when no CA is selected (synchronous version)."""
+        return await self.adefault()
+
     def disabled(self) -> "CertificateAuthorityQuerySet":
         """Return CAs that are disabled."""
         return self.filter(enabled=False)
@@ -330,15 +301,15 @@ def reasons(
 
     def scope(
         self,
-        ca: "CertificateAuthority",
+        serial: str,
         only_contains_ca_certs: bool = False,
         only_contains_user_certs: bool = False,
         only_contains_attribute_certs: bool = False,
         only_some_reasons: Optional[frozenset[x509.ReasonFlags]] = None,
     ) -> "CertificateRevocationListQuerySet":
         """Return CRLs with the given scope."""
         return self.filter(
-            ca=ca,
+            ca__serial=serial,
             only_contains_ca_certs=only_contains_ca_certs,
             only_contains_user_certs=only_contains_user_certs,
             only_contains_attribute_certs=only_contains_attribute_certs,

ca/django_ca/tests/views/test_certificate_revocation_list_view.py

@@ -27,6 +27,7 @@
 from django.urls import include, path, re_path, reverse
 
 import pytest
+from pytest_django import DjangoAssertNumQueries
 from pytest_django.fixtures import SettingsWrapper
 
 from django_ca import constants
@@ -108,17 +109,26 @@ def deprecated_scope() -> Iterator[None]:
         yield
 
 
-def test_full_crl(client: Client, default_url: str, root_crl: CertificateRevocationList) -> None:
+def test_full_crl(
+    django_assert_num_queries: DjangoAssertNumQueries,
+    client: Client,
+    default_url: str,
+    root_crl: CertificateRevocationList,
+) -> None:
     """Fetch a full CRL (= CA and user certs, all reasons)."""
-    response = client.get(default_url)
+    with django_assert_num_queries(0):
+        response = client.get(default_url)
     assert response.status_code == HTTPStatus.OK
     assert response["Content-Type"] == "application/pkix-crl"
     assert response.content == root_crl.data
 
 
-def test_ca_crl(client: Client, root_ca_crl: CertificateRevocationList) -> None:
+def test_ca_crl(
+    django_assert_num_queries: DjangoAssertNumQueries, client: Client, root_ca_crl: CertificateRevocationList
+) -> None:
     """Fetch a CA CRL."""
-    response = client.get(reverse("ca", kwargs={"serial": root_ca_crl.ca.serial}))
+    with django_assert_num_queries(0):
+        response = client.get(reverse("ca", kwargs={"serial": root_ca_crl.ca.serial}))
     assert response.status_code == HTTPStatus.OK
     assert response["Content-Type"] == "application/pkix-crl"
     assert response.content == root_ca_crl.data
@@ -132,18 +142,34 @@ def test_user_crl(client: Client, root_user_crl: CertificateRevocationList) -> N
     assert response.content == root_user_crl.data
 
 
-def test_with_cache_miss(client: Client, default_url: str, root_crl: CertificateRevocationList) -> None:
+def test_with_cache_miss(
+    django_assert_num_queries: DjangoAssertNumQueries,
+    client: Client,
+    default_url: str,
+    root_crl: CertificateRevocationList,
+) -> None:
     """Fetch a full CRL with a cache miss."""
     cache.clear()  # clear the cache to generate a cache miss
-    response = client.get(default_url)
+
+    with django_assert_num_queries(1) as captured:  # Only one query for fetching the CRL required
+        response = client.get(default_url)
+    assert 'FROM "django_ca_certificaterevocationlist" INNER JOIN' in captured.captured_queries[0]["sql"]
+
     assert response.status_code == HTTPStatus.OK
     assert response["Content-Type"] == "application/pkix-crl"
     assert response.content == root_crl.data
 
 
-def test_regenerate_full_crl(client: Client, usable_root: CertificateAuthority, default_url: str) -> None:
+def test_regenerate_full_crl(
+    django_assert_num_queries: DjangoAssertNumQueries,
+    client: Client,
+    usable_root: CertificateAuthority,
+    default_url: str,
+) -> None:
     """Fetch a full CRL where the CRL has to be regenerated."""
-    response = client.get(default_url)
+    with django_assert_num_queries(9):  # loads of queries required to regenerate a CRL
+        response = client.get(default_url)
+
     assert response.status_code == HTTPStatus.OK
     assert response["Content-Type"] == "application/pkix-crl"
     assert_crl(response.content, expected=[], encoding=Encoding.DER, signer=usable_root)

ca/django_ca/views.py

@@ -127,9 +127,7 @@ def get_key_backend_options(self, ca: CertificateAuthority) -> BaseModel:
         """
         return ca.key_backend.get_use_private_key_options(ca, {})
 
-    async def fetch_crl(
-        self, ca: CertificateAuthority, encoding: CertificateRevocationListEncodings
-    ) -> bytes:
+    async def fetch_crl(self, serial: str, encoding: CertificateRevocationListEncodings) -> bytes:
         """Actually fetch the CRL (nested function so that we can easily catch any exception)."""
         if self.scope is not _NOT_SET:
             warnings.warn(
@@ -164,33 +162,33 @@ async def fetch_crl(
         )
 
         cache_key = get_crl_cache_key(
-            ca.serial,
+            serial,
             encoding=encoding,
             only_contains_ca_certs=only_contains_ca_certs,
             only_contains_user_certs=only_contains_user_certs,
             only_contains_attribute_certs=only_contains_attribute_certs,
             only_some_reasons=self.only_some_reasons,
         )
 
-        encoded_crl: Optional[bytes] = cache.get(cache_key)
+        encoded_crl: Optional[bytes] = await cache.aget(cache_key)
 
         # CRL is not cached, try to retrieve it from the database.
         if encoded_crl is None:
             crl_qs = (
                 CertificateRevocationList.objects.scope(
-                    ca=ca,
+                    serial=serial,
                     only_contains_ca_certs=only_contains_ca_certs,
                     only_contains_user_certs=only_contains_user_certs,
                     only_contains_attribute_certs=only_contains_attribute_certs,
                     only_some_reasons=self.only_some_reasons,
-                )
-                .filter(data__isnull=False)  # only objects that have CRL data associated with it
-                .select_related("ca")
+                ).filter(data__isnull=False)  # Only objects that have CRL data associated with it
             )
             crl_obj: Optional[CertificateRevocationList] = await crl_qs.anewest()
 
             # CRL was not found in the database either, so we try to regenerate it.
             if crl_obj is None:
+                ca = await CertificateAuthority.objects.aget(serial=serial)
+
                 key_backend_options = self.get_key_backend_options(ca)
                 expires = datetime.now(tz=tz.utc) + timedelta(seconds=self.expires)
                 crl_obj = await CertificateRevocationList.objects.acreate_certificate_revocation_list(
@@ -204,7 +202,7 @@ async def fetch_crl(
                 )
 
             # Cache the CRL.
-            await crl_obj.acache()
+            await crl_obj.acache(serial)
 
             # Get object in the right encoding.
             if encoding == Encoding.PEM:
@@ -226,10 +224,8 @@ async def get(self, request: HttpRequest, serial: str) -> HttpResponse:
         else:
             encoding = self.type
 
-        ca = await CertificateAuthority.objects.aget(serial=serial)
-
         try:
-            crl = await self.fetch_crl(ca, encoding)
+            crl = await self.fetch_crl(serial, encoding)
         except Exception:  # pylint: disable=broad-exception-caught
             log.exception("Error generating a CRL")
             return HttpResponseServerError("Error while retrieving the CRL.", content_type="text/plain")