Identifier improvement

mpcontribs-api/src/mpcontribs_api/domains/_shared/models.py

@@ -9,7 +9,7 @@
 from pymongo.results import DeleteResult
 
 from mpcontribs_api import pagination
-from mpcontribs_api.domains._shared.types import MD5Hash, NFKCStr
+from mpcontribs_api.domains._shared.types import Identity, MD5Hash, NFKCStr
 from mpcontribs_api.projection import SparseFieldsModel
 
 
@@ -25,24 +25,25 @@ class BaseDocumentWithInput[TId](Document):
     """
 
     HAS_DERIVED_FIELDS: ClassVar[bool] = False
+    # The domain's ``Identity`` subclass. Describes how documents are uniquely addressed
+    identity_model: ClassVar[type[Identity]] = Identity
     # Required, non-null, resource-specific id. Overrides Document's optional ``PydanticObjectId`` id.
     id: TId = Field(alias="_id")  # pyright: ignore[reportGeneralTypeIssues, reportIncompatibleVariableOverride]
 
-    @classmethod
-    def identifier_fields(cls) -> frozenset[str]:
-        """Field names that uniquely identify a document in this collection.
-
-        This is the natural/unique key a caller can supply without first knowing the Mongo ``_id``
-        (e.g. ``{"name", "owner"}`` for a project group). The repository pairs these names with
-        caller-supplied values to locate a single resource, and rejects any value dict whose keys
-        don't match this set. Defaults to the primary key; subclasses with a meaningful compound key
-        override it.
+    def identifiers(self) -> dict[str, Any]:
+        """This document's natural-key field values, keyed by ``identity_model.model_fields``.
+
+        The natural/unique key is a caller-suppliable alternative to the Mongo ``_id`` (e.g.
+        ``{"name", "owner"}`` for a project group); it comes straight from the domain's ``Identity``
+        subclass so each domain declares its key exactly once.
         """
-        return frozenset({"id"})
+        return {field: getattr(self, field) for field in self.identity_model.model_fields}
 
-    def identifiers(self) -> dict[str, Any]:
-        """This document's identifier field values, keyed by :meth:`identifier_fields`."""
-        return {field: getattr(self, field) for field in self.identifier_fields()}
+    def identity(self) -> Identity:
+        """This document's identity as a concrete :class:`Identity`, built from its own fields."""
+        return self.identity_model.from_document(
+            {name: getattr(self, name) for name in self.identity_model.model_fields}
+        )
 
     def derived_field_updates(self) -> dict[str, Any]:
         """Server-derived fields to persist alongside a patch.
@@ -103,6 +104,15 @@ def canonical_md5(payload: Mapping[str, Any]) -> str:
     return hashlib.md5(normalized.encode("utf-8")).hexdigest()
 
 
+class ComponentIdentity(Identity):
+    """Identity of a content-addressed component: its ``md5`` content hash.
+
+    Shared by every component domain (structures/tables/attachments).
+    """
+
+    md5: MD5Hash
+
+
 class ComponentIn(BaseModel):
     """Base for component input payloads.
 
@@ -124,17 +134,13 @@ class Component(BaseDocumentWithInput[PydanticObjectId]):
     """
 
     HAS_DERIVED_FIELDS: ClassVar[bool] = True
+    identity_model: ClassVar[type[Identity]] = ComponentIdentity
     name: NFKCStr
     # Server-computed; the placeholder default is overwritten by ``_recompute_md5`` on validation.
     md5: MD5Hash = Field(default="0" * 32)
 
     hash_fields: ClassVar[frozenset[str]]
 
-    @classmethod
-    def identifier_fields(cls) -> frozenset[str]:
-        """A component is content-addressed: its ``md5`` uniquely identifies its content."""
-        return frozenset({"md5"})
-
     def derived_field_updates(self) -> dict[str, Any]:
         """Recompute ``md5`` from the (patched-in-memory) content so the write stays authoritative."""
         return {"md5": self.compute_md5()}

mpcontribs-api/src/mpcontribs_api/domains/_shared/repository.py

@@ -7,7 +7,7 @@
 from abc import ABC
 from collections.abc import AsyncIterable, AsyncIterator, Callable, Iterable, Mapping
 from contextlib import AbstractAsyncContextManager
-from typing import Any, ClassVar
+from typing import Any, ClassVar, cast
 
 import structlog
 from beanie import PydanticObjectId, UpdateResponse
@@ -23,7 +23,7 @@
 from mpcontribs_api.config import get_settings
 from mpcontribs_api.domains._shared.bulk import BulkFailure, BulkWriteSummary, bulk_failure_from_exception
 from mpcontribs_api.domains._shared.models import BaseDocumentWithInput, DeleteResponse, DocumentOut
-from mpcontribs_api.domains._shared.types import DownloadFormat, ShortMimeFormat
+from mpcontribs_api.domains._shared.types import DownloadFormat, Identity, ShortMimeFormat
 from mpcontribs_api.exceptions import ConflictError, DownloadError, NotFoundError, ValidationError
 from mpcontribs_api.pagination import CursorParams, Page, encode_cursor
 from mpcontribs_api.scope import Scope
@@ -124,18 +124,18 @@ async def read_many(
     def _identifier_query(self, identifiers: dict[str, Any]) -> dict[str, Any]:
         """Turn a ``{field: value}`` identifier dict into a scoped Mongo query fragment.
 
