Fix PyPI publish workflow, 100% coverage, aggregate option fix

.github/workflows/python-publish.yml

@@ -1,11 +1,4 @@
-# This workflow will upload a Python Package using Twine when a release is created
-# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python#publishing-to-package-registries
-
-# This workflow uses actions that are not certified by GitHub.
-# They are provided by a third-party and are governed by
-# separate terms of service, privacy policy, and support
-# documentation.
-
+# Build and upload to PyPI when a GitHub release is published.
 name: Upload Python Package
 
 on:
@@ -17,23 +10,19 @@ permissions:
 
 jobs:
   deploy:
-
     runs-on: ubuntu-latest
-
     steps:
-    - uses: actions/checkout@v3
-    - name: Set up Python
-      uses: actions/setup-python@v3
-      with:
-        python-version: '3.x'
-    - name: Install dependencies
-      run: |
-        python -m pip install --upgrade pip
-        pip install build
-    - name: Build package
-      run: python -m build
-    - name: Publish package
-      uses: pypa/gh-action-pypi-publish@27b31702a0e7fc50959f5ad993c78deac1bdfc29
-      with:
-        user: __token__
-        password: ${{ secrets.PYPI_API_TOKEN }}
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+      - name: Build package
+        run: |
+          python -m pip install --upgrade pip build twine
+          python -m build
+          twine check dist/*
+      - name: Publish package
+        uses: pypa/gh-action-pypi-publish@release/v1
+        with:
+          user: __token__
+          password: ${{ secrets.PYPI_API_TOKEN }}

CHANGELOG.md

@@ -32,5 +32,10 @@ Aggify is now a standalone, async-first MongoDB ODM with a Django-style API. **M
 - `$subtract` / `$divide` with 3 operands, mutation of shared `F` / `Q` objects, `$unwind` using the wrong path,
   silently dropped `$match` conditions, magic `let` substitution, `[n]` meaning `limit n`.
 
+### Fixed before release
+- Pipeline/queryset aggregate options (`default_max_time_ms`, `options(max_time_ms=..., allow_disk_use=...)`) were sent with the wrong
+  option names and rejected by the server; they are now translated to `maxTimeMS` / `allowDiskUse` / `batchSize`.
+- A queryset annotation missing from a result row is `None` instead of an unset attribute; dangling references keep their id on `fetch_related`.
+
 ### Breaking
 See the [upgrade guide](docs/migration/aggify-0.md).

aggify/contrib/django/forms.py

@@ -2,13 +2,16 @@
 
 from __future__ import annotations
 
+import inspect
 from typing import Any, Dict, Iterable, Optional, Type
 
 from django import forms
 
 from aggify import fields as f
 from aggify.exceptions import ValidationError as AggifyValidationError
 
+_URL_HAS_ASSUME_SCHEME = "assume_scheme" in inspect.signature(forms.URLField.__init__).parameters
+
 
 def _form_field(field: f.Field) -> Optional[forms.Field]:
     kw: Dict[str, Any] = {"required": field.required, "help_text": field.help_text}
@@ -20,6 +23,8 @@ def _form_field(field: f.Field) -> Optional[forms.Field]:
     if isinstance(field, f.EmailField):
         return forms.EmailField(max_length=field.max_length, **kw)
     if isinstance(field, f.URLField):
+        if _URL_HAS_ASSUME_SCHEME:  # pragma: no branch -- depends on the installed Django; Django >= 5.0: silence the "default scheme will change" deprecation
+            kw["assume_scheme"] = "https"
         return forms.URLField(max_length=field.max_length, **kw)
     if isinstance(field, f.StringField):
         return forms.CharField(min_length=field.min_length, max_length=field.max_length, strip=field.strip, **kw)

aggify/documents.py

@@ -158,8 +158,6 @@ def __new__(mcs, name: str, bases: Tuple[type, ...], attrs: Dict[str, Any], **kw
                     inherited.append((fname, fld.clone()))
         own = sorted(declared, key=lambda kv: kv[1]._creation_order)
         for fname, fld in [*inherited, *own]:
-            if fname in opts.fields and fname not in dict(own):
-                continue
             fld.contribute_to_class(cls, fname)
             opts.fields[fname] = fld
 
@@ -692,15 +690,19 @@ def fetch_related(self, *names: str) -> Document:
             field = self._meta.get_field(n)
             current = self._data.get(n)
             if current is not None and not hasattr(current, "_meta"):
-                self._data[n] = field.document_type.objects.filter(pk=current).first()
+                target = field.document_type.objects.filter(pk=current).first()
+                if target is not None:  # a dangling reference keeps its id
+                    self._data[n] = target
         return self
 
     async def afetch_related(self, *names: str) -> Document:
         for n in names:
             field = self._meta.get_field(n)
             current = self._data.get(n)
             if current is not None and not hasattr(current, "_meta"):
-                self._data[n] = await field.document_type.objects.filter(pk=current).afirst()
+                target = await field.document_type.objects.filter(pk=current).afirst()
+                if target is not None:
+                    self._data[n] = target
         return self
 
     # ------------------------------------------------------------------ #

aggify/fields.py

@@ -43,7 +43,7 @@
 from aggify import registry
 from aggify.conf import settings
 from aggify.exceptions import ValidationError
-from aggify.lookups import LOOKUPS, get_lookup, prepare_kind
+from aggify.lookups import get_lookup, prepare_kind
 from aggify.security import Raw, assert_safe_value, validate_dict_key
 
 T = TypeVar("T")
@@ -119,9 +119,10 @@ def contribute_to_class(self, model: Any, name: str) -> None:
         setattr(model, name, self)
 
     @overload
-    def __get__(self, instance: None, owner: Any) -> Field[T]: ...
+    def __get__(self, instance: None, owner: Any) -> Field[T]: ...  # pragma: no cover
+
     @overload
-    def __get__(self, instance: Any, owner: Any) -> T: ...
+    def __get__(self, instance: Any, owner: Any) -> T: ...  # pragma: no cover
 
     def __get__(self, instance: Any, owner: Any = None) -> Any:
         if instance is None:
@@ -410,8 +411,6 @@ def to_python(self, value: Any) -> Any:
                 return int(value.strip())
             except ValueError:
                 return value
-        if isinstance(value, bson.Int64):
-            return int(value)
         return value
 
     def to_mongo(self, value: Any) -> Any:
@@ -788,9 +787,7 @@ def validate(self, value: Any) -> None:
     def lookup_field(self, lookup: str, value: Any) -> Field:
         if lookup in ("exact", "ne") and isinstance(value, (list, tuple)):
             return self  # whole array comparison
-        if lookup in ("in", "nin", "all") or lookup in LOOKUPS:
-            return self.field
-        return self
+        return self.field
 
     def _prepare_exact(self, value: Any) -> Any:
         if isinstance(value, (list, tuple)):

aggify/pipeline.py

@@ -645,8 +645,7 @@ def _root_scope(self, expr: Any) -> Tuple[Any, Scope, Context]:
             f = r.field
             if isinstance(f, ListField):
                 f = f.field
-            if isinstance(f, EmbeddedField):
-                scope = Scope.for_model(f.document_type)
+            scope = Scope.for_model(f.document_type)  # _replace() has verified it is embedded
             return "$" + r.db_path, scope, ctx
         return wrap(expr).resolve(ctx), scope, ctx
 
@@ -809,11 +808,15 @@ def options(self, **opts: Any) -> Pipeline:
         new._options.update(opts)
         return new
 
-    def _aggregate_kwargs(self) -> Dict[str, Any]:
+    #: option names -> the spelling of the ``aggregate`` command (pymongo forwards unknown kwargs verbatim)
+    _COMMAND_OPTIONS = {"allow_disk_use": "allowDiskUse", "max_time_ms": "maxTimeMS", "batch_size": "batchSize"}
+
+    def _aggregate_kwargs(self, **extra: Any) -> Dict[str, Any]:
         kw = dict(self._options)
         if "max_time_ms" not in kw and settings.default_max_time_ms:
             kw["max_time_ms"] = settings.default_max_time_ms
-        return kw
+        kw.update(extra)
+        return {self._COMMAND_OPTIONS.get(k, k): v for k, v in kw.items()}
 
     def _alias(self) -> str:
         return self.model._meta.db_alias if self.model is not None else connection.DEFAULT_ALIAS

aggify/queryset.py

@@ -492,15 +492,14 @@ def _hydrate_row(self, row: Dict[str, Any], scope: Optional[Scope], loaded: Opti
         obj = self.model.from_mongo(row, loaded=loaded)
         scope_ = scope
         for name in self._annotations:
-            if name in row:
-                f = scope_.fields.get(name) if scope_ is not None else None
-                value = row[name]
-                if f is not None and value is not None:
-                    try:
-                        value = f.from_mongo(value)
-                    except Exception:  # noqa: S110 - keep the raw value if it cannot be converted
-                        pass
-                setattr(obj, name, value)
+            f = scope_.fields.get(name) if scope_ is not None else None
+            value = row.get(name)  # a missing key means "no value": None, like Django
+            if f is not None and value is not None:
+                try:
+                    value = f.from_mongo(value)
+                except Exception:  # noqa: S110 - keep the raw value if it cannot be converted
+                    pass
+            setattr(obj, name, value)
         for name in self._select_related:
             _attach_related(obj, self.model, name, row)
         return obj
@@ -593,9 +592,8 @@ def iterator(self, chunk_size: int = 1000) -> Iterator[M]:
         else:
             cursor = coll.aggregate(
                 plan._final_stages(),
-                batchSize=chunk_size,
                 **connection.session_kwargs(self.model._meta.db_alias),
-                **plan._aggregate_kwargs(),
+                **plan._aggregate_kwargs(batch_size=chunk_size),
             )
         try:
             for r in cursor:
@@ -764,8 +762,7 @@ def bulk_create(
         kw = connection.session_kwargs(self.model._meta.db_alias)
         try:
             for docs in batches:
-                if docs:
-                    coll.insert_many(docs, ordered=ordered, **kw)
+                coll.insert_many(docs, ordered=ordered, **kw)
         except BulkWriteError as exc:
             raise NotUniqueError(str(exc.details.get("writeErrors", [{}])[0].get("errmsg", exc))) from exc
         self._finish_bulk(objs)
@@ -867,9 +864,8 @@ async def aiterator(self, chunk_size: int = 1000) -> AsyncIterator[M]:
         else:
             cursor = await coll.aggregate(
                 plan._final_stages(),
-                batchSize=chunk_size,
                 **connection.session_kwargs(self.model._meta.db_alias),
-                **plan._aggregate_kwargs(),
+                **plan._aggregate_kwargs(batch_size=chunk_size),
             )
         try:
             async for r in cursor:
@@ -983,8 +979,7 @@ async def abulk_create(
         kw = connection.session_kwargs(self.model._meta.db_alias)
         try:
             for docs in batches:
-                if docs:
-                    await coll.insert_many(docs, ordered=ordered, **kw)
+                await coll.insert_many(docs, ordered=ordered, **kw)
         except BulkWriteError as exc:
             raise NotUniqueError(str(exc.details.get("writeErrors", [{}])[0].get("errmsg", exc))) from exc
         self._finish_bulk(objs)
@@ -1040,9 +1035,7 @@ def _invert(o: Any) -> Any:
         return o[1:] if o.startswith("-") else "-" + o.lstrip("+")
     if isinstance(o, OrderBy):
         return OrderBy(o.expression, not o.descending)
-    if isinstance(o, F):
-        return OrderBy(o, True)
-    return o
+    return OrderBy(o, True)
 
 
 def _getattr_field(obj: Any, name: str) -> Any:

aggify/registry.py

@@ -9,12 +9,8 @@
 _models: Dict[str, List[type]] = {}
 
 
-def register(model: type) -> None:
-    keys = {model.__name__}
-    label = getattr(getattr(model, "_meta", None), "label", None)
-    if label:
-        keys.add(label)
-    for key in keys:
+def register(model: Any) -> None:
+    for key in {model.__name__, model._meta.label}:
         bucket = _models.setdefault(key, [])
         # re-definition of the same class name (autoreload, tests) replaces the old entry
         bucket[:] = [
@@ -47,8 +43,4 @@ def resolve(ref: Union[str, type], *, relative_to: Any = None) -> type:
     return candidates[0]
 
 
-def clear() -> None:  # used by tests
-    _models.clear()
-
-
 Model = Type[Any]

aggify/schema.py

@@ -17,7 +17,7 @@
 from typing import Any, Dict, Iterable, List, Mapping, Optional, Tuple
 
 from aggify.conf import settings
-from aggify.exceptions import InvalidField, InvalidLookup, UnsafeQueryError
+from aggify.exceptions import InvalidField, UnsafeQueryError
 from aggify.fields import AnyField, DictField, EmbeddedField, Field, ListField, ReferenceField
 from aggify.lookups import LOOKUPS
 
@@ -235,9 +235,6 @@ def resolve_path(
             cur = None
         i += 1
 
-    if lookup is not None and lookup not in LOOKUPS:
-        raise InvalidLookup(lookup, path)
-
     return Resolved(
         db_path=".".join(db_parts),
         field=field,

aggify/updates.py

@@ -108,7 +108,7 @@ def build_update(model: Any, changes: Dict[str, Any]) -> Union[Dict[str, Any], L
             bucket[db_path] = value
         elif op == "rename":
             bucket[db_path] = validate_field_name(str(value), allow_dots=True)
-        elif op == "current_date":
+        else:  # current_date
             bucket[db_path] = True
     return ops
 
@@ -148,9 +148,7 @@ def _build_pipeline_update(
             set_[db_path] = {fn: [arr, [{"$literal": item}]]}
         else:
             raise InvalidArgument(f"{op}__ cannot be combined with expression assignments.")
-    stages: List[Dict[str, Any]] = []
-    if set_:
-        stages.append({"$set": set_})
+    stages: List[Dict[str, Any]] = [{"$set": set_}]  # an expression assignment is always present here
     if unset:
         stages.append({"$unset": unset})
     return stages

tests/contrib/conftest.py

@@ -0,0 +1,19 @@
+"""One Django configuration for every test module in this package (settings are process-global)."""
+
+import django
+from django.conf import settings
+
+from tests.conftest import MONGO_URI
+
+if not settings.configured:
+    settings.configure(
+        SECRET_KEY="test",
+        USE_TZ=True,
+        DATABASES={"default": {"ENGINE": "django.db.backends.sqlite3", "NAME": ":memory:"}},
+        INSTALLED_APPS=["django.contrib.contenttypes", "aggify.contrib.django"],
+        ROOT_URLCONF="tests.contrib.django_urls",
+        ALLOWED_HOSTS=["*"],
+        AGGIFY={"django-test": {"URI": MONGO_URI, "NAME": "aggify_django_tests", "OPTIONS": {"maxPoolSize": 7}}},
+        DEFAULT_AUTO_FIELD="django.db.models.AutoField",
+    )
+    django.setup()