diff --git a/mpcontribs-api/pyproject.toml b/mpcontribs-api/pyproject.toml index 6b89a5c76..e66fd3e17 100644 --- a/mpcontribs-api/pyproject.toml +++ b/mpcontribs-api/pyproject.toml @@ -32,13 +32,7 @@ authors = [ {name="The Materials Project", email="feedback@materialsproject.org"}, ] dependencies = [ - # "pint>=0.24", - # "psycopg2-binary", # "rq<=2.3.2", # see https://github.com/rq/Flask-RQ2/issues/620 - # "setproctitle", - # "uncertainties", - # "websocket_client", - # "zstandard", "aioboto3>=15.5.0", "beanie>=2.1.0", "fastapi[standard]>=0.136.3", diff --git a/mpcontribs-api/src/mpcontribs_api/api/v1/router.py b/mpcontribs-api/src/mpcontribs_api/api/v1/router.py index fe71d8326..168222680 100644 --- a/mpcontribs-api/src/mpcontribs_api/api/v1/router.py +++ b/mpcontribs-api/src/mpcontribs_api/api/v1/router.py @@ -3,7 +3,9 @@ from mpcontribs_api.domains.attachments.router import router as attachments_router from mpcontribs_api.domains.consumers.router import router as consumers_router from mpcontribs_api.domains.contributions.router import router as contributions_router +from mpcontribs_api.domains.initiatives.router import router as initiatives_router from mpcontribs_api.domains.limits.router import router as limits_router +from mpcontribs_api.domains.project_groups.router import router as project_groups_router from mpcontribs_api.domains.projects.router import router as projects_router from mpcontribs_api.domains.structures.router import router as structures_router from mpcontribs_api.domains.tables.router import router as tables_router @@ -12,7 +14,9 @@ router.include_router(attachments_router, prefix="/attachments", tags=["attachments"]) router.include_router(contributions_router, prefix="/contributions", tags=["contributions"]) +router.include_router(initiatives_router, prefix="/initiatives", tags=["initiatives"]) router.include_router(limits_router, prefix="/limits", tags=["limits"]) +router.include_router(project_groups_router, prefix="/project_groups", tags=["project_groups"]) router.include_router(projects_router, prefix="/projects", tags=["projects"]) router.include_router(structures_router, prefix="/structures", tags=["structures"]) router.include_router(tables_router, prefix="/tables", tags=["tables"]) diff --git a/mpcontribs-api/src/mpcontribs_api/app.py b/mpcontribs-api/src/mpcontribs_api/app.py index f869c4418..3878b703b 100644 --- a/mpcontribs-api/src/mpcontribs_api/app.py +++ b/mpcontribs-api/src/mpcontribs_api/app.py @@ -22,6 +22,8 @@ from mpcontribs_api.domains.consumers.models import Consumer from mpcontribs_api.domains.contributions.models import Contribution from mpcontribs_api.domains.healthcheck.router import router as healthcheck_router +from mpcontribs_api.domains.initiatives.models import Initiative +from mpcontribs_api.domains.project_groups.models import ProjectGroup from mpcontribs_api.domains.projects.models import Project from mpcontribs_api.domains.structures.models import Structure from mpcontribs_api.domains.tables.models import Table @@ -64,6 +66,8 @@ async def _setup_mongo(app: FastAPI, settings: Settings, stack: AsyncExitStack) database=client[settings.mongo.db_name], document_models=[ Project, + ProjectGroup, + Initiative, Contribution, Attachment, Structure, diff --git a/mpcontribs-api/src/mpcontribs_api/authz.py b/mpcontribs-api/src/mpcontribs_api/authz.py index bd332ac2c..e035010bd 100644 --- a/mpcontribs-api/src/mpcontribs_api/authz.py +++ b/mpcontribs-api/src/mpcontribs_api/authz.py @@ -33,6 +33,16 @@ ADMIN_GROUP = settings.mongo.admin_group +# prefix to user roles to disambiguate from project roles, which are bare ids +INITIATIVE_ROLE_PREFIX = "initiative:" + +# prefix for project-group roles: a group's _id (an ObjectId hex string) is granted as ``project-group:`` +PROJECT_GROUP_ROLE_PREFIX = "project-group:" + +# A role carrying one of these prefixes is scoped to a non-project resource; a role with none of +# them is a bare project id. +_RESOURCE_ROLE_PREFIXES = (INITIATIVE_ROLE_PREFIX, PROJECT_GROUP_ROLE_PREFIX) + class User(BaseModel): """User definition derived from request headers. @@ -64,7 +74,42 @@ def is_anonymous(self) -> bool: def is_admin(self) -> bool: return (not self.is_anonymous) and (ADMIN_GROUP in self.groups) - def has_role(self, role: str) -> bool: + @property + def project_roles(self) -> list[str]: + """The project ids this user carries, from their bare (unprefixed) roles. + + Resource-scoped roles (``initiative:``, ``project-group:``) and the admin sentinel are + excluded, leaving only bare project ids. + """ + return [role for role in self.groups if role != ADMIN_GROUP and not role.startswith(_RESOURCE_ROLE_PREFIXES)] + + @property + def initiative_roles(self) -> list[str]: + """The initiative slugs this user collaborates on, decoded from their ``initiative:`` roles.""" + return [role[len(INITIATIVE_ROLE_PREFIX) :] for role in self.groups if role.startswith(INITIATIVE_ROLE_PREFIX)] + + @property + def project_group_roles(self) -> list[str]: + """The project-group ids this user may access, decoded from their ``project-group:`` roles. + + Values are the raw hex strings; callers that query by ``_id`` must convert them + """ + return [ + role[len(PROJECT_GROUP_ROLE_PREFIX) :] for role in self.groups if role.startswith(PROJECT_GROUP_ROLE_PREFIX) + ] + + def has_role(self, role: str, *, resource: str | None = None) -> bool: + """Determine whether a user has a role assigned to them. + + Specifying resource as: + - ``INITIATIVE_ROLE_PREFIX`` looks for roles scoped to initiatives + - "project" looks for bare (unprefixed) project roles + - None looks for roles by matching the entire string + """ + if resource == INITIATIVE_ROLE_PREFIX[:-1]: + return role in self.initiative_roles + if resource == "project": + return role in self.project_roles return role in self.groups @property @@ -72,8 +117,16 @@ def writable_projects(self) -> frozenset[str]: """Projects this user may write to. Admins are unbounded (handled by can_write)""" if self.is_anonymous: return frozenset() - # exclude the admin sentinel so it never leaks into a $in / membership test - return frozenset(g for g in self.groups if g != ADMIN_GROUP) + # only bare project roles are writable projects; the admin sentinel and resource-scoped + # roles (initiative:/project-group:) must never leak into a $in / membership test + return frozenset(self.project_roles) + + def can_manage(self, id: str, resource: str) -> bool: + """Determines whether a user can manage a resource. + + If the user is known and either an admin or has a valid role assigned, they can manage + """ + return (not self.is_anonymous) and (self.is_admin or self.has_role(role=id, resource=resource)) def can_write(self, project: str) -> bool: """Single source of truth for write authorization.""" diff --git a/mpcontribs-api/src/mpcontribs_api/config.py b/mpcontribs-api/src/mpcontribs_api/config.py index dbfae43be..daeb78bf9 100644 --- a/mpcontribs-api/src/mpcontribs_api/config.py +++ b/mpcontribs-api/src/mpcontribs_api/config.py @@ -172,6 +172,26 @@ def _clamp_concurrency(self) -> Self: return self +class InitiativeSettings(BaseModel): + """Limits governing user-owned initiatives.""" + + max_unapproved_per_owner: int = Field( + default=3, + description="Maximum number of unapproved initiatives a single owner may have at once. Enforced on create.", + ) + max_projects_per_unapproved: int = Field( + default=2, + description="Maximum number of projects that may be assigned to an unapproved initiative. Enforced when a " + "project's initiative is set via PATCH.", + ) + + +class DomainSettings(BaseModel): + """Settings to configure the domain logic of MPContribs""" + + initiatives: InitiativeSettings = Field(default_factory=InitiativeSettings) + + class MPContribsSettings(BaseModel): max_contrib_data_depth: int = Field( default=7, description="The max number of levels allowed in a Contribution's data dictionary." @@ -218,6 +238,9 @@ class Settings(BaseSettings): # MPContribs_otel__* otel: ObservabilitySettings = Field(default_factory=ObservabilitySettings) + # MPContribs_domain_* + domain: DomainSettings = Field(default_factory=DomainSettings) + # MPContribs_consumer__* consumer: QuotaLimits = Field(default_factory=QuotaLimits) diff --git a/mpcontribs-api/src/mpcontribs_api/dependencies.py b/mpcontribs-api/src/mpcontribs_api/dependencies.py index 2f4b925f6..c2e6a639a 100644 --- a/mpcontribs-api/src/mpcontribs_api/dependencies.py +++ b/mpcontribs-api/src/mpcontribs_api/dependencies.py @@ -73,6 +73,18 @@ def require_user(user: UserDep) -> User: return user +def require_writer(user: UserDep) -> User: + """Require an authenticated caller who can write to at least one project. + + Controls access to creating components if you do not have contributions to attach them to. + Helps to limit orphanned components + """ + if user.is_anonymous: + raise AuthenticationError("authentication required") + if not (user.is_admin or user.writable_projects): + raise PermissionError("write access to at least one project is required") + + def require_admin(user: UserDep) -> User: """Require an authenticated admin caller. diff --git a/mpcontribs-api/src/mpcontribs_api/domains/_shared/components.py b/mpcontribs-api/src/mpcontribs_api/domains/_shared/components.py index 1af1edb05..fd2841eb2 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/_shared/components.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/_shared/components.py @@ -10,7 +10,6 @@ from mpcontribs_api.domains._shared.models import Component, ComponentIn, DeleteResponse, DocumentOut from mpcontribs_api.domains._shared.repository import MongoDbRepository from mpcontribs_api.domains._shared.types import MD5Hash -from mpcontribs_api.exceptions import NotFoundError class MongoDbComponentsRepository[ @@ -94,10 +93,6 @@ async def insert_component(self, component: TIn, *, session: AsyncClientSession """ return (await self.insert_components(components=[component], session=session))[0] - async def get_component_by_id(self, id: str, fields: frozenset[str] | None) -> TDoc | TOut | None: - """Find a single component by id. See ``get_by_id``.""" - return await self.get_by_id(self._convert_object_id(id), fields) - async def delete_components( self, filter: TFilter, @@ -115,37 +110,3 @@ async def delete_components( query = filter.filter(self.document_model.find(self._scope, session=session)) result = await query.delete(session=session) return DeleteResponse(num_deleted=result.deleted_count if result else 0) - - async def delete_component_by_id( - self, - id: str, - session: AsyncClientSession | None = None, - ) -> DeleteResponse: - """Deletes a single component by Id. - - Args: - id (str): the str representation of the component's ObjectId - session (AsyncClientSession | None): the current session, used to guarantee transactions - - Returns: - DeleteResponse: A report of the deletion - """ - return await self.delete_by_id(id=self._convert_object_id(id), session=session) - - async def patch_component_by_id(self, id: str, update: TPatch) -> TDoc: - """Partially update a component by id, recomputing its content hash. - - Components are content-addressed, so a content change must update ``md5``. Unlike the base - ``patch`` (an in-place ``$set``), this loads the full document, applies the set fields, - recomputes ``md5`` from ``hash_fields``, and saves — keeping md5 consistent with content. - """ - oid = self._convert_object_id(id) - doc = await self.document_model.find_one(self._scope, self.document_model.id == oid) - if doc is None: - raise NotFoundError(self._not_found(id)) - update_data = update.model_dump(exclude_unset=True) - for field, value in update_data.items(): - setattr(doc, field, value) - doc.md5 = doc.compute_md5() - await doc.save() - return doc diff --git a/mpcontribs-api/src/mpcontribs_api/domains/_shared/filters.py b/mpcontribs-api/src/mpcontribs_api/domains/_shared/filters.py index caa91f296..c2d4e7f8c 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/_shared/filters.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/_shared/filters.py @@ -2,6 +2,11 @@ from typing import Any from fastapi_filter.contrib.beanie import Filter +from fastapi_filter.contrib.beanie.filter import _odm_operator_transformer +from pydantic import ValidationInfo, field_validator + +# Register a custom __contains filter suffix to search where lists are a superset of a provided list +_odm_operator_transformer.setdefault("contains", lambda value: {"$all": value}) from mpcontribs_api.domains._shared.types import nfc_normalize @@ -24,6 +29,20 @@ def _normalize_query_values(value: Any) -> Any: class BaseFilter(Filter): """Base filter that bridges Beanie's ``_id`` alias and fastapi-filter's raw field names.""" + @field_validator("*", mode="before") + @classmethod + def _split_contains(cls, value: str | None, field: ValidationInfo) -> list[str] | str | None: + """Split a comma-separated ``__contains`` query string into a list. + + ``FilterDepends`` collapses list-typed filter fields to a single string query param and + relies on a before-validator to re-expand it. fastapi-filter only does this for ``__in`` + and ``__nin``; mirror it here for the ``contains`` operator so ``?tags__contains=a,c`` + parses into ``["a", "c"]``. + """ + if field.field_name is not None and field.field_name.endswith("__contains") and isinstance(value, str): + return value.split(",") if value else [] + return value + def _get_filter_conditions(self, nesting_depth: int = 1) -> list[tuple[Mapping[str, Any], Mapping[str, Any]]]: """Overrides Filter._get_filter_conditions to allow us to specify 'id' instead of '_id' in our models. diff --git a/mpcontribs-api/src/mpcontribs_api/domains/_shared/models.py b/mpcontribs-api/src/mpcontribs_api/domains/_shared/models.py index 236eb06c3..2b4b840e7 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/_shared/models.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/_shared/models.py @@ -5,7 +5,7 @@ from typing import Annotated, Any, ClassVar, Self from beanie import Document, PydanticObjectId -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, ConfigDict, Field, model_validator from pymongo.results import DeleteResult from mpcontribs_api import pagination @@ -24,9 +24,34 @@ class BaseDocumentWithInput[TId](Document): models subclass their document, so they can't be bound as a class type parameter). """ + HAS_DERIVED_FIELDS: ClassVar[bool] = False # 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. + """ + return frozenset({"id"}) + + 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 derived_field_updates(self) -> dict[str, Any]: + """Server-derived fields to persist alongside a patch. + + Called by the repository on a copy of this document that already has the patch applied + in memory; the returned mapping is merged into the same ``$set`` write. Default: none. + """ + return {} + @classmethod def from_input_model(cls, data: Any) -> Self: """Translate a validated input payload into a full stored document.""" @@ -43,8 +68,10 @@ class DocumentOut[TId](SparseFieldsModel): Mirrors :class:`BaseDocumentWithInput`: subclasses bind their id type as ``TId`` so each resource owns its id type, while the field (optional, since projections may omit it) and its alias wiring - are declared once here for the repository to read off any resource's output model. - """ + are declared once here for the repository to read off any resource's output model.""" + + # lets POST/PUT responses correctly bring ``_id`` into ``id``, without it ``id`` ends up as None + model_config = ConfigDict(populate_by_name=True) id: Annotated[TId | None, Field(alias="_id", serialization_alias="id")] = None @@ -96,12 +123,22 @@ class Component(BaseDocumentWithInput[PydanticObjectId]): never define a component's content identity. """ + HAS_DERIVED_FIELDS: ClassVar[bool] = True 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()} + # The md5 functions look redundant but aren't, we should keep both # Used in patching to compute the hash after an update - should not return self def compute_md5(self) -> str: diff --git a/mpcontribs-api/src/mpcontribs_api/domains/_shared/repository.py b/mpcontribs-api/src/mpcontribs_api/domains/_shared/repository.py index 66c940d3b..baacda8e2 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/_shared/repository.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/_shared/repository.py @@ -14,6 +14,7 @@ from fastapi_filter.contrib.beanie import Filter from pydantic import BaseModel from pymongo.asynchronous.client_session import AsyncClientSession +from pymongo.errors import DuplicateKeyError from types_aiobotocore_s3 import S3Client from mpcontribs_api.authz import User @@ -55,6 +56,7 @@ def __init__(self, user: User) -> None: Args: user (User): the current user requesting resources """ + self._user = user self._scope = self._build_scope(user) @staticmethod @@ -70,9 +72,17 @@ def _convert_object_id(self, id: str) -> PydanticObjectId: except InvalidId: raise ValidationError("Incorrect Id format. Must be MongoDB ObjectId format.", id=id) from None - def _not_found(self, id: str) -> str: - """Build a not-found message naming this repository's resource.""" - return f"{self.document_model.__name__} with id {id} not found" + def coerce_identifiers(self, identifiers: dict[str, Any]) -> dict[str, Any]: + """Return ``identifiers`` with a string ``id`` coerced to the model's primary-key type. + + Raises: + ValidationError: if ``id`` is a string that is not a valid ObjectId, for an + ObjectId-keyed model + """ + id = identifiers.get("id") + if isinstance(id, str) and self.document_model.model_fields["id"].annotation is PydanticObjectId: + return {**identifiers, "id": self._convert_object_id(id)} + return identifiers async def get_many( self, @@ -105,18 +115,42 @@ async def get_many( next_cursor = encode_cursor(str(items[-1].id)) if has_more and items else None return Page(items=items, next_cursor=next_cursor) - async def get_by_id(self, id: Any, fields: frozenset[str] | None = None) -> TDoc | TOut | None: - """Return a single scoped document by id, projected to the requested fields. + 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 + ``BaseFilter._get_filter_conditions``) since a raw dict query does not go through Beanie's + alias resolution. Args: - id (str): the id of the document to find + identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields``, + or ``{"id": }`` + """ + expected = self.document_model.identifier_fields() + if identifiers.keys() != expected and identifiers.keys() != {"id"}: + raise ValidationError( + "identifiers must match the model's identifier fields, or be a bare {'id': ...}", + expected=sorted(expected), + received=sorted(identifiers.keys()), + ) + return {("_id" if key == "id" else key): value for key, value in identifiers.items()} + + async def get_one( + self, + identifiers: dict[str, Any], + fields: frozenset[str] | None = None, + ) -> TOut | None: + """Return the single scoped document matching ``identifiers``, projected to ``fields``. + + Args: + identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields`` fields (frozenset[str] | None): fields to project; if None the full document is returned """ - return await self.document_model.find_one( - self._scope, - self.document_model.id == id, - projection_model=self.out_model.projection(fields), - ) + query = self._identifier_query(identifiers) + projection = self.out_model.projection(fields) + return await self.document_model.find_one(self._scope, query, projection_model=projection) # pyright: ignore[reportArgumentType] async def list_ids(self, filter: TFilter, session: AsyncClientSession | None = None) -> list[Any]: """Return just the ids of scoped documents matching ``filter``. @@ -134,31 +168,54 @@ async def list_ids(self, filter: TFilter, session: AsyncClientSession | None = N return [doc.id for doc in docs] async def insert_one(self, in_resource: TIn) -> TDoc: - """Insert a new document built from its input model, rejecting duplicate ids. + """Insert a new document built from its input model, rejecting an existing duplicate. + + Duplicates are determined by model-declared identifiers that uniquely identify a document. Args: in_resource (TIn): the validated input payload to translate and store """ document = self.document_model.from_input_model(in_resource) - existing = await self.document_model.find_one(self.document_model.id == document.id) - if existing: - raise ConflictError(f"Cannot insert document.\n Document with ID {document.id} exists") - await document.insert() + try: + await document.insert() + except DuplicateKeyError as exc: + raise ConflictError( + f"Cannot insert {self.document_model.__name__}: a conflicting document already exists", + identifiers=document.identifiers(), + ) from exc return document - async def delete_by_id(self, id: Any, session: AsyncClientSession | None = None) -> DeleteResponse: - """Delete a single scoped document by id. + async def delete(self, filter: TFilter, session: AsyncClientSession | None = None) -> DeleteResponse: + """Delete every scoped document matching an arbitrary ``filter``. + + This is the bulk path (e.g. "delete every ProjectGroup with owner == X"). It does not raise + on an empty match — a zero count is a valid, unambiguous outcome for a filter delete. Scoping + ensures callers cannot delete documents they are not permitted to see. + + Args: + filter (TFilter): the fastapi-filter query to apply on top of the user scope + session (AsyncClientSession | None): optional client session for transactions + """ + query = filter.filter(self.document_model.find(self._scope, session=session)) + result = await query.delete_many(session=session) + if result is None: + raise ValidationError("DeleteResult not returned internally") + return DeleteResponse.from_delete_result(result) - Scoping ensures callers cannot delete documents they are not permitted to see. + async def delete_one( + self, identifiers: dict[str, Any], session: AsyncClientSession | None = None + ) -> DeleteResponse: + """Delete the single scoped document matching ``identifiers``. Args: - id (str): the id of the document to delete + identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields`` + session (AsyncClientSession | None): optional client session for transactions """ - doc = await self.document_model.find_one(self._scope, self.document_model.id == id, session=session) - if not doc: - raise NotFoundError("Document with id not found", id=id) - await doc.delete(session=session) - return DeleteResponse(num_deleted=1) + query = self._identifier_query(identifiers) + result = await self.document_model.find_one(self._scope, query, session=session).delete(session=session) # pyright: ignore[reportArgumentType] + if result is None or result.deleted_count == 0: + raise NotFoundError(f"{self.document_model.__name__} not found", identifiers=identifiers) + return DeleteResponse.from_delete_result(result) async def delete_by_ids(self, ids: list[Any], session: AsyncClientSession | None = None) -> DeleteResponse: """Delete multiple scoped documents by id. @@ -179,38 +236,84 @@ async def delete_by_ids(self, ids: list[Any], session: AsyncClientSession | None raise ValidationError("DeleteResult not returned internally") return DeleteResponse.from_delete_result(delete_result) - async def patch(self, id: Any, update: TPatch) -> TDoc: - """Partially update a single scoped document by id. + def _patch_update_fields(self, update: TPatch) -> dict[str, Any]: + """Map a patch model to the MongoDB ``$set`` field dict. - Only fields explicitly set on ``update`` are applied. An empty patch is a no-op that still - returns the existing document for consistent behavior. Scoping ensures callers cannot patch - documents they are not permitted to see. + Defaults to the patch's set fields (``exclude_unset``), which replaces each named field + wholesale. Subclasses whose patch targets a nested sub-document override this to emit dotted + ``parent.child`` keys so only the named leaves change and their siblings are left intact. + """ + return update.model_dump(exclude_unset=True) - Args: - id (str): the id of the document to update - update (TPatch): the partial update to apply; unset fields are dropped + async def _patch_matching( + self, + match: Any, + update: TPatch, + not_found: NotFoundError, + session: AsyncClientSession | None = None, + extra_set: dict[str, Any] | None = None, + ) -> TDoc: + """Apply a partial update to the single scoped document matching ``match``. + + ``match`` is any beanie filter that keys at most one in-scope document. An empty patch + is a no-op that still returns the existing document; a missing target raises ``not_found``. + + ``extra_set`` carries server-resolved fields that the patch model cannot express — e.g. a + slug that a service has already resolved to a ``DBRef`` link. Its keys are merged into the + ``$set`` after the patch dump, so a non-empty ``extra_set`` also makes the update non-empty. """ # Only retain set fields (patch) - update_data = update.model_dump(exclude_unset=True) + update_data = self._patch_update_fields(update) + if extra_set: + update_data |= extra_set + existing = await self.document_model.find_one(self._scope, match, session=session) # If update is empty, return the model anyways (consistent behavior) if not update_data: - existing = await self.document_model.find_one(self._scope, self.document_model.id == id) if existing is None: - raise NotFoundError(self._not_found(id)) + raise not_found return existing + # Server-derived fields depend on the resulting document, which a bare $set never revalidates. + # Load, apply the patch in memory, and fold the recomputed values in. + if self.document_model.HAS_DERIVED_FIELDS: + if existing is None: + raise not_found + for field, value in update_data.items(): + setattr(existing, field, value) + update_data |= existing.derived_field_updates() + # Otherwise, update the fields fully (set) # Brendan TODO: Set will replace an entire field # - if we want to append to a list (ie. add a reference) we ned Push/AddToSet - query = self.document_model.find_one(self._scope, self.document_model.id == id).update( + query = self.document_model.find_one(self._scope, match, session=session).update( Set(update_data), response_type=UpdateResponse.NEW_DOCUMENT, ) updated = await query # pyright: ignore[reportGeneralTypeIssues] # beanie UpdateQuery is awaitable, but pyright doesn't see it if updated is None: - raise NotFoundError(self._not_found(id)) + raise not_found return updated + async def patch_one( + self, + identifiers: dict[str, Any], + update: TPatch, + session: AsyncClientSession | None = None, + extra_set: dict[str, Any] | None = None, + ) -> TDoc: + """Partially update the single scoped document matching ``identifiers``. + + Args: + identifiers (dict[str, Any]): identifier field values keyed by ``identifier_fields`` + 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`` + alongside the patch — for values the patch model cannot carry (e.g. a resolved link) + """ + query = self._identifier_query(identifiers) + not_found = NotFoundError(f"{self.document_model.__name__} not found", identifiers=identifiers) + return await self._patch_matching(query, update, not_found, session=session, extra_set=extra_set) + def _hash_payload(self, payload: dict[str, Any], *, separators: tuple[str, str] = (",", ":")) -> str: canonical = json.dumps( payload, diff --git a/mpcontribs-api/src/mpcontribs_api/domains/_shared/service.py b/mpcontribs-api/src/mpcontribs_api/domains/_shared/service.py index a8885f243..5f9f77e64 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/_shared/service.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/_shared/service.py @@ -1,6 +1,8 @@ from collections.abc import AsyncIterable from contextlib import AbstractAsyncContextManager +from typing import Any +from beanie import PydanticObjectId from fastapi_filter.contrib.beanie import Filter from pydantic import BaseModel from pymongo.asynchronous.client_session import AsyncClientSession @@ -70,16 +72,24 @@ async def get_many( pagination=pagination, filter=filter, fields=fields, restrict_ids=allowed ) - async def get_by_id(self, id: str, fields: frozenset[str] | None) -> TDoc | TOut | None: - """Find a single component by id, gated by contribution reachability. + async def _resolve_component_id(self, identifiers: dict[str, Any]) -> PydanticObjectId | None: + """Return the component ``_id`` after finding it via identifiers, or None if absent.""" + if "id" in identifiers: + return identifiers["id"] + existing = await self._components.get_one(identifiers, frozenset({"id"})) + return existing.id if existing is not None else None - Returns ``None`` (treated as not found) when no in-scope contribution references the id, - so callers cannot read a component belonging to a contribution they cannot see. + async def get_one(self, identifiers: dict[str, Any], fields: frozenset[str] | None) -> TDoc | TOut | None: + """Find a single component matching ``identifiers``, gated by contribution reachability. + + Returns ``None`` when no in-scope contribution references the component. + Accepts either the bare ``{"id": ...}`` form or the content-hash ``{"md5": ...}`` form. """ - oid = self._components._convert_object_id(id) - if not await self._contributions.referenced_component_ids(self._ref_field, [oid], scoped=True): + identifiers = self._components.coerce_identifiers(identifiers) + oid = await self._resolve_component_id(identifiers) + if oid is None or not await self._contributions.referenced_component_ids(self._ref_field, [oid], scoped=True): return None - return await self._components.get_component_by_id(id, fields) + return await self._components.get_one(identifiers, fields) async def insert( self, @@ -89,16 +99,19 @@ async def insert( """Bulk-insert components, deduplicated by content hash. See ``insert_components``.""" return await self._components.insert_components(components=components, session=session) - async def patch_by_id(self, id: str, update: TPatch) -> TDoc: - """Partially update a component by id, gated by contribution reachability. + async def patch_one(self, identifiers: dict[str, Any], update: TPatch) -> TDoc: + """Partially update a component matching ``identifiers``, gated by contribution reachability. + + Accepts either the bare ``{"id": ...}`` form or the content-hash ``{"md5": ...}`` form. Raises: - NotFoundError: when no in-scope contribution references the id + NotFoundError: when no in-scope contribution references the component """ - oid = self._components._convert_object_id(id) - if not await self._contributions.referenced_component_ids(self._ref_field, [oid], scoped=True): - raise NotFoundError(self._components._not_found(id)) - return await self._components.patch_component_by_id(id=id, update=update) + identifiers = self._components.coerce_identifiers(identifiers) + oid = await self._resolve_component_id(identifiers) + if oid is None or not await self._contributions.referenced_component_ids(self._ref_field, [oid], scoped=True): + raise NotFoundError(f"{self._components.document_model.__name__} not found", **identifiers) + return await self._components.patch_one(identifiers, update) async def download( self, @@ -146,11 +159,13 @@ async def delete(self, filter: TFilter) -> ComponentDeleteResponse: referenced_ids=sorted(referenced), ) - async def delete_by_id(self, id: str) -> ComponentDeleteResponse: - """Delete a single component by id, subject to the access and integrity gates. + async def delete_one(self, identifiers: dict[str, Any]) -> ComponentDeleteResponse: + """Delete a single component matching ``identifiers``, subject to the access and integrity gates. + + Accepts either the bare ``{"id": ...}`` form or the content-hash ``{"md5": ...}`` form. Args: - id (str): the str representation of the component's ObjectId + identifiers (dict[str, Any]): identifier field values, ``{"id": ...}`` or ``{"md5": ...}`` Returns: ComponentDeleteResponse: the deletion result, or a skipped result if still referenced @@ -158,10 +173,11 @@ async def delete_by_id(self, id: str) -> ComponentDeleteResponse: Raises: NotFoundError: if the component is not reachable via any in-scope contribution """ - oid = self._components._convert_object_id(id) - if not await self._contributions.referenced_component_ids(self._ref_field, [oid], scoped=True): - raise NotFoundError(self._components._not_found(id)) + identifiers = self._components.coerce_identifiers(identifiers) + oid = await self._resolve_component_id(identifiers) + if oid is None or not await self._contributions.referenced_component_ids(self._ref_field, [oid], scoped=True): + raise NotFoundError(f"{self._components.document_model.__name__} not found", **identifiers) if await self._contributions.referenced_component_ids(self._ref_field, [oid], scoped=False): return ComponentDeleteResponse(num_deleted=0, num_skipped=1, referenced_ids=[oid]) - deleted = await self._components.delete_by_id(oid) + deleted = await self._components.delete_one({"id": oid}) return ComponentDeleteResponse(num_deleted=deleted.num_deleted) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/_shared/types.py b/mpcontribs-api/src/mpcontribs_api/domains/_shared/types.py index f6b77f22b..b7ace7d38 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/_shared/types.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/_shared/types.py @@ -350,6 +350,23 @@ def to_snake_case(name: str) -> str: # Converts strs to pretty display form (keeps unicode and most formatting) DisplayStr = Annotated[str, BeforeValidator(func=nfc_normalize)] +# A URL-safe, human-readable slug +# carried in user.groups like ``initiative:`` +_SLUG_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") + + +def _validate_slug(v: str) -> str: + v = v.strip().lower() + if not _SLUG_RE.match(v): + raise ValidationError( + "slug must be lowercase alphanumeric words separated by single hyphens, e.g. 'battery-genome-2025'", + slug=v, + ) + return v + + +Slug = Annotated[str, Field(min_length=3, max_length=50), BeforeValidator(_validate_slug)] + def coerce_key(key: Any, *, require_ascii: bool = False, reserved: frozenset[str] | None = None) -> str: """Coerce one dict key to canonical ``snake_case``, enforcing the shared write-path key guards. diff --git a/mpcontribs-api/src/mpcontribs_api/domains/attachments/router.py b/mpcontribs-api/src/mpcontribs_api/domains/attachments/router.py index a6b6d1a81..2dda882c1 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/attachments/router.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/attachments/router.py @@ -13,7 +13,7 @@ download_filename, ) from mpcontribs_api.domains.attachments.dependencies import AttachmentServiceDep -from mpcontribs_api.domains.attachments.models import AttachmentFilter, AttachmentOut +from mpcontribs_api.domains.attachments.models import AttachmentFilter, AttachmentOut, AttachmentPatch from mpcontribs_api.pagination import CursorParams router = APIRouter() @@ -30,14 +30,15 @@ async def get_attachments( return await service.get_many(filter=filter, fields=selected, pagination=pagination) -@router.get("/{pk}") -async def get_attachment( +@router.get("/{id}") +async def get_one( service: AttachmentServiceDep, - pk: str, + id: str, fields: FieldSelector = None, ): + """Return a single attachment addressed by its ``_id``.""" selected = AttachmentOut.parse_fields(fields) - return await service.get_by_id(id=pk, fields=selected) + return await service.get_one(identifiers={"id": id}, fields=selected) @router.get("/download/{short_mime}") @@ -73,5 +74,16 @@ async def delete_attachments(service: AttachmentServiceDep, filter: AttachmentFi @router.delete("/{id}", response_model=ComponentDeleteResponse, dependencies=[Depends(require_user)]) -async def delete_attachment_by_id(service: AttachmentServiceDep, id: str): - return await service.delete_by_id(id=id) +async def delete_one(service: AttachmentServiceDep, id: str): + """Delete a single attachment addressed by its ``_id``.""" + return await service.delete_one(identifiers={"id": id}) + + +@router.patch("/{id}", dependencies=[Depends(require_user)]) +async def patch_one( + service: AttachmentServiceDep, + id: str, + update: AttachmentPatch, +): + """Patch a single attachment addressed by its ``_id`` or its content ``md5``.""" + return await service.patch_one(identifiers={"id": id}, update=update) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/consumers/dependencies.py b/mpcontribs-api/src/mpcontribs_api/domains/consumers/dependencies.py index ce53f7651..3e69d7bbb 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/consumers/dependencies.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/consumers/dependencies.py @@ -24,8 +24,8 @@ async def get_effective_limits(user: UserDep) -> ConsumerSettings: if user.consumer_id is None: return ConsumerSettings() - override = await MongoDbConsumerRepository(user).get_by_consumer_id(user.consumer_id) - return override.settings if override is not None else ConsumerSettings() + override = await MongoDbConsumerRepository(user).get_one({"consumer_id": user.consumer_id}) + return override.settings if override and override.settings else ConsumerSettings() ConsumerLimitsDep = Annotated[ConsumerSettings, Depends(get_effective_limits)] diff --git a/mpcontribs-api/src/mpcontribs_api/domains/consumers/models.py b/mpcontribs-api/src/mpcontribs_api/domains/consumers/models.py index a4f8a543c..6e6d218ae 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/consumers/models.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/consumers/models.py @@ -52,6 +52,11 @@ class Consumer(BaseDocumentWithInput[PydanticObjectId]): consumer_id: str settings: ConsumerSettings = Field(default_factory=ConsumerSettings) + @classmethod + def identifier_fields(cls) -> frozenset[str]: + """A consumer override is keyed by Kong's ``consumer_id`` (its unique natural key).""" + return frozenset({"consumer_id"}) + @classmethod def with_defaults(cls, consumer_id: str = "") -> Consumer: """In-memory Consumer whose ``settings`` carry the env-backed default limits. diff --git a/mpcontribs-api/src/mpcontribs_api/domains/consumers/repository.py b/mpcontribs-api/src/mpcontribs_api/domains/consumers/repository.py index 08463950c..a1db8e242 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/consumers/repository.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/consumers/repository.py @@ -1,8 +1,5 @@ from typing import Any -from beanie import UpdateResponse -from beanie.operators import Set - from mpcontribs_api.authz import User from mpcontribs_api.domains._shared.repository import MongoDbRepository from mpcontribs_api.domains.consumers.models import ( @@ -12,8 +9,6 @@ ConsumerOut, ConsumerPatch, ) -from mpcontribs_api.exceptions import ConflictError, NotFoundError -from mpcontribs_api.pagination import CursorParams class MongoDbConsumerRepository(MongoDbRepository[Consumer, ConsumerIn, ConsumerOut, ConsumerFilter, ConsumerPatch]): @@ -21,7 +16,9 @@ class MongoDbConsumerRepository(MongoDbRepository[Consumer, ConsumerIn, Consumer Consumer overrides are an admin-only resource: every route that reaches this repository is gated by ``require_admin``, so no per-user read scope is needed and ``_build_scope`` returns an - empty filter (admins see all overrides). + empty filter (admins see all overrides). Reads and deletes use the base repository directly + (keyed on ``consumer_id`` via ``Consumer.identifier_fields``); only the nested-``settings`` patch + shape is resource-specific. """ document_model = Consumer @@ -32,62 +29,12 @@ def _build_scope(user: User) -> dict[str, Any]: # Admin-only resource (routes enforce ``require_admin``); no visibility filter required. return {} - async def get_consumers( - self, - filter: ConsumerFilter, - pagination: CursorParams, - fields: frozenset[str] | None, - ): - """List consumer overrides. See ``get_many``.""" - return await self.get_many(pagination=pagination, filter=filter, fields=fields) - - async def get_consumer_by_id(self, id: str, fields: frozenset[str] | None): - """Find a single consumer override by its document id. See ``get_by_id``.""" - return await self.get_by_id(self._convert_object_id(id), fields) - - async def get_by_consumer_id(self, consumer_id: str) -> Consumer | None: - """Return the override document for a Kong ``consumer_id``, or ``None`` if none exists.""" - return await Consumer.find_one(Consumer.consumer_id == consumer_id) + def _patch_update_fields(self, update: ConsumerPatch) -> dict[str, Any]: + """Flatten the patch to dotted ``settings.`` keys. - async def insert_consumer(self, consumer: ConsumerIn) -> Consumer: - """Insert a new override, rejecting a duplicate ``consumer_id`` with a clean 409. - - The unique index on ``consumer_id`` is the hard guarantee; this pre-check turns the common - case into a readable conflict instead of a raw driver error. + The limits live under a nested ``settings`` sub-document; dotting the update makes a partial + patch change only the named limits and leave the siblings intact (a plain ``$set`` of + ``settings`` would replace the whole sub-document). """ - existing = await self.get_by_consumer_id(consumer.consumer_id) - if existing is not None: - raise ConflictError( - "An override for this consumer already exists", - consumer_id=consumer.consumer_id, - ) - return await self.insert_one(consumer) - - async def patch_consumer_by_id(self, id: str, update: ConsumerPatch) -> Consumer: - """Partially update an override's limits by document id. - - The limits live under a nested ``settings`` sub-document; the update is flattened to dotted - ``settings.`` keys so a partial patch changes only the named limits and leaves the - siblings intact (a plain ``$set`` of ``settings`` would replace the whole sub-document). - """ - object_id = self._convert_object_id(id) overrides = update.settings.model_dump(exclude_unset=True) if update.settings else {} - dotted = {f"settings.{field}": value for field, value in overrides.items()} - if not dotted: - # Empty patch is a no-op that still returns the existing document (consistent behavior). - existing = await Consumer.find_one(Consumer.id == object_id) - if existing is None: - raise NotFoundError(self._not_found(id)) - return existing - - updated = await Consumer.find_one(Consumer.id == object_id).update( - Set(dotted), - response_type=UpdateResponse.NEW_DOCUMENT, - ) # pyright: ignore[reportGeneralTypeIssues] # beanie UpdateQuery is awaitable, but pyright doesn't see it - if updated is None: - raise NotFoundError(self._not_found(id)) - return updated - - async def delete_consumer_by_id(self, id: str) -> None: - """Delete an override by document id. See ``delete_by_id``.""" - await self.delete_by_id(self._convert_object_id(id)) + return {f"settings.{field}": value for field, value in overrides.items()} diff --git a/mpcontribs-api/src/mpcontribs_api/domains/consumers/router.py b/mpcontribs-api/src/mpcontribs_api/domains/consumers/router.py index af19d78df..b515422e1 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/consumers/router.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/consumers/router.py @@ -31,7 +31,7 @@ async def get_consumers( if fields is None: fields = list(ConsumerOut.default_fields()) selected = ConsumerOut.parse_fields(fields) - return await repo.get_consumers(filter=filter, pagination=pagination, fields=selected) + return await repo.get_many(filter=filter, pagination=pagination, fields=selected) @router.get("/{id}") @@ -44,7 +44,7 @@ async def get_consumer_by_id( if fields is None: fields = list(ConsumerOut.default_fields()) selected = ConsumerOut.parse_fields(fields) - return await repo.get_consumer_by_id(id=id, fields=selected) + return await repo.get_one(repo.coerce_identifiers({"id": id}), selected) @router.post("", response_model=ConsumerOut, status_code=status.HTTP_201_CREATED) @@ -53,7 +53,7 @@ async def create_consumer( consumer: ConsumerIn, ): """Create a new consumer override, rejecting a duplicate ``consumer_id`` with 409 (admin only).""" - return await repo.insert_consumer(consumer) + return await repo.insert_one(consumer) @router.patch("/{id}", response_model=ConsumerOut) @@ -63,7 +63,7 @@ async def patch_consumer_by_id( update: ConsumerPatch, ): """Partially update a consumer override by document id (admin only).""" - return await repo.patch_consumer_by_id(id=id, update=update) + return await repo.patch_one(repo.coerce_identifiers({"id": id}), update) @router.delete("/{id}", status_code=status.HTTP_204_NO_CONTENT) @@ -72,5 +72,5 @@ async def delete_consumer_by_id( id: str, ): """Delete a consumer override by document id (admin only).""" - await repo.delete_consumer_by_id(id=id) + await repo.delete_one(repo.coerce_identifiers({"id": id})) return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/contributions/models.py b/mpcontribs-api/src/mpcontribs_api/domains/contributions/models.py index 3a3d15fea..b557693e9 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/contributions/models.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/contributions/models.py @@ -137,6 +137,11 @@ class Settings: IndexModel(keys=[("attachments.$id", ASCENDING)], name="ref_attachments"), ] + @classmethod + def identifier_fields(cls) -> frozenset[str]: + """A contribution's natural key is its full :class:`ContributionIdentity` composite.""" + return frozenset({"project", "material_id", "chemical_system_id", "formula", "unique_value", "condition_key"}) + class Contribution(ContributionBase, BaseDocumentWithInput[PydanticObjectId]): """Models what is actually stored in the database.""" diff --git a/mpcontribs-api/src/mpcontribs_api/domains/contributions/repository.py b/mpcontribs-api/src/mpcontribs_api/domains/contributions/repository.py index 787a7007c..c09818149 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/contributions/repository.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/contributions/repository.py @@ -101,55 +101,50 @@ async def get_contributions( """Query the Contribution collection, scoped to the current user. See ``get_many``.""" return await self.get_many(pagination=pagination, filter=filter, fields=fields) - async def get_contribution_by_id(self, id: str, fields: frozenset[str] | None): - """Find a single contribution by id, scoped to the current user. See ``get_by_id``.""" - return await self.get_by_id(self._convert_object_id(id), fields) - - async def patch_contribution_by_id( + async def patch_one( # pyright: ignore[reportIncompatibleMethodOverride] self, - id: str, + identifiers: dict[str, Any], update: ContributionPatch, unique_value: Scalar | None = _UNSET, *, replace_data: bool = False, existing_data: Any = None, - ): - """Partially update a contribution by id, scoped to the current user. + session: AsyncClientSession | None = None, + ) -> Contribution: + """Partially update the single scoped contribution matching ``identifiers``. ``unique_value`` is server-recomputed by the service when the patch changes ``data`` or - ``project`` (the inputs to identity); left as ``_UNSET`` it is not touched. When set, it is - folded into the ``$set`` so the identity index stays consistent with the patched ``data``. + ``project`` (the inputs to identity); left as ``_UNSET`` it delegates to the base partial + update. When set, it is folded into the ``$set`` so the identity index stays consistent with + the patched ``data``. ``data`` additively merges into the stored dict by default (unmentioned leaves survive, and a bare scalar routes onto a stored quantity leaf's ``value``); the merge is resolved against the caller-supplied ``existing_data``. Pass ``replace_data`` to overwrite the whole ``data`` dict instead. See ``_build_update_set``. """ + match = self._identifier_query(identifiers) + not_found = NotFoundError(f"{self.document_model.__name__} not found", identifiers=identifiers) try: if unique_value is _UNSET: - return await self.patch(self._convert_object_id(id), update) + return await self._patch_matching(match, update, not_found, session=session) update_data = _build_update_set( update.model_dump(exclude_unset=True), existing_data, replace_data=replace_data ) update_data["unique_value"] = unique_value - query = self.document_model.find_one( - self._scope, - self.document_model.id == self._convert_object_id(id), - ).update(Set(update_data), response_type=UpdateResponse.NEW_DOCUMENT) + query = self.document_model.find_one(self._scope, match, session=session).update( + Set(update_data), response_type=UpdateResponse.NEW_DOCUMENT + ) updated = await query # pyright: ignore[reportGeneralTypeIssues] # beanie UpdateQuery is awaitable if updated is None: - raise NotFoundError(self._not_found(id)) + raise not_found return updated except DuplicateKeyError as err: raise ConflictError( - f"contribution '{id}' cannot be patched: the resulting identity already exists", - id=id, + "contribution cannot be patched: the resulting identity already exists", + identifiers=identifiers, ) from err - async def delete_contribution_by_id(self, id: str) -> None: - """Delete a contribution by id, scoped to the current user. See ``delete_by_id``.""" - await self.delete_by_id(self._convert_object_id(id)) - async def delete_contributions( self, filter: ContributionFilter, @@ -333,10 +328,11 @@ async def aggregate_project_stats(self, project_id: str) -> ProjectAggregate: agg.columns = finalize_columns(acc) return agg - async def upsert_contribution_by_identifiers( + async def upsert_one( self, identifiers: dict[str, Any], contribution: ContributionIn, + session: AsyncClientSession | None = None, ) -> Contribution: """Atomically upsert a Contribution by its full identity. diff --git a/mpcontribs-api/src/mpcontribs_api/domains/contributions/router.py b/mpcontribs-api/src/mpcontribs_api/domains/contributions/router.py index 662cbc552..7ac123ea3 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/contributions/router.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/contributions/router.py @@ -138,38 +138,33 @@ async def download_contributions( @router.delete("/{id}", dependencies=[Depends(require_user)]) -async def delete_contribution_by_id( +async def delete_one( service: ContributionServiceDep, id: str, ): - return await service.delete_contributions(ContributionFilter.model_validate({"id": id})) + return await service.delete_one({"id": id}) @router.get("/{id}") -async def get_contribution_by_id( - repo: ContributionDep, +async def get_one( + service: ContributionServiceDep, id: str, fields: FieldSelector = None, ): selected = ContributionOut.parse_fields(fields) - return await repo.get_contribution_by_id(id=id, fields=selected) + return await service.get_one({"id": id}, fields=selected) @router.put("/{id}", dependencies=[Depends(require_user)]) -async def upsert_contribution_by_id(service: ContributionServiceDep, id: str, contribution: ContributionIn): - return await service.upsert_contribution_by_id(id=id, contribution=contribution) +async def upsert_one(service: ContributionServiceDep, id: str, contribution: ContributionIn): + # The by-id upsert resolves the server-owned ``unique_value`` and enforces the unapproved quota + # (see ``ContributionService.upsert_contribution_by_id``), which the generic identity upsert does not. + return await service.upsert_contribution_by_id(id, contribution) @router.patch("/{id}", dependencies=[Depends(require_user)]) -async def patch_contribution_by_id( - service: ContributionServiceDep, - id: str, - update: ContributionPatch, - replace_data: bool = False, -): - """Patch one contribution by id. - - ``data`` deep-merges into the stored ``data`` by default (unmentioned leaves survive); pass - ``?replace_data=true`` to overwrite the whole ``data`` dict instead. - """ - return await service.patch_contribution_by_id(id=id, update=update, replace_data=replace_data) +async def patch_one(service: ContributionServiceDep, id: str, update: ContributionPatch, replace_data: bool = False): + # The by-id patch re-resolves ``unique_value`` and validates the identifier hierarchy against the + # merged state (see ``ContributionService.patch_contribution_by_id``); ``?replace_data=true`` + # overwrites the whole ``data`` dict instead of deep-merging. + return await service.patch_contribution_by_id(id, update=update, replace_data=replace_data) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/contributions/service.py b/mpcontribs-api/src/mpcontribs_api/domains/contributions/service.py index e0239a0cf..f68773a7f 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/contributions/service.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/contributions/service.py @@ -2,7 +2,7 @@ from collections import defaultdict from collections.abc import Iterable from dataclasses import dataclass -from typing import cast +from typing import Any, cast import structlog from beanie import Link, PydanticObjectId @@ -29,6 +29,7 @@ ContributionFilter, ContributionIdentity, ContributionIn, + ContributionOut, ContributionPatch, Scalar, extract_unique_value, @@ -100,6 +101,42 @@ def _children(self) -> dict[str, MongoDbRepository]: "tables": self._tables, } + async def get_one( + self, identifiers: dict[str, Any], fields: frozenset[str] | None + ) -> Contribution | ContributionOut | None: + """Return the single scoped contribution matching ``identifiers``. + + Accepts either the bare ``{"id": ...}`` form or the semantic + ``{"project", "identifier", "version"}`` set, resolved by the base ``_identifier_query``. + """ + return await self._contributions.get_one(self._contributions.coerce_identifiers(identifiers), fields) + + async def patch_one(self, identifiers: dict[str, Any], update: ContributionPatch) -> Contribution: + """Partially update the single scoped contribution matching ``identifiers``.""" + return await self._contributions.patch_one(self._contributions.coerce_identifiers(identifiers), update) + + async def upsert_one(self, identifiers: dict[str, Any], contribution: ContributionIn) -> Contribution: + """Upsert the single scoped contribution matching ``identifiers``. See repository ``upsert_one``.""" + return await self._contributions.upsert_one(self._contributions.coerce_identifiers(identifiers), contribution) + + async def delete_one(self, identifiers: dict[str, Any]) -> BulkDeleteSummary: + """Delete a single contribution and its child components, matching ``identifiers``. + + Accepts either the bare ``{"id": ...}`` form or the semantic + ``{"project", "identifier", "version"}`` set. Cascades component deletion via + :meth:`delete_contributions` so children are never orphaned; a missing target is a zero-count + result (mirroring the bulk delete path, which does not 404). + """ + identifiers = self._contributions.coerce_identifiers(identifiers) + if set(identifiers) == {"id"}: + filter = ContributionFilter(id=identifiers["id"]) + else: + existing = await self._contributions.get_one(identifiers, frozenset({"id"})) + if existing is None: + return BulkDeleteSummary(num_deleted=0, num_children_deleted=0) + filter = ContributionFilter(id=existing.id) + return await self.delete_contributions(filter) + async def _unapproved_stored_count(self, project_id: str) -> int | None: """Contributions already stored for an unapproved ``project_id``, else ``None``. @@ -107,7 +144,7 @@ async def _unapproved_stored_count(self, project_id: str) -> int | None: be read in the current scope (existence/permission is enforced on insert, not here). The caller turns the count into a remaining allowance against the cap. """ - project = await self._projects.get_by_id(project_id, fields=frozenset({"is_approved"})) + project = await self._projects.get_one({"id": project_id}, frozenset({"is_approved"})) if not project or project.is_approved: return None # Soft limit: this count feeds a non-atomic check-then-write, so concurrent writes to the @@ -639,7 +676,7 @@ async def _bounded_upsert(item: PreparedWrite) -> Contribution | BulkFailure: identifiers = contrib.identity_dict(item.unique_value, item.condition_key) async with sem: try: - return await self._contributions.upsert_contribution_by_identifiers(identifiers, contrib) + return await self._contributions.upsert_one(identifiers, contrib) except Exception as exc: logger.error("upsert_contribution_failed", index=item.index, identifier=identifiers, exc_info=True) return bulk_failure_from_exception(item.index, identifiers, exc) @@ -748,7 +785,7 @@ async def upsert_contribution_by_id(self, id: str, contribution: ContributionIn) """Upsert a single contribution by Mongo id, resolving its server-owned ``unique_value``.""" if not self._user.can_write(contribution.project): raise PermissionError(f"not authorized to write to project '{contribution.project}'") - existing = await self._contributions.get_contribution_by_id(id, fields=None) + existing = await self._contributions.get_one(self._contributions.coerce_identifiers({"id": id}), None) if existing is None: stored = await self._unapproved_stored_count(contribution.project) cap = self._limits.max_unapproved_contributions_per_project @@ -782,13 +819,13 @@ async def patch_contribution_by_id( touches_unique = "data" in set_fields or "project" in set_fields touches_identity = bool(ContributionIdentity.HIERARCHY_FIELDS & set_fields.keys()) if not touches_unique and not touches_identity: - return await self._contributions.patch_contribution_by_id(id, update) + return await self._contributions.patch_one(self._contributions.coerce_identifiers({"id": id}), update) if replace_data and set_fields.get("data") is not None: # A whole-dict overwrite must satisfy the strict insert-path rules (no leaf fragments). validate_contribution_data(set_fields["data"]) - existing = await self._contributions.get_contribution_by_id(id, fields=None) + existing = await self._contributions.get_one(self._contributions.coerce_identifiers({"id": id}), None) if existing is None or existing.project is None: raise NotFoundError(f"contribution '{id}' not found") @@ -800,7 +837,7 @@ async def patch_contribution_by_id( existing_formula=existing.formula, ) if not touches_unique: - return await self._contributions.patch_contribution_by_id(id, update) + return await self._contributions.patch_one(self._contributions.coerce_identifiers({"id": id}), update) project = set_fields.get("project") or existing.project # Resolve unique_value against the data the write will actually leave behind: the merged view @@ -812,8 +849,12 @@ async def patch_contribution_by_id( else: data = QuantityLeaf.merge_data(existing.data, set_fields["data"]) unique_value = await self._resolve_unique_value(project, data) - return await self._contributions.patch_contribution_by_id( - id, update, unique_value=unique_value, replace_data=replace_data, existing_data=existing.data + return await self._contributions.patch_one( + self._contributions.coerce_identifiers({"id": id}), + update, + unique_value=unique_value, + replace_data=replace_data, + existing_data=existing.data, ) @staticmethod diff --git a/mpcontribs-api/src/mpcontribs_api/domains/initiatives/__init__.py b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/mpcontribs-api/src/mpcontribs_api/domains/initiatives/dependencies.py b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/dependencies.py new file mode 100644 index 000000000..a81d8c8d5 --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/dependencies.py @@ -0,0 +1,13 @@ +from typing import Annotated + +from fastapi import Depends + +from mpcontribs_api.dependencies import UserDep +from mpcontribs_api.domains.initiatives.repository import InitiativeRepository + + +def get_initiative_repository(user: UserDep) -> InitiativeRepository: + return InitiativeRepository(user) + + +InitiativeDep = Annotated[InitiativeRepository, Depends(get_initiative_repository)] diff --git a/mpcontribs-api/src/mpcontribs_api/domains/initiatives/models.py b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/models.py new file mode 100644 index 000000000..3f5a3db92 --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/models.py @@ -0,0 +1,135 @@ +from typing import Self + +from beanie import PydanticObjectId +from bson.errors import InvalidId +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator +from pymongo import ASCENDING, IndexModel + +from mpcontribs_api.domains._shared.filters import BaseFilter +from mpcontribs_api.domains._shared.models import BaseDocumentWithInput, DocumentOut +from mpcontribs_api.domains._shared.types import NFKCStr, PrefixedEmail, Slug +from mpcontribs_api.exceptions import ValidationError +from mpcontribs_api.projection import SparseFieldsModel + + +class Initiative(BaseDocumentWithInput[PydanticObjectId]): + """A canonical, authoritative grouping of projects into a larger organizational effort. + + Unlike an ad-hoc ``ProjectGroup`` (many-to-many, user-curated), an initiative is the single + canonical parent of its member projects: a project points at *at most one* initiative via + ``Project.initiative``. Membership is therefore derived from the projects collection — an + initiative stores no project list of its own. + + Collaborator rights are drawn from the caller's roles, mirroring how projects use groups: a + user may manage an initiative (add projects, patch it) if they own it, are an admin, or carry + the ``initiative:`` role. + """ + + slug: Slug + name: NFKCStr = Field(max_length=100) + owner: PrefixedEmail + is_public: bool = False + is_approved: bool = False + + class Settings: + name = "initiatives" + keep_nulls = False + indexes = [ + IndexModel(keys=[("slug", ASCENDING)], name="slug", unique=True), + IndexModel( + keys=[("owner", ASCENDING), ("is_approved", ASCENDING), ("is_public", ASCENDING)], + name="owner_is_approved_is_public", + ), + ] + validate_on_save = True + + @classmethod + def from_input_model(cls, data: InitiativeIn, owner: PrefixedEmail) -> Self: # pyright: ignore[reportIncompatibleMethodOverride] + return cls(_id=PydanticObjectId(), **data.model_dump(), owner=owner) + + @classmethod + def identifier_fields(cls) -> frozenset[str]: + """An ``Initiative`` is uniquely identified by its globally-unique ``slug``.""" + return frozenset({"slug"}) + + @model_validator(mode="after") + def _public_requires_approved(self) -> Self: + """An initiative cannot be public until it has been approved.""" + if self.is_public and not self.is_approved: + raise ValidationError("an initiative cannot be public until it is approved", slug=self.slug) + return self + + +class InitiativeIn(BaseModel): + """User-supplied fields for creating an initiative. + + ``owner`` is forced to the caller and ``is_public`` / ``is_approved`` always start ``False`` + (an admin approves later), so none of them are part of the input contract. + """ + + model_config = ConfigDict(extra="forbid") + + slug: Slug + name: NFKCStr = Field(max_length=100) + + +class InitiativeOut(DocumentOut[PydanticObjectId]): + slug: Slug | None = None + name: NFKCStr | None = None + owner: PrefixedEmail | None = None + is_public: bool | None = None + is_approved: bool | None = None + + @staticmethod + def default_fields() -> tuple[str, ...]: + return ("slug", "name", "owner", "is_public", "is_approved") + + +class InitiativePatch(SparseFieldsModel): + """Partial update to an initiative. + + ``slug`` and ``owner`` are immutable and intentionally absent. ``is_approved`` is admin-only + (enforced in the repository), and the ``is_public`` ⇒ ``is_approved`` invariant is re-checked + there against the resulting state, since a partial ``$set`` bypasses the document validator. + """ + + name: NFKCStr | None = Field(default=None, max_length=100) + is_public: bool | None = None + is_approved: bool | None = None + + +class InitiativeFilter(BaseFilter): + id: PydanticObjectId | None = None + id__in: list[PydanticObjectId] | None = None + id__neq: PydanticObjectId | None = None + + slug: Slug | None = None + slug__in: list[Slug] | None = None + slug__neq: Slug | None = None + + name: NFKCStr | None = None + name__in: list[NFKCStr] | None = None + name__neq: NFKCStr | None = None + + owner: PrefixedEmail | None = None + owner__in: list[PrefixedEmail] | None = None + owner__neq: PrefixedEmail | None = None + + is_public: bool | None = None + is_approved: bool | None = None + + order_by: list[str] | None = None + + class Constants(BaseFilter.Constants): + model = Initiative + + @field_validator("id", mode="before") + @classmethod + def convert_str_to_oid(cls, v: str): + try: + return PydanticObjectId(v) + except InvalidId as err: + raise ValidationError( + "Invalid ObjectId format. Must be 12-byte input or a 24-character hex string", + oid=v, + ) from err diff --git a/mpcontribs-api/src/mpcontribs_api/domains/initiatives/repository.py b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/repository.py new file mode 100644 index 000000000..8c3d8933b --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/repository.py @@ -0,0 +1,132 @@ +from typing import Any + +from pymongo.asynchronous.client_session import AsyncClientSession +from pymongo.errors import DuplicateKeyError + +from mpcontribs_api.authz import User +from mpcontribs_api.config import get_settings +from mpcontribs_api.domains._shared.models import DeleteResponse +from mpcontribs_api.domains._shared.repository import MongoDbRepository +from mpcontribs_api.domains.initiatives.models import ( + Initiative, + InitiativeFilter, + InitiativeIn, + InitiativeOut, + InitiativePatch, +) +from mpcontribs_api.exceptions import ConflictError, NotFoundError, PermissionError, ValidationError +from mpcontribs_api.pagination import CursorParams, Page + + +class InitiativeRepository( + MongoDbRepository[Initiative, InitiativeIn, InitiativeOut, InitiativeFilter, InitiativePatch] +): + document_model = Initiative + out_model = InitiativeOut + + def __init__(self, user: User) -> None: + super().__init__(user) + self._limits = get_settings().domain.initiatives + + @staticmethod + def _build_scope(user: User) -> dict[str, Any]: + """Scope reads to what the caller may see: public+approved, owned, or collaborated-on.""" + if user.is_admin: + return {} + ors: list[dict[str, Any]] = [{"is_public": True, "is_approved": True}] + if not user.is_anonymous: + ors.append({"owner": user.username}) + slugs = user.initiative_roles + if slugs: + ors.append({"slug": {"$in": sorted(slugs)}}) + return {"$or": ors} + + async def get_initiatives( + self, + pagination: CursorParams, + filter: InitiativeFilter, + fields: frozenset[str] | None, + ) -> Page[InitiativeOut]: + """Return a scoped, filtered, paginated page of initiatives. See ``get_many``.""" + return await self.get_many(pagination=pagination, filter=filter, fields=fields) + + async def insert_initiative(self, data: InitiativeIn) -> Initiative: + """Create an initiative owned by the caller, enforcing the per-owner unapproved quota. + + ``owner`` is forced to the caller and the initiative starts unapproved and private. A + non-admin who already owns ``max_unapproved_per_owner`` unapproved initiatives is rejected + with 409. A duplicate ``slug`` (globally unique) is also a 409. + """ + if self._user.username is None: + raise PermissionError(required_role="authenticated") + + if not self._user.is_admin: + unapproved = await self.document_model.find( + self.document_model.owner == self._user.username, + self.document_model.is_approved == False, # noqa: E712 — Beanie needs the value, not `is` + ).count() + if unapproved >= self._limits.max_unapproved_per_owner: + raise ConflictError( + "owner already has the maximum number of unapproved initiatives", + limit=self._limits.max_unapproved_per_owner, + ) + + initiative = self.document_model.from_input_model(data=data, owner=self._user.username) + try: + await initiative.insert() + except DuplicateKeyError as exc: # unique slug index + raise ConflictError("an initiative with this slug already exists", slug=data.slug) from exc + return initiative + + async def patch_one( # pyright: ignore[reportIncompatibleMethodOverride] + self, identifiers: dict[str, Any], update: InitiativePatch, session: AsyncClientSession | None = None + ) -> Initiative: + """Patch a scoped initiative by ``slug``, enforcing manage rights and approval rules. + + - The caller must be able to *manage* the initiative (owner/collaborator/admin) + - Only an admin may change ``is_approved``. + - The resulting state must satisfy ``is_public ⇒ is_approved`` (re-checked here because a + partial ``$set`` does not run the document validator). + + ``identifiers`` is the ``{"slug": ...}`` form; the auth checks run against the resolved + document, and the write itself is delegated to the base :meth:`MongoDbRepository.patch_one`. + """ + slug = identifiers["slug"] + existing = await self.document_model.find_one(self._scope, self.document_model.slug == slug) + if existing is None: + raise NotFoundError("Initiative not found", slug=slug) + if not ( + self._user.can_manage(id=existing.slug, resource="initiative") or self._user.username == existing.owner + ): + raise PermissionError(required_role="initiative-owner-collaborator-or-admin") + + data = update.model_dump(exclude_unset=True) + if "is_approved" in data and not self._user.is_admin: + raise PermissionError("only admins can set `is_approved`", required_role="admin") + + resulting_approved = data.get("is_approved", existing.is_approved) + resulting_public = data.get("is_public", existing.is_public) + if resulting_public and not resulting_approved: + raise ValidationError("an initiative cannot be public until it is approved", slug=slug) + + return await super().patch_one(identifiers, update, session=session) + + async def delete_one( + self, identifiers: dict[str, Any], session: AsyncClientSession | None = None + ) -> DeleteResponse: + """Delete a scoped initiative by ``slug``. Restricted to the owner or an admin. + + Collaborators may contribute projects but may not dissolve the effort. Deleting an + initiative does not touch member projects; their ``initiative`` link simply dangles until + re-pointed (reads resolve a missing link to null). + + ``identifiers`` is the ``{"slug": ...}`` form; the write is delegated to the base + :meth:`MongoDbRepository.delete_one`. + """ + slug = identifiers["slug"] + existing = await self.document_model.find_one(self._scope, self.document_model.slug == slug) + if existing is None: + raise NotFoundError("Initiative not found", slug=slug) + if not (self._user.is_admin or existing.owner == self._user.username): + raise PermissionError(required_role="owner-or-admin") + return await super().delete_one(identifiers, session=session) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/initiatives/router.py b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/router.py new file mode 100644 index 000000000..8a41815ae --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/initiatives/router.py @@ -0,0 +1,79 @@ +from typing import Annotated + +from fastapi import APIRouter, Depends, Response, status +from fastapi_filter import FilterDepends + +from mpcontribs_api.dependencies import require_user +from mpcontribs_api.domains._shared.types import FieldSelector +from mpcontribs_api.domains.initiatives.dependencies import InitiativeDep +from mpcontribs_api.domains.initiatives.models import ( + InitiativeFilter, + InitiativeIn, + InitiativeOut, + InitiativePatch, +) +from mpcontribs_api.pagination import CursorParams + +router = APIRouter() + + +@router.get("") +async def get_initiatives( + repo: InitiativeDep, + pagination: Annotated[CursorParams, Depends()], + filter: InitiativeFilter = FilterDepends(InitiativeFilter), + fields: FieldSelector = InitiativeOut.default_fields(), +): + """Return paginated initiatives matching a filter, scoped to the caller.""" + selected = InitiativeOut.parse_fields(fields) + return await repo.get_initiatives(pagination=pagination, filter=filter, fields=selected) + + +@router.get("/{slug}") +async def get_one( + repo: InitiativeDep, + slug: str, + fields: FieldSelector = InitiativeOut.default_fields(), +): + """Return the single initiative identified by ``slug``, scoped to the caller.""" + selected = InitiativeOut.parse_fields(fields) + return await repo.get_one({"slug": slug}, fields=selected) + + +@router.post( + "", response_model=InitiativeOut, status_code=status.HTTP_201_CREATED, dependencies=[Depends(require_user)] +) +async def insert_initiative( + repo: InitiativeDep, + initiative: InitiativeIn, +): + """Create a new initiative owned by the caller. + + Starts unapproved and private. Rejected with 409 if the caller already owns the maximum number + of unapproved initiatives, or if the slug is already taken. + """ + return await repo.insert_initiative(data=initiative) + + +@router.patch("/{slug}", response_model=InitiativeOut, dependencies=[Depends(require_user)]) +async def patch_one( + repo: InitiativeDep, + slug: str, + update: InitiativePatch, +): + """Partially update the initiative identified by ``slug``. + + Requires manage rights (owner/collaborator/admin). ``is_approved`` is admin-only, and an + initiative cannot be made public until it is approved. + """ + return await repo.patch_one({"slug": slug}, update=update) + + +@router.delete("/{slug}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(require_user)]) +async def delete_one( + repo: InitiativeDep, + slug: str, +): + """Delete the initiative identified by ``slug``. Restricted to its owner or an admin.""" + await repo.delete_one({"slug": slug}) + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/project_groups/dependencies.py b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/dependencies.py new file mode 100644 index 000000000..7220513ac --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/dependencies.py @@ -0,0 +1,25 @@ +from typing import Annotated + +from fastapi import Depends + +from mpcontribs_api.dependencies import UserDep +from mpcontribs_api.domains.project_groups.repository import ProjectGroupRepository +from mpcontribs_api.domains.project_groups.service import ProjectGroupService +from mpcontribs_api.domains.projects.repository import MongoDbProjectRepository + + +def get_project_group_repository(user: UserDep) -> ProjectGroupRepository: + return ProjectGroupRepository(user) + + +ProjectGroupDep = Annotated[ProjectGroupRepository, Depends(get_project_group_repository)] + + +def get_project_group_service(user: UserDep) -> ProjectGroupService: + return ProjectGroupService( + groups=ProjectGroupRepository(user), + projects=MongoDbProjectRepository(user), + ) + + +ProjectGroupServiceDep = Annotated[ProjectGroupService, Depends(get_project_group_service)] diff --git a/mpcontribs-api/src/mpcontribs_api/domains/project_groups/models.py b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/models.py new file mode 100644 index 000000000..2788c16aa --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/models.py @@ -0,0 +1,142 @@ +from beanie import Link, PydanticObjectId +from bson import DBRef +from bson.errors import InvalidId +from pydantic import BaseModel, ConfigDict, Field, field_validator +from pymongo import ASCENDING, IndexModel + +from mpcontribs_api.domains._shared.filters import BaseFilter +from mpcontribs_api.domains._shared.models import BaseDocumentWithInput, DocumentOut +from mpcontribs_api.domains._shared.types import PrefixedEmail, SearchStr, ShortStr +from mpcontribs_api.domains.projects.models import Project +from mpcontribs_api.exceptions import ValidationError +from mpcontribs_api.projection import SparseFieldsModel + + +class ProjectGroup(BaseDocumentWithInput[PydanticObjectId]): + name: SearchStr = Field(max_length=50) + owner: PrefixedEmail + description: str = Field(max_length=100) + is_public: bool = False + projects: list[Link[Project]] | None = None + + class Settings: + name = "project_groups" + indexes = [ + IndexModel( + keys=[("name", ASCENDING), ("owner", ASCENDING)], + name="name_owner", + unique=True, + ), + IndexModel( + keys=[("name", ASCENDING), ("owner", ASCENDING), ("is_public", ASCENDING)], + name="name_owner_is_public", + ), + ] + validate_on_save = True + + @classmethod + def identifier_fields(cls) -> frozenset[str]: + """A ``ProjectGroup`` is uniquely identified by its ``name`` + ``owner``.""" + return frozenset({"name", "owner"}) + + @classmethod + def from_input_model(cls, data: ProjectGroupIn) -> ProjectGroup: + """Build a stored group from input, assigning a fresh ``_id`` and resolving member ids to links""" + payload = data.model_dump() + project_ids = payload.pop("projects", None) or [] + payload["_id"] = PydanticObjectId() + payload["projects"] = [DBRef("projects", pid) for pid in project_ids] + return cls.model_validate(payload) + + @field_validator("projects") + @classmethod + def _reject_duplicate_refs( + cls, + value: list[Link[Project] | Project] | None, + ) -> list[Link[Project] | Project] | None: + if value is None: + return value + seen: set[ShortStr] = set() + for item in value: + ref_id = item.ref.id if isinstance(item, Link) else item.id + if ref_id in seen: + raise ValidationError( + message="duplicate Project reference in ProjectGroup", + duplicate_id=ref_id, + ) + seen.add(ref_id) + return value + + +class ProjectGroupIn(BaseModel): + """User-supplied fields for creating a project group""" + + model_config = ConfigDict(extra="forbid") + + name: SearchStr = Field(max_length=50) + owner: PrefixedEmail + description: str = Field(max_length=100) + is_public: bool = False + projects: list[ShortStr] = Field(default_factory=list) + + +class ProjectGroupOut(DocumentOut[PydanticObjectId]): + name: SearchStr | None = None + owner: PrefixedEmail | None = None + is_public: bool | None = None + projects: list[Link[Project]] | None = None + description: str | None = None + + @staticmethod + def default_fields() -> list[str]: + return [ + "name", + "description", + "projects", + ] + + +class ProjectGroupPatch(SparseFieldsModel): + name: SearchStr | None = None + owner: PrefixedEmail | None = None + is_public: bool | None = None + projects: list[Link[Project]] | None = None + description: str | None = None + + +class ProjectRefs(BaseModel): + """Request body for adding/removing projects from a group: the project ids to (un)link.""" + + project_ids: list[ShortStr] = Field(default_factory=list) + + +class ProjectGroupFilter(BaseFilter): + id: PydanticObjectId | None = None + id__in: list[PydanticObjectId] | None = None + id__neq: PydanticObjectId | None = None + + name: SearchStr | None = None + name__in: list[SearchStr] | None = None + name__neq: ShortStr | None = None + + owner: PrefixedEmail | None = None + owner__in: list[PrefixedEmail] | None = None + owner__neq: PrefixedEmail | None = None + + is_public: bool | None = None + + order_by: list[str] | None = None + + class Constants(BaseFilter.Constants): + model: ProjectGroup + + @field_validator("id", mode="before") + @classmethod + def convert_str_to_oid(cls, v: str): + try: + return PydanticObjectId(v) + except InvalidId as err: + raise ValidationError( + "Invalid ObjectId format. Must be 12-byte input or a 24-character hex string", + oid=v, + ) from err diff --git a/mpcontribs-api/src/mpcontribs_api/domains/project_groups/repository.py b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/repository.py new file mode 100644 index 000000000..31ed1f129 --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/repository.py @@ -0,0 +1,132 @@ +from typing import Any + +from beanie import PydanticObjectId, UpdateResponse +from beanie.operators import AddToSet, Pull +from bson import DBRef +from bson.errors import InvalidId +from pymongo.asynchronous.client_session import AsyncClientSession + +from mpcontribs_api.authz import User +from mpcontribs_api.domains._shared.models import DeleteResponse +from mpcontribs_api.domains._shared.repository import MongoDbRepository +from mpcontribs_api.domains._shared.types import ShortStr +from mpcontribs_api.domains.project_groups.models import ( + ProjectGroup, + ProjectGroupFilter, + ProjectGroupIn, + ProjectGroupOut, + ProjectGroupPatch, +) +from mpcontribs_api.exceptions import NotFoundError, PermissionError +from mpcontribs_api.pagination import CursorParams, Page + + +class ProjectGroupRepository( + MongoDbRepository[ProjectGroup, ProjectGroupIn, ProjectGroupOut, ProjectGroupFilter, ProjectGroupPatch] +): + document_model = ProjectGroup + out_model = ProjectGroupOut + + @staticmethod + def _build_scope(user: User) -> dict[str, Any]: + """Scope reads to what the caller may see: public groups, ones they own, or ones granted.""" + if user.is_admin: + return {} + ors: list[dict[str, Any]] = [{"is_public": True}] + if not user.is_anonymous: + ors.append({"owner": user.username}) + granted: list[PydanticObjectId] = [] + for raw in user.project_group_roles: + try: + granted.append(PydanticObjectId(raw)) + except InvalidId: + continue + if granted: + ors.append({"_id": {"$in": sorted(granted)}}) + return {"$or": ors} + + async def get_project_groups( + self, + pagination: CursorParams, + filter: ProjectGroupFilter, + fields: frozenset[str] | None, + ) -> Page[ProjectGroupOut]: + """Return paginated project groups matching a filter. + + Args: + pagination (CursorParams): arguments for cursor-based pagination + filter (ProjectGroupFilter): optional filters to select ProjectGroups + fields (frozenset[str] | None): the fields to return to a user + """ + return await self.get_many(pagination=pagination, filter=filter, fields=fields) + + async def insert_project_group(self, project_group: ProjectGroupIn) -> ProjectGroup: + return await self.insert_one(in_resource=project_group) + + async def delete_one( + self, identifiers: dict[str, Any], session: AsyncClientSession | None = None + ) -> DeleteResponse: + """Delete the single project group matching ``identifiers`` (``{name, owner}`` or ``{id}``). + + Absence (in scope) takes precedence over the ownership gate: a non-admin may only delete + their own group, so deleting another owner's visible (public) group is forbidden rather + than silently a no-op. The ``name`` + ``owner`` unique index makes the match unambiguous. + The auth check runs against the resolved document; the write is delegated to the base + :meth:`MongoDbRepository.delete_one`. + """ + doc = await self.document_model.find_one(self._scope, self._identifier_query(identifiers), session=session) # pyright: ignore[reportArgumentType] + if doc is None: + raise NotFoundError(f"{self.document_model.__name__} not found", **identifiers) + if not (self._user.is_admin or doc.owner == self._user.username): + raise PermissionError(required_role="owner-or-admin") + return await super().delete_one(identifiers, session=session) + + async def delete_project_groups(self, filter: ProjectGroupFilter) -> DeleteResponse: + """Bulk-delete project groups matching ``filter``, restricted to the caller's own. + + A non-admin's bulk delete is scoped to their own groups (overriding any ``owner`` in the + filter) so it can never remove public groups belonging to others. See ``delete``. + """ + if not self._user.is_admin: + filter.owner = self._user.username + return await self.delete(filter) + + async def add_project_refs( + self, + group_id: PydanticObjectId, + project_ids: list[ShortStr], + session: AsyncClientSession | None = None, + ) -> ProjectGroup | None: + """Atomically add project references to a scoped group, deduplicating existing members. + + Args: + group_id (PydanticObjectId): the id of the group to modify + project_ids (list[ShortStr]): project ids to add (already validated by the service) + session (AsyncClientSession | None): optional client session for transactions + """ + refs = [DBRef("projects", pid) for pid in project_ids] + query = self.document_model.find_one(self._scope, self.document_model.id == group_id, session=session).update( + AddToSet({"projects": {"$each": refs}}), + response_type=UpdateResponse.NEW_DOCUMENT, + ) + return await query # pyright: ignore[reportGeneralTypeIssues] # beanie UpdateQuery is awaitable + + async def delete_project_refs( + self, + group_id: PydanticObjectId, + project_ids: list[ShortStr], + session: AsyncClientSession | None = None, + ) -> ProjectGroup | None: + """Atomically delete project references from a scoped group. + + Args: + group_id (PydanticObjectId): the id of the group to modify + project_ids (list[ShortStr]): project ids to delete + session (AsyncClientSession | None): optional client session for transactions + """ + refs = [DBRef("projects", pid) for pid in project_ids] + query = self.document_model.find_one(self._scope, self.document_model.id == group_id, session=session).update( + Pull({"projects": {"$in": refs}}), + response_type=UpdateResponse.NEW_DOCUMENT, + ) + return await query # pyright: ignore[reportGeneralTypeIssues] # beanie UpdateQuery is awaitable diff --git a/mpcontribs-api/src/mpcontribs_api/domains/project_groups/router.py b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/router.py new file mode 100644 index 000000000..5854180d9 --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/router.py @@ -0,0 +1,177 @@ +from typing import Annotated + +from fastapi import APIRouter, Depends, Response, status +from fastapi_filter import FilterDepends + +from mpcontribs_api.dependencies import require_user +from mpcontribs_api.domains._shared.bulk import BulkWriteSummary +from mpcontribs_api.domains._shared.models import DeleteResponse +from mpcontribs_api.domains._shared.types import FieldSelector, PrefixedEmail, SearchStr +from mpcontribs_api.domains.project_groups.dependencies import ProjectGroupDep, ProjectGroupServiceDep +from mpcontribs_api.domains.project_groups.models import ( + ProjectGroupFilter, + ProjectGroupIn, + ProjectGroupOut, + ProjectGroupPatch, + ProjectRefs, +) +from mpcontribs_api.pagination import CursorParams + +router = APIRouter() + + +@router.get("") +async def get_project_groups( + repo: ProjectGroupDep, + pagination: Annotated[CursorParams, Depends()], + filter: ProjectGroupFilter = FilterDepends(ProjectGroupFilter), + fields: FieldSelector = ProjectGroupOut.default_fields(), +): + """Return paginated project groups matching a filter. + + Args: + repo (ProjectGroupDep): the project group repo we depend on + pagination (CursorParams): arguments for cursor-based pagination + filter (ProjectGroupFilter): optional filters to select ProjectGroups + fields (FieldSelector): the fields to return to a user + """ + selected = ProjectGroupOut.parse_fields(fields) + return await repo.get_project_groups(pagination=pagination, filter=filter, fields=selected) + + +@router.get("/item") +async def get_one( + service: ProjectGroupServiceDep, + name: SearchStr, + owner: PrefixedEmail, + fields: FieldSelector = ProjectGroupOut.default_fields(), +): + """Return the single project group identified by ``name`` + ``owner``. + + Args: + service (ProjectGroupServiceDep): the project group service we depend on + name (SearchStr): the project group's name + owner (PrefixedEmail): the project group's owner + fields (FieldSelector): the fields to return to a user + """ + selected = ProjectGroupOut.parse_fields(fields) + return await service.get_one({"name": name, "owner": owner}, fields=selected) + + +@router.post( + "", response_model=ProjectGroupOut, status_code=status.HTTP_201_CREATED, dependencies=[Depends(require_user)] +) +async def insert_project_group( + service: ProjectGroupServiceDep, + project_group: ProjectGroupIn, +): + """Insert a new project group. + + Each referenced project is verified against the projects collection (scoped to the caller); + creation is rejected with 404 if any project id is unknown or not visible. + + Args: + service (ProjectGroupServiceDep): the project group service we depend on + project_group (ProjectGroupIn): the project group to insert + """ + return await service.insert(project_group=project_group) + + +@router.patch("/item", response_model=ProjectGroupOut, dependencies=[Depends(require_user)]) +async def patch_one( + service: ProjectGroupServiceDep, + name: SearchStr, + owner: PrefixedEmail, + update: ProjectGroupPatch, +): + """Partially update the project group identified by ``name`` + ``owner``. + + Args: + service (ProjectGroupServiceDep): the project group service we depend on + name (SearchStr): the project group's name + owner (PrefixedEmail): the project group's owner + update (ProjectGroupPatch): the partial update to apply - unset fields are dropped + """ + return await service.patch_one({"name": name, "owner": owner}, update=update) + + +@router.delete("/item", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(require_user)]) +async def delete_one( + service: ProjectGroupServiceDep, + name: SearchStr, + owner: PrefixedEmail, +): + """Delete the single project group identified by ``name`` + ``owner``. + + Raises 404 if no such group is visible to the caller, 409 if the identifiers are ambiguous. + + Args: + service (ProjectGroupServiceDep): the project group service we depend on + name (SearchStr): the project group's name + owner (PrefixedEmail): the project group's owner + """ + await service.delete_one({"name": name, "owner": owner}) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + +@router.delete("", response_model=DeleteResponse, dependencies=[Depends(require_user)]) +async def delete_project_groups( + repo: ProjectGroupDep, + filter: ProjectGroupFilter = FilterDepends(ProjectGroupFilter), +): + """Bulk-delete every project group matching ``filter`` (e.g. all with a given owner). + + Args: + repo (ProjectGroupDep): the project group repo we depend on + filter (ProjectGroupFilter): the query selecting which project groups to delete + """ + return await repo.delete_project_groups(filter=filter) + + +@router.post("/item/projects", response_model=BulkWriteSummary[str], dependencies=[Depends(require_user)]) +async def add_projects_by_identifiers( + service: ProjectGroupServiceDep, + name: SearchStr, + owner: PrefixedEmail, + body: ProjectRefs, +): + """Add projects to the group identified by ``name`` + ``owner``. + + Each project is verified against the projects collection (scoped to the caller); unknown or + invisible projects are reported per-item in the response rather than failing the whole request. + """ + return await service.add_projects({"name": name, "owner": owner}, body.project_ids) + + +@router.delete("/item/projects", response_model=BulkWriteSummary[str], dependencies=[Depends(require_user)]) +async def delete_projects_by_identifiers( + service: ProjectGroupServiceDep, + name: SearchStr, + owner: PrefixedEmail, + body: ProjectRefs, +): + """Delete projects from the group identified by ``name`` + ``owner``. + + Ids that are not members of the group are reported per-item in the response. + """ + return await service.delete_projects({"name": name, "owner": owner}, body.project_ids) + + +@router.post("/{id}/projects", response_model=BulkWriteSummary[str], dependencies=[Depends(require_user)]) +async def add_projects_by_id( + service: ProjectGroupServiceDep, + id: str, + body: ProjectRefs, +): + """Add projects to the group identified by ``id``. See ``add_projects``.""" + return await service.add_projects({"id": id}, body.project_ids) + + +@router.delete("/{id}/projects", response_model=BulkWriteSummary[str], dependencies=[Depends(require_user)]) +async def delete_projects_by_id( + service: ProjectGroupServiceDep, + id: str, + body: ProjectRefs, +): + """Delete projects from the group identified by ``id``. See ``delete_projects``.""" + return await service.delete_projects({"id": id}, body.project_ids) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/project_groups/service.py b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/service.py new file mode 100644 index 000000000..be62035cd --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/project_groups/service.py @@ -0,0 +1,129 @@ +from typing import Any + +from beanie import Link + +from mpcontribs_api.domains._shared.bulk import BulkFailure, BulkWriteSummary +from mpcontribs_api.domains._shared.models import DeleteResponse +from mpcontribs_api.domains._shared.types import ShortStr +from mpcontribs_api.domains.project_groups.models import ( + ProjectGroup, + ProjectGroupIn, + ProjectGroupOut, + ProjectGroupPatch, +) +from mpcontribs_api.domains.project_groups.repository import ProjectGroupRepository +from mpcontribs_api.domains.projects.repository import MongoDbProjectRepository +from mpcontribs_api.exceptions import NotFoundError + +# Fields the membership operations need off a resolved group: its id (target of the update) and its +# current members (so deletion can tell members from non-members). +_GROUP_FIELDS = frozenset({"id", "projects"}) + + +class ProjectGroupService: + """Coordinates project-group membership changes across the groups and projects collections""" + + def __init__( + self, + groups: ProjectGroupRepository, + projects: MongoDbProjectRepository, + ) -> None: + self._groups = groups + self._projects = projects + + async def _project_exists(self, project_id: ShortStr) -> bool: + """Whether a project with ``project_id`` exists and is visible to the caller.""" + return await self._projects.get_one({"id": project_id}, fields=frozenset({"id"})) is not None + + async def insert(self, project_group: ProjectGroupIn) -> ProjectGroup: + """Insert a new group after verifying every referenced project exists and is visible. + + Non-admins are set as owner automatically, while admins can specify owners. + """ + user = self._groups._user + if not user.is_admin: + project_group = project_group.model_copy(update={"owner": user.username}) + missing = [pid for pid in project_group.projects if not await self._project_exists(pid)] + if missing: + raise NotFoundError("One or more projects not found or not visible", ids=missing) + return await self._groups.insert_project_group(project_group) + + async def get_one(self, identifiers: dict[str, Any], fields: frozenset[str] | None) -> ProjectGroupOut | None: + """Return the single group matching ``identifiers`` (``{"name", "owner"}`` or ``{"id"}``).""" + return await self._groups.get_one(identifiers, fields) + + async def patch_one(self, identifiers: dict[str, Any], update: ProjectGroupPatch) -> ProjectGroup: + """Patch the single group matching ``identifiers`` (``{"name", "owner"}`` or ``{"id"}``).""" + return await self._groups.patch_one(identifiers, update) + + async def delete_one(self, identifiers: dict[str, Any]) -> DeleteResponse: + """Delete the single group matching ``identifiers`` (``{"name", "owner"}`` or ``{"id"}``).""" + return await self._groups.delete_one(identifiers) + + async def _resolve_one(self, identifiers: dict[str, Any]) -> ProjectGroupOut: + """Resolve a visible group matching ``identifiers``, or raise ``NotFoundError``. + + ``identifiers`` is either the primary-key form ``{"id": }`` or the semantic + ``{"name": ..., "owner": ...}``. Propagates ``ConflictError`` from the repository if + ``(name, owner)`` identifiers are ambiguous. + """ + query = self._groups.coerce_identifiers(identifiers) + group = await self._groups.get_one(query, fields=_GROUP_FIELDS) + if group is None: + raise NotFoundError("ProjectGroup not found", **identifiers) + return group # pyright: ignore[reportReturnType] # projected reads return the out model + + async def _add(self, group: ProjectGroupOut, project_ids: list[ShortStr]) -> BulkWriteSummary[str]: + """Validate each project against the projects collection, then add the valid ones. + + A project that does not exist or is not visible to the caller is reported as a failed item; + the rest are added in a single atomic ``$addToSet`` (idempotent for existing members). + """ + failed: list[BulkFailure] = [] + valid: list[ShortStr] = [] + for index, pid in enumerate(project_ids): + if not await self._project_exists(pid): + failed.append( + BulkFailure( + index=index, + identifier={"id": pid}, + error_code="not_found", + message=f"Project {pid} not found or not visible", + ) + ) + elif pid not in valid: + valid.append(pid) + + if valid: + await self._groups.add_project_refs(group.id, valid) # pyright: ignore[reportArgumentType] # id is set on a resolved group + return BulkWriteSummary(total=len(project_ids), succeeded=valid, failed=failed) + + async def _delete(self, group: ProjectGroupOut, project_ids: list[ShortStr]) -> BulkWriteSummary[str]: + """Delete requested members from the group; non-members are reported as failed items.""" + current = {(link.ref.id if isinstance(link, Link) else link.id) for link in (group.projects or [])} + failed: list[BulkFailure] = [] + present: list[ShortStr] = [] + for index, pid in enumerate(project_ids): + if pid not in current: + failed.append( + BulkFailure( + index=index, + identifier={"id": pid}, + error_code="not_found", + message=f"Project {pid} is not a member of this group", + ) + ) + elif pid not in present: + present.append(pid) + + if present: + await self._groups.delete_project_refs(group.id, present) # pyright: ignore[reportArgumentType] # id is set on a resolved group + return BulkWriteSummary(total=len(project_ids), succeeded=present, failed=failed) + + async def add_projects(self, identifiers: dict[str, Any], project_ids: list[ShortStr]) -> BulkWriteSummary[str]: + """Add projects to the group matching ``identifiers`` (``{"id": ...}`` or ``{"name", "owner"}``).""" + return await self._add(await self._resolve_one(identifiers), project_ids) + + async def delete_projects(self, identifiers: dict[str, Any], project_ids: list[ShortStr]) -> BulkWriteSummary[str]: + """Delete projects from the group matching ``identifiers`` (``{"id": ...}`` or ``{"name", "owner"}``).""" + return await self._delete(await self._resolve_one(identifiers), project_ids) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/projects/dependencies.py b/mpcontribs-api/src/mpcontribs_api/domains/projects/dependencies.py index b74b44f93..393e4759e 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/projects/dependencies.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/projects/dependencies.py @@ -4,9 +4,11 @@ from mpcontribs_api.dependencies import UserDep from mpcontribs_api.domains.consumers.dependencies import ConsumerLimitsDep +from mpcontribs_api.domains.initiatives.repository import InitiativeRepository from mpcontribs_api.domains.projects.repository import ( MongoDbProjectRepository, ) +from mpcontribs_api.domains.projects.service import ProjectService def get_scoped_projects(user: UserDep, limits: ConsumerLimitsDep) -> MongoDbProjectRepository: @@ -14,3 +16,13 @@ def get_scoped_projects(user: UserDep, limits: ConsumerLimitsDep) -> MongoDbProj ProjectDep = Annotated[MongoDbProjectRepository, Depends(get_scoped_projects)] + + +def get_project_service(user: UserDep) -> ProjectService: + return ProjectService( + projects=MongoDbProjectRepository(user), + initiatives=InitiativeRepository(user), + ) + + +ProjectServiceDep = Annotated[ProjectService, Depends(get_project_service)] diff --git a/mpcontribs-api/src/mpcontribs_api/domains/projects/models.py b/mpcontribs-api/src/mpcontribs_api/domains/projects/models.py index 396505922..5e4a510c3 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/projects/models.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/projects/models.py @@ -1,11 +1,13 @@ from typing import Any, Literal +from beanie import Link from pydantic import BaseModel, ConfigDict, Field, HttpUrl, field_validator from mpcontribs_api import pagination from mpcontribs_api.domains._shared.filters import BaseFilter from mpcontribs_api.domains._shared.models import BaseDocumentWithInput, DocumentOut -from mpcontribs_api.domains._shared.types import PrefixedEmail, ShortStr +from mpcontribs_api.domains._shared.types import PrefixedEmail, SearchStr, ShortStr +from mpcontribs_api.domains.initiatives.models import Initiative from mpcontribs_api.exceptions import ValidationError @@ -42,17 +44,12 @@ def segments(self) -> tuple[str, ...]: class Stats(BaseModel): - columns: int - contributions: int - tables: int - structures: int - attachments: int - size: float - - @classmethod - def empty(cls) -> Stats: - """A zeroed rollup for a project with no contributions yet.""" - return cls(columns=0, contributions=0, tables=0, structures=0, attachments=0, size=0.0) + columns: int = 0 + contributions: int = 0 + tables: int = 0 + structures: int = 0 + attachments: int = 0 + size: float = 0 class Reference(BaseModel): @@ -73,6 +70,9 @@ class ProjectBase(BaseModel): unique_column: str | None = None # Optional + stats: Stats = Field(default_factory=Stats) + tags: list[SearchStr] | None = None + mp_category: str | None = None references: list[Reference] = Field(default_factory=list) long_title: str | None = None other: dict[str, Any] = Field(default_factory=dict) @@ -80,6 +80,9 @@ class ProjectBase(BaseModel): is_approved: bool = False license: Literal["CCA4", "CCPD"] | None = None + initiative: Link[Initiative] | None = None + + # Empty method for now. Keeping for business logic later # Validated on every representation (input and stored) so a bad unique_column is rejected immediately @field_validator("unique_column") @classmethod @@ -95,7 +98,7 @@ class Project(ProjectBase, BaseDocumentWithInput[ShortStr]): """Document model of what is actually stored.""" # Server-owned: derived from the project's contributions - stats: Stats = Field(default_factory=Stats.empty) + stats: Stats = Field(default_factory=Stats) columns: list[Column] = Field(default_factory=list) @classmethod @@ -111,6 +114,10 @@ def decode_cursor(cursor: str) -> str: """ return pagination.decode_cursor(cursor) + @classmethod + def server_managed_fields(cls) -> tuple: + return ("is_public", "is_approved", "stats", "mp_category") + class ProjectOut(DocumentOut[ShortStr]): """Full response of all public-facing fields.""" @@ -119,6 +126,8 @@ class ProjectOut(DocumentOut[ShortStr]): authors: str | None = None description: str | None = None title: ShortStr | None = None + tags: list[SearchStr] | None = None + mp_category: str | None = None owner: PrefixedEmail | None = None other: dict[str, Any] | None = None is_public: bool | None = None @@ -129,6 +138,7 @@ class ProjectOut(DocumentOut[ShortStr]): stats: Stats | None = None columns: list[Column] | None = None license: Literal["CCA4", "CCPD"] | None = None + initiative: Link[Initiative] | None = None @staticmethod def default_fields() -> tuple[str, ...]: @@ -152,6 +162,15 @@ class ProjectFilter(BaseFilter): owner__neq: PrefixedEmail | None = None owner__ilike: str | None = None + tags: list[SearchStr] | None = None # exact match of list + tags__in: list[SearchStr] | None = None # if at least one tag is present + tags__contains: list[SearchStr] | None = None # Project.tags must be a superset of these + + mp_category: str | None = None + mp_category__in: list[str] | None = None + mp_category__neq: str | None = None + mp_category__ilike: str | None = None + # fuzzy only long_title__ilike: str | None = None @@ -190,6 +209,7 @@ class ProjectPatch(BaseModel): title: ShortStr | None = None authors: str | None = None description: str | None = None + tags: list[SearchStr] | None = None owner: PrefixedEmail | None = None unique_column: str | None = None references: list[Reference] = Field(default_factory=list) @@ -200,6 +220,9 @@ class ProjectPatch(BaseModel): is_approved: bool | None = None license: Literal["CCA4", "CCPD"] | None = None + # str here, but ProjectService coerces to a Link + initiative: str | None = None + @field_validator("unique_column") @classmethod def _check_unique_column(cls, v: str | None) -> str | None: diff --git a/mpcontribs-api/src/mpcontribs_api/domains/projects/repository.py b/mpcontribs-api/src/mpcontribs_api/domains/projects/repository.py index f962086c3..073d05cd5 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/projects/repository.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/projects/repository.py @@ -1,8 +1,11 @@ from typing import Any +from beanie import PydanticObjectId from pymongo import UpdateOne +from pymongo.asynchronous.client_session import AsyncClientSession from mpcontribs_api.authz import User +from mpcontribs_api.domains._shared.models import DeleteResponse from mpcontribs_api.domains._shared.repository import MongoDbRepository from mpcontribs_api.domains.consumers.models import ConsumerSettings from mpcontribs_api.domains.projects.models import ( @@ -14,7 +17,7 @@ ProjectPatch, Stats, ) -from mpcontribs_api.exceptions import ConflictError, PermissionError +from mpcontribs_api.exceptions import ConflictError, NotFoundError, PermissionError, ValidationError from mpcontribs_api.pagination import CursorParams @@ -73,10 +76,6 @@ async def get_projects( """Query the Project collection, scoped to the current user. See ``get_many``.""" return await self.get_many(pagination=pagination, filter=filter, fields=fields) - async def get_project_by_id(self, id: str, fields: frozenset[str] | None): - """Find a single project by id, scoped to the current user. See ``get_by_id``.""" - return await self.get_by_id(id, fields) - async def unique_columns_by_id(self, ids: list[str]) -> dict[str, str | None]: """Return ``{project_id: unique_column}`` for the given project ids, scoped to the user. @@ -144,40 +143,82 @@ async def insert_project(self, id: str, project: ProjectIn) -> Project: await document.insert() return document - async def patch_project_by_id(self, id: str, update: ProjectPatch) -> Project: - """Partially update a project by id, scoped to the current user. See ``patch``. + async def patch_one( # pyright: ignore[reportIncompatibleMethodOverride] + self, + identifiers: dict[str, Any], + update: ProjectPatch, + session: AsyncClientSession | None = None, + extra_set: dict[str, Any] | None = None, + ) -> Project: + """Partially update a scoped project by id, enforcing approval rules. + + - Only an admin may change ``is_approved``. + - Resulting state must satisfy is_public <-> is_approved condition + + The ``initiative`` field is split out upstream in ``ProjectService.patch_one``, so it never + reaches this method as a bare slug; an assignment that also edits plain fields arrives with + the resolved link passed through ``extra_set`` (``{"initiative": }``) and is + written together with the plain fields in the single ``$set``. + """ + await self._enforce_patch_rules(identifiers["id"], update) + return await super().patch_one(identifiers, update, session=session, extra_set=extra_set) - ``is_approved`` is an admin-only curation flag: a non-admin that sets it (to any value) is - rejected. ``stats``/``columns`` are not on the patch model at all, so they cannot be - client-patched. Both remain server-owned. + async def _enforce_patch_rules(self, id: str, update: ProjectPatch) -> None: + """Enforce project patch invariants against the scoped target. - Raises: - PermissionError: if a non-admin caller sets ``is_approved`` + - Only an admin may change ``is_approved``. + - The resulting state must satisfy the is_public -> is_approved condition. + + Raises ``NotFoundError`` when the project is invisible to the caller or absent, so both the + plain and initiative-bearing patch paths reject unseen documents identically. """ - if update.is_approved is not None and not self._user.is_admin: + data = update.model_dump(exclude_unset=True) + if "is_approved" in data and not self._user.is_admin: raise PermissionError(required_role="admin") - # ``columns`` are server-owned and absent from ProjectPatch, so there is nothing to cap here. - return await self.patch(id, update) - async def delete_project_by_id(self, id: str) -> None: - """Delete a project by id, scoped to the current user. See ``delete_by_id``.""" - await self.delete_by_id(id) + existing = await self.document_model.find_one(self._scope, self.document_model.id == id) + if existing is None: + raise NotFoundError(f"{self.document_model.__name__} not found", id=id) + + resulting_approved = data.get("is_approved", existing.is_approved) + resulting_public = data.get("is_public", existing.is_public) + if resulting_public and not resulting_approved: + raise ValidationError("a project cannot be public until it is approved", id=id) - async def upsert_project_by_id(self, id: str, data: ProjectIn) -> Project: + async def delete_one( + self, identifiers: dict[str, Any], session: AsyncClientSession | None = None + ) -> DeleteResponse: + """Delete a scoped project by id. Restricted to the owner or an admin. + + Visibility (public/approved or group membership) is not enough to delete: a project can + only be dissolved by its owner (or an admin). A caller who cannot see the project gets a + 404; a caller who can see it but does not own it gets a 403. The auth check runs against the + resolved document; the write is delegated to the base :meth:`MongoDbRepository.delete_one`. + """ + id = identifiers["id"] + existing = await self.document_model.find_one(self._scope, self._identifier_query({"id": id})) + if existing is None: + raise NotFoundError(f"{self.document_model.__name__} not found", id=id) + if not (self._user.is_admin or existing.owner == self._user.username): + raise PermissionError(required_role="owner-or-admin") + return await super().delete_one(identifiers, session=session) + + async def upsert_one(self, identifiers: dict[str, Any], data: ProjectIn) -> Project: """Upsert a project by provided id, authorized to the current user. Update the document if the id exists, otherwise insert a new one under that id. - Authorization (the read scope is for visibility, not write access, so it is not - reused here): - **Existing project:** only its ``owner`` or an admin may overwrite it. The stored - ``owner`` is preserved — ownership cannot be reassigned through the request body. - - **New project:** ``owner`` is forced to the caller, ignoring any body value. + ``owner`` and all server-managed fields (see ``Project.server_managed_fields``) are + preserved - ``ProjectIn`` cannot carry them, so a PUT never resets approval, publication, + or stats. + - **New project:** ``owner`` is forced to the caller; server-managed fields keep their + defaults - Note: relies on the path param ``id`` for identity, not the body's id. + Note: relies on the identifier ``id`` for identity, not the body's id. Args: - id (str): the id of the project to upsert + identifiers (dict[str, Any]): the identifier of the project to upsert (``{"id": ...}``) data (ProjectIn): the data of the project to upsert Returns: @@ -190,6 +231,7 @@ async def upsert_project_by_id(self, id: str, data: ProjectIn) -> Project: if self._user.username is None: raise PermissionError(required_role="authenticated") + id = identifiers["id"] # ``columns`` are server-owned (derived from contributions) and absent from ProjectIn, so # there is no client-supplied column set to cap on the write path. existing = await self.document_model.find_one(self.document_model.id == id) @@ -200,6 +242,9 @@ async def upsert_project_by_id(self, id: str, data: ProjectIn) -> Project: # Ownership is immutable via upsert; keep the original owner. Updating an existing # project does not create a new one, so the per-user project cap does not apply. project.owner = existing.owner + # make sure a full replacement doesn't overwrite server-defined fields + for field in self.document_model.server_managed_fields(): + setattr(project, field, getattr(existing, field)) # Server-owned rollups are never taken from the request body; keep the stored values # (they self-heal on the next contribution write via ``ContributionService``). project.stats = existing.stats @@ -212,8 +257,30 @@ async def upsert_project_by_id(self, id: str, data: ProjectIn) -> Project: # New project: the caller owns it, regardless of the submitted owner. Enforce the # per-user cap against the caller before creating another project under their name. project.owner = self._user.username - # A new project starts unapproved; only an admin may create it pre-approved. + # Approval is admin-only; a non-admin's new project always starts unapproved. if not self._user.is_admin: project.is_approved = False await self._check_num_projects(self._user.username) + + if project.is_public and not project.is_approved: + raise ValidationError("a project cannot be public until it is approved", id=id) return await project.save() + + async def count_initiative_members(self, initiative_id: PydanticObjectId, exclude_project_id: str | None) -> int: + """Count projects assigned to an initiative, ignoring user scope. + + The unapproved-initiative member limit is an integrity constraint on the initiative's true + size, so it must count every member regardless of who can see them — a scoped count could + let a collaborator overshoot the cap with projects they cannot see. ``exclude_project_id`` + drops the project being (re)assigned so re-assigning an existing member is idempotent and + never trips the limit. + + Args: + initiative_id (PydanticObjectId): the initiative whose members to count + exclude_project_id (str | None): a project id to exclude from the count, if any + """ + collection = self.document_model.get_pymongo_collection() + query: dict[str, Any] = {"initiative.$id": initiative_id} + if exclude_project_id is not None: + query["_id"] = {"$ne": exclude_project_id} + return await collection.count_documents(query) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/projects/router.py b/mpcontribs-api/src/mpcontribs_api/domains/projects/router.py index e8f0408fc..4d4c8be8e 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/projects/router.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/projects/router.py @@ -6,7 +6,7 @@ from mpcontribs_api.dependencies import require_user from mpcontribs_api.domains._shared.types import FieldSelector -from mpcontribs_api.domains.projects.dependencies import ProjectDep +from mpcontribs_api.domains.projects.dependencies import ProjectDep, ProjectServiceDep from mpcontribs_api.domains.projects.models import ( ProjectFilter, ProjectIn, @@ -41,16 +41,16 @@ async def get_projects( @router.get("/{id}") -async def get_project_by_id( +async def get_one( id: str, - repo: ProjectDep, + service: ProjectServiceDep, fields: FieldSelector = None, ): """Gets a single project by its ID. Args: id (str): the id of the project to retrieve - repo (ProjectDep): the project repo we depend on + service (ProjectServiceDep): the project service we depend on fields (list[str] | None): optional ``_fields`` selection. Omitted -> server defaults; empty (``?_fields=``) -> identity fields only; ``_all`` -> the full document @@ -58,12 +58,12 @@ async def get_project_by_id( ProjectOut: the requested project, actual data returned is determined by the view the user requested """ selected = ProjectOut.parse_fields(fields) - return await repo.get_project_by_id(id=id, fields=selected) + return await service.get_one({"id": id}, fields=selected) @router.put("/{id}", response_model=ProjectOut, dependencies=[Depends(require_user)]) -async def upsert_project_by_id( - repo: ProjectDep, +async def upsert_one( + service: ProjectServiceDep, id: str, project: ProjectIn, ): @@ -73,19 +73,19 @@ async def upsert_project_by_id( Note: Relies on the path param 'id' for finding, rather than the body's id. Args: - repo (ProjectDep): the project repo we depend on + service (ProjectServiceDep): the project service we depend on id (str): the id of the project to retrieve project (ProjectIn): the data of the project to upsert Returns: ProjectOut: the full document that either replaced an old one or was inserted """ - return await repo.upsert_project_by_id(id=id, data=project) + return await service.upsert_one({"id": id}, data=project) @router.patch("/{id}", response_model=ProjectOut, dependencies=[Depends(require_user)]) -async def patch_project_by_id( - repo: ProjectDep, +async def patch_one( + service: ProjectServiceDep, id: str, update: ProjectPatch, ): @@ -93,8 +93,13 @@ async def patch_project_by_id( Note: overwrites fields with given values - arrays are not appended to. + The ``initiative`` field carries an initiative ``slug`` (or ``null`` to unassign). Setting it + is gated by the assignment service: the caller must be able to manage the target initiative + (owner/collaborator/admin) and an unapproved initiative may not exceed its member cap. Plain + field patches take the fast path straight to the repository. + Args: - repo (ProjectDep): the project repo we depend on + service (ProjectServiceDep): the project assignment service we depend on id (str): the id of the project to update update (ProjectPatch): the partial update to apply - unset fields are dropped - Note: If fields are intentionally set to None, None is applied to the field. @@ -102,21 +107,21 @@ async def patch_project_by_id( Returns: ProjectOut: the full Project with updates applied """ - return await repo.patch_project_by_id(id=id, update=update) + return await service.patch_one({"id": id}, update=update) @router.delete("/{id}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(require_user)]) -async def delete_project_by_id( - repo: ProjectDep, +async def delete_one( + service: ProjectServiceDep, id: str, ): """Deletes a project matching id. Args: - repo (ProjectDep): the project repo we depend on + service (ProjectServiceDep): the project service we depend on id (str): the id of the project to be deleted Returns: Response: a response with the 204 response code (rather than FastAPIs default 200) """ - await repo.delete_project_by_id(id=id) + await service.delete_one({"id": id}) return Response(status_code=HTTP_204_NO_CONTENT) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/projects/service.py b/mpcontribs-api/src/mpcontribs_api/domains/projects/service.py new file mode 100644 index 000000000..7ae294d3a --- /dev/null +++ b/mpcontribs-api/src/mpcontribs_api/domains/projects/service.py @@ -0,0 +1,93 @@ +from typing import Any + +from bson import DBRef + +from mpcontribs_api.config import get_settings +from mpcontribs_api.domains._shared.models import DeleteResponse +from mpcontribs_api.domains.initiatives.repository import InitiativeRepository +from mpcontribs_api.domains.projects.models import Project, ProjectIn, ProjectOut, ProjectPatch +from mpcontribs_api.domains.projects.repository import MongoDbProjectRepository +from mpcontribs_api.exceptions import ConflictError, NotFoundError, PermissionError + + +class ProjectService: + """Coordinates assigning a project to its canonical initiative across the two collections.""" + + def __init__( + self, + projects: MongoDbProjectRepository, + initiatives: InitiativeRepository, + ) -> None: + self._projects = projects + self._initiatives = initiatives + self._limits = get_settings().domain.initiatives + + async def patch_one(self, identifiers: dict[str, Any], update: ProjectPatch) -> Project: + """Apply a project patch, routing an ``initiative`` change through the assignment checks. + + ``initiative`` carries the target initiative's ``slug`` (or ``null`` to unassign). When + present it is split out of the patch (so it never reaches the raw ``$set`` as a bare + string), resolved to a link — running the both-rights and member-cap checks — then written + together with any co-submitted plain fields in a single atomic update, so the request never + persists a half-applied change. + """ + id = identifiers["id"] + if "initiative" not in update.model_fields_set: + return await self._projects.patch_one(identifiers, update) + + data = update.model_dump(exclude_unset=True) + slug = data.pop("initiative", None) + + # Resolve the target link (and run the both-rights + limit checks) before touching anything. + ref = await self._resolve_initiative_assignment(project_id=id, slug=slug) + + # `initiative` is server derived, so ProjectPatch can't handle it (expects str), so hand it in extra_set + return await self._projects.patch_one(identifiers, ProjectPatch(**data), extra_set={"initiative": ref}) + + async def get_one(self, identifiers: dict[str, Any], fields: frozenset[str] | None) -> Project | ProjectOut | None: + """Return the single scoped project matching ``identifiers`` (``{"id": ...}``).""" + return await self._projects.get_one(identifiers, fields) + + async def upsert_one(self, identifiers: dict[str, Any], data: ProjectIn) -> Project: + """Upsert the project addressed by ``identifiers`` (``{"id": ...}``). See repository.""" + return await self._projects.upsert_one(identifiers, data) + + async def delete_one(self, identifiers: dict[str, Any]) -> DeleteResponse: + """Delete the scoped project addressed by ``identifiers`` (``{"id": ...}``). See repository.""" + return await self._projects.delete_one(identifiers) + + async def _resolve_initiative_assignment(self, project_id: str, slug: str | None) -> DBRef | None: + """Validate an initiative assignment and return the link to store (or None to unassign). + + Unassigning needs only project-write access (already enforced downstream). Assigning + additionally requires that the caller can manage the target initiative and that an + unapproved target has room under its member cap. + """ + if slug is None: + return None + + initiative = await self._initiatives.get_one({"slug": slug}) + if initiative is None or initiative.id is None: + raise NotFoundError("Initiative not found or not visible", slug=slug) + + user = self._initiatives._user + if not (user.can_manage(id=slug, resource="initiative") or initiative.owner == user.username): + raise PermissionError( + message="user does not have adequate acceess to this resource", + required_role="initiative-owner-collaborator-or-admin", + resource_id=slug, + ) + + if not initiative.is_approved: + members = await self._projects.count_initiative_members( + initiative_id=initiative.id, + exclude_project_id=project_id, + ) + if members >= self._limits.max_projects_per_unapproved: + raise ConflictError( + message="unapproved initiative already has the maximum number of assigned projects", + slug=slug, + limit=self._limits.max_projects_per_unapproved, + ) + + return DBRef("initiatives", initiative.id) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/structures/router.py b/mpcontribs-api/src/mpcontribs_api/domains/structures/router.py index e27ae2821..998021ebc 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/structures/router.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/structures/router.py @@ -4,7 +4,7 @@ from fastapi.responses import StreamingResponse from fastapi_filter import FilterDepends -from mpcontribs_api.dependencies import S3Dep, require_user +from mpcontribs_api.dependencies import S3Dep, require_user, require_writer from mpcontribs_api.domains._shared.bulk import BulkWriteSummary from mpcontribs_api.domains._shared.models import ComponentDeleteResponse from mpcontribs_api.domains._shared.types import ( @@ -31,14 +31,15 @@ async def get_structures( return await service.get_many(filter=filter, fields=selected, pagination=pagination) -@router.get("/{pk}") -async def get_structure( +@router.get("/{id}") +async def get_one( service: StructureServiceDep, - pk: str, + id: str, fields: FieldSelector = None, ): + """Return a single structure addressed by its ``_id`` or its content ``md5``.""" selected = StructureOut.parse_fields(fields) - return await service.get_by_id(id=pk, fields=selected) + return await service.get_one(identifiers={"id": id}, fields=selected) @router.get("/download/{short_mime}") @@ -68,7 +69,7 @@ async def download_structure( ) -@router.post("", response_model=BulkWriteSummary[StructureOut], dependencies=[Depends(require_user)]) +@router.post("", response_model=BulkWriteSummary[StructureOut], dependencies=[Depends(require_writer)]) async def insert_structures( service: StructureServiceDep, structures: list[StructureIn], @@ -82,14 +83,16 @@ async def delete_structures(service: StructureServiceDep, filter: StructureFilte @router.delete("/{id}", response_model=ComponentDeleteResponse, dependencies=[Depends(require_user)]) -async def delete_structure_by_id(service: StructureServiceDep, id: str): - return await service.delete_by_id(id=id) +async def delete_one(service: StructureServiceDep, id: str): + """Delete a single structure addressed by its ``_id`` or its content ``md5``.""" + return await service.delete_one(identifiers={"id": id}) @router.patch("/{id}", dependencies=[Depends(require_user)]) -async def patch_structure_by_id( +async def patch_one( service: StructureServiceDep, id: str, update: StructurePatch, ): - return await service.patch_by_id(id=id, update=update) + """Patch a single structure addressed by its ``_id`` or its content ``md5``.""" + return await service.patch_one(identifiers={"id": id}, update=update) diff --git a/mpcontribs-api/src/mpcontribs_api/domains/tables/router.py b/mpcontribs-api/src/mpcontribs_api/domains/tables/router.py index 78d49dca0..a9d40f243 100644 --- a/mpcontribs-api/src/mpcontribs_api/domains/tables/router.py +++ b/mpcontribs-api/src/mpcontribs_api/domains/tables/router.py @@ -4,7 +4,7 @@ from fastapi.responses import StreamingResponse from fastapi_filter import FilterDepends -from mpcontribs_api.dependencies import S3Dep, require_user +from mpcontribs_api.dependencies import S3Dep, require_user, require_writer from mpcontribs_api.domains._shared.bulk import BulkWriteSummary from mpcontribs_api.domains._shared.models import ComponentDeleteResponse from mpcontribs_api.domains._shared.types import ( @@ -31,14 +31,15 @@ async def get_tables( return await service.get_many(filter=filter, fields=selected, pagination=pagination) -@router.get("/{pk}") -async def get_table( +@router.get("/{id}") +async def get_one( service: TableServiceDep, - pk: str, + id: str, fields: FieldSelector = None, ): + """Return a single table addressed by its ``_id`` or its content ``md5``.""" selected = TableOut.parse_fields(fields) - return await service.get_by_id(id=pk, fields=selected) + return await service.get_one(identifiers={"id": id}, fields=selected) @router.get("/download/{short_mime}") @@ -68,7 +69,7 @@ async def download_table( ) -@router.post("", response_model=BulkWriteSummary[Table], dependencies=[Depends(require_user)]) +@router.post("", response_model=BulkWriteSummary[Table], dependencies=[Depends(require_writer)]) async def insert_tables( service: TableServiceDep, tables: list[TableIn], @@ -82,14 +83,16 @@ async def delete_tables(service: TableServiceDep, filter: TableFilter = FilterDe @router.delete("/{id}", response_model=ComponentDeleteResponse, dependencies=[Depends(require_user)]) -async def delete_table_by_id(service: TableServiceDep, id: str): - return await service.delete_by_id(id=id) +async def delete_one(service: TableServiceDep, id: str): + """Delete a single table addressed by its ``_id``""" + return await service.delete_one(identifiers={"id": id}) @router.patch("/{id}", dependencies=[Depends(require_user)]) -async def patch_table_by_id( +async def patch_one( service: TableServiceDep, id: str, update: TablePatch, ): - return await service.patch_by_id(id=id, update=update) + """Patch a single table addressed by its ``_id``.""" + return await service.patch_one(identifiers={"id": id}, update=update) diff --git a/mpcontribs-api/tests/integration/db/conftest.py b/mpcontribs-api/tests/integration/db/conftest.py index edc2abc15..0fe90e4f1 100644 --- a/mpcontribs-api/tests/integration/db/conftest.py +++ b/mpcontribs-api/tests/integration/db/conftest.py @@ -7,6 +7,8 @@ from mpcontribs_api.domains.attachments.models import Attachment from mpcontribs_api.domains.consumers.models import Consumer from mpcontribs_api.domains.contributions.models import Contribution +from mpcontribs_api.domains.initiatives.models import Initiative +from mpcontribs_api.domains.project_groups.models import ProjectGroup from mpcontribs_api.domains.projects.models import Project from mpcontribs_api.domains.structures.models import Structure from mpcontribs_api.domains.tables.models import Table @@ -79,7 +81,7 @@ async def db(mongo_client): await database.drop_collection(collection) await init_beanie( database=database, - document_models=[Project, Contribution, Structure, Table, Attachment, Consumer], + document_models=[Project, ProjectGroup, Initiative, Contribution, Structure, Table, Attachment, Consumer], ) yield database @@ -112,6 +114,20 @@ async def clean_components(db): await db[collection].delete_many({}) +@pytest_asyncio.fixture(autouse=True) +async def clean_project_groups(db): + await db["project_groups"].delete_many({}) + yield + await db["project_groups"].delete_many({}) + + +@pytest_asyncio.fixture(autouse=True) +async def clean_initiatives(db): + await db["initiatives"].delete_many({}) + yield + await db["initiatives"].delete_many({}) + + @pytest_asyncio.fixture(autouse=True) async def clean_consumers(db): await db["mp_consumers"].delete_many({}) diff --git a/mpcontribs-api/tests/integration/db/test_component_reachability.py b/mpcontribs-api/tests/integration/db/test_component_reachability.py index eae386fa3..01bff7d3d 100644 --- a/mpcontribs-api/tests/integration/db/test_component_reachability.py +++ b/mpcontribs-api/tests/integration/db/test_component_reachability.py @@ -55,7 +55,7 @@ class TestComponentReadReachability: async def test_get_by_id_returns_reachable_component(self, db): att = await _attachment(1) await _contribution("mp-pub", is_public=True, attachments=[att]) - result = await _service(ANON).get_by_id(str(att.id), fields=None) + result = await _service(ANON).get_one({"id": str(att.id)}, fields=None) assert result is not None assert result.id == att.id @@ -63,13 +63,13 @@ async def test_get_by_id_hides_unreachable_component(self, db): att = await _attachment(2) # Referenced only by a private contribution -> anonymous cannot reach it. await _contribution("mp-priv", is_public=False, attachments=[att]) - result = await _service(ANON).get_by_id(str(att.id), fields=None) + result = await _service(ANON).get_one({"id": str(att.id)}, fields=None) assert result is None async def test_get_by_id_hides_orphan_component(self, db): # No contribution references this attachment at all. att = await _attachment(3) - result = await _service(ANON).get_by_id(str(att.id), fields=None) + result = await _service(ANON).get_one({"id": str(att.id)}, fields=None) assert result is None async def test_get_many_only_lists_reachable(self, db): diff --git a/mpcontribs-api/tests/integration/db/test_components_repository.py b/mpcontribs-api/tests/integration/db/test_components_repository.py index 8a3845aaf..d48b025a0 100644 --- a/mpcontribs-api/tests/integration/db/test_components_repository.py +++ b/mpcontribs-api/tests/integration/db/test_components_repository.py @@ -100,7 +100,7 @@ async def test_single_insert_persists(self, db): # --------------------------------------------------------------------------- -# delete_components / delete_component_by_id +# delete_components / delete_one # --------------------------------------------------------------------------- @@ -113,9 +113,9 @@ async def test_filtered_delete_removes_only_matches(self, db): assert remaining == {keep.md5} async def test_delete_by_id_removes_one(self, db): - """delete_component_by_id matches a string id by converting it to ObjectId.""" + """The inherited base delete_one removes a single component by its primary key.""" [doc] = await _repo().insert_components([_attachment(1)]) - result = await _repo().delete_component_by_id(str(doc.id)) + result = await _repo().delete_one({"id": doc.id}) assert result.num_deleted == 1 assert await _count() == 0 @@ -123,32 +123,58 @@ async def test_delete_by_unknown_id_raises(self, db): from mpcontribs_api.exceptions import NotFoundError with pytest.raises(NotFoundError): - await _repo().delete_component_by_id(str(PydanticObjectId())) + await _repo().delete_one({"id": PydanticObjectId()}) + + async def test_delete_by_md5_removes_one(self, db): + """A component is addressable by its content md5 (its declared identifier) as well as by id.""" + [doc] = await _repo().insert_components([_attachment(1)]) + result = await _repo().delete_one({"md5": doc.md5}) + assert result.num_deleted == 1 + assert await _count() == 0 + + +# --------------------------------------------------------------------------- +# get_one / patch_one address a component by id or by its content md5 +# --------------------------------------------------------------------------- + + +class TestAddressComponentByMd5: + async def test_get_one_by_md5(self, db): + [doc] = await _repo().insert_components([_attachment(1)]) + by_md5 = await _repo().get_one({"md5": doc.md5}, fields=None) + by_id = await _repo().get_one({"id": doc.id}, fields=None) + assert by_md5 is not None + assert by_md5.id == by_id.id == doc.id + + async def test_patch_one_by_md5(self, db): + [doc] = await _repo().insert_components([_attachment(1, name="data.csv")]) + updated = await _repo().patch_one({"md5": doc.md5}, AttachmentPatch(name="renamed.png")) + assert updated.name == "renamed.png" # --------------------------------------------------------------------------- -# patch_component_by_id +# patch_one recomputes the derived md5 # --------------------------------------------------------------------------- class TestPatchComponent: async def test_patch_updates_field(self, db): [doc] = await _repo().insert_components([_attachment(1, name="data.csv")]) - updated = await _repo().patch_component_by_id(str(doc.id), AttachmentPatch(name="renamed.png")) + updated = await _repo().patch_one({"id": doc.id}, AttachmentPatch(name="renamed.png")) assert updated.name == "renamed.png" async def test_empty_patch_returns_existing(self, db): [doc] = await _repo().insert_components([_attachment(1, name="data.csv")]) - updated = await _repo().patch_component_by_id(str(doc.id), AttachmentPatch()) + updated = await _repo().patch_one({"id": doc.id}, AttachmentPatch()) assert updated.id == doc.id async def test_patch_content_recomputes_md5(self, db): # name is not a hash field, so renaming must NOT change md5. [doc] = await _repo().insert_components([_attachment(1)]) - renamed = await _repo().patch_component_by_id(str(doc.id), AttachmentPatch(name="renamed.png")) + renamed = await _repo().patch_one({"id": doc.id}, AttachmentPatch(name="renamed.png")) assert renamed.md5 == doc.md5 # content IS a hash field, so changing it must recompute md5. - rehashed = await _repo().patch_component_by_id(str(doc.id), AttachmentPatch(content=999)) + rehashed = await _repo().patch_one({"id": doc.id}, AttachmentPatch(content=999)) assert rehashed.md5 != doc.md5 persisted = await Attachment.find_one(Attachment.id == doc.id) assert persisted.md5 == rehashed.md5 @@ -215,7 +241,7 @@ async def test_table_frame_round_trips_via_storage_shape(self, db): assert raw["total_data_rows"] == 2 # Read back: reassembled into the same DataFrame (index folded back as the first column). - out = await repo.get_component_by_id(str(doc.id), TableOut.parse_fields(["data"])) + out = await repo.get_one({"id": doc.id}, TableOut.parse_fields(["data"])) assert out.data.columns == ["T [K]", "1e16", "1e17"] assert out.data.equals(frame) # The raw storage keys must not leak onto the response model. diff --git a/mpcontribs-api/tests/integration/db/test_consumers_repository.py b/mpcontribs-api/tests/integration/db/test_consumers_repository.py index 4fd78cec7..9abecdf0b 100644 --- a/mpcontribs-api/tests/integration/db/test_consumers_repository.py +++ b/mpcontribs-api/tests/integration/db/test_consumers_repository.py @@ -25,68 +25,69 @@ def _repo() -> MongoDbConsumerRepository: # --------------------------------------------------------------------------- -# insert_consumer / get_by_consumer_id +# insert_one / get_one (by consumer_id) # --------------------------------------------------------------------------- class TestInsertAndLookup: async def test_insert_then_lookup_by_consumer_id(self, db): - await _repo().insert_consumer(ConsumerIn(consumer_id="kong-1")) - found = await _repo().get_by_consumer_id("kong-1") + await _repo().insert_one(ConsumerIn(consumer_id="kong-1")) + found = await _repo().get_one({"consumer_id": "kong-1"}) assert found is not None assert found.consumer_id == "kong-1" async def test_duplicate_consumer_id_raises_conflict(self, db): - await _repo().insert_consumer(ConsumerIn(consumer_id="kong-dup")) + await _repo().insert_one(ConsumerIn(consumer_id="kong-dup")) with pytest.raises(ConflictError): - await _repo().insert_consumer(ConsumerIn(consumer_id="kong-dup")) + await _repo().insert_one(ConsumerIn(consumer_id="kong-dup")) async def test_lookup_missing_returns_none(self, db): - assert await _repo().get_by_consumer_id("kong-absent") is None + assert await _repo().get_one({"consumer_id": "kong-absent"}) is None async def test_partial_override_snapshots_defaults_for_siblings(self, db): # Admin overrides only max_projects; the stored document must carry a fully-resolved # settings block, with untouched limits snapshotted from the global defaults. - await _repo().insert_consumer( + await _repo().insert_one( ConsumerIn(consumer_id="kong-partial", settings=ConsumerSettings(max_projects=1)) ) - stored = await _repo().get_by_consumer_id("kong-partial") + stored = await _repo().get_one({"consumer_id": "kong-partial"}) assert stored is not None + assert stored.settings is not None assert stored.settings.max_projects == 1 assert stored.settings.max_columns == get_settings().consumer.max_columns # --------------------------------------------------------------------------- -# get_consumer_by_id (document id) +# get_one (document id) # --------------------------------------------------------------------------- class TestGetByDocumentId: async def test_returns_out_model(self, db): - created = await _repo().insert_consumer(ConsumerIn(consumer_id="kong-doc")) - result = await _repo().get_consumer_by_id(id=str(created.id), fields=None) + created = await _repo().insert_one(ConsumerIn(consumer_id="kong-doc")) + result = await _repo().get_one({"id": created.id}, None) assert result is not None assert result.consumer_id == "kong-doc" async def test_missing_returns_none(self, db): from beanie import PydanticObjectId - result = await _repo().get_consumer_by_id(id=str(PydanticObjectId()), fields=None) + result = await _repo().get_one({"id": PydanticObjectId()}, None) assert result is None # --------------------------------------------------------------------------- -# patch_consumer_by_id — partial, sibling-preserving +# patch_one — partial, sibling-preserving # --------------------------------------------------------------------------- class TestPatchConsumer: async def test_patch_changes_only_named_limit(self, db): - created = await _repo().insert_consumer(ConsumerIn(consumer_id="kong-patch")) + created = await _repo().insert_one(ConsumerIn(consumer_id="kong-patch")) original_columns = created.settings.max_columns - updated = await _repo().patch_consumer_by_id( - id=str(created.id), + updated = await _repo().patch_one( + {"id": created.id}, update=ConsumerPatch(settings=ConsumerSettings(max_projects=1)), ) assert updated.settings.max_projects == 1 @@ -94,8 +95,8 @@ async def test_patch_changes_only_named_limit(self, db): assert updated.settings.max_columns == original_columns async def test_empty_patch_returns_existing_unchanged(self, db): - created = await _repo().insert_consumer(ConsumerIn(consumer_id="kong-noop")) - result = await _repo().patch_consumer_by_id(id=str(created.id), update=ConsumerPatch()) + created = await _repo().insert_one(ConsumerIn(consumer_id="kong-noop")) + result = await _repo().patch_one({"id": created.id}, update=ConsumerPatch()) assert result.consumer_id == "kong-noop" assert result.settings.max_projects == created.settings.max_projects @@ -103,28 +104,28 @@ async def test_patch_missing_raises_not_found(self, db): from beanie import PydanticObjectId with pytest.raises(NotFoundError): - await _repo().patch_consumer_by_id( - id=str(PydanticObjectId()), + await _repo().patch_one( + {"id": PydanticObjectId()}, update=ConsumerPatch(settings=ConsumerSettings(max_projects=1)), ) # --------------------------------------------------------------------------- -# delete_consumer_by_id +# delete_one # --------------------------------------------------------------------------- class TestDeleteConsumer: async def test_delete_removes_override(self, db): - created = await _repo().insert_consumer(ConsumerIn(consumer_id="kong-del")) - await _repo().delete_consumer_by_id(id=str(created.id)) - assert await _repo().get_by_consumer_id("kong-del") is None + created = await _repo().insert_one(ConsumerIn(consumer_id="kong-del")) + await _repo().delete_one({"id": created.id}) + assert await _repo().get_one({"consumer_id": "kong-del"}) is None async def test_delete_missing_raises_not_found(self, db): from beanie import PydanticObjectId with pytest.raises(NotFoundError): - await _repo().delete_consumer_by_id(id=str(PydanticObjectId())) + await _repo().delete_one({"id": PydanticObjectId()}) # --------------------------------------------------------------------------- @@ -140,7 +141,7 @@ async def test_no_consumer_id_returns_defaults_without_lookup(self, db): assert limits.max_projects == get_settings().consumer.max_projects async def test_stored_override_is_returned(self, db): - await _repo().insert_consumer( + await _repo().insert_one( ConsumerIn(consumer_id="kong-eff", settings=ConsumerSettings(max_projects=42)) ) user = User(consumer_id="kong-eff", username="google:alice@example.com", groups=frozenset()) diff --git a/mpcontribs-api/tests/integration/db/test_contributions_repository.py b/mpcontribs-api/tests/integration/db/test_contributions_repository.py index a1c24c174..031adf5d1 100644 --- a/mpcontribs-api/tests/integration/db/test_contributions_repository.py +++ b/mpcontribs-api/tests/integration/db/test_contributions_repository.py @@ -79,6 +79,29 @@ def _noop_filter() -> ContributionFilter: return ContributionFilter() +def _identity( + project="test-proj", + material_id="mp-1", + chemical_system_id="Fe-O", + formula="Fe2O3", + unique_value=None, + condition_key="", +) -> dict: + """The full composite natural key (see ``Contribution.identifier_fields``) for a semantic lookup. + + Mirrors the defaults ``_insert``/``_contrib_in`` seed, so ``_identity(material_id=...)`` addresses + a document created with the matching ``identifier=...``. + """ + return { + "project": project, + "material_id": material_id, + "chemical_system_id": chemical_system_id, + "formula": formula, + "unique_value": unique_value, + "condition_key": condition_key, + } + + # --------------------------------------------------------------------------- # insert_contribution (single) # --------------------------------------------------------------------------- @@ -305,64 +328,132 @@ async def test_filter_by_is_public(self, db): assert all(c.is_public is True for c in page.items) # --------------------------------------------------------------------------- -# get_contribution_by_id +# get_one (by id) # --------------------------------------------------------------------------- class TestGetContributionById: async def test_returns_doc_for_valid_id(self, db): doc = await _insert(identifier="get-id") - result = await _repo(ADMIN).get_contribution_by_id(str(doc.id), fields=None) + result = await _repo(ADMIN).get_one({"id": doc.id}, fields=None) assert result is not None assert result.material_id == "get-id" async def test_returns_none_for_missing_id(self, db): - result = await _repo(ADMIN).get_contribution_by_id(str(PydanticObjectId()), fields=None) + result = await _repo(ADMIN).get_one({"id": PydanticObjectId()}, fields=None) assert result is None async def test_admin_can_get_private_doc(self, db): doc = await _insert(identifier="get-priv", is_public=False) - result = await _repo(ADMIN).get_contribution_by_id(str(doc.id), fields=None) + result = await _repo(ADMIN).get_one({"id": doc.id}, fields=None) assert result is not None async def test_anon_cannot_get_private_doc(self, db): doc = await _insert(identifier="get-anon-priv", is_public=False) - result = await _repo(ANON).get_contribution_by_id(str(doc.id), fields=None) + result = await _repo(ANON).get_one({"id": doc.id}, fields=None) assert result is None async def test_anon_can_get_public_doc(self, db): doc = await _insert(identifier="get-anon-pub", is_public=True) - result = await _repo(ANON).get_contribution_by_id(str(doc.id), fields=None) + result = await _repo(ANON).get_one({"id": doc.id}, fields=None) assert result is not None - async def test_raises_validation_error_for_bad_id_format(self, db): - with pytest.raises(ValidationError): - await _repo(ADMIN).get_contribution_by_id("not-an-objectid", fields=None) - async def test_projection_limits_fields(self, db): doc = await _insert(identifier="get-proj", is_public=True) fields = ContributionOut.parse_fields(["formula"]) - result = await _repo(ADMIN).get_contribution_by_id(str(doc.id), fields=fields) + result = await _repo(ADMIN).get_one({"id": doc.id}, fields=fields) assert result is not None assert result.formula == "Fe2O3" assert not hasattr(result, "data") # --------------------------------------------------------------------------- -# patch_contribution_by_id (scoped partial update by id) +# get_one (by the semantic composite identity) +# --------------------------------------------------------------------------- + + +class TestGetContributionBySemanticIdentifiers: + async def test_finds_existing_doc(self, db): + await _insert(project="find-proj", identifier="find-id") + result = await _repo(ADMIN).get_one(_identity(project="find-proj", material_id="find-id"), fields=None) + assert result is not None + assert result.project == "find-proj" + assert result.material_id == "find-id" + + async def test_returns_none_for_missing_combination(self, db): + await _insert(project="miss-proj", identifier="miss-id") + result = await _repo(ADMIN).get_one(_identity(project="miss-proj", material_id="wrong-id"), fields=None) + assert result is None + + async def test_scope_prevents_anon_finding_private(self, db): + await _insert(project="anon-scope", identifier="priv-doc", is_public=False) + result = await _repo(ANON).get_one(_identity(project="anon-scope", material_id="priv-doc"), fields=None) + assert result is None + + async def test_scope_allows_anon_finding_public(self, db): + await _insert(project="anon-scope-pub", identifier="pub-doc", is_public=True) + result = await _repo(ANON).get_one(_identity(project="anon-scope-pub", material_id="pub-doc"), fields=None) + assert result is not None + + async def test_composite_identity_is_unique_lookup(self, db): + await _insert(project="same-proj", identifier="id-a") + await _insert(project="same-proj", identifier="id-b") + result = await _repo(ADMIN).get_one(_identity(project="same-proj", material_id="id-a"), fields=None) + assert result is not None + assert result.material_id == "id-a" + + async def test_partial_identifier_set_is_rejected(self, db): + # The semantic set must be the complete composite key, not a subset. + await _insert(project="partial-proj", identifier="partial-id") + with pytest.raises(ValidationError): + await _repo(ADMIN).get_one({"project": "partial-proj", "material_id": "partial-id"}, fields=None) + + +# --------------------------------------------------------------------------- +# patch_one (scoped partial update by id or composite identity) # --------------------------------------------------------------------------- class TestPatchContributionById: + async def test_updates_formula(self, db): + doc = await _insert(identifier="patch-formula") + await _repo(ADMIN).patch_one({"id": doc.id}, ContributionPatch(formula="Li2O")) + found = await Contribution.find_one(Contribution.id == doc.id) + assert found.formula == "Li2O" + + async def test_unset_fields_not_overwritten(self, db): + doc = await _insert(identifier="patch-preserve", formula="Fe2O3") + await _repo(ADMIN).patch_one({"id": doc.id}, ContributionPatch(needs_build=False)) + found = await Contribution.find_one(Contribution.id == doc.id) + assert found.formula == "Fe2O3" + + async def test_empty_patch_is_a_noop(self, db): + doc = await _insert(identifier="patch-empty", formula="Fe2O3") + result = await _repo(ADMIN).patch_one({"id": doc.id}, ContributionPatch()) + assert result is not None + found = await Contribution.find_one(Contribution.id == doc.id) + assert found.formula == "Fe2O3" + + async def test_patch_by_semantic_identifiers(self, db): + # The same patch reachable through the full composite identity. + await _insert(project="patch-sem", identifier="sem-id", formula="Fe2O3") + await _repo(ADMIN).patch_one( + _identity(project="patch-sem", material_id="sem-id"), ContributionPatch(formula="Li2O") + ) + found = await Contribution.find_one(Contribution.project == "patch-sem") + assert found.formula == "Li2O" + async def test_raises_validation_error_for_bad_id(self, db): + repo = _repo(ADMIN) with pytest.raises(ValidationError): - await _repo(ADMIN).patch_contribution_by_id("bad-id", ContributionPatch(formula="Fe2O3")) + await repo.patch_one(repo.coerce_identifiers({"id": "bad-id"}), ContributionPatch(formula="Fe2O3")) async def test_anon_cannot_patch_private_doc(self, db): from mpcontribs_api.exceptions import NotFoundError + doc = await _insert(identifier="patch-anon-priv", is_public=False) with pytest.raises(NotFoundError): - await _repo(ANON).patch_contribution_by_id(str(doc.id), ContributionPatch(formula="Fe2O3")) + await _repo(ANON).patch_one({"id": doc.id}, ContributionPatch(formula="Fe2O3")) async def test_patch_onto_existing_identity_raises_conflict(self, db): # Two contributions differing only by material_id, so distinct identities. @@ -371,33 +462,35 @@ async def test_patch_onto_existing_identity_raises_conflict(self, db): # Patching victim's material_id onto the first doc's makes the identities collide; the unique # index rejects the write, which the repo surfaces as a ConflictError (409) not a raw 500. with pytest.raises(ConflictError): - await _repo(ADMIN).patch_contribution_by_id(str(victim.id), ContributionPatch(material_id="mp-100")) + await _repo(ADMIN).patch_one({"id": victim.id}, ContributionPatch(material_id="mp-100")) # --------------------------------------------------------------------------- -# delete_contribution_by_id +# delete_one (by id) # --------------------------------------------------------------------------- class TestDeleteContributionById: async def test_deleted_doc_not_found_afterwards(self, db): doc = await _insert(identifier="del-me") - await _repo(ADMIN).delete_contribution_by_id(str(doc.id)) + await _repo(ADMIN).delete_one({"id": doc.id}) found = await Contribution.find_one(Contribution.id == doc.id) assert found is None async def test_delete_nonexistent_throws_error(self, db): with pytest.raises(NotFoundError, match="not found"): - await _repo(ADMIN).delete_contribution_by_id(str(PydanticObjectId())) + await _repo(ADMIN).delete_one({"id": PydanticObjectId()}) - async def test_raises_validation_error_for_bad_id(self, db): - with pytest.raises(ValidationError): - await _repo(ADMIN).delete_contribution_by_id("not-an-id") + async def test_delete_by_semantic_identifiers(self, db): + await _insert(project="del-sem", identifier="mp-9001") + await _repo(ADMIN).delete_one(_identity(project="del-sem", material_id="mp-9001")) + found = await Contribution.find_one(Contribution.project == "del-sem") + assert found is None async def test_anon_cannot_delete_private_doc(self, db): doc = await _insert(identifier="del-anon-priv", is_public=False) with pytest.raises(NotFoundError, match="not found"): - await _repo(ANON).delete_contribution_by_id(str(doc.id)) + await _repo(ANON).delete_one({"id": doc.id}) # Scope prevents anonymous from seeing the doc, so it is never deleted. still_there = await Contribution.find_one(Contribution.id == doc.id) assert still_there is not None @@ -450,14 +543,24 @@ async def test_insert_when_id_absent_persists_document(self, db): assert stored.material_id == "mp-4002" async def test_update_when_id_present_applies_change(self, db): - existing = await _insert(identifier="mp-4003") - payload = _contrib_in(identifier="mp-4003", formula="Li2O", _id=existing.id) + existing = await _insert(identifier="mp-4001") + payload = _contrib_in(identifier="mp-4001", formula="Li2O", _id=existing.id) result = await _repo(ADMIN).upsert_contribution_by_id(str(existing.id), payload) assert isinstance(result, Contribution) stored = await Contribution.find_one(Contribution.id == existing.id) assert stored is not None assert stored.formula == "Li2O" + async def test_upsert_by_identity_updates_in_place(self, db): + # The identity-keyed upsert (upsert_one) targets the composite natural key, not the _id, and + # updates the matching document in place rather than creating a duplicate. + await _insert(project="ups-sem", identifier="mp-5001", formula="Fe2O3") + payload = _contrib_in(project="ups-sem", identifier="mp-5001", formula="Fe2O3") + result = await _repo(ADMIN).upsert_one(_identity(project="ups-sem", material_id="mp-5001"), payload) + assert isinstance(result, Contribution) + count = await Contribution.find(Contribution.project == "ups-sem").count() + assert count == 1 + async def test_update_clears_unique_value_when_resolved_to_none(self, db): # A doc previously stamped with a unique_value whose project later drops its unique_column: # re-resolving to None must clear the stored value, not leave it stale (exclude_none would). diff --git a/mpcontribs-api/tests/integration/db/test_initiatives_repository.py b/mpcontribs-api/tests/integration/db/test_initiatives_repository.py new file mode 100644 index 000000000..5881002c0 --- /dev/null +++ b/mpcontribs-api/tests/integration/db/test_initiatives_repository.py @@ -0,0 +1,233 @@ +import pytest + +from mpcontribs_api.authz import User +from mpcontribs_api.config import get_settings +from mpcontribs_api.domains.initiatives.models import ( + Initiative, + InitiativeFilter, + InitiativeIn, + InitiativePatch, +) +from mpcontribs_api.domains.initiatives.repository import InitiativeRepository +from mpcontribs_api.exceptions import ConflictError, NotFoundError, PermissionError, ValidationError +from mpcontribs_api.pagination import CursorParams + +# Share the session event loop (see the projects repo test for why). +pytestmark = [pytest.mark.db, pytest.mark.asyncio(loop_scope="session")] + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +ADMIN = User(username="google:admin@example.com", groups=frozenset({"admin"})) +ALICE = User(username="google:alice@example.com", groups=frozenset({"mp-team"})) +BOB = User(username="google:bob@example.com", groups=frozenset({"mp-team"})) +ANON = User() + +ALICE_EMAIL = "google:alice@example.com" +BOB_EMAIL = "google:bob@example.com" + + +def _repo(user: User) -> InitiativeRepository: + return InitiativeRepository(user) + + +def _collaborator(slug: str, username: str = BOB_EMAIL) -> User: + """A user whose role grants them collaborator rights on ``slug``.""" + return User(username=username, groups=frozenset({f"initiative:{slug}"})) + + +async def _insert(slug: str, owner_user: User = ALICE, name: str = "An Initiative") -> Initiative: + return await _repo(owner_user).insert_initiative(InitiativeIn(slug=slug, name=name)) + + +async def _approve(slug: str) -> Initiative: + return await _repo(ADMIN).patch_one({"slug": slug}, InitiativePatch(is_approved=True)) + + +# --------------------------------------------------------------------------- +# Create + owner forcing +# --------------------------------------------------------------------------- + + +class TestInsert: + async def test_forces_owner_and_starts_private_unapproved(self, db): + created = await _insert("battery-genome", ALICE) + assert created.owner == ALICE_EMAIL + assert created.is_public is False + assert created.is_approved is False + + async def test_duplicate_slug_is_conflict(self, db): + await _insert("dup-slug", ALICE) + with pytest.raises(ConflictError): + await _insert("dup-slug", BOB) # globally unique, even across owners + + async def test_anonymous_cannot_create(self, db): + with pytest.raises(PermissionError): + await _repo(ANON).insert_initiative(InitiativeIn(slug="anon-init", name="x")) + + async def test_invalid_slug_rejected(self, db): + with pytest.raises(ValidationError): + InitiativeIn(slug="Not A Slug!", name="x") + + +class TestUnapprovedPerOwnerLimit: + async def test_owner_capped_at_configured_unapproved(self, db): + limit = get_settings().domain.initiatives.max_unapproved_per_owner + for i in range(limit): + await _insert(f"cap-{i}", ALICE) + with pytest.raises(ConflictError): + await _insert("cap-over", ALICE) + + async def test_approved_do_not_count_against_quota(self, db): + limit = get_settings().domain.initiatives.max_unapproved_per_owner + for i in range(limit): + await _insert(f"quota-{i}", ALICE) + await _approve("quota-0") # frees a slot + # A fresh unapproved initiative now fits again. + assert await _insert("quota-extra", ALICE) is not None + + async def test_admin_is_exempt(self, db): + limit = get_settings().domain.initiatives.max_unapproved_per_owner + for i in range(limit + 2): + await _repo(ADMIN).insert_initiative(InitiativeIn(slug=f"admin-{i}", name="x")) + + +# --------------------------------------------------------------------------- +# Approval + public invariant +# --------------------------------------------------------------------------- + + +class TestApprovalAndPublic: + async def test_only_admin_may_approve(self, db): + await _insert("approve-me", ALICE) + with pytest.raises(PermissionError): + await _repo(ALICE).patch_one({"slug": "approve-me"}, InitiativePatch(is_approved=True)) + approved = await _approve("approve-me") + assert approved.is_approved is True + + async def test_cannot_make_public_while_unapproved(self, db): + await _insert("public-fail", ALICE) + with pytest.raises(ValidationError): + await _repo(ALICE).patch_one({"slug": "public-fail"}, InitiativePatch(is_public=True)) + + async def test_public_allowed_once_approved(self, db): + await _insert("public-ok", ALICE) + await _approve("public-ok") + patched = await _repo(ALICE).patch_one({"slug": "public-ok"}, InitiativePatch(is_public=True)) + assert patched.is_public is True + + async def test_admin_can_approve_and_publish_together(self, db): + await _insert("publish-both", ALICE) + patched = await _repo(ADMIN).patch_one({"slug": "publish-both"}, InitiativePatch(is_approved=True, is_public=True)) + assert patched.is_approved is True and patched.is_public is True + + +# --------------------------------------------------------------------------- +# Manage rights (patch) + read scope +# --------------------------------------------------------------------------- + + +class TestManageAndScope: + async def test_owner_can_rename(self, db): + await _insert("rename-me", ALICE) + patched = await _repo(ALICE).patch_one({"slug": "rename-me"}, InitiativePatch(name="Renamed")) + assert patched.name == "Renamed" + + async def test_collaborator_can_patch(self, db): + await _insert("collab-patch", ALICE) + patched = await _repo(_collaborator("collab-patch")).patch_one({"slug": "collab-patch"}, InitiativePatch(name="By Collaborator")) + assert patched.name == "By Collaborator" + + async def test_visible_but_unmanaged_cannot_patch(self, db): + # An approved+public initiative is visible to everyone, but a stranger still cannot manage it. + await _insert("visible-public", ALICE) + await _approve("visible-public") + await _repo(ALICE).patch_one({"slug": "visible-public"}, InitiativePatch(is_public=True)) + stranger = User(username="google:carol@example.com", groups=frozenset()) + with pytest.raises(PermissionError): + await _repo(stranger).patch_one({"slug": "visible-public"}, InitiativePatch(name="hijack")) + + async def test_private_unapproved_scope(self, db): + await _insert("scoped-priv", ALICE) + assert await _repo(ALICE).get_one({"slug": "scoped-priv"}, fields=None) is not None # owner + assert await _repo(ADMIN).get_one({"slug": "scoped-priv"}, fields=None) is not None # admin + assert await _repo(_collaborator("scoped-priv")).get_one({"slug": "scoped-priv"}, fields=None) is not None + assert await _repo(ANON).get_one({"slug": "scoped-priv"}, fields=None) is None # anon + assert await _repo(BOB).get_one({"slug": "scoped-priv"}, fields=None) is None # unrelated user + + async def test_public_approved_visible_to_anon(self, db): + await _insert("scoped-pub", ALICE) + await _approve("scoped-pub") + await _repo(ALICE).patch_one({"slug": "scoped-pub"}, InitiativePatch(is_public=True)) + assert await _repo(ANON).get_one({"slug": "scoped-pub"}, fields=None) is not None + + +# --------------------------------------------------------------------------- +# Delete (owner or admin only) +# --------------------------------------------------------------------------- + + +class TestDelete: + async def test_owner_can_delete(self, db): + await _insert("del-owner", ALICE) + result = await _repo(ALICE).delete_one({"slug": "del-owner"}) + assert result.num_deleted == 1 + assert await _repo(ADMIN).get_one({"slug": "del-owner"}, fields=None) is None + + async def test_collaborator_cannot_delete(self, db): + await _insert("del-collab", ALICE) + with pytest.raises(PermissionError): + await _repo(_collaborator("del-collab")).delete_one({"slug": "del-collab"}) + + async def test_missing_is_not_found(self, db): + with pytest.raises(NotFoundError): + await _repo(ADMIN).delete_one({"slug": "nope-missing"}) + + +# --------------------------------------------------------------------------- +# Listing + filtering (scoped) +# --------------------------------------------------------------------------- + + +class TestListAndFilter: + async def test_list_scoped_to_caller(self, db): + await _insert("mine-1", ALICE) + await _insert("bobs-1", BOB) # Bob's private initiative, invisible to Alice + page = await _repo(ALICE).get_initiatives(CursorParams(), InitiativeFilter(), fields=None) + slugs = {i.slug for i in page.items} + assert "mine-1" in slugs + assert "bobs-1" not in slugs + + async def test_filter_by_is_approved(self, db): + await _insert("appr-1", ALICE) + await _insert("unappr-1", ALICE) + await _approve("appr-1") + page = await _repo(ADMIN).get_initiatives(CursorParams(), InitiativeFilter(is_approved=True), fields=None) + slugs = {i.slug for i in page.items} + assert "appr-1" in slugs + assert "unappr-1" not in slugs + + async def test_filter_by_owner(self, db): + await _insert("owned-alice", ALICE) + await _insert("owned-bob", BOB) + page = await _repo(ADMIN).get_initiatives(CursorParams(), InitiativeFilter(owner=BOB_EMAIL), fields=None) + assert {i.slug for i in page.items} == {"owned-bob"} + + +# --------------------------------------------------------------------------- +# Admin bypass +# --------------------------------------------------------------------------- + + +class TestAdminBypass: + async def test_admin_can_patch_non_owned(self, db): + await _insert("admin-patch", ALICE) + patched = await _repo(ADMIN).patch_one({"slug": "admin-patch"}, InitiativePatch(name="Admin Renamed")) + assert patched.name == "Admin Renamed" + + async def test_admin_can_delete_non_owned(self, db): + await _insert("admin-del", ALICE) + result = await _repo(ADMIN).delete_one({"slug": "admin-del"}) + assert result.num_deleted == 1 diff --git a/mpcontribs-api/tests/integration/db/test_initiatives_service.py b/mpcontribs-api/tests/integration/db/test_initiatives_service.py new file mode 100644 index 000000000..2a59037d4 --- /dev/null +++ b/mpcontribs-api/tests/integration/db/test_initiatives_service.py @@ -0,0 +1,203 @@ +import pytest +from beanie import Link + +from mpcontribs_api.authz import User +from mpcontribs_api.config import get_settings +from mpcontribs_api.domains.initiatives.models import InitiativeIn, InitiativePatch +from mpcontribs_api.domains.initiatives.repository import InitiativeRepository +from mpcontribs_api.domains.projects.models import Project, ProjectIn, ProjectPatch +from mpcontribs_api.domains.projects.repository import MongoDbProjectRepository +from mpcontribs_api.domains.projects.service import ProjectService +from mpcontribs_api.exceptions import ConflictError, NotFoundError, PermissionError + +pytestmark = [pytest.mark.db, pytest.mark.asyncio(loop_scope="session")] + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +ADMIN = User(username="google:admin@example.com", groups=frozenset({"admin"})) +ALICE = User(username="google:alice@example.com", groups=frozenset({"mp-team"})) +CAROL = User(username="google:carol@example.com", groups=frozenset()) + +ALICE_EMAIL = "google:alice@example.com" +BOB_EMAIL = "google:bob@example.com" +CAROL_EMAIL = "google:carol@example.com" + + +def _service(user: User) -> ProjectService: + return ProjectService( + projects=MongoDbProjectRepository(user), + initiatives=InitiativeRepository(user), + ) + + +def _collaborator(slug: str, username: str = BOB_EMAIL) -> User: + return User(username=username, groups=frozenset({f"initiative:{slug}"})) + + +async def _insert_project(pid: str, owner: str = ALICE_EMAIL) -> Project: + return await MongoDbProjectRepository(ADMIN).insert_project( + pid, + ProjectIn( + title=pid[:30], + authors="Author", + description="desc", + owner=owner, + ), + ) + + +async def _insert_initiative(slug: str, owner_user: User = ALICE): + return await InitiativeRepository(owner_user).insert_initiative(InitiativeIn(slug=slug, name="Init")) + + +def _assigned_id(project: Project): + """The initiative _id a returned project points at, or None.""" + link = project.initiative + if link is None: + return None + return link.ref.id if isinstance(link, Link) else link.id + + +# --------------------------------------------------------------------------- +# Happy-path assignment +# --------------------------------------------------------------------------- + + +class TestAssign: + async def test_owner_of_both_can_assign(self, db): + await _insert_project("proj-a", owner=ALICE_EMAIL) + init = await _insert_initiative("init-a", ALICE) + updated = await _service(ALICE).patch_one({"id": "proj-a"}, ProjectPatch(initiative="init-a")) + assert _assigned_id(updated) == init.id + + async def test_collaborator_can_assign_own_project(self, db): + await _insert_project("proj-b", owner=BOB_EMAIL) + init = await _insert_initiative("init-collab", ALICE) + bob = _collaborator("init-collab") + updated = await _service(bob).patch_one({"id": "proj-b"}, ProjectPatch(initiative="init-collab")) + assert _assigned_id(updated) == init.id + + async def test_plain_patch_passes_through_untouched(self, db): + await _insert_project("proj-plain", owner=ALICE_EMAIL) + init = await _insert_initiative("init-plain", ALICE) + await _service(ALICE).patch_one({"id": "proj-plain"}, ProjectPatch(initiative="init-plain")) + # A patch that does not mention `initiative` must not disturb the existing assignment. + updated = await _service(ALICE).patch_one({"id": "proj-plain"}, ProjectPatch(title="new-title")) + assert updated.title == "new-title" + assert _assigned_id(updated) == init.id + + async def test_unassign_clears_link(self, db): + await _insert_project("proj-un", owner=ALICE_EMAIL) + await _insert_initiative("init-un", ALICE) + await _service(ALICE).patch_one({"id": "proj-un"}, ProjectPatch(initiative="init-un")) + updated = await _service(ALICE).patch_one({"id": "proj-un"}, ProjectPatch(initiative=None)) + assert _assigned_id(updated) is None + + +# --------------------------------------------------------------------------- +# Both-rights enforcement +# --------------------------------------------------------------------------- + + +class TestBothRights: + async def test_visible_but_unmanaged_initiative_rejected(self, db): + # Carol owns her project (project-write ok) and can *see* this public+approved initiative, + # but she neither owns nor collaborates on it, so she still cannot assign to it. + await _insert_project("proj-c", owner=CAROL_EMAIL) + await _insert_initiative("init-c", ALICE) + await InitiativeRepository(ADMIN).patch_one( + {"slug": "init-c"}, InitiativePatch(is_approved=True, is_public=True) + ) + with pytest.raises(PermissionError): + await _service(CAROL).patch_one({"id": "proj-c"}, ProjectPatch(initiative="init-c")) + + async def test_invisible_initiative_is_not_found(self, db): + # Alice's private initiative is invisible to Carol, so it reads as not-found (not a 403). + await _insert_project("proj-c2", owner=CAROL_EMAIL) + await _insert_initiative("init-priv", ALICE) + with pytest.raises(NotFoundError): + await _service(CAROL).patch_one({"id": "proj-c2"}, ProjectPatch(initiative="init-priv")) + + async def test_manager_without_project_write_rejected(self, db): + # Alice manages the initiative but cannot see/write Bob's private project. + await _insert_project("proj-bob", owner=BOB_EMAIL) + await _insert_initiative("init-d", ALICE) + with pytest.raises(NotFoundError): + await _service(ALICE).patch_one({"id": "proj-bob"}, ProjectPatch(initiative="init-d")) + + async def test_assign_to_missing_initiative_is_not_found(self, db): + await _insert_project("proj-ghost", owner=ALICE_EMAIL) + with pytest.raises(NotFoundError): + await _service(ALICE).patch_one({"id": "proj-ghost"}, ProjectPatch(initiative="ghost-init")) + + +# --------------------------------------------------------------------------- +# Member cap on unapproved initiatives +# --------------------------------------------------------------------------- + + +class TestMemberCap: + async def test_unapproved_capped_at_configured_members(self, db): + cap = get_settings().domain.initiatives.max_projects_per_unapproved + await _insert_initiative("init-cap", ALICE) + for i in range(cap): + await _insert_project(f"cap-proj-{i}", owner=ALICE_EMAIL) + await _service(ALICE).patch_one({"id": f"cap-proj-{i}"}, ProjectPatch(initiative="init-cap")) + await _insert_project("cap-proj-over", owner=ALICE_EMAIL) + with pytest.raises(ConflictError): + await _service(ALICE).patch_one({"id": "cap-proj-over"}, ProjectPatch(initiative="init-cap")) + + async def test_reassigning_existing_member_is_idempotent(self, db): + cap = get_settings().domain.initiatives.max_projects_per_unapproved + await _insert_initiative("init-idem", ALICE) + for i in range(cap): + await _insert_project(f"idem-proj-{i}", owner=ALICE_EMAIL) + await _service(ALICE).patch_one({"id": f"idem-proj-{i}"}, ProjectPatch(initiative="init-idem")) + # At the cap, re-assigning a project that is already a member must not trip the limit. + again = await _service(ALICE).patch_one({"id": "idem-proj-0"}, ProjectPatch(initiative="init-idem")) + assert again.initiative is not None + + async def test_approved_initiative_has_no_member_cap(self, db, monkeypatch): + cap = get_settings().domain.initiatives.max_projects_per_unapproved + # Lift the orthogonal per-user project quota so seeding cap+2 owned projects doesn't trip it; + # this test isolates the *initiative member* cap, not the project-count cap. + monkeypatch.setattr(get_settings().consumer, "max_projects", cap + 5) + await _insert_initiative("init-approved", ALICE) + await InitiativeRepository(ADMIN).patch_one({"slug": "init-approved"}, InitiativePatch(is_approved=True)) + for i in range(cap + 2): # comfortably past the unapproved cap + await _insert_project(f"appr-proj-{i}", owner=ALICE_EMAIL) + await _service(ALICE).patch_one({"id": f"appr-proj-{i}"}, ProjectPatch(initiative="init-approved")) + count = await MongoDbProjectRepository(ADMIN).count_initiative_members( + initiative_id=(await InitiativeRepository(ADMIN).get_one({"slug": "init-approved"})).id, # type: ignore[union-attr] + exclude_project_id=None, + ) + assert count == cap + 2 + + +# --------------------------------------------------------------------------- +# Admin bypass + unassignment rights +# --------------------------------------------------------------------------- + + +class TestAdminAndUnassign: + async def test_admin_can_assign_to_any_initiative(self, db): + # Alice's private initiative is manageable by an admin even though the admin holds no role. + await _insert_project("adm-proj", owner=ALICE_EMAIL) + init = await _insert_initiative("adm-init", ALICE) + updated = await _service(ADMIN).patch_one({"id": "adm-proj"}, ProjectPatch(initiative="adm-init")) + assert _assigned_id(updated) == init.id + + async def test_project_owner_can_unassign_without_initiative_rights(self, db): + # A collaborator assigns Bob's project; Bob, lacking any initiative role, can still detach + # his own project — unassignment needs only project-write access. + await _insert_project("detach-proj", owner=BOB_EMAIL) + await _insert_initiative("detach-init", ALICE) + await _service(_collaborator("detach-init", username=BOB_EMAIL)).patch_one( + {"id": "detach-proj"}, ProjectPatch(initiative="detach-init") + ) + bob_plain = User(username=BOB_EMAIL, groups=frozenset()) + updated = await _service(bob_plain).patch_one({"id": "detach-proj"}, ProjectPatch(initiative=None)) + assert _assigned_id(updated) is None diff --git a/mpcontribs-api/tests/integration/db/test_project_groups_repository.py b/mpcontribs-api/tests/integration/db/test_project_groups_repository.py new file mode 100644 index 000000000..739cdaa52 --- /dev/null +++ b/mpcontribs-api/tests/integration/db/test_project_groups_repository.py @@ -0,0 +1,247 @@ +import pytest +from beanie import PydanticObjectId + +from mpcontribs_api.authz import User +from mpcontribs_api.domains.project_groups.models import ( + ProjectGroup, + ProjectGroupFilter, + ProjectGroupIn, + ProjectGroupPatch, +) +from mpcontribs_api.domains.project_groups.repository import ProjectGroupRepository +from mpcontribs_api.exceptions import ConflictError, NotFoundError, PermissionError, ValidationError +from mpcontribs_api.pagination import CursorParams + +# Share the session event loop (see the projects repo test for why). +pytestmark = [pytest.mark.db, pytest.mark.asyncio(loop_scope="session")] + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +ADMIN = User(username="google:admin@example.com", groups=frozenset({"admin"})) +ALICE = User(username="google:alice@example.com", groups=frozenset({"mp-team"})) +BOB = User(username="google:bob@example.com", groups=frozenset()) +ANON = User() + +ALICE_EMAIL = "google:alice@example.com" +BOB_EMAIL = "google:bob@example.com" + + +def _repo(user: User) -> ProjectGroupRepository: + return ProjectGroupRepository(user) + + +def _group_in(name: str, owner: str = ALICE_EMAIL, **overrides) -> ProjectGroupIn: + defaults = { + "name": name, + "owner": owner, + "projects": [], + "description": "a group", + } + defaults.update(overrides) + return ProjectGroupIn(**defaults) + + +async def _insert(name: str, owner: str = ALICE_EMAIL, **overrides) -> ProjectGroup: + return await _repo(ADMIN).insert_project_group(_group_in(name, owner, **overrides)) + + +# --------------------------------------------------------------------------- +# get_one +# --------------------------------------------------------------------------- + + +class TestGetOne: + async def test_returns_group_by_identifiers(self, db): + await _insert("group-a") + found = await _repo(ADMIN).get_one({"name": "group-a", "owner": ALICE_EMAIL}, fields=None) + assert found is not None + assert found.name == "group-a" + assert found.owner == ALICE_EMAIL + + async def test_returns_none_when_absent(self, db): + found = await _repo(ADMIN).get_one({"name": "missing", "owner": ALICE_EMAIL}, fields=None) + assert found is None + + async def test_out_of_scope_returns_none(self, db): + # Alice's private group is invisible to an anonymous caller. + await _insert("group-priv") + found = await _repo(ANON).get_one({"name": "group-priv", "owner": ALICE_EMAIL}, fields=None) + assert found is None + + +# --------------------------------------------------------------------------- +# project-group: role scoping +# --------------------------------------------------------------------------- + + +def _role_user(group_id, username: str = "google:carol@example.com") -> User: + """A non-owner authenticated user granted access to one group via its project-group role.""" + return User(username=username, groups=frozenset({f"project-group:{group_id}"})) + + +class TestGroupRoleScope: + async def test_role_grants_visibility(self, db): + group = await _insert("role-vis") # Alice's private group + found = await _repo(_role_user(group.id)).get_one({"name": "role-vis", "owner": ALICE_EMAIL}, fields=None) + assert found is not None + assert found.id == group.id + + async def test_without_role_not_visible(self, db): + await _insert("role-none") + found = await _repo(BOB).get_one({"name": "role-none", "owner": ALICE_EMAIL}, fields=None) + assert found is None + + async def test_malformed_role_is_ignored(self, db): + await _insert("role-bad") + member = User(username="google:carol@example.com", groups=frozenset({"project-group:not-an-oid"})) + # A malformed role id must not raise; it simply grants nothing. + found = await _repo(member).get_one({"name": "role-bad", "owner": ALICE_EMAIL}, fields=None) + assert found is None + + async def test_role_appears_in_listing(self, db): + group = await _insert("role-list") + page = await _repo(_role_user(group.id)).get_project_groups( + pagination=CursorParams(), filter=ProjectGroupFilter(), fields=None + ) + assert group.id in {g.id for g in page.items} + + async def test_role_grants_scope_but_not_delete(self, db): + # Scope makes the group visible, but deletion remains owner-or-admin (403 for a role holder). + group = await _insert("role-del") + with pytest.raises(PermissionError): + await _repo(_role_user(group.id)).delete_one({"name": "role-del", "owner": ALICE_EMAIL}) + assert await ProjectGroup.find_one(ProjectGroup.name == "role-del") is not None + + +# --------------------------------------------------------------------------- +# delete_one (identifier-keyed, single-resource, raises) +# --------------------------------------------------------------------------- + + +class TestDeleteOne: + async def test_deletes_matching_group(self, db): + await _insert("del-a") + result = await _repo(ADMIN).delete_one({"name": "del-a", "owner": ALICE_EMAIL}) + assert result.num_deleted == 1 + assert await ProjectGroup.find_one(ProjectGroup.name == "del-a") is None + + async def test_absent_raises_not_found(self, db): + with pytest.raises(NotFoundError): + await _repo(ADMIN).delete_one({"name": "nope", "owner": ALICE_EMAIL}) + + async def test_out_of_scope_raises_not_found(self, db): + # Alice's group is out of scope for anon, so it "does not exist" for them. + await _insert("del-scoped") + with pytest.raises(NotFoundError): + await _repo(ANON).delete_one({"name": "del-scoped", "owner": ALICE_EMAIL}) + # ...and it is untouched. + assert await ProjectGroup.find_one(ProjectGroup.name == "del-scoped") is not None + + async def test_owner_can_delete_own(self, db): + await _insert("del-own", owner=ALICE_EMAIL) + result = await _repo(ALICE).delete_one({"name": "del-own", "owner": ALICE_EMAIL}) + assert result.num_deleted == 1 + + async def test_visible_public_non_owner_forbidden(self, db): + # Bob can *see* Alice's public group but does not own it → 403, and it is left intact. + await _insert("del-pub", owner=ALICE_EMAIL, is_public=True) + with pytest.raises(PermissionError): + await _repo(BOB).delete_one({"name": "del-pub", "owner": ALICE_EMAIL}) + assert await ProjectGroup.find_one(ProjectGroup.name == "del-pub") is not None + + async def test_wrong_identifier_keys_raise_validation(self, db): + with pytest.raises(ValidationError): + await _repo(ADMIN).delete_one({"name": "x"}) # missing 'owner' + + +# --------------------------------------------------------------------------- +# patch_one +# --------------------------------------------------------------------------- + + +class TestPatchOne: + async def test_updates_field(self, db): + await _insert("patch-a", description="before") + updated = await _repo(ADMIN).patch_one({"name": "patch-a", "owner": ALICE_EMAIL}, ProjectGroupPatch(description="after")) + assert updated.description == "after" + + async def test_absent_raises_not_found(self, db): + with pytest.raises(NotFoundError): + await _repo(ADMIN).patch_one({"name": "ghost", "owner": ALICE_EMAIL}, ProjectGroupPatch(description="x")) + + +# --------------------------------------------------------------------------- +# delete (arbitrary-filter bulk) +# --------------------------------------------------------------------------- + + +class TestDeleteByFilter: + async def test_bulk_deletes_all_matching_owner(self, db): + await _insert("bulk-1") + await _insert("bulk-2") + await _insert("other", owner="google:bob@example.com") + result = await _repo(ADMIN).delete_project_groups( + filter=ProjectGroupFilter(owner=ALICE_EMAIL) + ) + assert result.num_deleted == 2 + assert await ProjectGroup.find_one(ProjectGroup.owner == "google:bob@example.com") is not None + + async def test_no_match_returns_zero(self, db): + result = await _repo(ADMIN).delete_project_groups( + filter=ProjectGroupFilter(owner="google:nobody@example.com") + ) + assert result.num_deleted == 0 + + async def test_non_admin_bulk_restricted_to_own(self, db): + # A broad filter from a non-admin is pinned to their own groups: a public group owned by + # someone else must survive even though the filter would otherwise match it. + await _insert("own-bulk", owner=ALICE_EMAIL, is_public=True) + await _insert("other-bulk", owner=BOB_EMAIL, is_public=True) + result = await _repo(ALICE).delete_project_groups(filter=ProjectGroupFilter(is_public=True)) + assert result.num_deleted == 1 + assert await ProjectGroup.find_one(ProjectGroup.name == "own-bulk") is None + assert await ProjectGroup.find_one(ProjectGroup.name == "other-bulk") is not None + + +# --------------------------------------------------------------------------- +# insert_project_group +# +# The input model carries no ``_id`` (the server assigns the ObjectId) and takes +# plain project ids, which from_input_model resolves into stored Links/DBRefs. +# --------------------------------------------------------------------------- + + +class TestInsertProjectGroup: + async def test_assigns_object_id(self, db): + group = await _insert("ins-oid") + assert isinstance(group.id, PydanticObjectId) + + async def test_resolves_project_ids_to_links(self, db): + await _insert("ins-with-projects", projects=["mp-alpha", "mp-beta"]) + doc = await ProjectGroup.find_one(ProjectGroup.name == "ins-with-projects") + assert doc is not None + assert doc.projects is not None + assert {link.ref.collection for link in doc.projects} == {"projects"} + assert sorted(link.ref.id for link in doc.projects) == ["mp-alpha", "mp-beta"] + + async def test_empty_projects_default(self, db): + await _insert("ins-no-projects") + doc = await ProjectGroup.find_one(ProjectGroup.name == "ins-no-projects") + assert doc is not None + assert doc.projects == [] + + async def test_duplicate_identifiers_raise_conflict(self, db): + # A ProjectGroup's identity is name + owner, not its server-assigned _id (a fresh ObjectId is + # minted per insert). insert_one must reject a second group with the same name+owner cleanly. + await _insert("ins-dup") + with pytest.raises(ConflictError): + await _insert("ins-dup") + + async def test_same_name_different_owner_allowed(self, db): + # name alone is not the identity: the same name under a different owner is a distinct group. + await _insert("ins-shared-name", owner=ALICE_EMAIL) + other = await _insert("ins-shared-name", owner="google:bob@example.com") + assert other.owner == "google:bob@example.com" diff --git a/mpcontribs-api/tests/integration/db/test_project_groups_service.py b/mpcontribs-api/tests/integration/db/test_project_groups_service.py new file mode 100644 index 000000000..1ce0c1ed1 --- /dev/null +++ b/mpcontribs-api/tests/integration/db/test_project_groups_service.py @@ -0,0 +1,150 @@ +import pytest +from beanie import PydanticObjectId + +from mpcontribs_api.authz import User +from mpcontribs_api.domains.project_groups.models import ProjectGroup, ProjectGroupIn +from mpcontribs_api.domains.project_groups.repository import ProjectGroupRepository +from mpcontribs_api.domains.project_groups.service import ProjectGroupService +from mpcontribs_api.domains.projects.models import ProjectIn +from mpcontribs_api.domains.projects.repository import MongoDbProjectRepository +from mpcontribs_api.exceptions import ConflictError, NotFoundError + +pytestmark = [pytest.mark.db, pytest.mark.asyncio(loop_scope="session")] + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +ADMIN = User(username="google:admin@example.com", groups=frozenset({"admin"})) +ALICE = User(username="google:alice@example.com", groups=frozenset({"mp-team"})) +ANON = User() + +ALICE_EMAIL = "google:alice@example.com" +BOB_EMAIL = "google:bob@example.com" + + +def _service(user: User = ADMIN) -> ProjectGroupService: + return ProjectGroupService(groups=ProjectGroupRepository(user), projects=MongoDbProjectRepository(user)) + + +async def _insert_project(pid: str, owner: str = ALICE_EMAIL, **overrides): + payload = { + "title": pid[:30], + "authors": "Author", + "description": "desc", + "owner": owner, + } + payload.update(overrides) + return await MongoDbProjectRepository(ADMIN).insert_project(pid, ProjectIn(**payload)) + + +async def _insert_group(name: str, owner: str = ALICE_EMAIL) -> ProjectGroup: + return await ProjectGroupRepository(ADMIN).insert_project_group( + ProjectGroupIn(name=name, owner=owner, projects=[], description="d") + ) + + +async def _members(group_id: PydanticObjectId) -> list[str]: + doc = await ProjectGroup.find_one(ProjectGroup.id == group_id) + assert doc is not None + return sorted(link.ref.id for link in (doc.projects or [])) + + +# --------------------------------------------------------------------------- +# add +# --------------------------------------------------------------------------- + + +class TestAdd: + async def test_add_by_id_links_projects(self, db): + group = await _insert_group("add-id") + await _insert_project("mp-1") + await _insert_project("mp-2") + summary = await _service().add_projects({"id": str(group.id)}, ["mp-1", "mp-2"]) + assert summary.succeeded == ["mp-1", "mp-2"] + assert summary.failed == [] + assert await _members(group.id) == ["mp-1", "mp-2"] + + async def test_add_by_identifiers_links_projects(self, db): + group = await _insert_group("add-ident") + await _insert_project("mp-x") + summary = await _service().add_projects({"name": "add-ident", "owner": ALICE_EMAIL}, ["mp-x"]) + assert summary.succeeded == ["mp-x"] + assert await _members(group.id) == ["mp-x"] + + async def test_add_is_idempotent(self, db): + group = await _insert_group("add-idem") + await _insert_project("mp-1") + await _service().add_projects({"id": str(group.id)}, ["mp-1"]) + await _service().add_projects({"id": str(group.id)}, ["mp-1"]) + assert await _members(group.id) == ["mp-1"] + + async def test_missing_project_fails_and_leaves_group_unchanged(self, db): + group = await _insert_group("add-missing") + summary = await _service().add_projects({"id": str(group.id)}, ["ghost"]) + assert summary.succeeded == [] + assert summary.failed[0].error_code == "not_found" + assert await _members(group.id) == [] + + async def test_out_of_scope_project_fails(self, db): + # Bob's private project is invisible to Alice, so she cannot link it. + group = await _insert_group("add-scope") + await _insert_project("mp-bob", owner=BOB_EMAIL) + summary = await _service(ALICE).add_projects({"id": str(group.id)}, ["mp-bob"]) + assert summary.succeeded == [] + assert summary.failed[0].error_code == "not_found" + assert await _members(group.id) == [] + + async def test_group_not_visible_raises_not_found(self, db): + group = await _insert_group("add-priv") # owned by Alice, invisible to anon + with pytest.raises(NotFoundError): + await _service(ANON).add_projects({"id": str(group.id)}, []) + + +# --------------------------------------------------------------------------- +# delete +# --------------------------------------------------------------------------- + + +class TestInsert: + async def test_non_admin_owner_forced_to_caller(self, db): + # Alice submits Bob as owner; the caller's identity must win so she can manage the group. + group = await _service(ALICE).insert( + ProjectGroupIn(name="ins-forced", owner=BOB_EMAIL, projects=[], description="d") + ) + assert group.owner == ALICE_EMAIL + + async def test_admin_may_set_owner_on_behalf(self, db): + group = await _service(ADMIN).insert( + ProjectGroupIn(name="ins-onbehalf", owner=BOB_EMAIL, projects=[], description="d") + ) + assert group.owner == BOB_EMAIL + + +class TestDelete: + async def test_delete_by_id_unlinks_project(self, db): + group = await _insert_group("rm-id") + await _insert_project("mp-1") + await _insert_project("mp-2") + await _service().add_projects({"id": str(group.id)}, ["mp-1", "mp-2"]) + summary = await _service().delete_projects({"id": str(group.id)}, ["mp-1"]) + assert summary.succeeded == ["mp-1"] + assert await _members(group.id) == ["mp-2"] + + async def test_delete_by_identifiers_unlinks_project(self, db): + group = await _insert_group("rm-ident") + await _insert_project("mp-1") + await _service().add_projects({"id": str(group.id)}, ["mp-1"]) + summary = await _service().delete_projects({"name": "rm-ident", "owner": ALICE_EMAIL}, ["mp-1"]) + assert summary.succeeded == ["mp-1"] + assert await _members(group.id) == [] + + async def test_delete_non_member_reported_as_failure(self, db): + group = await _insert_group("rm-nonmember") + await _insert_project("mp-1") + await _service().add_projects({"id": str(group.id)}, ["mp-1"]) + summary = await _service().delete_projects({"id": str(group.id)}, ["ghost"]) + assert summary.succeeded == [] + assert summary.failed[0].error_code == "not_found" + assert await _members(group.id) == ["mp-1"] diff --git a/mpcontribs-api/tests/integration/db/test_projects_repository.py b/mpcontribs-api/tests/integration/db/test_projects_repository.py index 063ecda32..f4761f487 100644 --- a/mpcontribs-api/tests/integration/db/test_projects_repository.py +++ b/mpcontribs-api/tests/integration/db/test_projects_repository.py @@ -4,8 +4,8 @@ from mpcontribs_api.domains.projects.models import Column, Project, ProjectIn, ProjectOut, ProjectPatch, Stats from mpcontribs_api.domains.consumers.models import ConsumerSettings from mpcontribs_api.domains.projects.repository import MongoDbProjectRepository +from mpcontribs_api.exceptions import ConflictError, NotFoundError, PermissionError, ValidationError from mpcontribs_api.exceptions import PermissionError as AppPermissionError -from mpcontribs_api.exceptions import ConflictError, NotFoundError, ValidationError from mpcontribs_api.pagination import CursorParams # All tests in this module share the session event loop so they can reuse the @@ -33,7 +33,7 @@ def _cols(n: int) -> list[dict[str, str]]: def _project_in(id: str, **overrides) -> ProjectIn: - # No id (it comes from the path) and no stats/columns (server-owned) on the input model. + """Build a user-supplied ``ProjectIn`` (content fields only — no server-managed id/stats).""" defaults = { "title": id[:30], "authors": "Test Author", @@ -45,6 +45,11 @@ def _project_in(id: str, **overrides) -> ProjectIn: async def _insert(id: str, **overrides) -> Project: + """Seed a project via the repository's insert path. + + ``ProjectIn`` carries ``is_public`` / ``is_approved`` (the scope tests seed specific states + through overrides); the id comes from the path, and stats/columns keep their server defaults. + """ project_in = _project_in(id, **overrides) return await _repo(ADMIN).insert_project(id, project_in) @@ -66,15 +71,12 @@ async def test_duplicate_id_raises_conflict(self, db): with pytest.raises(ConflictError): await _insert("ins-dup") - async def test_default_not_public(self, db): + async def test_insert_defaults_private_and_unapproved(self, db): + # ProjectIn carries no is_public/is_approved, so an inserted project is private and unapproved. await _insert("ins-priv") found = await Project.find_one(Project.id == "ins-priv") assert found.is_public is False - - async def test_explicit_public(self, db): - await _insert("ins-pub", is_public=True, is_approved=True) - found = await Project.find_one(Project.id == "ins-pub") - assert found.is_public is True + assert found.is_approved is False # --------------------------------------------------------------------------- @@ -119,29 +121,29 @@ def _noop_filter(): # --------------------------------------------------------------------------- -# get_project_by_id +# get_one # --------------------------------------------------------------------------- class TestGetProjectById: async def test_returns_project_for_valid_id(self, db): await _insert("get-by-id") - result = await _repo(ADMIN).get_project_by_id(id="get-by-id", fields=None) + result = await _repo(ADMIN).get_one({"id": "get-by-id"}, fields=None) assert result is not None assert result.id == "get-by-id" async def test_returns_none_for_missing_id(self, db): - result = await _repo(ADMIN).get_project_by_id(id="does-not-exist", fields=None) + result = await _repo(ADMIN).get_one({"id": "does-not-exist"}, fields=None) assert result is None async def test_admin_can_get_private_project(self, db): await _insert("get-priv", is_public=False) - result = await _repo(ADMIN).get_project_by_id(id="get-priv", fields=None) + result = await _repo(ADMIN).get_one({"id": "get-priv"}, fields=None) assert result is not None async def test_anon_cannot_get_private_project(self, db): await _insert("get-priv-anon", is_public=False) - result = await _repo(ANON).get_project_by_id(id="get-priv-anon", fields=None) + result = await _repo(ANON).get_one({"id": "get-priv-anon"}, fields=None) assert result is None @@ -151,7 +153,7 @@ async def test_anon_cannot_get_private_project(self, db): # Regression: Beanie stores the primary key under Mongo's ``_id`` (``id`` is an # alias), but fastapi-filter keys queries on the raw field name. Without the # ``id`` -> ``_id`` remap in BaseFilter these filters matched nothing even -# though get_project_by_id (which queries ``_id`` directly) found the document. +# though get_one (which queries ``_id`` directly) found the document. # --------------------------------------------------------------------------- @@ -191,6 +193,57 @@ async def test_filter_by_id_neq_excludes(self, db): assert "filter-id-neq-drop" not in ids +# --------------------------------------------------------------------------- +# get_projects — tags filtering +# +# ``tags__contains`` maps to MongoDB ``$all``: a project matches only when its +# tags are a superset of every value supplied (the query list is a subset of +# the stored array). Contrast with ``tags__in`` ($in), which matches on any +# single overlapping tag. +# --------------------------------------------------------------------------- + + +class TestGetProjectsTagsFilter: + async def test_contains_requires_all_tags_as_subset(self, db): + from mpcontribs_api.domains.projects.models import ProjectFilter + + await _insert("tags-superset", tags=["alpha", "beta", "gamma"]) + await _insert("tags-partial", tags=["alpha", "beta"]) + await _insert("tags-none", tags=["delta"]) + page = await _repo(ADMIN).get_projects( + filter=ProjectFilter(tags__contains=["alpha", "gamma"]), + pagination=CursorParams(), + fields=None, + ) + assert {p.id for p in page.items} == {"tags-superset"} + + async def test_contains_single_tag(self, db): + from mpcontribs_api.domains.projects.models import ProjectFilter + + await _insert("tags-single-hit", tags=["alpha", "beta"]) + await _insert("tags-single-miss", tags=["beta", "gamma"]) + page = await _repo(ADMIN).get_projects( + filter=ProjectFilter(tags__contains=["alpha"]), + pagination=CursorParams(), + fields=None, + ) + assert {p.id for p in page.items} == {"tags-single-hit"} + + async def test_contains_parses_comma_string(self, db): + from mpcontribs_api.domains.projects.models import ProjectFilter + + await _insert("tags-csv-hit", tags=["alpha", "beta", "gamma"]) + await _insert("tags-csv-miss", tags=["alpha"]) + # FilterDepends collapses the list query param to a comma string; the + # BaseFilter validator must re-expand it. + page = await _repo(ADMIN).get_projects( + filter=ProjectFilter(tags__contains="alpha,beta"), + pagination=CursorParams(), + fields=None, + ) + assert {p.id for p in page.items} == {"tags-csv-hit"} + + # --------------------------------------------------------------------------- # Field projection # --------------------------------------------------------------------------- @@ -270,7 +323,7 @@ async def test_all_items_covered_across_pages(self, db): # --------------------------------------------------------------------------- -# patch_project_by_id +# patch_one # --------------------------------------------------------------------------- @@ -278,7 +331,7 @@ class TestPatchProject: async def test_updates_single_field(self, db): await _insert("patch-me") patch = ProjectPatch(title="Updated Title") - await _repo(ADMIN).patch_project_by_id(id="patch-me", update=patch) + await _repo(ADMIN).patch_one({"id": "patch-me"}, patch) found = await Project.find_one(Project.id == "patch-me") assert found.title == "Updated Title" @@ -286,68 +339,100 @@ async def test_unset_fields_not_overwritten(self, db): await _insert("patch-preserve") original = await Project.find_one(Project.id == "patch-preserve") patch = ProjectPatch(title="New Title") - await _repo(ADMIN).patch_project_by_id(id="patch-preserve", update=patch) + await _repo(ADMIN).patch_one({"id": "patch-preserve"}, patch) found = await Project.find_one(Project.id == "patch-preserve") assert found.authors == original.authors async def test_not_found_raises(self, db): patch = ProjectPatch(title="Won't work") with pytest.raises(NotFoundError): - await _repo(ADMIN).patch_project_by_id(id="no-such-id", update=patch) + await _repo(ADMIN).patch_one({"id": "no-such-id"}, patch) async def test_empty_patch_returns_existing(self, db): await _insert("patch-empty") - result = await _repo(ADMIN).patch_project_by_id(id="patch-empty", update=ProjectPatch()) + result = await _repo(ADMIN).patch_one({"id": "patch-empty"}, ProjectPatch()) assert result.id == "patch-empty" # --------------------------------------------------------------------------- -# delete_project_by_id (soft-delete via DocumentWithSoftDelete) +# delete_one (soft-delete via DocumentWithSoftDelete) # --------------------------------------------------------------------------- class TestDeleteProject: async def test_deleted_project_not_in_default_query(self, db): await _insert("del-me", is_public=True, is_approved=True) - await _repo(ADMIN).delete_project_by_id(id="del-me") + await _repo(ADMIN).delete_one({"id": "del-me"}) page = await _repo(ADMIN).get_projects(filter=_noop_filter(), pagination=CursorParams(), fields=None) ids = {p.id for p in page.items} assert "del-me" not in ids async def test_delete_nonexistent_throws_error(self, db): - # delete_project_by_id does find_one().delete() — Error if not found + # delete_one does find_one().delete() — Error if not found with pytest.raises(NotFoundError, match="not found"): - await _repo(ADMIN).delete_project_by_id(id="ghost-id") + await _repo(ADMIN).delete_one({"id": "ghost-id"}) + + async def test_owner_can_delete_own_project(self, db): + await _insert("del-own", owner="google:alice@example.com") + await _repo(ALICE).delete_one({"id": "del-own"}) + assert await Project.find_one(Project.id == "del-own") is None + + async def test_admin_can_delete_any_project(self, db): + await _insert("del-admin", owner="google:alice@example.com") + await _repo(ADMIN).delete_one({"id": "del-admin"}) + assert await Project.find_one(Project.id == "del-admin") is None + + async def test_group_member_non_owner_cannot_delete(self, db): + # A user whose group contains the project slug can *see* it, but only the owner may delete. + member = User(username="google:carol@example.com", groups=frozenset({"del-grp"})) + await _insert("del-grp", owner="google:alice@example.com") + with pytest.raises(PermissionError): + await _repo(member).delete_one({"id": "del-grp"}) + assert await Project.find_one(Project.id == "del-grp") is not None + + async def test_visible_public_non_owner_cannot_delete(self, db): + # BOB can see the public+approved project but does not own it → 403, not a silent success. + await _insert("del-pub", owner="google:alice@example.com", is_public=True, is_approved=True) + with pytest.raises(PermissionError): + await _repo(BOB).delete_one({"id": "del-pub"}) + assert await Project.find_one(Project.id == "del-pub") is not None + + async def test_out_of_scope_delete_not_found(self, db): + # BOB cannot see Alice's private project → 404 (existence is not leaked as a 403). + await _insert("del-hidden", owner="google:alice@example.com", is_public=False) + with pytest.raises(NotFoundError): + await _repo(BOB).delete_one({"id": "del-hidden"}) + assert await Project.find_one(Project.id == "del-hidden") is not None # --------------------------------------------------------------------------- -# upsert_project_by_id +# upsert_one # --------------------------------------------------------------------------- class TestUpsertProject: async def test_upsert_creates_new_project(self, db): data = _project_in("upsert-new") - await _repo(ADMIN).upsert_project_by_id(id="upsert-new", data=data) + await _repo(ADMIN).upsert_one({"id": "upsert-new"}, data=data) found = await Project.find_one(Project.id == "upsert-new") assert found is not None async def test_upsert_updates_existing_project(self, db): await _insert("upsert-existing") data = _project_in("upsert-existing", title="Replaced Title") - await _repo(ADMIN).upsert_project_by_id(id="upsert-existing", data=data) + await _repo(ADMIN).upsert_one({"id": "upsert-existing"}, data=data) found = await Project.find_one(Project.id == "upsert-existing") assert found.title == "Replaced Title" async def test_upsert_uses_path_id_not_body_id(self, db): data = _project_in("body-id") - await _repo(ADMIN).upsert_project_by_id(id="path-id", data=data) + await _repo(ADMIN).upsert_one({"id": "path-id"}, data=data) found = await Project.find_one(Project.id == "path-id") assert found is not None # --------------------------------------------------------------------------- -# upsert_project_by_id — authorization (owner-or-admin) +# upsert_one — authorization (owner-or-admin) # --------------------------------------------------------------------------- BOB = User(username="google:bob@example.com", groups=frozenset()) @@ -357,14 +442,14 @@ class TestUpsertProjectAuthorization: async def test_owner_can_overwrite_own_project(self, db): await _insert("auth-own", owner="google:alice@example.com") data = _project_in("auth-own", owner="google:alice@example.com", title="Owner Edit") - await _repo(ALICE).upsert_project_by_id(id="auth-own", data=data) + await _repo(ALICE).upsert_one({"id": "auth-own"}, data=data) found = await Project.find_one(Project.id == "auth-own") assert found.title == "Owner Edit" async def test_admin_can_overwrite_any_project(self, db): await _insert("auth-admin", owner="google:alice@example.com") data = _project_in("auth-admin", owner="google:alice@example.com", title="Admin Edit") - await _repo(ADMIN).upsert_project_by_id(id="auth-admin", data=data) + await _repo(ADMIN).upsert_one({"id": "auth-admin"}, data=data) found = await Project.find_one(Project.id == "auth-admin") assert found.title == "Admin Edit" @@ -374,14 +459,14 @@ async def test_non_owner_cannot_overwrite(self, db): from mpcontribs_api.exceptions import PermissionError as AppPermissionError with pytest.raises(AppPermissionError): - await _repo(BOB).upsert_project_by_id(id="auth-other", data=data) + await _repo(BOB).upsert_one({"id": "auth-other"}, data=data) found = await Project.find_one(Project.id == "auth-other") assert found.title == "Original" async def test_new_project_sets_owner_to_caller(self, db): # Body carries a foreign owner; the authenticated caller's identity must win on insert. data = _project_in("auth-newowner", owner="google:alice@example.com") - await _repo(BOB).upsert_project_by_id(id="auth-newowner", data=data) + await _repo(BOB).upsert_one({"id": "auth-newowner"}, data=data) found = await Project.find_one(Project.id == "auth-newowner") assert found.owner == "google:bob@example.com" @@ -389,12 +474,94 @@ async def test_update_preserves_original_owner(self, db): await _insert("auth-preserve", owner="google:alice@example.com") # Alice tries to reassign ownership via the body; owner must stay hers. data = _project_in("auth-preserve", owner="google:bob@example.com", title="Edit") - await _repo(ALICE).upsert_project_by_id(id="auth-preserve", data=data) + await _repo(ALICE).upsert_one({"id": "auth-preserve"}, data=data) found = await Project.find_one(Project.id == "auth-preserve") assert found.owner == "google:alice@example.com" # --------------------------------------------------------------------------- +# is_approved is admin-only (via PATCH — ProjectIn cannot carry it) +# --------------------------------------------------------------------------- + + +class TestApprovalIsAdminOnly: + async def test_non_admin_cannot_patch_is_approved(self, db): + await _insert("appr-patch", owner="google:alice@example.com") + with pytest.raises(PermissionError): + await _repo(ALICE).patch_one({"id": "appr-patch"}, ProjectPatch(is_approved=True)) + found = await Project.find_one(Project.id == "appr-patch") + assert found.is_approved is False + + async def test_admin_can_patch_is_approved(self, db): + await _insert("appr-patch-admin", owner="google:alice@example.com") + await _repo(ADMIN).patch_one({"id": "appr-patch-admin"}, ProjectPatch(is_approved=True)) + found = await Project.find_one(Project.id == "appr-patch-admin") + assert found.is_approved is True + + +# --------------------------------------------------------------------------- +# a project cannot be public unless approved (enforced on PATCH) +# --------------------------------------------------------------------------- + + +class TestPublicRequiresApproved: + async def test_patch_public_on_unapproved_rejected(self, db): + await _insert("pub-unappr", owner="google:alice@example.com", is_approved=False) + with pytest.raises(ValidationError, match="approved"): + await _repo(ADMIN).patch_one({"id": "pub-unappr"}, ProjectPatch(is_public=True)) + found = await Project.find_one(Project.id == "pub-unappr") + assert found.is_public is False + + async def test_patch_public_and_approved_together_succeeds(self, db): + await _insert("pub-both", owner="google:alice@example.com", is_approved=False) + await _repo(ADMIN).patch_one( + {"id": "pub-both"}, ProjectPatch(is_public=True, is_approved=True) + ) + found = await Project.find_one(Project.id == "pub-both") + assert found.is_public is True + assert found.is_approved is True + + async def test_patch_public_on_approved_succeeds(self, db): + await _insert("pub-approved", owner="google:alice@example.com", is_approved=True) + await _repo(ADMIN).patch_one({"id": "pub-approved"}, ProjectPatch(is_public=True)) + found = await Project.find_one(Project.id == "pub-approved") + assert found.is_public is True + + +# --------------------------------------------------------------------------- +# upsert (PUT) cannot set server-managed fields; it preserves them on update +# --------------------------------------------------------------------------- + + +class TestUpsertServerManagedFields: + async def test_new_project_is_private_and_unapproved(self, db): + # ProjectIn has no is_public/is_approved, so a new PUT project starts safe by default. + await _repo(BOB).upsert_one({"id": "srv-new"}, data=_project_in("srv-new")) + found = await Project.find_one(Project.id == "srv-new") + assert found.is_public is False + assert found.is_approved is False + + async def test_admin_upsert_cannot_approve_via_body(self, db): + # Approval is PATCH-only even for an admin; a PUT can never approve a project. + await _repo(ADMIN).upsert_one({"id": "srv-admin-new"}, data=_project_in("srv-admin-new")) + found = await Project.find_one(Project.id == "srv-admin-new") + assert found.is_approved is False + + async def test_update_preserves_public_and_approved(self, db): + # A full-replace PUT by the owner must not wipe server-managed publication/approval. + await _insert("srv-preserve", owner="google:alice@example.com", is_public=True, is_approved=True) + data = _project_in("srv-preserve", owner="google:alice@example.com", title="Renamed Title") + await _repo(ALICE).upsert_one({"id": "srv-preserve"}, data=data) + found = await Project.find_one(Project.id == "srv-preserve") + assert found.title == "Renamed Title" # content fields still update + assert found.is_public is True + assert found.is_approved is True + + async def test_update_preserves_stats(self, db): + await _insert("srv-stats", owner="google:alice@example.com", stats=Stats(contributions=7)) + await _repo(ALICE).upsert_one({"id": "srv-stats"}, data=_project_in("srv-stats")) + found = await Project.find_one(Project.id == "srv-stats") + assert found.stats.contributions == 7 # Server-owned fields: stats / columns are derived, is_approved is admin-only # --------------------------------------------------------------------------- @@ -408,27 +575,27 @@ async def test_upsert_update_preserves_stats_and_columns(self, db): stored.columns = [Column(path="data.band_gap", min=0.0, max=1.0, unit="eV")] await stored.save() # A full overwrite must not clobber them. - await _repo(ADMIN).upsert_project_by_id(id="srv-preserve", data=_project_in("srv-preserve", title="Edited")) + await _repo(ADMIN).upsert_one({"id": "srv-preserve"}, _project_in("srv-preserve", title="Edited")) found = await Project.find_one(Project.id == "srv-preserve") assert found.title == "Edited" assert found.stats.contributions == 5 assert [c.path for c in found.columns] == ["data.band_gap"] async def test_upsert_new_starts_with_empty_stats(self, db): - await _repo(ALICE).upsert_project_by_id(id="srv-new-empty", data=_project_in("srv-new-empty")) + await _repo(ALICE).upsert_one({"id": "srv-new-empty"}, _project_in("srv-new-empty")) found = await Project.find_one(Project.id == "srv-new-empty") - assert found.stats == Stats.empty() + assert found.stats == Stats() assert found.columns == [] async def test_non_admin_cannot_approve_new_project_via_upsert(self, db): data = _project_in("srv-approve-new", is_approved=True) - await _repo(ALICE).upsert_project_by_id(id="srv-approve-new", data=data) + await _repo(ALICE).upsert_one({"id": "srv-approve-new"}, data) found = await Project.find_one(Project.id == "srv-approve-new") assert found.is_approved is False async def test_admin_can_approve_new_project_via_upsert(self, db): data = _project_in("srv-approve-admin", is_approved=True) - await _repo(ADMIN).upsert_project_by_id(id="srv-approve-admin", data=data) + await _repo(ADMIN).upsert_one({"id": "srv-approve-admin"}, data) found = await Project.find_one(Project.id == "srv-approve-admin") assert found.is_approved is True @@ -436,26 +603,26 @@ async def test_non_admin_cannot_change_approval_via_upsert(self, db): await _insert("srv-approve-existing", owner="google:alice@example.com", is_approved=True) # The owner (non-admin) overwrites and tries to un-approve; approval must stick. data = _project_in("srv-approve-existing", owner="google:alice@example.com", is_approved=False) - await _repo(ALICE).upsert_project_by_id(id="srv-approve-existing", data=data) + await _repo(ALICE).upsert_one({"id": "srv-approve-existing"}, data) found = await Project.find_one(Project.id == "srv-approve-existing") assert found.is_approved is True async def test_non_admin_cannot_approve_via_patch(self, db): await _insert("srv-patch-approve", owner="google:alice@example.com") with pytest.raises(AppPermissionError): - await _repo(ALICE).patch_project_by_id(id="srv-patch-approve", update=ProjectPatch(is_approved=True)) + await _repo(ALICE).patch_one({"id": "srv-patch-approve"}, update=ProjectPatch(is_approved=True)) found = await Project.find_one(Project.id == "srv-patch-approve") assert found.is_approved is False async def test_admin_can_approve_via_patch(self, db): await _insert("srv-patch-admin", owner="google:alice@example.com") - await _repo(ADMIN).patch_project_by_id(id="srv-patch-admin", update=ProjectPatch(is_approved=True)) + await _repo(ADMIN).patch_one({"id": "srv-patch-admin"}, update=ProjectPatch(is_approved=True)) found = await Project.find_one(Project.id == "srv-patch-admin") assert found.is_approved is True async def test_non_admin_plain_patch_is_allowed(self, db): await _insert("srv-patch-plain", owner="google:alice@example.com") - await _repo(ALICE).patch_project_by_id(id="srv-patch-plain", update=ProjectPatch(title="New Title")) + await _repo(ALICE).patch_one({"id": "srv-patch-plain"}, update=ProjectPatch(title="New Title")) found = await Project.find_one(Project.id == "srv-patch-plain") assert found.title == "New Title" # Per-user project-count quota (max_projects) @@ -494,7 +661,7 @@ async def test_upsert_new_project_over_cap_rejected(self, db, monkeypatch): await _insert("owned-1", owner=ALICE_EMAIL) data = _project_in("new-proj", owner=ALICE_EMAIL) with pytest.raises(AppPermissionError): - await _repo(ALICE).upsert_project_by_id(id="new-proj", data=data) + await _repo(ALICE).upsert_one({"id": "new-proj"}, data) async def test_upsert_existing_project_allowed_at_cap(self, db, monkeypatch): # Regression: updating a project you already own must never be blocked by the cap, even @@ -504,7 +671,7 @@ async def test_upsert_existing_project_allowed_at_cap(self, db, monkeypatch): monkeypatch.setattr(get_settings().consumer, "max_projects", 1) await _insert("owned-only", owner=ALICE_EMAIL) data = _project_in("owned-only", owner=ALICE_EMAIL, title="Updated Title") - result = await _repo(ALICE).upsert_project_by_id(id="owned-only", data=data) + result = await _repo(ALICE).upsert_one({"id": "owned-only"}, data) assert result.title == "Updated Title" async def test_injected_consumer_override_lowers_cap(self, db): diff --git a/mpcontribs-api/tests/integration/db/test_stats_recompute.py b/mpcontribs-api/tests/integration/db/test_stats_recompute.py index 18fcae5d4..cbc216962 100644 --- a/mpcontribs-api/tests/integration/db/test_stats_recompute.py +++ b/mpcontribs-api/tests/integration/db/test_stats_recompute.py @@ -60,13 +60,12 @@ def _service(client) -> ContributionService: ) -async def _make_project(unique_identifiers: bool = True) -> Project: +async def _make_project() -> Project: project_in = ProjectIn( title="Stats Project", authors="Test Author", description="Recompute lifecycle fixture", owner="google:admin@example.com", - unique_identifiers=unique_identifiers, ) return await MongoDbProjectRepository(ADMIN).insert_project(PID, project_in) diff --git a/mpcontribs-api/tests/integration/test_component_routes.py b/mpcontribs-api/tests/integration/test_component_routes.py index c1b49cd8b..6452a4c1a 100644 --- a/mpcontribs-api/tests/integration/test_component_routes.py +++ b/mpcontribs-api/tests/integration/test_component_routes.py @@ -127,15 +127,15 @@ def test_post_forwards_to_service(self, client, structure_service): class TestStructuresByIdRouting: def test_get_by_id_conventional_path(self, client, structure_service): - structure_service.get_by_id.return_value = SAMPLE_STRUCTURE + structure_service.get_one.return_value = SAMPLE_STRUCTURE assert client.get(f"/api/v1/structures/{PydanticObjectId()}").status_code == 200 def test_delete_by_id_conventional_path(self, client, structure_service): - structure_service.delete_by_id.return_value = ComponentDeleteResponse(num_deleted=1) + structure_service.delete_one.return_value = ComponentDeleteResponse(num_deleted=1) assert client.delete(f"/api/v1/structures/{PydanticObjectId()}").status_code == 200 def test_patch_by_id_conventional_path(self, client, structure_service): - structure_service.patch_by_id.return_value = SAMPLE_STRUCTURE + structure_service.patch_one.return_value = SAMPLE_STRUCTURE r = client.patch(f"/api/v1/structures/{PydanticObjectId()}", json={"name": "renamed"}) assert r.status_code == 200 @@ -186,15 +186,15 @@ def test_post_forwards_to_service(self, client, table_service): class TestTablesByIdRouting: def test_get_by_id_conventional_path(self, client, table_service): - table_service.get_by_id.return_value = SAMPLE_TABLE + table_service.get_one.return_value = SAMPLE_TABLE assert client.get(f"/api/v1/tables/{PydanticObjectId()}").status_code == 200 def test_delete_by_id_conventional_path(self, client, table_service): - table_service.delete_by_id.return_value = ComponentDeleteResponse(num_deleted=1) + table_service.delete_one.return_value = ComponentDeleteResponse(num_deleted=1) assert client.delete(f"/api/v1/tables/{PydanticObjectId()}").status_code == 200 def test_patch_by_id_conventional_path(self, client, table_service): - table_service.patch_by_id.return_value = SAMPLE_TABLE + table_service.patch_one.return_value = SAMPLE_TABLE r = client.patch(f"/api/v1/tables/{PydanticObjectId()}", json={"name": "x"}) assert r.status_code == 200 @@ -212,14 +212,14 @@ def test_list_calls_attachment_service(self, client, attachment_service): attachment_service.get_many.assert_awaited_once() def test_get_by_id_calls_attachment_service(self, client, attachment_service): - attachment_service.get_by_id.return_value = None + attachment_service.get_one.return_value = None client.get(f"/api/v1/attachments/{PydanticObjectId()}") - attachment_service.get_by_id.assert_awaited_once() + attachment_service.get_one.assert_awaited_once() def test_delete_by_id_calls_attachment_service(self, client, attachment_service): - attachment_service.delete_by_id.return_value = ComponentDeleteResponse(num_deleted=1) + attachment_service.delete_one.return_value = ComponentDeleteResponse(num_deleted=1) client.delete(f"/api/v1/attachments/{PydanticObjectId()}") - attachment_service.delete_by_id.assert_awaited_once() + attachment_service.delete_one.assert_awaited_once() def test_batch_delete_calls_attachment_service(self, client, attachment_service): attachment_service.delete.return_value = ComponentDeleteResponse(num_deleted=0) @@ -310,12 +310,12 @@ def test_structures_delete_anon_401(self, client, structure_service): def test_structure_delete_by_id_anon_401(self, client, structure_service): r = client.delete(f"/api/v1/structures/{PydanticObjectId()}", headers=FORCE_ANON_HEADERS) assert r.status_code == 401 - structure_service.delete_by_id.assert_not_called() + structure_service.delete_one.assert_not_called() def test_structure_patch_by_id_anon_401(self, client, structure_service): r = client.patch(f"/api/v1/structures/{PydanticObjectId()}", json={"name": "x"}, headers=FORCE_ANON_HEADERS) assert r.status_code == 401 - structure_service.patch_by_id.assert_not_called() + structure_service.patch_one.assert_not_called() def test_tables_delete_anon_401(self, client, table_service): r = client.delete("/api/v1/tables", headers=FORCE_ANON_HEADERS) @@ -325,9 +325,41 @@ def test_tables_delete_anon_401(self, client, table_service): def test_attachment_delete_by_id_anon_401(self, client, attachment_service): r = client.delete(f"/api/v1/attachments/{PydanticObjectId()}", headers=FORCE_ANON_HEADERS) assert r.status_code == 401 - attachment_service.delete_by_id.assert_not_called() + attachment_service.delete_one.assert_not_called() def test_structures_get_still_open_to_anon(self, client, structure_service): structure_service.get_many.return_value = Page(items=[], next_cursor=None) r = client.get("/api/v1/structures", headers=FORCE_ANON_HEADERS) assert r.status_code == 200 + + +# =========================================================================== +# Component inserts require the caller be a writer of at least one project +# (authenticated alone is not enough — require_writer). +# =========================================================================== + +# Authenticated, but carries no groups -> no writable projects. Override the default +# groups header (AUTHED_HEADERS sets mp-team) to empty so the caller is a non-writer. +NON_WRITER_HEADERS = { + "x-consumer-username": "google:nogroups@example.com", + "x-authenticated-groups": "", +} + + +class TestComponentInsertRequiresWriter: + def test_structures_post_non_writer_403(self, client, structure_service): + r = client.post("/api/v1/structures", json=[], headers=NON_WRITER_HEADERS) + assert r.status_code == 403 + structure_service.insert.assert_not_called() + + def test_tables_post_non_writer_403(self, client, table_service): + r = client.post("/api/v1/tables", json=[], headers=NON_WRITER_HEADERS) + assert r.status_code == 403 + table_service.insert.assert_not_called() + + def test_structures_post_writer_allowed(self, client, structure_service): + # The default AUTHED_HEADERS identity carries the mp-team group -> writer. + structure_service.insert.return_value = {"total": 0, "succeeded": [], "failed": []} + r = client.post("/api/v1/structures", json=[]) + assert r.status_code == 200 + structure_service.insert.assert_awaited_once() diff --git a/mpcontribs-api/tests/integration/test_contributions_routes.py b/mpcontribs-api/tests/integration/test_contributions_routes.py index a61ea72af..4df2d4c38 100644 --- a/mpcontribs-api/tests/integration/test_contributions_routes.py +++ b/mpcontribs-api/tests/integration/test_contributions_routes.py @@ -137,8 +137,8 @@ def test_malformed_body_returns_422(self, client, contribution_service): class TestContributionByIdRouting: """RED: routes mount as /contributions{id} not /contributions/{id}.""" - def test_get_by_id_conventional_path(self, client, contribution_repo): - contribution_repo.get_contribution_by_id.return_value = SAMPLE_OUT + def test_get_by_id_conventional_path(self, client, contribution_service): + contribution_service.get_one.return_value = SAMPLE_OUT assert client.get(f"/api/v1/contributions/{PydanticObjectId()}").status_code == 200 def test_patch_by_id_conventional_path(self, client, contribution_service): @@ -152,9 +152,7 @@ def test_put_by_id_conventional_path(self, client, contribution_service): assert r.status_code == 200 def test_delete_by_id_conventional_path(self, client, contribution_service): - contribution_service.delete_contributions.return_value = BulkDeleteSummary( - num_deleted=1, num_children_deleted=0 - ) + contribution_service.delete_one.return_value = BulkDeleteSummary(num_deleted=1, num_children_deleted=0) assert client.delete(f"/api/v1/contributions/{PydanticObjectId()}").status_code == 200 def test_download_route_conventional_path(self, client, contribution_repo): @@ -169,23 +167,17 @@ def test_download_route_conventional_path(self, client, contribution_repo): class TestDeleteContributionByIdWiring: def test_delete_delegates_to_service(self, client, contribution_service): - contribution_service.delete_contributions.return_value = BulkDeleteSummary( - num_deleted=1, num_children_deleted=2 - ) + contribution_service.delete_one.return_value = BulkDeleteSummary(num_deleted=1, num_children_deleted=2) oid = PydanticObjectId() - # NOTE: glued path is intentional here — see module docstring. r = client.delete(f"/api/v1/contributions/{oid}") assert r.status_code == 200 - contribution_service.delete_contributions.assert_awaited_once() + contribution_service.delete_one.assert_awaited_once() - def test_delete_builds_filter_from_path_id(self, client, contribution_service): - contribution_service.delete_contributions.return_value = BulkDeleteSummary( - num_deleted=1, num_children_deleted=0 - ) + def test_delete_passes_id_identifiers_to_service(self, client, contribution_service): + contribution_service.delete_one.return_value = BulkDeleteSummary(num_deleted=1, num_children_deleted=0) oid = PydanticObjectId() client.delete(f"/api/v1/contributions/{oid}") - passed_filter = contribution_service.delete_contributions.call_args.args[0] - assert passed_filter.id == oid + assert contribution_service.delete_one.call_args.args[0] == {"id": str(oid)} # =========================================================================== diff --git a/mpcontribs-api/tests/integration/test_initiatives.py b/mpcontribs-api/tests/integration/test_initiatives.py new file mode 100644 index 000000000..516bb5b46 --- /dev/null +++ b/mpcontribs-api/tests/integration/test_initiatives.py @@ -0,0 +1,135 @@ +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from mpcontribs_api.domains.initiatives.dependencies import get_initiative_repository +from mpcontribs_api.exceptions import NotFoundError +from mpcontribs_api.pagination import Page +from tests.integration.conftest import AUTHED_HEADERS, FORCE_ANON_HEADERS + +SAMPLE_OID = "6eb7cf5a86d9755df3a6c593" + + +@pytest.fixture +def initiative_repo(test_app): + repo = AsyncMock() + test_app.dependency_overrides[get_initiative_repository] = lambda: repo + yield repo + test_app.dependency_overrides.pop(get_initiative_repository, None) + + +def _stored(**overrides): + """Stand-in for the stored document: exposes ``.id`` like a Beanie Document.""" + attrs = { + "id": SAMPLE_OID, + "slug": "battery-genome", + "name": "Battery Genome", + "owner": "google:alice@example.com", + "is_public": False, + "is_approved": False, + } + attrs.update(overrides) + return SimpleNamespace(**attrs) + + +# --------------------------------------------------------------------------- +# POST /api/v1/initiatives +# --------------------------------------------------------------------------- + + +class TestInsert: + def test_returns_201_and_echoes_id(self, client, initiative_repo): + initiative_repo.insert_initiative.return_value = _stored() + r = client.post( + "/api/v1/initiatives", + json={"slug": "battery-genome", "name": "Battery Genome"}, + headers=AUTHED_HEADERS, + ) + assert r.status_code == 201 + assert r.json()["id"] == SAMPLE_OID + + def test_anonymous_rejected_401(self, client, initiative_repo): + r = client.post( + "/api/v1/initiatives", + json={"slug": "battery-genome", "name": "Battery Genome"}, + headers=FORCE_ANON_HEADERS, + ) + assert r.status_code == 401 + + def test_invalid_slug_returns_422(self, client, initiative_repo): + r = client.post( + "/api/v1/initiatives", + json={"slug": "Not A Slug!", "name": "x"}, + headers=AUTHED_HEADERS, + ) + assert r.status_code == 422 + + +# --------------------------------------------------------------------------- +# GET /api/v1/initiatives (+ /{slug}) +# --------------------------------------------------------------------------- + + +class TestGet: + def test_list_returns_200(self, client, initiative_repo): + initiative_repo.get_initiatives.return_value = Page(items=[], next_cursor=None) + r = client.get("/api/v1/initiatives", headers=AUTHED_HEADERS) + assert r.status_code == 200 + + def test_get_by_slug_returns_200(self, client, initiative_repo): + initiative_repo.get_one.return_value = _stored() + r = client.get("/api/v1/initiatives/battery-genome", headers=AUTHED_HEADERS) + assert r.status_code == 200 + assert r.json()["slug"] == "battery-genome" + + +# --------------------------------------------------------------------------- +# PATCH /api/v1/initiatives/{slug} +# --------------------------------------------------------------------------- + + +class TestPatch: + def test_patch_returns_200(self, client, initiative_repo): + initiative_repo.patch_one.return_value = _stored(name="Renamed") + r = client.patch( + "/api/v1/initiatives/battery-genome", + json={"name": "Renamed"}, + headers=AUTHED_HEADERS, + ) + assert r.status_code == 200 + assert r.json()["name"] == "Renamed" + + def test_anonymous_rejected_401(self, client, initiative_repo): + r = client.patch( + "/api/v1/initiatives/battery-genome", + json={"name": "Renamed"}, + headers=FORCE_ANON_HEADERS, + ) + assert r.status_code == 401 + + def test_not_found_propagates_404(self, client, initiative_repo): + initiative_repo.patch_one.side_effect = NotFoundError("nope") + r = client.patch( + "/api/v1/initiatives/missing", + json={"name": "Renamed"}, + headers=AUTHED_HEADERS, + ) + assert r.status_code == 404 + + +# --------------------------------------------------------------------------- +# DELETE /api/v1/initiatives/{slug} +# --------------------------------------------------------------------------- + + +class TestDelete: + def test_delete_returns_204(self, client, initiative_repo): + initiative_repo.delete_one.return_value = None + r = client.delete("/api/v1/initiatives/battery-genome", headers=AUTHED_HEADERS) + assert r.status_code == 204 + assert r.content == b"" + + def test_anonymous_rejected_401(self, client, initiative_repo): + r = client.delete("/api/v1/initiatives/battery-genome", headers=FORCE_ANON_HEADERS) + assert r.status_code == 401 diff --git a/mpcontribs-api/tests/integration/test_project_groups.py b/mpcontribs-api/tests/integration/test_project_groups.py new file mode 100644 index 000000000..e12ff146f --- /dev/null +++ b/mpcontribs-api/tests/integration/test_project_groups.py @@ -0,0 +1,76 @@ +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest + +from mpcontribs_api.domains.project_groups.dependencies import get_project_group_service +from tests.integration.conftest import AUTHED_HEADERS + +# A valid 24-char hex ObjectId string (ProjectGroupOut.id is a PydanticObjectId). +SAMPLE_OID = "6eb7cf5a86d9755df3a6c593" + + +@pytest.fixture +def group_service(test_app): + service = AsyncMock() + test_app.dependency_overrides[get_project_group_service] = lambda: service + yield service + test_app.dependency_overrides.pop(get_project_group_service, None) + + +# --------------------------------------------------------------------------- +# POST /api/v1/project_groups +# +# The handler returns the stored document, which FastAPI coerces into +# ProjectGroupOut with from_attributes=True. A Beanie Document exposes ``.id`` +# (not ``._id``), so the response model must populate its id by field name as +# well as the ``_id`` alias — otherwise every create/update response would +# serialise ``id: null`` and callers couldn't see what id their group got. +# --------------------------------------------------------------------------- + + +class TestInsertProjectGroupResponse: + def _body(self, **overrides): + body = { + "name": "my-group", + "owner": "google:alice@example.com", + "description": "d", + "projects": [], + } + body.update(overrides) + return body + + def _inserted(self, **overrides): + """Stand-in for the stored document: exposes ``.id`` like a Beanie Document.""" + attrs = { + "id": SAMPLE_OID, + "name": "my-group", + "owner": "google:alice@example.com", + "is_public": False, + "projects": None, + "description": "d", + } + attrs.update(overrides) + return SimpleNamespace(**attrs) + + def test_returns_201(self, client, group_service): + group_service.insert.return_value = self._inserted() + r = client.post("/api/v1/project_groups", json=self._body(), headers=AUTHED_HEADERS) + assert r.status_code == 201 + + def test_response_includes_generated_id(self, client, group_service): + group_service.insert.return_value = self._inserted() + body = client.post("/api/v1/project_groups", json=self._body(), headers=AUTHED_HEADERS).json() + assert body["id"] == SAMPLE_OID + + def test_response_echoes_full_document(self, client, group_service): + group_service.insert.return_value = self._inserted(name="echo-group", is_public=True) + body = client.post( + "/api/v1/project_groups", + json=self._body(name="echo-group", is_public=True), + headers=AUTHED_HEADERS, + ).json() + assert body["id"] == SAMPLE_OID + assert body["name"] == "echo-group" + assert body["owner"] == "google:alice@example.com" + assert body["is_public"] is True diff --git a/mpcontribs-api/tests/integration/test_projects.py b/mpcontribs-api/tests/integration/test_projects.py index bfaa29639..48c74a382 100644 --- a/mpcontribs-api/tests/integration/test_projects.py +++ b/mpcontribs-api/tests/integration/test_projects.py @@ -1,6 +1,8 @@ +from unittest.mock import AsyncMock + import pytest -from mpcontribs_api.domains.projects.dependencies import get_scoped_projects +from mpcontribs_api.domains.projects.dependencies import get_project_service, get_scoped_projects from mpcontribs_api.domains.projects.models import ProjectOut, Stats from mpcontribs_api.exceptions import ConflictError, NotFoundError from mpcontribs_api.pagination import Page @@ -37,6 +39,15 @@ def project_repo(test_app, mock_project_repo): test_app.dependency_overrides.pop(get_scoped_projects, None) +@pytest.fixture +def project_service(test_app): + """Override the assignment service the PATCH route depends on with an async mock.""" + service = AsyncMock() + test_app.dependency_overrides[get_project_service] = lambda: service + yield service + test_app.dependency_overrides.pop(get_project_service, None) + + # --------------------------------------------------------------------------- # GET /api/v1/projects # --------------------------------------------------------------------------- @@ -127,11 +138,11 @@ def test_all_sentinel_forwards_none(self, client, project_repo): _, kwargs = project_repo.get_projects.call_args assert kwargs["fields"] is None - def test_empty_fields_detail_forwards_identity_only(self, client, project_repo): - # Same three-way rule on the detail route. - project_repo.get_project_by_id.return_value = SAMPLE_PROJECT + def test_empty_fields_detail_forwards_identity_only(self, client, project_service): + # Same three-way rule on the detail route (now served through ProjectService). + project_service.get_one.return_value = SAMPLE_PROJECT client.get("/api/v1/projects/mp-sample", params={"_fields": ""}, headers=AUTHED_HEADERS) - _, kwargs = project_repo.get_project_by_id.call_args + _, kwargs = project_service.get_one.call_args assert kwargs["fields"] == frozenset({"id"}) @@ -141,44 +152,43 @@ def test_empty_fields_detail_forwards_identity_only(self, client, project_repo): class TestGetProjectById: - def test_found_returns_200(self, client, project_repo): - project_repo.get_project_by_id.return_value = SAMPLE_PROJECT + def test_found_returns_200(self, client, project_service): + project_service.get_one.return_value = SAMPLE_PROJECT r = client.get("/api/v1/projects/mp-sample", headers=AUTHED_HEADERS) assert r.status_code == 200 - def test_response_contains_project_data(self, client, project_repo): - project_repo.get_project_by_id.return_value = SAMPLE_PROJECT + def test_response_contains_project_data(self, client, project_service): + project_service.get_one.return_value = SAMPLE_PROJECT body = client.get("/api/v1/projects/mp-sample", headers=AUTHED_HEADERS).json() assert body["id"] == "mp-sample" assert body["title"] == "Sample Project" - def test_not_found_returns_404(self, client, project_repo): - project_repo.get_project_by_id.side_effect = NotFoundError("project not found") + def test_not_found_returns_404(self, client, project_service): + project_service.get_one.side_effect = NotFoundError("project not found") r = client.get("/api/v1/projects/nonexistent", headers=AUTHED_HEADERS) assert r.status_code == 404 - def test_not_found_error_code(self, client, project_repo): - project_repo.get_project_by_id.side_effect = NotFoundError("project not found") + def test_not_found_error_code(self, client, project_service): + project_service.get_one.side_effect = NotFoundError("project not found") body = client.get("/api/v1/projects/nonexistent", headers=AUTHED_HEADERS).json() assert body["error"]["code"] == "not_found" - def test_id_forwarded_to_repo(self, client, project_repo): - project_repo.get_project_by_id.return_value = SAMPLE_PROJECT + def test_id_forwarded_to_service(self, client, project_service): + project_service.get_one.return_value = SAMPLE_PROJECT client.get("/api/v1/projects/my-specific-id", headers=AUTHED_HEADERS) - _, kwargs = project_repo.get_project_by_id.call_args - assert kwargs["id"] == "my-specific-id" + assert project_service.get_one.call_args.args[0] == {"id": "my-specific-id"} - def test_fields_param_forwarded(self, client, project_repo): - project_repo.get_project_by_id.return_value = SAMPLE_PROJECT + def test_fields_param_forwarded(self, client, project_service): + project_service.get_one.return_value = SAMPLE_PROJECT client.get("/api/v1/projects/mp-sample", params={"_fields": "title"}, headers=AUTHED_HEADERS) - _, kwargs = project_repo.get_project_by_id.call_args + _, kwargs = project_service.get_one.call_args assert kwargs["fields"] is not None assert "title" in kwargs["fields"] - def test_no_fields_param_uses_default_fields(self, client, project_repo): - project_repo.get_project_by_id.return_value = SAMPLE_PROJECT + def test_no_fields_param_uses_default_fields(self, client, project_service): + project_service.get_one.return_value = SAMPLE_PROJECT client.get("/api/v1/projects/mp-sample", headers=AUTHED_HEADERS) - _, kwargs = project_repo.get_project_by_id.call_args + _, kwargs = project_service.get_one.call_args assert kwargs["fields"] is not None assert "title" in kwargs["fields"] @@ -189,8 +199,8 @@ def test_no_fields_param_uses_default_fields(self, client, project_repo): class TestPatchProject: - def test_valid_patch_returns_200(self, client, project_repo): - project_repo.patch_project_by_id.return_value = SAMPLE_PROJECT + def test_valid_patch_returns_200(self, client, project_service): + project_service.patch_one.return_value = SAMPLE_PROJECT r = client.patch( "/api/v1/projects/mp-sample", json={"title": "Updated Title"}, @@ -198,9 +208,9 @@ def test_valid_patch_returns_200(self, client, project_repo): ) assert r.status_code == 200 - def test_patch_response_is_project_out(self, client, project_repo): + def test_patch_response_is_project_out(self, client, project_service): updated = ProjectOut(id="mp-sample", title="Updated Title") - project_repo.patch_project_by_id.return_value = updated + project_service.patch_one.return_value = updated body = client.patch( "/api/v1/projects/mp-sample", json={"title": "Updated Title"}, @@ -208,8 +218,8 @@ def test_patch_response_is_project_out(self, client, project_repo): ).json() assert body["title"] == "Updated Title" - def test_not_found_returns_404(self, client, project_repo): - project_repo.patch_project_by_id.side_effect = NotFoundError("not found") + def test_not_found_returns_404(self, client, project_service): + project_service.patch_one.side_effect = NotFoundError("not found") r = client.patch( "/api/v1/projects/missing", json={"title": "x" * 5}, @@ -217,7 +227,7 @@ def test_not_found_returns_404(self, client, project_repo): ) assert r.status_code == 404 - def test_invalid_title_too_short_returns_422(self, client, project_repo): + def test_invalid_title_too_short_returns_422(self, client, project_service): r = client.patch( "/api/v1/projects/mp-sample", json={"title": "ab"}, @@ -225,16 +235,16 @@ def test_invalid_title_too_short_returns_422(self, client, project_repo): ) assert r.status_code == 422 - def test_id_and_update_forwarded_to_repo(self, client, project_repo): - project_repo.patch_project_by_id.return_value = SAMPLE_PROJECT + def test_id_and_update_forwarded_to_service(self, client, project_service): + project_service.patch_one.return_value = SAMPLE_PROJECT client.patch( "/api/v1/projects/mp-sample", json={"title": "New Name"}, headers=AUTHED_HEADERS, ) - _, kwargs = project_repo.patch_project_by_id.call_args - assert kwargs["id"] == "mp-sample" - assert kwargs["update"].title == "New Name" + call = project_service.patch_one.call_args + assert call.args[0] == {"id": "mp-sample"} + assert call.kwargs["update"].title == "New Name" # --------------------------------------------------------------------------- @@ -243,21 +253,20 @@ def test_id_and_update_forwarded_to_repo(self, client, project_repo): class TestDeleteProject: - def test_delete_returns_204(self, client, project_repo): - project_repo.delete_project_by_id.return_value = None + def test_delete_returns_204(self, client, project_service): + project_service.delete_one.return_value = None r = client.delete("/api/v1/projects/mp-sample", headers=AUTHED_HEADERS) assert r.status_code == 204 - def test_delete_response_has_no_body(self, client, project_repo): - project_repo.delete_project_by_id.return_value = None + def test_delete_response_has_no_body(self, client, project_service): + project_service.delete_one.return_value = None r = client.delete("/api/v1/projects/mp-sample", headers=AUTHED_HEADERS) assert r.content == b"" - def test_id_forwarded_to_repo(self, client, project_repo): - project_repo.delete_project_by_id.return_value = None + def test_id_forwarded_to_service(self, client, project_service): + project_service.delete_one.return_value = None client.delete("/api/v1/projects/mp-sample", headers=AUTHED_HEADERS) - _, kwargs = project_repo.delete_project_by_id.call_args - assert kwargs["id"] == "mp-sample" + assert project_service.delete_one.call_args.args[0] == {"id": "mp-sample"} # --------------------------------------------------------------------------- @@ -268,7 +277,7 @@ def test_id_forwarded_to_repo(self, client, project_repo): class TestUpsertProject: def _valid_body(self, **overrides): body = { - "_id": "mp-sample", + "id": "mp-sample", "title": "Test Project", "authors": "Alice", "description": "A project", @@ -278,17 +287,17 @@ def _valid_body(self, **overrides): body.update(overrides) return body - def test_valid_upsert_returns_200(self, client, project_repo): - project_repo.upsert_project_by_id.return_value = SAMPLE_PROJECT + def test_valid_upsert_returns_200(self, client, project_service): + project_service.upsert_one.return_value = SAMPLE_PROJECT r = client.put("/api/v1/projects/mp-sample", json=self._valid_body(), headers=AUTHED_HEADERS) assert r.status_code == 200 - def test_conflict_returns_409(self, client, project_repo): - project_repo.upsert_project_by_id.side_effect = ConflictError("already exists") + def test_conflict_returns_409(self, client, project_service): + project_service.upsert_one.side_effect = ConflictError("already exists") r = client.put("/api/v1/projects/mp-sample", json=self._valid_body(), headers=AUTHED_HEADERS) assert r.status_code == 409 - def test_missing_required_field_returns_422(self, client, project_repo): + def test_missing_required_field_returns_422(self, client, project_service): body = self._valid_body() del body["title"] r = client.put("/api/v1/projects/mp-sample", json=body, headers=AUTHED_HEADERS) @@ -303,7 +312,7 @@ def test_missing_required_field_returns_422(self, client, project_repo): class TestProjectMutationsRequireAuth: def _body(self): return { - "_id": "mp-sample", + "id": "mp-sample", "title": "Test Project", "authors": "Alice", "description": "A project", @@ -311,19 +320,19 @@ def _body(self): "stats": {"columns": 0, "contributions": 0, "tables": 0, "structures": 0, "attachments": 0, "size": 0.0}, } - def test_anonymous_put_returns_401(self, client, project_repo): - project_repo.upsert_project_by_id.return_value = SAMPLE_PROJECT + def test_anonymous_put_returns_401(self, client, project_service): + project_service.upsert_one.return_value = SAMPLE_PROJECT r = client.put("/api/v1/projects/mp-sample", json=self._body(), headers=ANON_HEADERS) assert r.status_code == 401 assert r.json()["error"]["code"] == "authentication_error" - project_repo.upsert_project_by_id.assert_not_called() + project_service.upsert_one.assert_not_called() - def test_anonymous_patch_returns_401(self, client, project_repo): + def test_anonymous_patch_returns_401(self, client, project_service): r = client.patch("/api/v1/projects/mp-sample", json={"title": "Updated Title"}, headers=ANON_HEADERS) assert r.status_code == 401 - project_repo.patch_project_by_id.assert_not_called() + project_service.patch_one.assert_not_called() - def test_anonymous_delete_returns_401(self, client, project_repo): + def test_anonymous_delete_returns_401(self, client, project_service): r = client.delete("/api/v1/projects/mp-sample", headers=ANON_HEADERS) assert r.status_code == 401 - project_repo.delete_project_by_id.assert_not_called() + project_service.delete_one.assert_not_called() diff --git a/mpcontribs-api/tests/unit/domains/test_component_service.py b/mpcontribs-api/tests/unit/domains/test_component_service.py index d75b02c00..c9f981e37 100644 --- a/mpcontribs-api/tests/unit/domains/test_component_service.py +++ b/mpcontribs-api/tests/unit/domains/test_component_service.py @@ -15,6 +15,13 @@ def _oid() -> PydanticObjectId: return PydanticObjectId() +def _coerce_identifiers(identifiers: dict) -> dict: + """Stub for the repo's ObjectId-keyed ``coerce_identifiers`` (string id -> ObjectId).""" + if isinstance(identifiers.get("id"), str): + return {**identifiers, "id": PydanticObjectId(identifiers["id"])} + return identifiers + + def _make_service( *, candidate_ids: list[PydanticObjectId], @@ -29,9 +36,8 @@ def _make_service( components = AsyncMock(name="components") components.list_ids = AsyncMock(return_value=candidate_ids) components.delete_by_ids = AsyncMock(side_effect=lambda ids: DeleteResponse(num_deleted=len(ids))) - components.delete_by_id = AsyncMock(return_value=DeleteResponse(num_deleted=1)) - components._convert_object_id = MagicMock(side_effect=lambda s: PydanticObjectId(s)) - components._not_found = MagicMock(return_value="not found") + components.delete_one = AsyncMock(return_value=DeleteResponse(num_deleted=1)) + components.coerce_identifiers = MagicMock(side_effect=_coerce_identifiers) contributions = AsyncMock(name="contributions") @@ -121,30 +127,30 @@ async def test_delete_by_id_not_reachable_raises_not_found(): svc, _, _ = _make_service(candidate_ids=[], reachable=set(), referenced=set()) with pytest.raises(NotFoundError): - await svc.delete_by_id(str(oid)) + await svc.delete_one({"id": str(oid)}) async def test_delete_by_id_referenced_is_skipped(): oid = _oid() svc, components, _ = _make_service(candidate_ids=[], reachable={oid}, referenced={oid}) - result = await svc.delete_by_id(str(oid)) + result = await svc.delete_one({"id": str(oid)}) assert result.num_deleted == 0 assert result.num_skipped == 1 assert result.referenced_ids == [oid] - components.delete_by_id.assert_not_awaited() + components.delete_one.assert_not_awaited() async def test_delete_by_id_reachable_and_unreferenced_deletes(): oid = _oid() svc, components, _ = _make_service(candidate_ids=[], reachable={oid}, referenced=set()) - result = await svc.delete_by_id(str(oid)) + result = await svc.delete_one({"id": str(oid)}) assert result.num_deleted == 1 assert result.num_skipped == 0 - components.delete_by_id.assert_awaited_once_with(oid) + components.delete_one.assert_awaited_once_with({"id": oid}) # --------------------------------------------------------------------------- @@ -155,8 +161,7 @@ async def test_delete_by_id_reachable_and_unreferenced_deletes(): def _make_read_service(*, reachable: set[PydanticObjectId]) -> tuple[ComponentService, AsyncMock, AsyncMock]: """ComponentService whose contribution repo reports `reachable` ids as in-scope.""" components = AsyncMock(name="components") - components._convert_object_id = MagicMock(side_effect=lambda s: PydanticObjectId(s)) - components._not_found = MagicMock(return_value="not found") + components.coerce_identifiers = MagicMock(side_effect=_coerce_identifiers) contributions = AsyncMock(name="contributions") @@ -175,21 +180,21 @@ async def test_get_by_id_unreachable_returns_none_without_fetch(): oid = _oid() svc, components, _ = _make_read_service(reachable=set()) - result = await svc.get_by_id(str(oid), fields=None) + result = await svc.get_one({"id": str(oid)}, fields=None) assert result is None - components.get_component_by_id.assert_not_awaited() + components.get_one.assert_not_awaited() async def test_get_by_id_reachable_fetches_component(): oid = _oid() svc, components, _ = _make_read_service(reachable={oid}) - components.get_component_by_id = AsyncMock(return_value="the-component") + components.get_one = AsyncMock(return_value="the-component") - result = await svc.get_by_id(str(oid), fields=None) + result = await svc.get_one({"id": str(oid)}, fields=None) assert result == "the-component" - components.get_component_by_id.assert_awaited_once() + components.get_one.assert_awaited_once() async def test_get_many_restricts_to_reachable_ids(): @@ -211,16 +216,16 @@ async def test_patch_by_id_unreachable_raises_not_found(): svc, components, _ = _make_read_service(reachable=set()) with pytest.raises(NotFoundError): - await svc.patch_by_id(str(oid), update=MagicMock()) - components.patch_component_by_id.assert_not_awaited() + await svc.patch_one({"id": str(oid)}, update=MagicMock()) + components.patch_one.assert_not_awaited() async def test_patch_by_id_reachable_patches(): oid = _oid() svc, components, _ = _make_read_service(reachable={oid}) - components.patch_component_by_id = AsyncMock(return_value="patched") + components.patch_one = AsyncMock(return_value="patched") - result = await svc.patch_by_id(str(oid), update=MagicMock()) + result = await svc.patch_one({"id": str(oid)}, update=MagicMock()) assert result == "patched" - components.patch_component_by_id.assert_awaited_once() + components.patch_one.assert_awaited_once() diff --git a/mpcontribs-api/tests/unit/domains/test_consumers_models.py b/mpcontribs-api/tests/unit/domains/test_consumers_models.py index 573ba5560..d372f74d5 100644 --- a/mpcontribs-api/tests/unit/domains/test_consumers_models.py +++ b/mpcontribs-api/tests/unit/domains/test_consumers_models.py @@ -31,7 +31,7 @@ def test_partial_override_keeps_other_defaults(self): assert settings.max_columns == get_settings().consumer.max_columns def test_only_explicit_field_is_marked_set(self): - # patch_consumer_by_id relies on exclude_unset to touch only the named limit, so a partial + # patch_one relies on exclude_unset to touch only the named limit, so a partial # override must report exactly the fields the admin supplied. settings = ConsumerSettings(max_columns=5) assert settings.model_dump(exclude_unset=True) == {"max_columns": 5} diff --git a/mpcontribs-api/tests/unit/domains/test_contribution_service.py b/mpcontribs-api/tests/unit/domains/test_contribution_service.py index 2a5a7dbcb..b62d99aa0 100644 --- a/mpcontribs-api/tests/unit/domains/test_contribution_service.py +++ b/mpcontribs-api/tests/unit/domains/test_contribution_service.py @@ -224,7 +224,7 @@ def _make_service( def _approved_projects_repo() -> AsyncMock: """A projects repo whose every project reads as approved (quota does not apply).""" repo = AsyncMock() - repo.get_by_id = AsyncMock(return_value=MagicMock(is_approved=True)) + repo.get_one = AsyncMock(return_value=MagicMock(is_approved=True)) repo.unique_columns_by_id.side_effect = lambda ids: {pid: None for pid in ids} return repo @@ -232,7 +232,7 @@ def _approved_projects_repo() -> AsyncMock: def _unapproved_projects_repo() -> AsyncMock: """A projects repo whose every project reads as unapproved (quota applies).""" repo = AsyncMock() - repo.get_by_id = AsyncMock(return_value=MagicMock(is_approved=False)) + repo.get_one = AsyncMock(return_value=MagicMock(is_approved=False)) repo.unique_columns_by_id.side_effect = lambda ids: {pid: None for pid in ids} return repo @@ -317,13 +317,13 @@ async def test_oversize_contribution_goes_to_failures_without_db(self): def _projects_repo_by_approval(approval: dict[str, bool]) -> AsyncMock: - """Projects repo whose ``get_by_id`` reports approval per project id from ``approval``.""" + """Projects repo whose ``get_one`` reports approval per project id from ``approval``.""" repo = AsyncMock() - async def _get_by_id(project_id, fields=None): - return MagicMock(is_approved=approval[project_id]) + async def _get_one(identifiers, fields=None): + return MagicMock(is_approved=approval[identifiers["id"]]) - repo.get_by_id = AsyncMock(side_effect=_get_by_id) + repo.get_one = AsyncMock(side_effect=_get_one) repo.unique_columns_by_id.side_effect = lambda ids: {pid: None for pid in ids} return repo @@ -466,7 +466,7 @@ async def test_only_new_documents_count_against_cap(self, monkeypatch): # cap 3, 2 stored -> one slot for a new document; updating an existing one is free. monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 3) contrib_repo = AsyncMock() - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") contrib_repo.count_contributions_for_project.return_value = 2 svc, *_ = _make_service(contributions=contrib_repo, projects=_unapproved_projects_repo()) # Set after _make_service, which stubs existing_identities to an empty set. 'a' already exists. @@ -486,13 +486,13 @@ async def test_only_new_documents_count_against_cap(self, monkeypatch): assert len(summary.succeeded) == 2 assert [f.index for f in summary.failed] == [2] assert summary.failed[0].error_code == "permission_denied" - upserted = {c.args[1].material_id for c in contrib_repo.upsert_contribution_by_identifiers.call_args_list} + upserted = {c.args[1].material_id for c in contrib_repo.upsert_one.call_args_list} assert upserted == {_mp_id_for("a"), _mp_id_for("b")} async def test_pure_updates_are_never_capped(self, monkeypatch): monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 1) contrib_repo = AsyncMock() - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") contrib_repo.count_contributions_for_project.return_value = 99 # far over cap svc, *_ = _make_service(contributions=contrib_repo, projects=_unapproved_projects_repo()) # Every contribution in the batch is an existing document -> all are free updates. @@ -507,12 +507,12 @@ async def test_pure_updates_are_never_capped(self, monkeypatch): assert len(summary.succeeded) == 3 assert summary.failed == [] - assert contrib_repo.upsert_contribution_by_identifiers.call_count == 3 + assert contrib_repo.upsert_one.call_count == 3 async def test_approved_project_skips_quota(self, monkeypatch): monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 1) contrib_repo = AsyncMock() - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") svc, *_ = _make_service(contributions=contrib_repo, projects=_approved_projects_repo()) summary = await svc.upsert_contributions([_contrib_in(identifier=f"mp-{i}") for i in range(3)]) @@ -532,7 +532,7 @@ class TestUpsertContributionByIdQuota: async def test_update_existing_allowed_even_over_cap(self, monkeypatch): monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 1) contrib_repo = AsyncMock() - contrib_repo.get_contribution_by_id.return_value = MagicMock(spec=Contribution) # id exists -> update + contrib_repo.get_one.return_value = MagicMock(spec=Contribution) # id exists -> update contrib_repo.count_contributions_for_project.return_value = 99 contrib_repo.upsert_contribution_by_id.return_value = MagicMock(spec=Contribution) svc, *_ = _make_service(contributions=contrib_repo, projects=_unapproved_projects_repo()) @@ -545,7 +545,7 @@ async def test_update_existing_allowed_even_over_cap(self, monkeypatch): async def test_new_insert_over_cap_rejected(self, monkeypatch): monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 2) contrib_repo = AsyncMock() - contrib_repo.get_contribution_by_id.return_value = None # id absent -> would insert + contrib_repo.get_one.return_value = None # id absent -> would insert contrib_repo.count_contributions_for_project.return_value = 5 # over cap svc, *_ = _make_service(contributions=contrib_repo, projects=_unapproved_projects_repo()) @@ -557,7 +557,7 @@ async def test_new_insert_over_cap_rejected(self, monkeypatch): async def test_new_insert_under_cap_allowed(self, monkeypatch): monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 5) contrib_repo = AsyncMock() - contrib_repo.get_contribution_by_id.return_value = None + contrib_repo.get_one.return_value = None contrib_repo.count_contributions_for_project.return_value = 1 contrib_repo.upsert_contribution_by_id.return_value = MagicMock(spec=Contribution) svc, *_ = _make_service(contributions=contrib_repo, projects=_unapproved_projects_repo()) @@ -570,7 +570,7 @@ async def test_new_insert_at_exactly_cap_rejected(self, monkeypatch): # stored == cap: the project is full, so a brand-new document is rejected (no cap+1 slack). monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 2) contrib_repo = AsyncMock() - contrib_repo.get_contribution_by_id.return_value = None + contrib_repo.get_one.return_value = None contrib_repo.count_contributions_for_project.return_value = 2 svc, *_ = _make_service(contributions=contrib_repo, projects=_unapproved_projects_repo()) @@ -582,7 +582,7 @@ async def test_new_insert_at_exactly_cap_rejected(self, monkeypatch): async def test_new_insert_approved_project_unlimited(self, monkeypatch): monkeypatch.setattr(get_settings().consumer, "max_unapproved_contributions_per_project", 1) contrib_repo = AsyncMock() - contrib_repo.get_contribution_by_id.return_value = None + contrib_repo.get_one.return_value = None contrib_repo.upsert_contribution_by_id.return_value = MagicMock(spec=Contribution) svc, *_ = _make_service(contributions=contrib_repo, projects=_approved_projects_repo()) @@ -927,21 +927,21 @@ async def test_insert_project_not_found_is_validation_failure(self): async def test_upsert_does_not_conflict_on_existing_identity(self): """Upsert targets an existing identity (update), so it must not pre-reject as a conflict.""" svc, contrib_repo, *_ = _make_service() - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") await svc.upsert_contributions([_contrib_in(identifier="mp-1")]) # existing_identities is not consulted on the upsert path contrib_repo.existing_identities.assert_not_called() - contrib_repo.upsert_contribution_by_identifiers.assert_called_once() + contrib_repo.upsert_one.assert_called_once() async def test_upsert_passes_resolved_unique_value_in_identifiers(self): svc, contrib_repo, *_ = _make_service(unique_column="sample_id") - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") await svc.upsert_contributions([_contrib_in(data={"sample_id": "A"})]) - identifiers = contrib_repo.upsert_contribution_by_identifiers.call_args.args[0] + identifiers = contrib_repo.upsert_one.call_args.args[0] assert identifiers["unique_value"] == "A" async def test_upsert_missing_unique_column_value_is_validation_failure(self): @@ -952,7 +952,7 @@ async def test_upsert_missing_unique_column_value_is_validation_failure(self): assert summary.succeeded == [] assert [f.error_code for f in summary.failed] == ["validation_error"] assert "unique_column" in summary.failed[0].message - contrib_repo.upsert_contribution_by_identifiers.assert_not_called() + contrib_repo.upsert_one.assert_not_called() # --------------------------------------------------------------------------- @@ -1003,7 +1003,7 @@ async def test_raises_before_any_db_write(self): dirty = _contrib_in(structures=[_structure_in()]) with pytest.raises(ValidationError): await svc.upsert_contributions([dirty]) - contrib_repo.upsert_contribution_by_identifiers.assert_not_called() + contrib_repo.upsert_one.assert_not_called() contrib_repo.insert_contribution.assert_not_called() @@ -1015,7 +1015,7 @@ async def test_raises_before_any_db_write(self): class TestUpsertContributionsAtomic: async def test_calls_atomic_repo_method_once_per_item(self): svc, contrib_repo, *_ = _make_service() - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") contribs = [_contrib_in(identifier=f"mp-{i}") for i in range(3)] summary = await svc.upsert_contributions(contribs) @@ -1023,19 +1023,18 @@ async def test_calls_atomic_repo_method_once_per_item(self): assert summary.total == 3 assert len(summary.succeeded) == 3 assert summary.failed == [] - assert contrib_repo.upsert_contribution_by_identifiers.call_count == 3 - # The legacy read-then-write path must not be used - contrib_repo.update_contribution.assert_not_called() + assert contrib_repo.upsert_one.call_count == 3 + # The atomic upsert path is used, not the bulk insert path. contrib_repo.insert_contribution.assert_not_called() async def test_passes_identifiers_dict_and_input_to_repo(self): svc, contrib_repo, *_ = _make_service() - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") contrib = _contrib_in(project="my-proj", material_id="mp-99") await svc.upsert_contributions([contrib]) - call = contrib_repo.upsert_contribution_by_identifiers.call_args + call = contrib_repo.upsert_one.call_args assert call.args[0] == { "project": "my-proj", "material_id": "mp-99", @@ -1058,7 +1057,7 @@ async def _upsert(identifiers, contrib): returned[contrib.material_id] = doc return doc - contrib_repo.upsert_contribution_by_identifiers.side_effect = _upsert + contrib_repo.upsert_one.side_effect = _upsert contribs = [_contrib_in(identifier=f"mp-{i}") for i in range(3)] summary = await svc.upsert_contributions(contribs) @@ -1071,7 +1070,7 @@ async def test_empty_batch_returns_empty_summary(self): assert summary.total == 0 assert summary.succeeded == [] assert summary.failed == [] - contrib_repo.upsert_contribution_by_identifiers.assert_not_called() + contrib_repo.upsert_one.assert_not_called() async def test_same_key_concurrent_upserts_both_go_through_atomic_call(self): """Race-safety regression: two items with the same (project, identifier) in one batch @@ -1079,7 +1078,7 @@ async def test_same_key_concurrent_upserts_both_go_through_atomic_call(self): tiebreaker — the service must not pre-deduplicate or otherwise swallow one. """ svc, contrib_repo, *_ = _make_service() - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") contribs = [ _contrib_in(project="prj", identifier="same"), @@ -1088,7 +1087,7 @@ async def test_same_key_concurrent_upserts_both_go_through_atomic_call(self): summary = await svc.upsert_contributions(contribs) assert len(summary.succeeded) == 2 - assert contrib_repo.upsert_contribution_by_identifiers.call_count == 2 + assert contrib_repo.upsert_one.call_count == 2 async def test_one_failure_is_reported_not_raised(self): svc, contrib_repo, *_ = _make_service() @@ -1098,7 +1097,7 @@ async def _upsert(identifiers, contrib): raise ConflictError("boom") return MagicMock(spec=Contribution, project="proj") - contrib_repo.upsert_contribution_by_identifiers.side_effect = _upsert + contrib_repo.upsert_one.side_effect = _upsert contribs = [_contrib_in(identifier=f"mp-{i}") for i in range(3)] summary = await svc.upsert_contributions(contribs) @@ -1166,7 +1165,7 @@ async def test_insert_unauthorized_and_oversize_yield_single_failure(self): async def test_upsert_rejects_unauthorized_project_per_item(self): svc, contrib_repo, *_ = _make_service(user=_member_user("allowed")) - contrib_repo.upsert_contribution_by_identifiers.return_value = MagicMock(spec=Contribution, project="proj") + contrib_repo.upsert_one.return_value = MagicMock(spec=Contribution, project="proj") contribs = [ _contrib_in(project="allowed", identifier="ok"), @@ -1180,7 +1179,7 @@ async def test_upsert_rejects_unauthorized_project_per_item(self): assert summary.failed[0].error_code == "permission_denied" assert "forbidden" in summary.failed[0].message # Only the authorized item reached the atomic repo method - contrib_repo.upsert_contribution_by_identifiers.assert_called_once() + contrib_repo.upsert_one.assert_called_once() async def test_upsert_anonymous_authorized_for_nothing(self): svc, contrib_repo, *_ = _make_service(user=User()) # anonymous: no username, no groups @@ -1190,7 +1189,7 @@ async def test_upsert_anonymous_authorized_for_nothing(self): assert summary.total == 1 assert summary.succeeded == [] assert [f.error_code for f in summary.failed] == ["permission_denied"] - contrib_repo.upsert_contribution_by_identifiers.assert_not_called() + contrib_repo.upsert_one.assert_not_called() async def test_upsert_by_id_rejects_unauthorized_project(self): # A member of "allowed" cannot write "forbidden" through the by-id endpoint. The check runs @@ -1201,7 +1200,7 @@ async def test_upsert_by_id_rejects_unauthorized_project(self): with pytest.raises(PermissionError, match="forbidden"): await svc.upsert_contribution_by_id("someid", _contrib_in(project="forbidden")) - contrib_repo.get_contribution_by_id.assert_not_called() + contrib_repo.get_one.assert_not_called() contrib_repo.upsert_contribution_by_id.assert_not_called() async def test_upsert_by_id_unauthorized_cannot_overwrite_public_contribution(self): @@ -1209,7 +1208,7 @@ async def test_upsert_by_id_unauthorized_cannot_overwrite_public_contribution(se # repository read scope would admit a public row, authorization is enforced up front. svc, contrib_repo, *_ = _make_service(user=_member_user("allowed")) # An existing (readable) contribution must not lower the bar — authz still rejects the write. - contrib_repo.get_contribution_by_id.return_value = MagicMock(spec=Contribution) + contrib_repo.get_one.return_value = MagicMock(spec=Contribution) with pytest.raises(PermissionError, match="forbidden"): await svc.upsert_contribution_by_id("someid", _contrib_in(project="forbidden")) @@ -1222,14 +1221,14 @@ async def test_upsert_by_id_anonymous_authorized_for_nothing(self): with pytest.raises(PermissionError): await svc.upsert_contribution_by_id("someid", _contrib_in(project="any")) - contrib_repo.get_contribution_by_id.assert_not_called() + contrib_repo.get_one.assert_not_called() contrib_repo.upsert_contribution_by_id.assert_not_called() async def test_upsert_by_id_authorized_member_proceeds(self): # A member writing to their own project passes authorization; updating an existing row is # not gated by the quota, so the write goes through. contrib_repo = AsyncMock() - contrib_repo.get_contribution_by_id.return_value = MagicMock(spec=Contribution) # exists -> update + contrib_repo.get_one.return_value = MagicMock(spec=Contribution) # exists -> update contrib_repo.upsert_contribution_by_id.return_value = MagicMock(spec=Contribution) svc, *_ = _make_service( contributions=contrib_repo, projects=_unapproved_projects_repo(), user=_member_user("allowed") @@ -1241,7 +1240,7 @@ async def test_upsert_by_id_authorized_member_proceeds(self): async def test_upsert_by_id_admin_bypasses_authorization(self): contrib_repo = AsyncMock() - contrib_repo.get_contribution_by_id.return_value = MagicMock(spec=Contribution) + contrib_repo.get_one.return_value = MagicMock(spec=Contribution) contrib_repo.upsert_contribution_by_id.return_value = MagicMock(spec=Contribution) svc, *_ = _make_service(contributions=contrib_repo, projects=_approved_projects_repo()) # admin default @@ -1500,33 +1499,33 @@ class TestPatchIdentifierHierarchy: async def test_patch_material_id_onto_doc_without_formula_raises(self): svc, contrib_repo, *_ = _make_service() existing = _existing_doc(material_id=None, chemical_system_id="Fe-O", formula=None) - contrib_repo.get_contribution_by_id.return_value = existing + contrib_repo.get_one.return_value = existing with pytest.raises(ValidationError, match="formula is required when material_id"): await svc.patch_contribution_by_id(str(existing.id), ContributionPatch(material_id="mp-1")) # Rejected before any write. - contrib_repo.patch_contribution_by_id.assert_not_called() + contrib_repo.patch_one.assert_not_called() async def test_patch_material_id_when_existing_has_formula_ok(self): svc, contrib_repo, *_ = _make_service() existing = _existing_doc(material_id=None, chemical_system_id="Fe-O", formula="Fe2O3") - contrib_repo.get_contribution_by_id.return_value = existing - contrib_repo.patch_contribution_by_id.return_value = MagicMock(spec=Contribution) + contrib_repo.get_one.return_value = existing + contrib_repo.patch_one.return_value = MagicMock(spec=Contribution) await svc.patch_contribution_by_id(str(existing.id), ContributionPatch(material_id="mp-1")) - contrib_repo.patch_contribution_by_id.assert_called_once() + contrib_repo.patch_one.assert_called_once() async def test_metadata_only_patch_skips_existing_read(self): svc, contrib_repo, *_ = _make_service() - contrib_repo.patch_contribution_by_id.return_value = MagicMock(spec=Contribution) + contrib_repo.patch_one.return_value = MagicMock(spec=Contribution) await svc.patch_contribution_by_id("some-id", ContributionPatch(is_public=True)) # No identity/unique inputs touched -> no re-read, straight to the plain patch. - contrib_repo.get_contribution_by_id.assert_not_called() - contrib_repo.patch_contribution_by_id.assert_called_once() + contrib_repo.get_one.assert_not_called() + contrib_repo.patch_one.assert_called_once() # --------------------------------------------------------------------------- @@ -1539,26 +1538,26 @@ async def test_data_patch_defaults_to_merge_and_forwards_replace_false(self): svc, contrib_repo, *_ = _make_service() existing = _existing_doc(material_id=None, chemical_system_id="Fe-O", formula="Fe2O3") existing.data = {"x": 1.0} - contrib_repo.get_contribution_by_id.return_value = existing - contrib_repo.patch_contribution_by_id.return_value = MagicMock(spec=Contribution) + contrib_repo.get_one.return_value = existing + contrib_repo.patch_one.return_value = MagicMock(spec=Contribution) await svc.patch_contribution_by_id(str(existing.id), ContributionPatch(data={"y": 9.0})) # The repo performs the actual dotted-$set merge; the service just forwards replace_data=False. - assert contrib_repo.patch_contribution_by_id.call_args.kwargs["replace_data"] is False + assert contrib_repo.patch_one.call_args.kwargs["replace_data"] is False async def test_replace_data_flag_forwarded_to_repo(self): svc, contrib_repo, *_ = _make_service() existing = _existing_doc(material_id=None, chemical_system_id="Fe-O", formula="Fe2O3") existing.data = {"x": 1.0} - contrib_repo.get_contribution_by_id.return_value = existing - contrib_repo.patch_contribution_by_id.return_value = MagicMock(spec=Contribution) + contrib_repo.get_one.return_value = existing + contrib_repo.patch_one.return_value = MagicMock(spec=Contribution) await svc.patch_contribution_by_id( str(existing.id), ContributionPatch(data={"y": 9.0}), replace_data=True ) - assert contrib_repo.patch_contribution_by_id.call_args.kwargs["replace_data"] is True + assert contrib_repo.patch_one.call_args.kwargs["replace_data"] is True async def test_merge_resolves_unique_value_from_merged_state(self): # The unique_column value lives in the stored data and is NOT in the patch. A merge preserves @@ -1566,13 +1565,13 @@ async def test_merge_resolves_unique_value_from_merged_state(self): svc, contrib_repo, *_ = _make_service(unique_column="sample_id") existing = _existing_doc(material_id=None, chemical_system_id="Fe-O", formula="Fe2O3") existing.data = {"sample_id": 42, "x": 1.0} - contrib_repo.get_contribution_by_id.return_value = existing - contrib_repo.patch_contribution_by_id.return_value = MagicMock(spec=Contribution) + contrib_repo.get_one.return_value = existing + contrib_repo.patch_one.return_value = MagicMock(spec=Contribution) await svc.patch_contribution_by_id(str(existing.id), ContributionPatch(data={"y": 9.0})) # Resolved from {sample_id:42, x:1, y:9}, so the untouched unique_value survives the merge. - assert contrib_repo.patch_contribution_by_id.call_args.kwargs["unique_value"] == 42 + assert contrib_repo.patch_one.call_args.kwargs["unique_value"] == 42 async def test_replace_resolves_unique_value_from_patch_data_only(self): # On replace the stored data is discarded, so a unique_column absent from the patch is a @@ -1580,10 +1579,10 @@ async def test_replace_resolves_unique_value_from_patch_data_only(self): svc, contrib_repo, *_ = _make_service(unique_column="sample_id") existing = _existing_doc(material_id=None, chemical_system_id="Fe-O", formula="Fe2O3") existing.data = {"sample_id": 42} - contrib_repo.get_contribution_by_id.return_value = existing + contrib_repo.get_one.return_value = existing with pytest.raises(ValidationError, match="unique_column"): await svc.patch_contribution_by_id( str(existing.id), ContributionPatch(data={"y": 9.0}), replace_data=True ) - contrib_repo.patch_contribution_by_id.assert_not_called() + contrib_repo.patch_one.assert_not_called() diff --git a/mpcontribs-api/tests/unit/domains/test_initiatives_models.py b/mpcontribs-api/tests/unit/domains/test_initiatives_models.py new file mode 100644 index 000000000..956c82c13 --- /dev/null +++ b/mpcontribs-api/tests/unit/domains/test_initiatives_models.py @@ -0,0 +1,100 @@ +import pytest +from beanie import PydanticObjectId +from pydantic import ValidationError as PydanticValidationError + +from mpcontribs_api.domains.initiatives.models import ( + Initiative, + InitiativeIn, + InitiativeOut, + InitiativePatch, +) +from mpcontribs_api.exceptions import ValidationError + +OID = PydanticObjectId() +OWNER = "google:alice@example.com" + + +def _init(**overrides) -> Initiative: + payload = {"_id": OID, "slug": "battery-genome", "name": "Battery Genome", "owner": OWNER} + payload.update(overrides) + return Initiative.model_validate(payload) + + +# --------------------------------------------------------------------------- +# Slug validation / normalisation +# --------------------------------------------------------------------------- + + +class TestSlug: + def test_lowercases_and_strips(self): + assert _init(slug=" Battery-Genome-2025 ").slug == "battery-genome-2025" + + @pytest.mark.parametrize("bad", ["has space", "under_score", "trailing-", "-leading", "sym!bol", "Dou--ble"]) + def test_rejects_malformed_slug(self, bad): + with pytest.raises(ValidationError): + _init(slug=bad) + + def test_rejects_too_short_via_length(self): + # "ab" is well-formed but below the 3-char minimum, so the length constraint rejects it. + with pytest.raises(PydanticValidationError): + _init(slug="ab") + + +# --------------------------------------------------------------------------- +# Initiative document invariants +# --------------------------------------------------------------------------- + + +class TestInitiative: + def test_defaults_private_unapproved(self): + init = _init() + assert init.is_public is False + assert init.is_approved is False + + def test_public_and_approved_ok(self): + init = _init(is_public=True, is_approved=True) + assert init.is_public is True + + def test_public_without_approved_rejected(self): + with pytest.raises(ValidationError): + _init(is_public=True, is_approved=False) + + +# --------------------------------------------------------------------------- +# InitiativeIn (create contract) +# --------------------------------------------------------------------------- + + +class TestInitiativeIn: + def test_minimal_valid(self): + data = InitiativeIn(slug="battery-genome", name="Battery Genome") + assert data.slug == "battery-genome" + + @pytest.mark.parametrize("field", ["owner", "is_public", "is_approved", "id", "unknown"]) + def test_forbids_server_controlled_and_unknown_fields(self, field): + # owner and the flags are forced server-side; none are part of the input contract. + with pytest.raises(PydanticValidationError): + InitiativeIn(slug="battery-genome", name="Battery Genome", **{field: "x"}) + + def test_normalises_slug(self): + assert InitiativeIn(slug="Battery-Genome", name="x").slug == "battery-genome" + + +# --------------------------------------------------------------------------- +# InitiativeOut / InitiativePatch shape +# --------------------------------------------------------------------------- + + +class TestOutAndPatch: + def test_out_populates_id_from_alias(self): + out = InitiativeOut.model_validate({"_id": OID, "slug": "battery-genome"}) + assert out.id == OID + + def test_patch_tracks_only_set_fields(self): + patch = InitiativePatch(name="Renamed") + assert patch.model_dump(exclude_unset=True) == {"name": "Renamed"} + + def test_patch_has_no_slug_or_owner_field(self): + # slug and owner are immutable, so they are not part of the patch surface. + assert "slug" not in InitiativePatch.model_fields + assert "owner" not in InitiativePatch.model_fields diff --git a/mpcontribs-api/tests/unit/domains/test_project_group_service.py b/mpcontribs-api/tests/unit/domains/test_project_group_service.py new file mode 100644 index 000000000..52a90d68b --- /dev/null +++ b/mpcontribs-api/tests/unit/domains/test_project_group_service.py @@ -0,0 +1,186 @@ +from unittest.mock import AsyncMock, MagicMock + +import pytest +from beanie import Link, PydanticObjectId +from bson import DBRef + +from mpcontribs_api.authz import User +from mpcontribs_api.domains.project_groups.models import ProjectGroupIn, ProjectGroupOut +from mpcontribs_api.domains.project_groups.service import ProjectGroupService +from mpcontribs_api.domains.projects.models import Project +from mpcontribs_api.exceptions import ConflictError, NotFoundError + +pytestmark = pytest.mark.asyncio + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_service(group: ProjectGroupOut | None, *, visible_projects: set[str] | None = None, ambiguous: bool = False): + """Build a service over stubbed repos. + + ``group`` is what the groups repo resolves to (None => not found). ``visible_projects`` gates + which project ids the projects repo reports as existing/visible. ``ambiguous`` makes identifier + resolution raise ConflictError (duplicate under the unique key). + """ + visible = visible_projects or set() + groups = AsyncMock() + projects = AsyncMock() + # insert() forces owner to the caller for non-admins; give the stub an admin user so these + # payload-identity assertions exercise the pass-through path (owner-forcing is covered end-to-end + # in the db service test). + groups._user = User(username="google:admin@example.com", groups=frozenset({"admin"})) + + if ambiguous: + groups.get_one.side_effect = ConflictError("ambiguous") + else: + groups.get_one.return_value = group + # coerce_identifiers is a sync repo method; keep it sync so the service gets a real dict, not a coroutine. + def _coerce_identifiers(identifiers): + if isinstance(identifiers.get("id"), str): + return {**identifiers, "id": PydanticObjectId(identifiers["id"])} + return identifiers + + groups.coerce_identifiers = MagicMock(side_effect=_coerce_identifiers) + groups.add_project_refs.return_value = group + groups.delete_project_refs.return_value = group + + async def _get_project(identifiers, fields=None): + pid = identifiers["id"] + return {"_id": pid} if pid in visible else None + + projects.get_one.side_effect = _get_project + + return ProjectGroupService(groups=groups, projects=projects), groups, projects + + +def _group(project_ids: list[str] | None = None) -> ProjectGroupOut: + group = ProjectGroupOut.model_validate( + {"_id": PydanticObjectId(), "name": "g", "owner": "google:a@b.com", "projects": []} + ) + # Members are stored as Links (DBRefs); set them directly to sidestep Link revalidation. + group.projects = [Link(DBRef("projects", pid), Project) for pid in (project_ids or [])] + return group + + +# --------------------------------------------------------------------------- +# insert +# --------------------------------------------------------------------------- + + +class TestInsert: + def _payload(self, projects: list[str]) -> ProjectGroupIn: + return ProjectGroupIn(name="g", owner="google:a@b.com", description="d", projects=projects) + + async def test_all_projects_valid_inserts(self): + service, groups, _ = _make_service(None, visible_projects={"mp-1", "mp-2"}) + groups.insert_project_group.return_value = "stored" + payload = self._payload(["mp-1", "mp-2"]) + result = await service.insert(payload) + assert result == "stored" + groups.insert_project_group.assert_awaited_once_with(payload) + + async def test_missing_project_raises_not_found_and_skips_insert(self): + service, groups, _ = _make_service(None, visible_projects={"mp-1"}) + with pytest.raises(NotFoundError) as exc: + await service.insert(self._payload(["mp-1", "ghost"])) + assert exc.value.context["ids"] == ["ghost"] + groups.insert_project_group.assert_not_awaited() + + async def test_empty_projects_inserts_without_validation(self): + service, groups, projects = _make_service(None) + payload = self._payload([]) + await service.insert(payload) + projects.get_one.assert_not_awaited() + groups.insert_project_group.assert_awaited_once_with(payload) + + +# --------------------------------------------------------------------------- +# Group resolution +# --------------------------------------------------------------------------- + + +class TestGroupResolution: + async def test_add_by_id_missing_group_raises_not_found(self): + service, _, _ = _make_service(None) + with pytest.raises(NotFoundError): + await service.add_projects({"id": "0" * 24}, ["mp-1"]) + + async def test_add_by_identifiers_missing_group_raises_not_found(self): + service, _, _ = _make_service(None) + with pytest.raises(NotFoundError): + await service.add_projects({"name": "g", "owner": "google:a@b.com"}, ["mp-1"]) + + async def test_ambiguous_identifiers_propagate_conflict(self): + service, _, _ = _make_service(_group(), ambiguous=True) + with pytest.raises(ConflictError): + await service.add_projects({"name": "g", "owner": "google:a@b.com"}, ["mp-1"]) + + +# --------------------------------------------------------------------------- +# add +# --------------------------------------------------------------------------- + + +class TestAddProjects: + async def test_valid_projects_are_added(self): + group = _group() + service, groups, _ = _make_service(group, visible_projects={"mp-1", "mp-2"}) + summary = await service.add_projects({"id": str(group.id)}, ["mp-1", "mp-2"]) + assert summary.total == 2 + assert summary.succeeded == ["mp-1", "mp-2"] + assert summary.failed == [] + groups.add_project_refs.assert_awaited_once_with(group.id, ["mp-1", "mp-2"]) + + async def test_missing_project_reported_as_failure(self): + group = _group() + service, groups, _ = _make_service(group, visible_projects={"mp-1"}) + summary = await service.add_projects({"id": str(group.id)}, ["mp-1", "ghost"]) + assert summary.succeeded == ["mp-1"] + assert len(summary.failed) == 1 + assert summary.failed[0].index == 1 + assert summary.failed[0].error_code == "not_found" + # only the valid id is written + groups.add_project_refs.assert_awaited_once_with(group.id, ["mp-1"]) + + async def test_no_valid_projects_skips_update(self): + group = _group() + service, groups, _ = _make_service(group, visible_projects=set()) + summary = await service.add_projects({"id": str(group.id)}, ["ghost"]) + assert summary.succeeded == [] + assert len(summary.failed) == 1 + groups.add_project_refs.assert_not_awaited() + + async def test_duplicate_input_added_once(self): + group = _group() + service, groups, _ = _make_service(group, visible_projects={"mp-1"}) + summary = await service.add_projects({"id": str(group.id)}, ["mp-1", "mp-1"]) + assert summary.succeeded == ["mp-1"] + groups.add_project_refs.assert_awaited_once_with(group.id, ["mp-1"]) + + +# --------------------------------------------------------------------------- +# delete +# --------------------------------------------------------------------------- + + +class TestDeleteProjects: + async def test_members_deleted_non_members_reported(self): + group = _group(["mp-1", "mp-2"]) + service, groups, _ = _make_service(group) + summary = await service.delete_projects({"id": str(group.id)}, ["mp-1", "ghost"]) + assert summary.succeeded == ["mp-1"] + assert len(summary.failed) == 1 + assert summary.failed[0].index == 1 + assert summary.failed[0].error_code == "not_found" + groups.delete_project_refs.assert_awaited_once_with(group.id, ["mp-1"]) + + async def test_no_members_skips_update(self): + group = _group(["mp-1"]) + service, groups, _ = _make_service(group) + summary = await service.delete_projects({"id": str(group.id)}, ["ghost"]) + assert summary.succeeded == [] + assert len(summary.failed) == 1 + groups.delete_project_refs.assert_not_awaited() diff --git a/mpcontribs-api/tests/unit/domains/test_projects_models.py b/mpcontribs-api/tests/unit/domains/test_projects_models.py index 8602e9805..5a35cbdac 100644 --- a/mpcontribs-api/tests/unit/domains/test_projects_models.py +++ b/mpcontribs-api/tests/unit/domains/test_projects_models.py @@ -95,9 +95,15 @@ def test_zero_values_allowed(self): stats = Stats(columns=0, contributions=0, tables=0, structures=0, attachments=0, size=0.0) assert stats.contributions == 0 - def test_missing_field_raises(self): - with pytest.raises(PydanticValidationError): - Stats(columns=1, contributions=2, tables=3, structures=4, attachments=5) # missing size + def test_fields_default_to_zero(self): + # Stats is server-computed and every field defaults to zero, so an empty Stats is valid. + stats = Stats() + assert stats.columns == 0 + assert stats.contributions == 0 + assert stats.tables == 0 + assert stats.structures == 0 + assert stats.attachments == 0 + assert stats.size == 0.0 # --------------------------------------------------------------------------- @@ -282,7 +288,7 @@ def test_from_input_model_defaults(self): def test_from_input_model_starts_with_empty_server_owned_fields(self): # stats/columns aren't on the input model and default empty on the document. project = Project.from_input_model(self._make_input(), id="test-proj") - assert project.stats == Stats.empty() + assert project.stats == Stats() assert project.columns == [] diff --git a/mpcontribs-api/tests/unit/domains/test_search_str_tags.py b/mpcontribs-api/tests/unit/domains/test_search_str_tags.py new file mode 100644 index 000000000..ebaabe31b --- /dev/null +++ b/mpcontribs-api/tests/unit/domains/test_search_str_tags.py @@ -0,0 +1,126 @@ +"""Fuzz the ``SearchStr`` normalizer behind ``tags`` (and the other places it is reused). + +``tags`` is ``list[SearchStr]`` on ``ProjectIn`` / ``ProjectOut`` / ``ProjectPatch`` and on every +``ProjectFilter`` operator (``tags`` / ``tags__in`` / ``tags__contains``); ``SearchStr`` also backs +other fields such as ``ProjectGroupFilter.name``. ``SearchStr`` runs ``_nfkc_casefold`` — NFKC +compatibility fold, whitespace strip, then casefold — so a stored tag and a query tag always +collapse to the same bytes. These tests hit the tricky unicode edges and prove the normalizer is +applied identically wherever it is declared. + +All non-ASCII codepoints use ``\\u`` escapes so the source is unambiguous byte-for-byte. +""" + +import random + +import pytest +from pydantic import TypeAdapter + +from mpcontribs_api.domains._shared.types import SearchStr +from mpcontribs_api.domains.project_groups.models import ProjectGroupFilter +from mpcontribs_api.domains.projects.models import ProjectFilter, ProjectIn, ProjectPatch + +_search = TypeAdapter(SearchStr) + + +# (id, raw, expected) — each row targets a distinct edge of NFKC + strip + casefold. +_CASES = [ + ("ascii_casefold", "BandGap", "bandgap"), + ("hyphen_preserved", "Band-Gap", "band-gap"), # SearchStr does not strip punctuation + ("eszett_grows_length", "Straße", "strasse"), # U+00DF casefolds to "ss" (grows length) + ("greek_final_sigma", "ΟΔΟΣ", "οδοσ"), # Σ -> σ (not ς) + ("ligature_fi", "file", "file"), # NFKC decomposes the fi ligature + ("micro_sign_to_mu", "µ", "μ"), # MICRO SIGN -> GREEK SMALL LETTER MU + ("kelvin_sign", "K", "k"), # KELVIN SIGN -> latin k + ("fullwidth_to_ascii", "ＡＢ", "ab"), # fullwidth A B -> ascii + ("superscript_digit", "m²", "m2"), # m² -> m2 + ("combining_composes", "é", "é"), # e + COMBINING ACUTE -> precomposed é + ("roman_numeral", "Ⅷ", "viii"), # ROMAN NUMERAL EIGHT -> viii + ("nbsp_trimmed", " tag ", "tag"), # NBSP folds to space, then strip + ("mixed_whitespace_trimmed", " Tag\n", "tag"), + ("turkish_dotted_I", "İ", "i̇"), # İ casefolds to i + COMBINING DOT ABOVE + ("empty_after_strip", " ", ""), # no min length: whitespace-only -> "" +] + + +@pytest.mark.parametrize("raw,expected", [(r, e) for _, r, e in _CASES], ids=[i for i, _, _ in _CASES]) +def test_searchstr_normalization(raw, expected): + out = _search.validate_python(raw) + assert out == expected + # every realistic tag must be a stable key: re-folding it changes nothing + assert _search.validate_python(out) == out + + +def test_searchstr_fuzz_output_is_stripped_and_casefolded(): + """Whatever the input's unicode form, the output is always trimmed and fully casefolded. + + These are the invariants that hold universally. Idempotency does *not* hold for every input -- + see ``test_searchstr_casefold_expansion_breaks_idempotency`` -- so it is asserted only for the + realistic cases above, not fuzzed here. + + Seeded so the run is deterministic. Draws from ranges that break naive normalizers (latin-1 + supplement, combining marks, greek, fullwidth, ligatures, roman numerals, and whitespace). + """ + rng = random.Random(1729) + pool = ( + [chr(c) for c in range(0x20, 0x7F)] # ascii printable + + [chr(c) for c in range(0xA0, 0x100)] # latin-1 supplement (µ, ß, é, NBSP, ...) + + [chr(c) for c in range(0x300, 0x370)] # combining marks + + [chr(c) for c in range(0x391, 0x3CA)] # greek letters + + [chr(c) for c in range(0xFF01, 0xFF5F)] # fullwidth forms + + [chr(c) for c in range(0xFB00, 0xFB07)] # latin ligatures + + [chr(c) for c in range(0x2160, 0x2180)] # roman numerals + + ["\t", "\n", "\r", "\x20", " ", " "] # tab/nl/cr/space/NBSP/em-space + ) + for _ in range(2000): + raw = "".join(rng.choice(pool) for _ in range(rng.randint(0, 8))) + out = _search.validate_python(raw) + assert out == out.strip(), f"leaked surrounding whitespace for {raw!r}" + assert out == out.casefold(), f"not casefold-stable for {raw!r}" + + +# ligature + uppercase + trailing NBSP -> "file": exercises fold, casefold, and trim at once. +_MESSY_TAG = " fiLE " + +_TAG_FIELD_EXTRACTORS = [ + ( + "project_in_tags", + lambda t: ProjectIn( + title="title-x", + authors="a", + description="d", + owner="google:a@b.com", + tags=[t], + ).tags, + ), + ("project_patch_tags", lambda t: ProjectPatch(tags=[t]).tags), + ("filter_tags", lambda t: ProjectFilter(tags=[t]).tags), + ("filter_tags__in", lambda t: ProjectFilter(tags__in=[t]).tags__in), + ("filter_tags__contains", lambda t: ProjectFilter(tags__contains=[t]).tags__contains), + # "etc.": the same SearchStr normalizer, reused on a non-tag list field on another model. + ("project_group_filter_name__in", lambda t: ProjectGroupFilter(name__in=[t]).name__in), +] + + +@pytest.mark.parametrize( + "extract", [e for _, e in _TAG_FIELD_EXTRACTORS], ids=[i for i, _ in _TAG_FIELD_EXTRACTORS] +) +def test_searchstr_normalized_across_models(extract): + assert extract(_MESSY_TAG) == ["file"] + + +@pytest.mark.xfail( + strict=True, + reason="_nfkc_casefold is not idempotent when casefold expands a char sitting before a combining mark", +) +def test_searchstr_casefold_expansion_breaks_idempotency(): + """Documents a real edge: a casefold-expanding char (ß -> ss) followed by a combining mark. + + NFKC runs before casefold, so ``ß`` + combining circumflex stays decomposed through the first + fold (-> ``ss`` + circumflex). Re-folding then NFKC-composes ``s`` + circumflex into ``ŝ``, so + the value is not stable under a second pass. Because ``ProjectOut.tags`` is also + ``list[SearchStr]``, a stored tag re-normalizes on read and can round-trip to a different + string. xfail(strict) so this flips to a failure the moment the normalizer is made idempotent + (e.g. a trailing NFKC pass after casefold). + """ + once = _search.validate_python("ß̂") # eszett + COMBINING CIRCUMFLEX ACCENT + assert _search.validate_python(once) == once