-        The keys must be either the model's :meth:`identifier_fields` exactly, or the bare
-        primary-key form ``{"id": ...}`` (which addresses any document by its ``_id`` regardless of
-        its semantic identifier). ``id`` is remapped to Mongo's ``_id`` (mirroring
+        The keys must be either the model's natural key (``identity_model.model_fields``) exactly, or
+        the bare primary-key form ``{"id": ...}`` (which addresses any document by its ``_id``
+        regardless of its semantic identifier). ``id`` is remapped to Mongo's ``_id`` (mirroring
         ``BaseFilter._get_filter_conditions``) since a raw dict query does not go through Beanie's
         alias resolution.
 
         Args:
-            identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields``,
+            identifiers (dict[str, Any]): identifier field values keyed by the model's natural key,
                 or ``{"id": <primary key>}``
         """
         identifiers = self.coerce_identifiers(identifiers)
-        expected = self.document_model.identifier_fields()
+        expected = self.document_model.identity_model.model_fields.keys()
         if identifiers.keys() != expected and identifiers.keys() != {"id"}:
             raise ValidationError(
                 "identifiers must match the model's identifier fields, or be a bare {'id': ...}",
@@ -153,7 +153,8 @@ async def read_one(
         """Return the single scoped document matching ``identifiers``, projected to ``fields``.
 
         Args:
-            identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields``
+            identifiers (dict[str, Any]): identifier field values keyed by the model's natural key, or the bare
+                primary-key form ``{"id": <primary key>}``
             fields (frozenset[str] | None): fields to project; if None the full document is returned
             session (AsyncClientSession | None): optional client session for transactions
         """
@@ -217,15 +218,49 @@ async def insert_many(self, documents: list[TDoc], session: AsyncClientSession |
         """
         return await self.document_model.insert_many(documents, ordered=False, session=session)
 
+    def _scoped_identity_match(self, identity: Identity) -> dict[str, Any]:
+        """Scoped Mongo match locating the single document with ``identity`` (its full natural key)."""
+        match = {("_id" if key == "id" else key): value for key, value in identity.as_dict().items()}
+        return {"$and": [self._scope, match]} if self._scope else match
+
     async def upsert_one(self, document: TDoc, session: AsyncClientSession | None = None) -> TDoc:
-        """Insert ``document`` or replace the existing one with the same ``_id`` (PUT semantics).
+        """Insert ``document`` or merge it into the existing one with the same identity (natural key).
 
-        Domains whose upsert key is a compound identity rather than ``_id`` (e.g. contributions) override this.
+        Null fields are dropped from the ``$set`` (``keep_nulls`` parity); on insert the full ``document`` is written.
+        For PUT-by-``_id`` replace semantics use :meth:`replace_one`.
 
         Args:
             document (TDoc): the fully-built document to persist
             session (AsyncClientSession | None): optional client session for transactions
         """
+        match = self._scoped_identity_match(document.identity())
+        update_data = document.model_dump(exclude={"id"}, exclude_none=True)
+        try:
+            result = await self.document_model.find_one(match, session=session).upsert(  # pyright: ignore[reportGeneralTypeIssues] # beanie UpdateQuery is awaitable
+                Set(update_data),
+                on_insert=document,
+                response_type=UpdateResponse.NEW_DOCUMENT,
+                session=session,
+            )
+        except DuplicateKeyError as exc:
+            raise ConflictError(
+                f"Cannot upsert {self.document_model.__name__}: a conflicting document already exists",
+                identifiers=document.identifiers(),
+            ) from exc
+        return cast(TDoc, result)  # upsert always returns the resulting document
+
+    async def replace_one(self, id: Any, document: TDoc, session: AsyncClientSession | None = None) -> TDoc:
+        """Insert ``document`` under ``id`` or fully replace the existing one at that ``_id`` (PUT).
+
+        This replaces the whole document at a primary key — omitted/null fields are cleared — matching HTTP PUT.
+        ``document.id`` is set to ``id`` so the write always lands under the caller's key.
+
+        Args:
+            id (Any): the primary key to write under
+            document (TDoc): the fully-built replacement document
+            session (AsyncClientSession | None): optional client session for transactions
+        """
+        document.id = id
         try:
             return await document.save(session=session)
         except DuplicateKeyError as exc:
@@ -291,7 +326,8 @@ async def delete_one(
         """Delete the single scoped document matching ``identifiers``.
 
         Args:
-            identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields``
+            identifiers (dict[str, Any]): identifier field values keyed by the model's natural key, or the bare
+                primary-key form ``{"id": <primary key>}``
             session (AsyncClientSession | None): optional client session for transactions
         """
         query = self._identifier_query(identifiers)
@@ -367,7 +403,8 @@ async def update_one(
         """Partially update the single scoped document matching ``identifiers``.
 
         Args:
-            identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields``
+            identifiers (dict[str, Any]): identifier field values keyed by the model's natural key, or the bare
+                primary-key form ``{"id": <primary key>}``
             update (TPatch): the partial update to apply; unset fields are dropped
             session (AsyncClientSession | None): optional client session for transactions
             extra_set (dict[str, Any] | None): server-resolved fields to merge into the ``$set``

mpcontribs-api/src/mpcontribs_api/domains/_shared/types.py

@@ -1,15 +1,12 @@
 import re
 import unicodedata
-from abc import ABC
 from collections.abc import Callable, Mapping
-from dataclasses import MISSING, dataclass, fields
 from enum import StrEnum
-from functools import cache
-from typing import Annotated, Any, Self, get_args, get_type_hints
+from typing import Annotated, Any, Self
 
 import polars as pl
 from fastapi import Query
-from pydantic import BeforeValidator, Field, PlainSerializer, WithJsonSchema
+from pydantic import BaseModel, BeforeValidator, ConfigDict, Field, PlainSerializer, WithJsonSchema
 from pymatgen.core import Element
 from pymongo import ASCENDING, IndexModel
 
@@ -424,62 +421,45 @@ def map_keys(value: Any, *, coerce: Callable[[Any], str], on_scalar: Callable[[A
     return on_scalar(value)
 
 
-@cache
-def _optional_field_names(cls: type) -> frozenset[str]:
-    """Names of ``cls``'s dataclass fields whose type admits ``None`` (resolved through string annotations)."""
-    hints = get_type_hints(cls)
-    return frozenset(f.name for f in fields(cls) if type(None) in get_args(hints.get(f.name)))
-
-
-# dataclass construction is cheaper than Pydantic.BaseModel
-@dataclass(frozen=True, slots=True)
-class Identity(ABC):  # noqa: B024  # base kept abstract as a marker; from_document is a shared concrete helper
+class Identity(BaseModel):
     """The full identity of a document model.
 
     Field declaration order IS the identity/index column order: ``index_model`` and ``projection``
-    iterate ``dataclasses.fields`` in that order, so the order is declared exactly once (below).
+    iterate ``model_fields`` (which preserves definition order) so the order is declared exactly once
+    on each subclass.
     """
 
     # WARNING: the order the fields are specified in reflects their ordering for indices. Changing the order
     # creates index migration. Only change intentionally
+    model_config = ConfigDict(frozen=True)
 
     def as_dict(self) -> dict[str, Any]:
         """Identity as a flat dict keyed by field name (for Mongo match clauses and upsert)."""
-        return {f.name: getattr(self, f.name) for f in fields(self)}
-
-    @classmethod
-    def model_fields(cls) -> frozenset[str]:
-        """Returns the field names as a frozenset"""
-        return frozenset(f.name for f in fields(cls))
+        return {name: getattr(self, name) for name in type(self).model_fields}
 
     @classmethod
     def from_document(cls, doc: Mapping[str, Any]) -> Self:
         """Build from a raw Mongo document/projection, tolerating null-stripped fields.
 
-        Generic over any ``@dataclass`` subclass: iterates the concrete class's own fields,
-        falling back to each field's default (or ``None`` for a defaultless Optional field) when the
-        document omits it, since Mongo strips nulls (``keep_nulls=False``). Required non-null fields
-        that are absent surface as a ``TypeError`` from the constructor.
+        Uses `model_construct` to avoid having to revalidate data that is already valid (stored), and to avoid
+        errors on legacy documents.
         """
-        optional = _optional_field_names(cls)
-        kwargs: dict[str, Any] = {}
-        for f in fields(cls):
-            if doc.get(f.name) is not None:
-                kwargs[f.name] = doc[f.name]
-            elif f.default is not MISSING:
-                kwargs[f.name] = f.default
-            elif f.default_factory is not MISSING:
-                kwargs[f.name] = f.default_factory()
-            elif f.name in optional:
-                kwargs[f.name] = None
-        return cls(**kwargs)
+        present = {name: doc[name] for name in cls.model_fields if doc.get(name) is not None}
+        missing = [name for name, field in cls.model_fields.items() if field.is_required() and name not in present]
+        if missing:
+            raise TypeError(f"{cls.__name__}.from_document missing required field(s): {sorted(missing)}")
+        return cls.model_construct(**present)
 
     @classmethod
-    def index_model(cls, name: str = "project_identity", *, unique: bool = True) -> IndexModel:
-        """The unique index enforcing identity — keys follow the field order so they can't drift."""
-        return IndexModel(keys=[(f.name, ASCENDING) for f in fields(cls)], name=name, unique=unique)
+    def index_model(cls, name: str = "identity", *, unique: bool = True) -> IndexModel:
+        """The unique index enforcing identity — keys follow the field order so they can't drift.
+
+        ``name`` should be passed explicitly to match the collection's deployed index name; renaming
+        a live index forces a drop/recreate migration.
+        """
+        return IndexModel(keys=[(name_, ASCENDING) for name_ in cls.model_fields], name=name, unique=unique)
 
     @classmethod
     def projection(cls) -> dict[str, int]:
         """A Mongo projection selecting exactly the identity fields."""
-        return {f.name: 1 for f in fields(cls)}
+        return {name: 1 for name in cls.model_fields}

mpcontribs-api/src/mpcontribs_api/domains/attachments/models.py

@@ -2,7 +2,7 @@
 from pydantic import field_validator
 
 from mpcontribs_api.domains._shared.filters import BaseFilter
-from mpcontribs_api.domains._shared.models import Component, ComponentIn, DocumentOut
+from mpcontribs_api.domains._shared.models import Component, ComponentIdentity, ComponentIn, DocumentOut
 from mpcontribs_api.domains._shared.types import FileLike, MD5Hash, MimeFormat, NFKCStr
 from mpcontribs_api.exceptions import ValidationError
 from mpcontribs_api.projection import SparseFieldsModel
@@ -27,6 +27,7 @@ class Attachment(Component):
 
     class Settings:
         name = "attachments"
+        indexes = [ComponentIdentity.index_model(name="md5")]
 
     @field_validator("name", mode="before")
     @classmethod

mpcontribs-api/src/mpcontribs_api/domains/attachments/router.py

@@ -5,7 +5,7 @@
 from fastapi_filter import FilterDepends
 
 from mpcontribs_api.dependencies import S3Dep, require_user
-from mpcontribs_api.domains._shared.models import ComponentDeleteResponse
+from mpcontribs_api.domains._shared.models import ComponentDeleteResponse, ComponentIdentity
 from mpcontribs_api.domains._shared.types import (
     DownloadFormat,
     FieldSelector,
@@ -30,6 +30,33 @@ async def read_many(
     return await service.read_many(filter=filter, fields=selected, pagination=pagination)
 
 
+@router.get("/item")
+async def read_one_by_identity(
+    service: AttachmentServiceDep,
+    identity: Annotated[ComponentIdentity, Depends()],
+    fields: FieldSelector = None,
+):
+    """Return a single attachment addressed by its content ``md5`` (its natural key)."""
+    selected = AttachmentOut.parse_fields(fields)
+    return await service.read_one(identifiers=identity.as_dict(), fields=selected)
+
+
+@router.delete("/item", response_model=ComponentDeleteResponse, dependencies=[Depends(require_user)])
+async def delete_one_by_identity(service: AttachmentServiceDep, identity: Annotated[ComponentIdentity, Depends()]):
+    """Delete a single attachment addressed by its content ``md5`` (its natural key)."""
+    return await service.delete_one(identifiers=identity.as_dict())
+
+
+@router.patch("/item", dependencies=[Depends(require_user)])
+async def update_one_by_identity(
+    service: AttachmentServiceDep,
+    identity: Annotated[ComponentIdentity, Depends()],
+    update: AttachmentPatch,
+):
+    """Patch a single attachment addressed by its content ``md5`` (its natural key)."""
+    return await service.update_one(identifiers=identity.as_dict(), update=update)
+
+
 @router.get("/{id}")
 async def read_one(
     service: AttachmentServiceDep,
@@ -85,5 +112,5 @@ async def update_one(
     id: str,
     update: AttachmentPatch,
 ):
-    """Patch a single attachment addressed by its ``_id`` or its content ``md5``."""
+    """Patch a single attachment addressed by its ``_id``."""
     return await service.update_one(identifiers={"id": id}, update=update)