From 30ce8f9466459ac7371e07cc5e4d77ffe10293b3 Mon Sep 17 00:00:00 2001 From: Emily Marshall Date: Thu, 18 Jun 2026 19:00:53 +1000 Subject: [PATCH 1/5] feat: add tolid table and endpoints for handling tolid requests --- README.md | 2 + alembic/versions/0005_add_tolid_requests.py | 94 ++++++ app/api/v1/endpoints/broker.py | 107 +++++++ app/models/__init__.py | 1 + app/models/project.py | 1 + app/models/tolid_request.py | 60 ++++ app/schemas/tolid.py | 61 ++++ app/services/tolid_service.py | 210 ++++++++++++++ schema.sql | 32 +++ tests/unit/endpoints/test_endpoints_broker.py | 14 +- .../endpoints/test_endpoints_broker_tolids.py | 270 ++++++++++++++++++ tests/unit/services/test_tolid_service.py | 131 +++++++++ 12 files changed, 980 insertions(+), 3 deletions(-) create mode 100644 alembic/versions/0005_add_tolid_requests.py create mode 100644 app/models/tolid_request.py create mode 100644 app/schemas/tolid.py create mode 100644 app/services/tolid_service.py create mode 100644 tests/unit/endpoints/test_endpoints_broker_tolids.py create mode 100644 tests/unit/services/test_tolid_service.py diff --git a/README.md b/README.md index e40a7a1..f86f804 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ A dedicated broker workflow enables integration with external submission pipelin - Claim drafts and obtain a lease: `/api/v1/broker/organisms/{taxon_id}/claim` - ENA broker contract endpoints: `/api/v1/broker/claims/ready`, `/api/v1/broker/claims/entity`, `/api/v1/broker/validation`, `/api/v1/broker/reports/{attempt_id}` - Renew lease, finalise, and report results: `/api/v1/broker/attempts/{attempt_id}/...` + - Lightweight ToLID persistence/reporting endpoints: `/api/v1/broker/tolids/...` - Attempt listing and summaries for dashboard views - Bulk import endpoints for organisms, samples, and experiments - XML export endpoints for downstream systems @@ -238,6 +239,7 @@ pyproject.toml, uv.lock, docker-compose.yml, Dockerfile, schema.sql, scripts/, d ``` For the flat ENA broker contract used by Canopy, see [docs/ena_broker_contract.md](docs/ena_broker_contract.md). + For the lightweight ToLID persistence/reporting flow, see [docs/tolid_broker_api.md](docs/tolid_broker_api.md). For a deeper overview of attempt leasing and statuses, see the `broker` endpoints in `app/api/v1/endpoints/broker.py` and the interactive docs. ## Bulk Import API diff --git a/alembic/versions/0005_add_tolid_requests.py b/alembic/versions/0005_add_tolid_requests.py new file mode 100644 index 0000000..48af004 --- /dev/null +++ b/alembic/versions/0005_add_tolid_requests.py @@ -0,0 +1,94 @@ +"""Add durable ToLID request state. + +Revision ID: 0005_add_tolid_requests +Revises: 0004_qc_reads_assembly_refs +Create Date: 2026-06-16 +""" + +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +from alembic import op + +revision = "0005_add_tolid_requests" +down_revision = "0004_qc_reads_assembly_refs" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + tolid_status = sa.Enum( + "not_requested", + "pending", + "assigned", + "failed", + name="tolid_request_status", + ) + tolid_status.create(op.get_bind(), checkfirst=True) + + op.create_table( + "tolid_request", + sa.Column("id", postgresql.UUID(as_uuid=True), primary_key=True, nullable=False), + sa.Column( + "sample_id", + postgresql.UUID(as_uuid=True), + sa.ForeignKey("sample.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("tolid_external_id", sa.Text(), nullable=False), + sa.Column( + "taxon_id", + sa.Integer(), + sa.ForeignKey("organism.taxon_id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("scientific_name", sa.Text(), nullable=True), + sa.Column("tolid", sa.Text(), nullable=True), + sa.Column("request_id", sa.Text(), nullable=True), + sa.Column( + "status", + tolid_status, + nullable=False, + server_default="not_requested", + ), + sa.Column("last_requested_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("error_message", sa.Text(), nullable=True), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + nullable=False, + server_default=sa.func.now(), + ), + sa.Column( + "updated_at", + sa.DateTime(timezone=True), + nullable=False, + server_default=sa.func.now(), + ), + ) + + op.create_index("uq_tolid_request_sample_id", "tolid_request", ["sample_id"], unique=True) + op.create_index("idx_tolid_request_status", "tolid_request", ["status"]) + op.create_index( + "idx_tolid_request_status_last_requested_at", + "tolid_request", + ["status", "last_requested_at"], + ) + op.create_index("idx_tolid_request_request_id", "tolid_request", ["request_id"]) + + +def downgrade() -> None: + op.drop_index("idx_tolid_request_request_id", table_name="tolid_request") + op.drop_index("idx_tolid_request_status_last_requested_at", table_name="tolid_request") + op.drop_index("idx_tolid_request_status", table_name="tolid_request") + op.drop_index("uq_tolid_request_sample_id", table_name="tolid_request") + op.drop_table("tolid_request") + + tolid_status = sa.Enum( + "not_requested", + "pending", + "assigned", + "failed", + name="tolid_request_status", + ) + tolid_status.drop(op.get_bind(), checkfirst=True) diff --git a/app/api/v1/endpoints/broker.py b/app/api/v1/endpoints/broker.py index c7f39ac..9641c58 100644 --- a/app/api/v1/endpoints/broker.py +++ b/app/api/v1/endpoints/broker.py @@ -11,6 +11,7 @@ from sqlalchemy.orm import Session from app.core.dependencies import get_current_active_user, get_db, has_role +from app.core.errors import AppError from app.core.policy import policy from app.models.accession_registry import AccessionRegistry from app.models.broker import SubmissionAttempt, SubmissionEvent @@ -38,6 +39,8 @@ BrokerValidationRequest, BrokerValidationResponse, ) +from app.schemas.tolid import TolidRequestBrokerView, TolidRequestReport, TolidRequestStatus +from app.services.tolid_service import tolid_request_service router = APIRouter() CLAIMABLE_SUBMISSION_STATES = ("draft", "ready") @@ -145,6 +148,25 @@ class ReportResult(BaseModel): updated_counts: Dict[str, int] +def _sync_tolid_request_from_sample_submission( + db: Session, *, sample_submission: SampleSubmission, accession: Optional[str] +) -> None: + if not accession or sample_submission.sample_id is None: + return + + try: + tolid_request_service.ensure_row_for_sample( + db, + sample_id=sample_submission.sample_id, + tolid_external_id=accession, + ) + except AppError: + logger.warning( + "Skipping ToLID sync for sample submission %s because sample metadata was unavailable", + sample_submission.id, + ) + + def _normalise_tax_id_value(value: str | int) -> int: try: return int(str(value)) @@ -2229,6 +2251,12 @@ def report_results( # On conflict by (authority, accession) or (authority, entity_type, entity_id), do nothing stmt = stmt.on_conflict_do_nothing(index_elements=[AccessionRegistry.accession]) db.execute(stmt) + if item.status == "accepted": + _sync_tolid_request_from_sample_submission( + db, + sample_submission=sub, + accession=item.accession, + ) # Clear lease on finalise (anything other than submitting) if item.status != "submitting": @@ -2504,6 +2532,85 @@ def report_results( ) +@router.get("/tolids/requestable", response_model=List[TolidRequestBrokerView]) +@policy("broker:read") +def get_requestable_tolids( + *, + db: Session = Depends(get_db), + taxon_id: Optional[int] = Query(None, description="Filter by organism taxon_id"), + sample_id: Optional[UUID] = Query(None, description="Filter by sample_id"), + sample_ids: Optional[List[UUID]] = Query(None, description="Filter by sample_ids"), + skip: int = Query(0, ge=0), + limit: int = Query(100, ge=1, le=1000), + current_user: User = Depends(get_current_active_user), +) -> List[TolidRequestBrokerView]: + """Return specimen ToLID rows that have not yet been requested.""" + return tolid_request_service.list_rows( + db, + row_status=TolidRequestStatus.NOT_REQUESTED, + taxon_id=taxon_id, + sample_id=sample_id, + sample_ids=sample_ids, + skip=skip, + limit=limit, + ) + + +@router.get("/tolids/pending", response_model=List[TolidRequestBrokerView]) +@policy("broker:read") +def get_pending_tolids( + *, + db: Session = Depends(get_db), + taxon_id: Optional[int] = Query(None, description="Filter by organism taxon_id"), + sample_id: Optional[UUID] = Query(None, description="Filter by sample_id"), + sample_ids: Optional[List[UUID]] = Query(None, description="Filter by sample_ids"), + requested_before: Optional[datetime] = Query( + None, description="Return only rows requested before this timestamp" + ), + skip: int = Query(0, ge=0), + limit: int = Query(100, ge=1, le=1000), + current_user: User = Depends(get_current_active_user), +) -> List[TolidRequestBrokerView]: + """Return specimen ToLID rows still waiting on remote assignment.""" + return tolid_request_service.list_rows( + db, + row_status=TolidRequestStatus.PENDING, + taxon_id=taxon_id, + sample_id=sample_id, + sample_ids=sample_ids, + requested_before=requested_before, + skip=skip, + limit=limit, + ) + + +@router.get("/tolids/{sample_id}", response_model=TolidRequestBrokerView) +@policy("broker:read") +def get_tolid_by_sample( + *, + sample_id: UUID, + db: Session = Depends(get_db), + current_user: User = Depends(get_current_active_user), +) -> TolidRequestBrokerView: + """Return the durable ToLID state for one specimen sample.""" + return tolid_request_service.get_row_for_sample(db, sample_id) + + +@router.post("/tolids/{sample_id}/report", response_model=TolidRequestBrokerView) +@policy("broker:claim") +def report_tolid_result( + *, + sample_id: UUID, + payload: TolidRequestReport = Body(...), + db: Session = Depends(get_db), + current_user: User = Depends(get_current_active_user), +) -> TolidRequestBrokerView: + """Persist the broker-reported ToLID result for one specimen sample.""" + row = tolid_request_service.report_result(db, sample_id=sample_id, report=payload) + db.commit() + return row + + # ---------- Dashboard Helpers ---------- def expire_stale_leases(db: Session) -> Dict[str, int]: """Expire all submissions with expired leases by resetting them to 'draft' status. diff --git a/app/models/__init__.py b/app/models/__init__.py index f10afb6..0a223ef 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -10,5 +10,6 @@ from app.models.read import Read from app.models.sample import Sample, SampleSubmission from app.models.taxonomy_info import TaxonomyInfo +from app.models.tolid_request import TolidRequest from app.models.token import RefreshToken from app.models.user import User diff --git a/app/models/project.py b/app/models/project.py index c5b79a7..f8043c8 100644 --- a/app/models/project.py +++ b/app/models/project.py @@ -84,6 +84,7 @@ class ProjectSubmission(Base): response_payload = Column(JSONB, nullable=True) accession = Column(Text, nullable=True) + submitted_at = Column(DateTime(timezone=True), nullable=True) created_at = Column(DateTime(timezone=True), nullable=False, server_default=func.now()) updated_at = Column( diff --git a/app/models/tolid_request.py b/app/models/tolid_request.py new file mode 100644 index 0000000..89133a2 --- /dev/null +++ b/app/models/tolid_request.py @@ -0,0 +1,60 @@ +import uuid + +from sqlalchemy import Column, DateTime, ForeignKey, Index, Integer, Text, func +from sqlalchemy import Enum as SQLAlchemyEnum +from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.orm import backref, relationship + +from app.db.session import Base + + +class TolidRequest(Base): + """Durable ToLID request state for a specimen sample.""" + + __tablename__ = "tolid_request" + + id = Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + sample_id = Column(UUID(as_uuid=True), ForeignKey("sample.id", ondelete="CASCADE"), nullable=False) + tolid_external_id = Column(Text, nullable=False) + taxon_id = Column( + Integer, ForeignKey("organism.taxon_id", ondelete="CASCADE"), nullable=False + ) + scientific_name = Column(Text, nullable=True) + tolid = Column(Text, nullable=True) + request_id = Column(Text, nullable=True) + status = Column( + SQLAlchemyEnum( + "not_requested", + "pending", + "assigned", + "failed", + name="tolid_request_status", + ), + nullable=False, + default="not_requested", + ) + last_requested_at = Column(DateTime(timezone=True), nullable=True) + error_message = Column(Text, nullable=True) + created_at = Column(DateTime(timezone=True), nullable=False, server_default=func.now()) + updated_at = Column( + DateTime(timezone=True), + nullable=False, + server_default=func.now(), + onupdate=func.now(), + ) + + sample = relationship( + "Sample", + backref=backref("tolid_request_record", uselist=False, cascade="all, delete-orphan"), + ) + organism = relationship( + "Organism", + backref=backref("tolid_request_records", cascade="all, delete-orphan"), + ) + + __table_args__ = ( + Index("uq_tolid_request_sample_id", "sample_id", unique=True), + Index("idx_tolid_request_status", "status"), + Index("idx_tolid_request_status_last_requested_at", "status", "last_requested_at"), + Index("idx_tolid_request_request_id", "request_id"), + ) diff --git a/app/schemas/tolid.py b/app/schemas/tolid.py new file mode 100644 index 0000000..16879a7 --- /dev/null +++ b/app/schemas/tolid.py @@ -0,0 +1,61 @@ +from datetime import datetime +from enum import Enum +from typing import Optional +from uuid import UUID + +from pydantic import BaseModel, ConfigDict, model_validator + + +class TolidRequestStatus(str, Enum): + NOT_REQUESTED = "not_requested" + PENDING = "pending" + ASSIGNED = "assigned" + FAILED = "failed" + + +class TolidRequestBrokerView(BaseModel): + sample_id: UUID + specimen_id: Optional[str] = None + tolid_external_id: str + taxon_id: int + scientific_name: Optional[str] = None + status: TolidRequestStatus + request_id: Optional[str] = None + tolid: Optional[str] = None + last_requested_at: Optional[datetime] = None + error_message: Optional[str] = None + + +class TolidRequestReport(BaseModel): + status: TolidRequestStatus + tolid: Optional[str] = None + request_id: Optional[str] = None + last_requested_at: Optional[datetime] = None + error_message: Optional[str] = None + + @model_validator(mode="after") + def _validate_status_requirements(self): + if self.status == TolidRequestStatus.NOT_REQUESTED: + raise ValueError("status must be one of pending, assigned, or failed") + if self.status == TolidRequestStatus.ASSIGNED and not self.tolid: + raise ValueError("assigned status requires tolid") + if self.status == TolidRequestStatus.PENDING and not self.request_id: + raise ValueError("pending status requires request_id") + return self + + +class TolidRequestInDB(BaseModel): + id: UUID + sample_id: UUID + tolid_external_id: str + taxon_id: int + scientific_name: Optional[str] = None + tolid: Optional[str] = None + request_id: Optional[str] = None + status: TolidRequestStatus + last_requested_at: Optional[datetime] = None + error_message: Optional[str] = None + created_at: datetime + updated_at: datetime + + model_config = ConfigDict(from_attributes=True) diff --git a/app/services/tolid_service.py b/app/services/tolid_service.py new file mode 100644 index 0000000..b23004e --- /dev/null +++ b/app/services/tolid_service.py @@ -0,0 +1,210 @@ +from datetime import datetime +from typing import Iterable, List, Optional +from uuid import UUID + +from fastapi import status +from sqlalchemy.orm import Session + +from app.core.errors import AppError +from app.models.organism import Organism +from app.models.sample import Sample, SampleSubmission +from app.models.tolid_request import TolidRequest +from app.schemas.common import SampleKind +from app.schemas.tolid import TolidRequestBrokerView, TolidRequestReport, TolidRequestStatus + + +class TolidRequestService: + """Service helpers for durable ToLID request state.""" + + @staticmethod + def _is_specimen(sample: Optional[Sample]) -> bool: + if sample is None: + return False + return sample.kind == SampleKind.SPECIMEN or sample.kind == SampleKind.SPECIMEN.value + + @staticmethod + def _status_value(row: TolidRequest) -> str: + return row.status.value if hasattr(row.status, "value") else row.status + + def _get_sample(self, db: Session, sample_id: UUID) -> Sample: + sample = db.query(Sample).filter(Sample.id == sample_id).first() + if not sample: + raise AppError( + status_code=status.HTTP_404_NOT_FOUND, + code="sample_not_found", + message=f"Sample {sample_id} not found", + ) + return sample + + def _get_scientific_name(self, db: Session, taxon_id: int) -> Optional[str]: + organism = db.query(Organism).filter(Organism.taxon_id == taxon_id).first() + return organism.scientific_name if organism else None + + def _find_row(self, db: Session, sample_id: UUID) -> Optional[TolidRequest]: + return db.query(TolidRequest).filter(TolidRequest.sample_id == sample_id).first() + + def _fallback_external_id(self, db: Session, sample: Sample) -> Optional[str]: + submissions = db.query(SampleSubmission).filter(SampleSubmission.sample_id == sample.id).all() + for submission in submissions: + if submission.accession: + return submission.accession + return sample.biosample_accession + + def _to_broker_view(self, row: TolidRequest) -> TolidRequestBrokerView: + specimen_id = row.sample.specimen_id if getattr(row, "sample", None) else None + return TolidRequestBrokerView( + sample_id=row.sample_id, + specimen_id=specimen_id, + tolid_external_id=row.tolid_external_id, + taxon_id=row.taxon_id, + scientific_name=row.scientific_name, + status=row.status, + request_id=row.request_id, + tolid=row.tolid, + last_requested_at=row.last_requested_at, + error_message=row.error_message, + ) + + def list_rows( + self, + db: Session, + *, + row_status: TolidRequestStatus, + taxon_id: Optional[int] = None, + sample_id: Optional[UUID] = None, + sample_ids: Optional[Iterable[UUID]] = None, + requested_before: Optional[datetime] = None, + skip: int = 0, + limit: int = 100, + ) -> List[TolidRequestBrokerView]: + rows = db.query(TolidRequest).all() + sample_ids_set = set(sample_ids) if sample_ids else None + filtered = [] + for row in rows: + sample = getattr(row, "sample", None) + if not self._is_specimen(sample): + continue + if self._status_value(row) != row_status.value: + continue + if taxon_id is not None and row.taxon_id != taxon_id: + continue + if sample_id is not None and row.sample_id != sample_id: + continue + if sample_ids_set is not None and row.sample_id not in sample_ids_set: + continue + if requested_before is not None: + if row.last_requested_at is None or row.last_requested_at >= requested_before: + continue + filtered.append(row) + + return [self._to_broker_view(row) for row in filtered[skip : skip + limit]] + + def get_row_for_sample(self, db: Session, sample_id: UUID) -> TolidRequestBrokerView: + row = self._find_row(db, sample_id) + if not row: + raise AppError( + status_code=status.HTTP_404_NOT_FOUND, + code="tolid_request_not_found", + message=f"ToLID request for sample {sample_id} not found", + ) + if not self._is_specimen(getattr(row, "sample", None)): + raise AppError( + status_code=status.HTTP_404_NOT_FOUND, + code="tolid_request_not_found", + message=f"ToLID request for sample {sample_id} not found", + ) + return self._to_broker_view(row) + + def ensure_row_for_sample( + self, + db: Session, + *, + sample_id: UUID, + tolid_external_id: str, + scientific_name: Optional[str] = None, + ) -> Optional[TolidRequest]: + sample = self._get_sample(db, sample_id) + if not self._is_specimen(sample): + return None + + row = self._find_row(db, sample_id) + if scientific_name is None: + scientific_name = self._get_scientific_name(db, sample.taxon_id) + + if row: + row.tolid_external_id = tolid_external_id + row.taxon_id = sample.taxon_id + row.scientific_name = scientific_name + row.sample = sample + db.add(row) + return row + + row = TolidRequest( + sample_id=sample.id, + tolid_external_id=tolid_external_id, + taxon_id=sample.taxon_id, + scientific_name=scientific_name, + status=TolidRequestStatus.NOT_REQUESTED.value, + ) + row.sample = sample + db.add(row) + return row + + def report_result( + self, + db: Session, + *, + sample_id: UUID, + report: TolidRequestReport, + ) -> TolidRequestBrokerView: + sample = self._get_sample(db, sample_id) + if not self._is_specimen(sample): + raise AppError( + status_code=status.HTTP_409_CONFLICT, + code="tolid_not_specimen", + message="ToLID requests are only supported for specimen samples", + ) + + row = self._find_row(db, sample_id) + if row is None: + tolid_external_id = self._fallback_external_id(db, sample) + if not tolid_external_id: + raise AppError( + status_code=status.HTTP_409_CONFLICT, + code="tolid_external_id_missing", + message="Cannot create ToLID request row before an ENA sample accession is known", + ) + row = self.ensure_row_for_sample( + db, + sample_id=sample_id, + tolid_external_id=tolid_external_id, + ) + if row is None: + raise AppError( + status_code=status.HTTP_409_CONFLICT, + code="tolid_not_specimen", + message="ToLID requests are only supported for specimen samples", + ) + + row.sample = sample + row.status = report.status.value + row.last_requested_at = report.last_requested_at + + if report.status == TolidRequestStatus.ASSIGNED: + row.tolid = report.tolid + row.request_id = report.request_id + row.error_message = None + sample.tolid = report.tolid + db.add(sample) + elif report.status == TolidRequestStatus.PENDING: + row.request_id = report.request_id + row.error_message = None + elif report.status == TolidRequestStatus.FAILED: + row.request_id = report.request_id + row.error_message = report.error_message + + db.add(row) + return self._to_broker_view(row) + + +tolid_request_service = TolidRequestService() diff --git a/schema.sql b/schema.sql index a1e26d7..29a708f 100644 --- a/schema.sql +++ b/schema.sql @@ -27,6 +27,7 @@ CREATE TYPE assembly_file_type AS ENUM ( CREATE TYPE entity_type AS ENUM ('organism', 'sample', 'experiment', 'read', 'assembly', 'project', 'qc_read'); CREATE TYPE project_type AS ENUM ('root', 'genomic_data', 'assembly'); CREATE TYPE sample_kind AS ENUM ('specimen', 'derived'); +CREATE TYPE tolid_request_status AS ENUM ('not_requested', 'pending', 'assigned', 'failed'); -- ========================================== -- Users and Authentication -- ========================================== @@ -336,6 +337,37 @@ CREATE INDEX IF NOT EXISTS idx_sample_organism_specimen_lookup -- UNIQUE (sample_id, authority) WHERE status = 'accepted' AND accession IS NOT NULL +-- ========================================== +-- ToLID request table +-- ========================================== + +CREATE TABLE tolid_request ( + id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), + sample_id UUID NOT NULL REFERENCES sample(id) ON DELETE CASCADE, + tolid_external_id TEXT NOT NULL, + taxon_id INT NOT NULL REFERENCES organism(taxon_id) ON DELETE CASCADE, + scientific_name TEXT, + tolid TEXT, + request_id TEXT, + status tolid_request_status NOT NULL DEFAULT 'not_requested', + last_requested_at TIMESTAMPTZ, + error_message TEXT, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE UNIQUE INDEX IF NOT EXISTS uq_tolid_request_sample_id + ON tolid_request (sample_id); + +CREATE INDEX IF NOT EXISTS idx_tolid_request_status + ON tolid_request (status); + +CREATE INDEX IF NOT EXISTS idx_tolid_request_status_last_requested_at + ON tolid_request (status, last_requested_at); + +CREATE INDEX IF NOT EXISTS idx_tolid_request_request_id + ON tolid_request (request_id); + -- ========================================== -- Experiment tables -- ========================================== diff --git a/tests/unit/endpoints/test_endpoints_broker.py b/tests/unit/endpoints/test_endpoints_broker.py index 48f8eff..4e44a46 100644 --- a/tests/unit/endpoints/test_endpoints_broker.py +++ b/tests/unit/endpoints/test_endpoints_broker.py @@ -520,8 +520,13 @@ def test_broker_report_results_project_rejected_clears_lease(): att_id = uuid4() sub_id = uuid4() proj_id = uuid4() - sub = SimpleNamespace( - id=sub_id, project_id=proj_id, status="submitting", attempt_id=att_id, authority="ENA" + submitted_at = datetime(2024, 1, 1, tzinfo=timezone.utc) + sub = ProjectSubmission( + id=sub_id, + project_id=proj_id, + status="submitting", + attempt_id=att_id, + authority="ENA", ) db = FakeSession({ProjectSubmission: [sub]}) payload = broker.ReportRequest( @@ -531,7 +536,9 @@ def test_broker_report_results_project_rejected_clears_lease(): reads=[], projects=[ broker.ReportItem( - id=sub_id, status="rejected", submitted_at=datetime(2024, 1, 1, tzinfo=timezone.utc) + id=sub_id, + status="rejected", + submitted_at=submitted_at, ) ], ) @@ -539,5 +546,6 @@ def test_broker_report_results_project_rejected_clears_lease(): attempt_id=att_id, payload=payload, db=db, current_user=_broker_user() ) assert result.updated_counts["projects"] == 1 + assert sub.submitted_at == submitted_at assert sub.attempt_id is None assert getattr(sub, "finalised_attempt_id", None) == att_id diff --git a/tests/unit/endpoints/test_endpoints_broker_tolids.py b/tests/unit/endpoints/test_endpoints_broker_tolids.py new file mode 100644 index 0000000..adacba1 --- /dev/null +++ b/tests/unit/endpoints/test_endpoints_broker_tolids.py @@ -0,0 +1,270 @@ +from datetime import datetime, timezone +from types import SimpleNamespace +from uuid import uuid4 + +from app.api.v1.endpoints import broker +from app.models.organism import Organism +from app.models.sample import Sample, SampleSubmission +from app.models.tolid_request import TolidRequest +from app.schemas.tolid import TolidRequestReport + + +def _broker_user(): + return SimpleNamespace(is_superuser=False, roles=["broker"], is_active=True) + + +class _Query: + def __init__(self, data): + self.data = list(data) + + def filter(self, *_a, **_k): + return self + + def all(self): + return list(self.data) + + def first(self): + return self.data[0] if self.data else None + + +class _Session: + def __init__(self, data_map=None): + self.data_map = data_map or {} + self.added = [] + self.committed = False + self.flushed = False + self.executed = [] + + def query(self, model): + return _Query(self.data_map.get(model, [])) + + def add(self, obj): + self.data_map.setdefault(type(obj), []) + if obj not in self.data_map[type(obj)]: + self.data_map[type(obj)].append(obj) + self.added.append(obj) + + def flush(self): + self.flushed = True + + def commit(self): + self.committed = True + + def execute(self, stmt): + self.executed.append(stmt) + + +def _make_tolid_row(*, status, taxon_id=1729, specimen_kind="specimen", specimen_id="SPEC-1"): + sample = Sample(id=uuid4(), taxon_id=taxon_id, kind=specimen_kind, specimen_id=specimen_id) + row = TolidRequest( + id=uuid4(), + sample_id=sample.id, + tolid_external_id=f"SAMEA-{specimen_id}", + taxon_id=taxon_id, + scientific_name=f"Species {taxon_id}", + status=status, + ) + row.sample = sample + return row, sample + + +def test_requestable_endpoint_only_returns_not_requested_rows_and_specimens(): + requestable_row, _ = _make_tolid_row(status="not_requested", specimen_id="SPEC-1") + pending_row, _ = _make_tolid_row(status="pending", specimen_id="SPEC-2") + derived_row, _ = _make_tolid_row( + status="not_requested", + specimen_kind="derived", + specimen_id="SPEC-3", + ) + db = _Session({TolidRequest: [requestable_row, pending_row, derived_row]}) + + out = broker.get_requestable_tolids( + db=db, + taxon_id=None, + sample_id=None, + sample_ids=None, + skip=0, + limit=100, + current_user=_broker_user(), + ) + + assert [row.sample_id for row in out] == [requestable_row.sample_id] + assert out[0].status == "not_requested" + + +def test_pending_endpoint_only_returns_pending_rows_and_supports_taxon_filter(): + first_pending, _ = _make_tolid_row(status="pending", taxon_id=1729, specimen_id="SPEC-1") + first_pending.last_requested_at = datetime(2024, 1, 1, tzinfo=timezone.utc) + second_pending, _ = _make_tolid_row(status="pending", taxon_id=9999, specimen_id="SPEC-2") + second_pending.last_requested_at = datetime(2024, 1, 3, tzinfo=timezone.utc) + requestable_row, _ = _make_tolid_row(status="not_requested", taxon_id=1729, specimen_id="SPEC-3") + db = _Session({TolidRequest: [first_pending, second_pending, requestable_row]}) + + out = broker.get_pending_tolids( + db=db, + taxon_id=1729, + sample_id=None, + sample_ids=None, + requested_before=datetime(2024, 1, 2, tzinfo=timezone.utc), + skip=0, + limit=100, + current_user=_broker_user(), + ) + + assert [row.sample_id for row in out] == [first_pending.sample_id] + assert out[0].status == "pending" + + +def test_get_tolid_by_sample_returns_expected_row(): + row, sample = _make_tolid_row(status="pending", specimen_id="SPEC-1") + row.request_id = "REQ-1" + row.error_message = "Still waiting" + db = _Session({TolidRequest: [row], Sample: [sample]}) + + out = broker.get_tolid_by_sample( + sample_id=sample.id, + db=db, + current_user=_broker_user(), + ) + + assert out.sample_id == sample.id + assert out.specimen_id == "SPEC-1" + assert out.request_id == "REQ-1" + assert out.error_message == "Still waiting" + + +def test_report_tolid_result_updates_pending_assigned_and_failed_states(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="SPEC-1") + row = TolidRequest( + id=uuid4(), + sample_id=sample.id, + tolid_external_id="SAMEA0001", + taxon_id=1729, + scientific_name="Species 1729", + status="not_requested", + ) + row.sample = sample + db = _Session({Sample: [sample], TolidRequest: [row], SampleSubmission: []}) + + pending = broker.report_tolid_result( + sample_id=sample.id, + payload=TolidRequestReport( + status="pending", + request_id="REQ-1", + last_requested_at=datetime(2024, 1, 1, tzinfo=timezone.utc), + ), + db=db, + current_user=_broker_user(), + ) + assert pending.status == "pending" + assert row.request_id == "REQ-1" + + assigned = broker.report_tolid_result( + sample_id=sample.id, + payload=TolidRequestReport( + status="assigned", + tolid="tol123", + request_id="REQ-1", + last_requested_at=datetime(2024, 1, 2, tzinfo=timezone.utc), + ), + db=db, + current_user=_broker_user(), + ) + assert assigned.status == "assigned" + assert row.tolid == "tol123" + assert sample.tolid == "tol123" + + failed = broker.report_tolid_result( + sample_id=sample.id, + payload=TolidRequestReport( + status="failed", + request_id="REQ-2", + error_message="remote error", + last_requested_at=datetime(2024, 1, 3, tzinfo=timezone.utc), + ), + db=db, + current_user=_broker_user(), + ) + assert failed.status == "failed" + assert row.error_message == "remote error" + assert db.committed is True + + +def test_report_results_auto_creates_tolid_row_for_accepted_specimen_submission(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="SPEC-1") + organism = Organism(taxon_id=1729, scientific_name="Species 1729") + submission = SampleSubmission( + id=uuid4(), + sample_id=sample.id, + status="submitting", + attempt_id=uuid4(), + authority="ENA", + prepared_payload={}, + project_id=uuid4(), + ) + db = _Session({SampleSubmission: [submission], Sample: [sample], Organism: [organism]}) + + result = broker.report_results( + attempt_id=submission.attempt_id, + payload=broker.ReportRequest( + attempt_id=submission.attempt_id, + samples=[ + broker.ReportItem( + id=submission.id, + status="accepted", + accession="SAMEA0001", + submitted_at=datetime(2024, 1, 1, tzinfo=timezone.utc), + ) + ], + experiments=[], + reads=[], + projects=[], + ), + db=db, + current_user=_broker_user(), + ) + + assert result.updated_counts["samples"] == 1 + rows = db.data_map[TolidRequest] + assert len(rows) == 1 + assert rows[0].sample_id == sample.id + assert rows[0].tolid_external_id == "SAMEA0001" + assert rows[0].status == "not_requested" + assert rows[0].scientific_name == "Species 1729" + + +def test_report_results_does_not_create_tolid_row_for_non_specimen_sample(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="derived", specimen_id="SPEC-1") + organism = Organism(taxon_id=1729, scientific_name="Species 1729") + submission = SampleSubmission( + id=uuid4(), + sample_id=sample.id, + status="submitting", + attempt_id=uuid4(), + authority="ENA", + prepared_payload={}, + project_id=uuid4(), + ) + db = _Session({SampleSubmission: [submission], Sample: [sample], Organism: [organism]}) + + broker.report_results( + attempt_id=submission.attempt_id, + payload=broker.ReportRequest( + attempt_id=submission.attempt_id, + samples=[ + broker.ReportItem( + id=submission.id, + status="accepted", + accession="SAMEA0001", + submitted_at=datetime(2024, 1, 1, tzinfo=timezone.utc), + ) + ], + experiments=[], + reads=[], + projects=[], + ), + db=db, + current_user=_broker_user(), + ) + + assert TolidRequest not in db.data_map diff --git a/tests/unit/services/test_tolid_service.py b/tests/unit/services/test_tolid_service.py new file mode 100644 index 0000000..3499fd7 --- /dev/null +++ b/tests/unit/services/test_tolid_service.py @@ -0,0 +1,131 @@ +from datetime import datetime, timezone +from uuid import uuid4 + +from app.models.sample import Sample, SampleSubmission +from app.models.tolid_request import TolidRequest +from app.schemas.tolid import TolidRequestReport +from app.services.tolid_service import tolid_request_service + + +class _Query: + def __init__(self, data): + self.data = list(data) + + def filter(self, *_a, **_k): + return self + + def all(self): + return list(self.data) + + def first(self): + return self.data[0] if self.data else None + + +class _Session: + def __init__(self, data_map): + self.data_map = data_map + self.added = [] + + def query(self, model): + return _Query(self.data_map.get(model, [])) + + def add(self, obj): + self.data_map.setdefault(type(obj), []) + if obj not in self.data_map[type(obj)]: + self.data_map[type(obj)].append(obj) + self.added.append(obj) + + +def test_tolid_request_model_has_expected_indexes(): + table = TolidRequest.__table__ + indexes = {index.name: index for index in table.indexes} + + assert table.name == "tolid_request" + assert "sample_id" in table.c + assert indexes["uq_tolid_request_sample_id"].unique is True + assert "idx_tolid_request_status" in indexes + assert "idx_tolid_request_status_last_requested_at" in indexes + assert "idx_tolid_request_request_id" in indexes + + +def test_ensure_row_for_sample_updates_existing_row_instead_of_creating_duplicate(): + sample_id = uuid4() + sample = Sample(id=sample_id, taxon_id=1729, kind="specimen", specimen_id="SPEC-1") + existing = TolidRequest( + id=uuid4(), + sample_id=sample_id, + tolid_external_id="SAMEA0001", + taxon_id=1729, + scientific_name="Old name", + status="not_requested", + ) + existing.sample = sample + db = _Session({Sample: [sample], TolidRequest: [existing]}) + + out = tolid_request_service.ensure_row_for_sample( + db, + sample_id=sample_id, + tolid_external_id="SAMEA9999", + scientific_name="New name", + ) + + assert out is existing + assert existing.tolid_external_id == "SAMEA9999" + assert existing.scientific_name == "New name" + assert len(db.data_map[TolidRequest]) == 1 + + +def test_ensure_row_for_sample_skips_non_specimen_samples(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="derived", specimen_id="SPEC-1") + db = _Session({Sample: [sample]}) + + out = tolid_request_service.ensure_row_for_sample( + db, + sample_id=sample.id, + tolid_external_id="SAMEA0001", + scientific_name="Species name", + ) + + assert out is None + assert TolidRequest not in db.data_map + + +def test_report_result_assigned_updates_sample_tolid(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="SPEC-1") + submission = SampleSubmission( + id=uuid4(), + sample_id=sample.id, + status="accepted", + accession="SAMEA0001", + authority="ENA", + prepared_payload={}, + project_id=uuid4(), + submitted_at=datetime(2024, 1, 1, tzinfo=timezone.utc), + ) + row = TolidRequest( + id=uuid4(), + sample_id=sample.id, + tolid_external_id="SAMEA0001", + taxon_id=1729, + scientific_name="Species name", + status="pending", + request_id="REQ-1", + ) + row.sample = sample + db = _Session({Sample: [sample], SampleSubmission: [submission], TolidRequest: [row]}) + + view = tolid_request_service.report_result( + db, + sample_id=sample.id, + report=TolidRequestReport( + status="assigned", + tolid="tol123", + request_id="REQ-1", + last_requested_at=datetime(2024, 1, 2, tzinfo=timezone.utc), + ), + ) + + assert row.status == "assigned" + assert row.tolid == "tol123" + assert sample.tolid == "tol123" + assert view.tolid == "tol123" From 1d6989cde30967d48ab49352697b0b483e85551e Mon Sep 17 00:00:00 2001 From: Emily Marshall Date: Thu, 18 Jun 2026 19:10:53 +1000 Subject: [PATCH 2/5] feat: add tolid table and endpoints for handling tolid requests --- alembic/versions/0005_add_tolid_requests.py | 13 +- app/api/v1/endpoints/broker.py | 51 +-- app/schemas/tolid.py | 7 +- app/services/tolid_service.py | 162 +++++---- docs/tolid_broker_api.md | 335 ++++++++++++++++++ .../endpoints/test_endpoints_broker_tolids.py | 203 ++++++----- tests/unit/services/test_tolid_service.py | 71 ++-- 7 files changed, 595 insertions(+), 247 deletions(-) create mode 100644 docs/tolid_broker_api.md diff --git a/alembic/versions/0005_add_tolid_requests.py b/alembic/versions/0005_add_tolid_requests.py index 48af004..90eaca9 100644 --- a/alembic/versions/0005_add_tolid_requests.py +++ b/alembic/versions/0005_add_tolid_requests.py @@ -17,7 +17,7 @@ def upgrade() -> None: - tolid_status = sa.Enum( + tolid_status = postgresql.ENUM( "not_requested", "pending", "assigned", @@ -47,7 +47,14 @@ def upgrade() -> None: sa.Column("request_id", sa.Text(), nullable=True), sa.Column( "status", - tolid_status, + postgresql.ENUM( + "not_requested", + "pending", + "assigned", + "failed", + name="tolid_request_status", + create_type=False, + ), nullable=False, server_default="not_requested", ), @@ -84,7 +91,7 @@ def downgrade() -> None: op.drop_index("uq_tolid_request_sample_id", table_name="tolid_request") op.drop_table("tolid_request") - tolid_status = sa.Enum( + tolid_status = postgresql.ENUM( "not_requested", "pending", "assigned", diff --git a/app/api/v1/endpoints/broker.py b/app/api/v1/endpoints/broker.py index 9641c58..aeba69c 100644 --- a/app/api/v1/endpoints/broker.py +++ b/app/api/v1/endpoints/broker.py @@ -11,7 +11,6 @@ from sqlalchemy.orm import Session from app.core.dependencies import get_current_active_user, get_db, has_role -from app.core.errors import AppError from app.core.policy import policy from app.models.accession_registry import AccessionRegistry from app.models.broker import SubmissionAttempt, SubmissionEvent @@ -148,25 +147,6 @@ class ReportResult(BaseModel): updated_counts: Dict[str, int] -def _sync_tolid_request_from_sample_submission( - db: Session, *, sample_submission: SampleSubmission, accession: Optional[str] -) -> None: - if not accession or sample_submission.sample_id is None: - return - - try: - tolid_request_service.ensure_row_for_sample( - db, - sample_id=sample_submission.sample_id, - tolid_external_id=accession, - ) - except AppError: - logger.warning( - "Skipping ToLID sync for sample submission %s because sample metadata was unavailable", - sample_submission.id, - ) - - def _normalise_tax_id_value(value: str | int) -> int: try: return int(str(value)) @@ -2251,13 +2231,6 @@ def report_results( # On conflict by (authority, accession) or (authority, entity_type, entity_id), do nothing stmt = stmt.on_conflict_do_nothing(index_elements=[AccessionRegistry.accession]) db.execute(stmt) - if item.status == "accepted": - _sync_tolid_request_from_sample_submission( - db, - sample_submission=sub, - accession=item.accession, - ) - # Clear lease on finalise (anything other than submitting) if item.status != "submitting": sub.attempt_id = None @@ -2532,28 +2505,16 @@ def report_results( ) -@router.get("/tolids/requestable", response_model=List[TolidRequestBrokerView]) +@router.get("/tolids/by-specimen-accession/{specimen_id}", response_model=TolidRequestBrokerView) @policy("broker:read") -def get_requestable_tolids( +def get_tolid_by_specimen_accession( *, + specimen_id: str, db: Session = Depends(get_db), - taxon_id: Optional[int] = Query(None, description="Filter by organism taxon_id"), - sample_id: Optional[UUID] = Query(None, description="Filter by sample_id"), - sample_ids: Optional[List[UUID]] = Query(None, description="Filter by sample_ids"), - skip: int = Query(0, ge=0), - limit: int = Query(100, ge=1, le=1000), current_user: User = Depends(get_current_active_user), -) -> List[TolidRequestBrokerView]: - """Return specimen ToLID rows that have not yet been requested.""" - return tolid_request_service.list_rows( - db, - row_status=TolidRequestStatus.NOT_REQUESTED, - taxon_id=taxon_id, - sample_id=sample_id, - sample_ids=sample_ids, - skip=skip, - limit=limit, - ) +) -> TolidRequestBrokerView: + """Resolve a specimen sample by ENA accession and return current or virtual ToLID state.""" + return tolid_request_service.get_by_specimen_accession(db, specimen_id) @router.get("/tolids/pending", response_model=List[TolidRequestBrokerView]) diff --git a/app/schemas/tolid.py b/app/schemas/tolid.py index 16879a7..8f4d4b4 100644 --- a/app/schemas/tolid.py +++ b/app/schemas/tolid.py @@ -1,6 +1,6 @@ from datetime import datetime from enum import Enum -from typing import Optional +from typing import Any, Dict, Optional from uuid import UUID from pydantic import BaseModel, ConfigDict, model_validator @@ -15,8 +15,7 @@ class TolidRequestStatus(str, Enum): class TolidRequestBrokerView(BaseModel): sample_id: UUID - specimen_id: Optional[str] = None - tolid_external_id: str + specimen_id: str taxon_id: int scientific_name: Optional[str] = None status: TolidRequestStatus @@ -24,6 +23,8 @@ class TolidRequestBrokerView(BaseModel): tolid: Optional[str] = None last_requested_at: Optional[datetime] = None error_message: Optional[str] = None + kind: str + sample_payload: Optional[Dict[str, Any]] = None class TolidRequestReport(BaseModel): diff --git a/app/services/tolid_service.py b/app/services/tolid_service.py index b23004e..5f2712a 100644 --- a/app/services/tolid_service.py +++ b/app/services/tolid_service.py @@ -1,7 +1,8 @@ from datetime import datetime -from typing import Iterable, List, Optional +from typing import Dict, Iterable, List, Optional from uuid import UUID +from fastapi.encoders import jsonable_encoder from fastapi import status from sqlalchemy.orm import Session @@ -27,7 +28,7 @@ def _status_value(row: TolidRequest) -> str: return row.status.value if hasattr(row.status, "value") else row.status def _get_sample(self, db: Session, sample_id: UUID) -> Sample: - sample = db.query(Sample).filter(Sample.id == sample_id).first() + sample = next((row for row in db.query(Sample).all() if row.id == sample_id), None) if not sample: raise AppError( status_code=status.HTTP_404_NOT_FOUND, @@ -37,32 +38,72 @@ def _get_sample(self, db: Session, sample_id: UUID) -> Sample: return sample def _get_scientific_name(self, db: Session, taxon_id: int) -> Optional[str]: - organism = db.query(Organism).filter(Organism.taxon_id == taxon_id).first() + organism = next((row for row in db.query(Organism).all() if row.taxon_id == taxon_id), None) return organism.scientific_name if organism else None def _find_row(self, db: Session, sample_id: UUID) -> Optional[TolidRequest]: - return db.query(TolidRequest).filter(TolidRequest.sample_id == sample_id).first() + return next((row for row in db.query(TolidRequest).all() if row.sample_id == sample_id), None) def _fallback_external_id(self, db: Session, sample: Sample) -> Optional[str]: - submissions = db.query(SampleSubmission).filter(SampleSubmission.sample_id == sample.id).all() + submissions = db.query(SampleSubmission).all() for submission in submissions: - if submission.accession: + if submission.sample_id == sample.id and submission.accession: return submission.accession return sample.biosample_accession - def _to_broker_view(self, row: TolidRequest) -> TolidRequestBrokerView: - specimen_id = row.sample.specimen_id if getattr(row, "sample", None) else None + def _find_sample_by_accession(self, db: Session, specimen_id: str) -> Optional[Sample]: + for submission in db.query(SampleSubmission).all(): + if submission.accession != specimen_id or submission.sample_id is None: + continue + sample = next( + (row for row in db.query(Sample).all() if row.id == submission.sample_id), + None, + ) + if sample is not None: + return sample + + return next( + (row for row in db.query(Sample).all() if row.biosample_accession == specimen_id), + None, + ) + + def _sample_payload(self, sample: Sample) -> Dict: + return jsonable_encoder( + {column.name: getattr(sample, column.name) for column in sample.__table__.columns} + ) + + def _resolved_scientific_name( + self, *, db: Session, sample: Sample, row: Optional[TolidRequest] + ) -> Optional[str]: + if row is not None and row.scientific_name: + return row.scientific_name + organism = getattr(sample, "organism", None) + if organism is not None and organism.scientific_name: + return organism.scientific_name + return self._get_scientific_name(db, sample.taxon_id) + + def _to_broker_view( + self, + *, + db: Session, + sample: Sample, + specimen_id: str, + row: Optional[TolidRequest], + ) -> TolidRequestBrokerView: return TolidRequestBrokerView( - sample_id=row.sample_id, + sample_id=sample.id, specimen_id=specimen_id, - tolid_external_id=row.tolid_external_id, - taxon_id=row.taxon_id, - scientific_name=row.scientific_name, - status=row.status, - request_id=row.request_id, - tolid=row.tolid, - last_requested_at=row.last_requested_at, - error_message=row.error_message, + taxon_id=sample.taxon_id, + scientific_name=self._resolved_scientific_name(db=db, sample=sample, row=row), + status=( + self._status_value(row) if row is not None else TolidRequestStatus.NOT_REQUESTED.value + ), + request_id=(row.request_id if row is not None else None), + tolid=(row.tolid if row is not None else None), + last_requested_at=(row.last_requested_at if row is not None else None), + error_message=(row.error_message if row is not None else None), + kind=sample.kind.value if hasattr(sample.kind, "value") else sample.kind, + sample_payload=self._sample_payload(sample), ) def list_rows( @@ -97,58 +138,52 @@ def list_rows( continue filtered.append(row) - return [self._to_broker_view(row) for row in filtered[skip : skip + limit]] + return [ + self._to_broker_view( + db=db, + sample=row.sample, + specimen_id=row.tolid_external_id, + row=row, + ) + for row in filtered[skip : skip + limit] + ] def get_row_for_sample(self, db: Session, sample_id: UUID) -> TolidRequestBrokerView: - row = self._find_row(db, sample_id) - if not row: + sample = self._get_sample(db, sample_id) + if not self._is_specimen(sample): raise AppError( status_code=status.HTTP_404_NOT_FOUND, code="tolid_request_not_found", message=f"ToLID request for sample {sample_id} not found", ) - if not self._is_specimen(getattr(row, "sample", None)): + + row = self._find_row(db, sample_id) + specimen_id = row.tolid_external_id if row is not None else self._fallback_external_id(db, sample) + if not specimen_id: raise AppError( status_code=status.HTTP_404_NOT_FOUND, code="tolid_request_not_found", message=f"ToLID request for sample {sample_id} not found", ) - return self._to_broker_view(row) + return self._to_broker_view(db=db, sample=sample, specimen_id=specimen_id, row=row) - def ensure_row_for_sample( - self, - db: Session, - *, - sample_id: UUID, - tolid_external_id: str, - scientific_name: Optional[str] = None, - ) -> Optional[TolidRequest]: - sample = self._get_sample(db, sample_id) + def get_by_specimen_accession(self, db: Session, specimen_id: str) -> TolidRequestBrokerView: + sample = self._find_sample_by_accession(db, specimen_id) + if sample is None: + raise AppError( + status_code=status.HTTP_404_NOT_FOUND, + code="sample_accession_not_found", + message=f"No sample found for specimen accession {specimen_id}", + ) if not self._is_specimen(sample): - return None + raise AppError( + status_code=status.HTTP_409_CONFLICT, + code="tolid_not_specimen", + message="ToLID requests are only supported for specimen samples", + ) - row = self._find_row(db, sample_id) - if scientific_name is None: - scientific_name = self._get_scientific_name(db, sample.taxon_id) - - if row: - row.tolid_external_id = tolid_external_id - row.taxon_id = sample.taxon_id - row.scientific_name = scientific_name - row.sample = sample - db.add(row) - return row - - row = TolidRequest( - sample_id=sample.id, - tolid_external_id=tolid_external_id, - taxon_id=sample.taxon_id, - scientific_name=scientific_name, - status=TolidRequestStatus.NOT_REQUESTED.value, - ) - row.sample = sample - db.add(row) - return row + row = self._find_row(db, sample.id) + return self._to_broker_view(db=db, sample=sample, specimen_id=specimen_id, row=row) def report_result( self, @@ -174,19 +209,17 @@ def report_result( code="tolid_external_id_missing", message="Cannot create ToLID request row before an ENA sample accession is known", ) - row = self.ensure_row_for_sample( - db, - sample_id=sample_id, + row = TolidRequest( + sample_id=sample.id, tolid_external_id=tolid_external_id, + taxon_id=sample.taxon_id, + scientific_name=self._get_scientific_name(db, sample.taxon_id), ) - if row is None: - raise AppError( - status_code=status.HTTP_409_CONFLICT, - code="tolid_not_specimen", - message="ToLID requests are only supported for specimen samples", - ) row.sample = sample + row.tolid_external_id = self._fallback_external_id(db, sample) or row.tolid_external_id + row.taxon_id = sample.taxon_id + row.scientific_name = row.scientific_name or self._get_scientific_name(db, sample.taxon_id) row.status = report.status.value row.last_requested_at = report.last_requested_at @@ -204,7 +237,8 @@ def report_result( row.error_message = report.error_message db.add(row) - return self._to_broker_view(row) + specimen_id = row.tolid_external_id or self._fallback_external_id(db, sample) + return self._to_broker_view(db=db, sample=sample, specimen_id=specimen_id, row=row) tolid_request_service = TolidRequestService() diff --git a/docs/tolid_broker_api.md b/docs/tolid_broker_api.md new file mode 100644 index 0000000..a42c058 --- /dev/null +++ b/docs/tolid_broker_api.md @@ -0,0 +1,335 @@ +# ToLID Broker API + +This document describes the simplified ToLID flow between Canopy and the external broker worker. + +Canopy is the durable store for ToLID state. +The broker is responsible for calling the remote ToLID service. +Canopy does not call the remote ToLID API directly. + +Unlike the ENA broker claim flow, the ToLID flow does not use claim, lease, renew, or finalise semantics. + +## Overview + +The intended flow is: + +1. Broker submits a specimen sample to ENA. +2. Broker receives an ENA sample accession such as `ERS123456`. +3. Broker asks Canopy for specimen metadata using that accession. +4. Broker calls the remote ToLID service. +5. Broker reports one of these results back to Canopy: + - `pending` with `request_id` + - `assigned` with `tolid` + - `failed` with `error_message` +6. Broker later fetches `pending` rows from Canopy and retries them as needed. + +## Key Semantic Change + +Canopy no longer requires a pre-populated ToLID row before the first request. + +Absence of a `tolid_request` row means: + +- the sample has not yet started the ToLID flow +- the sample may still be requestable if it is a specimen sample and has an ENA sample accession + +The `tolid_request` table now represents durable ToLID state once work has actually started or completed. + +## Authentication + +All ToLID broker endpoints require authentication. + +- Read endpoints use the `broker:read` policy. +- Report/update endpoints use the `broker:claim` policy, matching the existing broker write surface. + +## Data Model + +Canopy stores ToLID request state in `tolid_request`. + +Fields: + +- `sample_id`: UUID, FK to `sample.id`, unique +- `tolid_external_id`: string, usually the ENA sample accession sent to the ToLID service +- `taxon_id`: integer, FK to `organism.taxon_id` +- `scientific_name`: string nullable +- `tolid`: string nullable +- `request_id`: string nullable +- `status`: `not_requested | pending | assigned | failed` +- `last_requested_at`: timezone-aware timestamp nullable +- `error_message`: string nullable +- `created_at`, `updated_at` + +Notes: + +- In current usage, Canopy does not need to persist `not_requested` rows ahead of time. +- `not_requested` is mainly a virtual response state returned when the sample is eligible but no ToLID row exists yet. + +Indexes: + +- unique index on `sample_id` +- index on `status` +- index on `(status, last_requested_at)` +- index on `request_id` + +## Eligibility Rules + +- ToLID requests are only supported for specimen samples. +- Direct accession lookup validates that the resolved sample has `kind = specimen`. +- The pending endpoint only returns rows whose linked sample has `kind = specimen`. + +## Endpoints Overview + +| Method | Endpoint | Description | +|--------|----------|-------------| +| `GET` | `/api/v1/broker/tolids/by-specimen-accession/{specimen_id}` | Resolve a specimen sample from an ENA accession and return current or virtual ToLID state | +| `GET` | `/api/v1/broker/tolids/pending` | List specimen ToLID rows in `pending` state | +| `GET` | `/api/v1/broker/tolids/{sample_id}` | Get one ToLID row by Canopy sample ID, returning virtual `not_requested` state if no row exists but the sample has an accession | +| `POST` | `/api/v1/broker/tolids/{sample_id}/report` | Create or update persisted ToLID state for one sample | + +## What No Longer Happens + +Canopy no longer auto-creates `not_requested` ToLID rows when sample submission results are reported. + +Normal broker sample submission reporting still stores the ENA accession on the submission side, but ToLID state is only persisted when the broker explicitly reports ToLID progress or results. + +## 1. Lookup a Specimen Sample by ENA Accession + +Returns the specimen sample metadata needed to build a first-time ToLID request. + +If a `tolid_request` row already exists, the current persisted state is returned. +If no row exists yet, Canopy returns a virtual/default state with `status = not_requested`. + +**Endpoint:** `GET /api/v1/broker/tolids/by-specimen-accession/{specimen_id}` + +Example: + +```bash +curl -s "http://localhost:8000/api/v1/broker/tolids/by-specimen-accession/ERS123456" \ + -H "Authorization: Bearer $TOKEN" +``` + +**Response example when no ToLID row exists yet:** + +```json +{ + "sample_id": "f47ac10b-58cc-4372-a567-0e02b2c3d479", + "specimen_id": "ERS123456", + "taxon_id": 1931064, + "scientific_name": "Manorina melanotis", + "status": "not_requested", + "request_id": null, + "tolid": null, + "last_requested_at": null, + "error_message": null, + "kind": "specimen", + "sample_payload": { + "id": "f47ac10b-58cc-4372-a567-0e02b2c3d479", + "specimen_id": "ATOL-SPEC-001", + "taxon_id": 1931064 + } +} +``` + +Response semantics: + +- `specimen_id` in this API means the ENA sample accession used for ToLID lookup +- the original Canopy sample record is available in `sample_payload` + +## 2. Get Pending ToLIDs + +Returns persisted ToLID rows in `pending` state. + +**Endpoint:** `GET /api/v1/broker/tolids/pending` + +**Query parameters:** + +| Name | Required | Description | +|------|----------|-------------| +| `taxon_id` | No | Filter by `organism.taxon_id` | +| `sample_id` | No | Filter to one sample | +| `sample_ids` | No | Filter to a set of samples | +| `requested_before` | No | Return only rows with `last_requested_at` earlier than this timestamp | +| `skip` | No | Pagination offset, default `0` | +| `limit` | No | Pagination limit, default `100`, max `1000` | + +**Response example:** + +```json +[ + { + "sample_id": "f47ac10b-58cc-4372-a567-0e02b2c3d479", + "specimen_id": "ERS123456", + "taxon_id": 1931064, + "scientific_name": "Manorina melanotis", + "status": "pending", + "request_id": "REQ-123", + "tolid": null, + "last_requested_at": "2026-06-17T10:15:00Z", + "error_message": null, + "kind": "specimen", + "sample_payload": { + "id": "f47ac10b-58cc-4372-a567-0e02b2c3d479", + "specimen_id": "ATOL-SPEC-001", + "taxon_id": 1931064 + } + } +] +``` + +## 3. Get One ToLID Row by Sample + +Returns one sample’s ToLID state using the Canopy `sample_id`. + +If the sample is a specimen sample and Canopy can resolve an ENA sample accession, this endpoint returns: + +- persisted state if a `tolid_request` row exists +- virtual `not_requested` state if no row exists yet + +**Endpoint:** `GET /api/v1/broker/tolids/{sample_id}` + +## 4. Report a ToLID Result + +Creates or updates persisted ToLID state for one sample. + +If no `tolid_request` row exists yet, Canopy creates it lazily during this call. + +**Endpoint:** `POST /api/v1/broker/tolids/{sample_id}/report` + +Supported statuses: + +- `pending` +- `assigned` +- `failed` + +`not_requested` is not accepted by this reporting endpoint. + +### Pending + +Use when the remote ToLID service has accepted the request but has not yet assigned a ToLID. + +**Request body:** + +```json +{ + "status": "pending", + "request_id": "REQ-123", + "last_requested_at": "2026-06-17T10:15:00Z" +} +``` + +Behavior: + +- creates the row if needed +- stores `request_id` +- sets `status = pending` +- stores `last_requested_at` +- clears `error_message` + +### Assigned + +Use when the remote ToLID service has assigned a real ToLID. + +**Request body:** + +```json +{ + "status": "assigned", + "tolid": "tolExample1", + "request_id": "REQ-123", + "last_requested_at": "2026-06-17T10:30:00Z" +} +``` + +Behavior: + +- creates the row if needed +- stores `tolid` +- stores `request_id` if present +- sets `status = assigned` +- stores `last_requested_at` +- clears `error_message` +- mirrors the assigned value onto `sample.tolid` + +### Failed + +Use when the broker wants Canopy to persist a terminal failure state. + +**Request body:** + +```json +{ + "status": "failed", + "request_id": "REQ-123", + "last_requested_at": "2026-06-17T10:45:00Z", + "error_message": "Remote service rejected the request" +} +``` + +Behavior: + +- creates the row if needed +- sets `status = failed` +- stores `request_id` if present +- stores `error_message` +- stores `last_requested_at` if present + +## Validation Rules + +- `assigned` requires `tolid` +- `pending` requires `request_id` +- direct lookup rejects non-specimen samples +- report rejects non-specimen samples +- lazy row creation during report requires Canopy to be able to resolve an ENA sample accession from the sample submission state + +## Example curl Commands + +```bash +TOKEN= +SAMPLE_ID= +``` + +### Lookup by accession for the first request + +```bash +curl -s "http://localhost:8000/api/v1/broker/tolids/by-specimen-accession/ERS123456" \ + -H "Authorization: Bearer $TOKEN" +``` + +### Fetch pending rows for retry/polling + +```bash +curl -s "http://localhost:8000/api/v1/broker/tolids/pending?taxon_id=1729" \ + -H "Authorization: Bearer $TOKEN" +``` + +### Report a pending request + +```bash +curl -s -X POST "http://localhost:8000/api/v1/broker/tolids/$SAMPLE_ID/report" \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{ + "status": "pending", + "request_id": "REQ-123", + "last_requested_at": "2026-06-17T10:15:00Z" + }' +``` + +### Report an assigned ToLID + +```bash +curl -s -X POST "http://localhost:8000/api/v1/broker/tolids/$SAMPLE_ID/report" \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{ + "status": "assigned", + "tolid": "tolExample1", + "request_id": "REQ-123", + "last_requested_at": "2026-06-17T10:30:00Z" + }' +``` + +## Notes + +- `assigned` is treated as terminal. +- `failed` is treated as terminal unless reset manually later. +- Canopy does not decide retry timing for `pending` rows. +- The broker decides when to retry and uses the `pending` endpoint to fetch work. diff --git a/tests/unit/endpoints/test_endpoints_broker_tolids.py b/tests/unit/endpoints/test_endpoints_broker_tolids.py index adacba1..20423a1 100644 --- a/tests/unit/endpoints/test_endpoints_broker_tolids.py +++ b/tests/unit/endpoints/test_endpoints_broker_tolids.py @@ -54,51 +54,110 @@ def execute(self, stmt): self.executed.append(stmt) -def _make_tolid_row(*, status, taxon_id=1729, specimen_kind="specimen", specimen_id="SPEC-1"): - sample = Sample(id=uuid4(), taxon_id=taxon_id, kind=specimen_kind, specimen_id=specimen_id) +def _make_tolid_row(*, status, accession, taxon_id=1729, specimen_kind="specimen"): + sample = Sample(id=uuid4(), taxon_id=taxon_id, kind=specimen_kind, specimen_id="CANOPY-SPEC-1") + organism = Organism(taxon_id=taxon_id, scientific_name=f"Species {taxon_id}") + sample.organism = organism row = TolidRequest( id=uuid4(), sample_id=sample.id, - tolid_external_id=f"SAMEA-{specimen_id}", + tolid_external_id=accession, taxon_id=taxon_id, scientific_name=f"Species {taxon_id}", status=status, ) row.sample = sample - return row, sample + submission = SampleSubmission( + id=uuid4(), + sample_id=sample.id, + status="accepted", + accession=accession, + authority="ENA", + prepared_payload={}, + project_id=uuid4(), + ) + return row, sample, submission, organism -def test_requestable_endpoint_only_returns_not_requested_rows_and_specimens(): - requestable_row, _ = _make_tolid_row(status="not_requested", specimen_id="SPEC-1") - pending_row, _ = _make_tolid_row(status="pending", specimen_id="SPEC-2") - derived_row, _ = _make_tolid_row( - status="not_requested", - specimen_kind="derived", - specimen_id="SPEC-3", +def test_lookup_by_specimen_accession_returns_virtual_not_requested_state_without_row(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="CANOPY-SPEC-1") + organism = Organism(taxon_id=1729, scientific_name="Species 1729") + sample.organism = organism + submission = SampleSubmission( + id=uuid4(), + sample_id=sample.id, + status="accepted", + accession="ERS123456", + authority="ENA", + prepared_payload={}, + project_id=uuid4(), ) - db = _Session({TolidRequest: [requestable_row, pending_row, derived_row]}) + db = _Session({Sample: [sample], SampleSubmission: [submission], Organism: [organism]}) - out = broker.get_requestable_tolids( + out = broker.get_tolid_by_specimen_accession( + specimen_id="ERS123456", db=db, - taxon_id=None, - sample_id=None, - sample_ids=None, - skip=0, - limit=100, current_user=_broker_user(), ) - assert [row.sample_id for row in out] == [requestable_row.sample_id] - assert out[0].status == "not_requested" + assert out.sample_id == sample.id + assert out.specimen_id == "ERS123456" + assert out.status == "not_requested" + assert out.kind == "specimen" + assert out.sample_payload["specimen_id"] == "CANOPY-SPEC-1" + + +def test_lookup_by_specimen_accession_returns_existing_tolid_state(): + row, sample, submission, organism = _make_tolid_row(status="pending", accession="ERS123456") + row.request_id = "REQ-1" + row.error_message = "Still waiting" + db = _Session( + { + TolidRequest: [row], + Sample: [sample], + SampleSubmission: [submission], + Organism: [organism], + } + ) + + out = broker.get_tolid_by_specimen_accession( + specimen_id="ERS123456", + db=db, + current_user=_broker_user(), + ) + + assert out.sample_id == sample.id + assert out.specimen_id == "ERS123456" + assert out.request_id == "REQ-1" + assert out.error_message == "Still waiting" def test_pending_endpoint_only_returns_pending_rows_and_supports_taxon_filter(): - first_pending, _ = _make_tolid_row(status="pending", taxon_id=1729, specimen_id="SPEC-1") + first_pending, sample_1, submission_1, organism_1 = _make_tolid_row( + status="pending", + accession="ERS123456", + taxon_id=1729, + ) first_pending.last_requested_at = datetime(2024, 1, 1, tzinfo=timezone.utc) - second_pending, _ = _make_tolid_row(status="pending", taxon_id=9999, specimen_id="SPEC-2") + second_pending, sample_2, submission_2, organism_2 = _make_tolid_row( + status="pending", + accession="ERS999999", + taxon_id=9999, + ) second_pending.last_requested_at = datetime(2024, 1, 3, tzinfo=timezone.utc) - requestable_row, _ = _make_tolid_row(status="not_requested", taxon_id=1729, specimen_id="SPEC-3") - db = _Session({TolidRequest: [first_pending, second_pending, requestable_row]}) + assigned_row, sample_3, submission_3, organism_3 = _make_tolid_row( + status="assigned", + accession="ERS000000", + taxon_id=1729, + ) + db = _Session( + { + TolidRequest: [first_pending, second_pending, assigned_row], + Sample: [sample_1, sample_2, sample_3], + SampleSubmission: [submission_1, submission_2, submission_3], + Organism: [organism_1, organism_2, organism_3], + } + ) out = broker.get_pending_tolids( db=db, @@ -113,13 +172,23 @@ def test_pending_endpoint_only_returns_pending_rows_and_supports_taxon_filter(): assert [row.sample_id for row in out] == [first_pending.sample_id] assert out[0].status == "pending" + assert out[0].specimen_id == "ERS123456" -def test_get_tolid_by_sample_returns_expected_row(): - row, sample = _make_tolid_row(status="pending", specimen_id="SPEC-1") - row.request_id = "REQ-1" - row.error_message = "Still waiting" - db = _Session({TolidRequest: [row], Sample: [sample]}) +def test_get_tolid_by_sample_returns_virtual_state_using_accession(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="CANOPY-SPEC-1") + organism = Organism(taxon_id=1729, scientific_name="Species 1729") + sample.organism = organism + submission = SampleSubmission( + id=uuid4(), + sample_id=sample.id, + status="accepted", + accession="ERS123456", + authority="ENA", + prepared_payload={}, + project_id=uuid4(), + ) + db = _Session({Sample: [sample], SampleSubmission: [submission], Organism: [organism]}) out = broker.get_tolid_by_sample( sample_id=sample.id, @@ -128,23 +197,24 @@ def test_get_tolid_by_sample_returns_expected_row(): ) assert out.sample_id == sample.id - assert out.specimen_id == "SPEC-1" - assert out.request_id == "REQ-1" - assert out.error_message == "Still waiting" + assert out.specimen_id == "ERS123456" + assert out.status == "not_requested" -def test_report_tolid_result_updates_pending_assigned_and_failed_states(): - sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="SPEC-1") - row = TolidRequest( +def test_report_tolid_result_creates_row_lazily_and_updates_states(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="CANOPY-SPEC-1") + organism = Organism(taxon_id=1729, scientific_name="Species 1729") + sample.organism = organism + submission = SampleSubmission( id=uuid4(), sample_id=sample.id, - tolid_external_id="SAMEA0001", - taxon_id=1729, - scientific_name="Species 1729", - status="not_requested", + status="accepted", + accession="ERS123456", + authority="ENA", + prepared_payload={}, + project_id=uuid4(), ) - row.sample = sample - db = _Session({Sample: [sample], TolidRequest: [row], SampleSubmission: []}) + db = _Session({Sample: [sample], SampleSubmission: [submission], Organism: [organism]}) pending = broker.report_tolid_result( sample_id=sample.id, @@ -156,7 +226,9 @@ def test_report_tolid_result_updates_pending_assigned_and_failed_states(): db=db, current_user=_broker_user(), ) + row = db.data_map[TolidRequest][0] assert pending.status == "pending" + assert pending.specimen_id == "ERS123456" assert row.request_id == "REQ-1" assigned = broker.report_tolid_result( @@ -190,9 +262,10 @@ def test_report_tolid_result_updates_pending_assigned_and_failed_states(): assert db.committed is True -def test_report_results_auto_creates_tolid_row_for_accepted_specimen_submission(): - sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="SPEC-1") +def test_report_results_does_not_auto_create_tolid_row_for_accepted_specimen_submission(): + sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="CANOPY-SPEC-1") organism = Organism(taxon_id=1729, scientific_name="Species 1729") + sample.organism = organism submission = SampleSubmission( id=uuid4(), sample_id=sample.id, @@ -212,7 +285,7 @@ def test_report_results_auto_creates_tolid_row_for_accepted_specimen_submission( broker.ReportItem( id=submission.id, status="accepted", - accession="SAMEA0001", + accession="ERS123456", submitted_at=datetime(2024, 1, 1, tzinfo=timezone.utc), ) ], @@ -225,46 +298,4 @@ def test_report_results_auto_creates_tolid_row_for_accepted_specimen_submission( ) assert result.updated_counts["samples"] == 1 - rows = db.data_map[TolidRequest] - assert len(rows) == 1 - assert rows[0].sample_id == sample.id - assert rows[0].tolid_external_id == "SAMEA0001" - assert rows[0].status == "not_requested" - assert rows[0].scientific_name == "Species 1729" - - -def test_report_results_does_not_create_tolid_row_for_non_specimen_sample(): - sample = Sample(id=uuid4(), taxon_id=1729, kind="derived", specimen_id="SPEC-1") - organism = Organism(taxon_id=1729, scientific_name="Species 1729") - submission = SampleSubmission( - id=uuid4(), - sample_id=sample.id, - status="submitting", - attempt_id=uuid4(), - authority="ENA", - prepared_payload={}, - project_id=uuid4(), - ) - db = _Session({SampleSubmission: [submission], Sample: [sample], Organism: [organism]}) - - broker.report_results( - attempt_id=submission.attempt_id, - payload=broker.ReportRequest( - attempt_id=submission.attempt_id, - samples=[ - broker.ReportItem( - id=submission.id, - status="accepted", - accession="SAMEA0001", - submitted_at=datetime(2024, 1, 1, tzinfo=timezone.utc), - ) - ], - experiments=[], - reads=[], - projects=[], - ), - db=db, - current_user=_broker_user(), - ) - assert TolidRequest not in db.data_map diff --git a/tests/unit/services/test_tolid_service.py b/tests/unit/services/test_tolid_service.py index 3499fd7..d3262a6 100644 --- a/tests/unit/services/test_tolid_service.py +++ b/tests/unit/services/test_tolid_service.py @@ -1,6 +1,7 @@ from datetime import datetime, timezone from uuid import uuid4 +from app.models.organism import Organism from app.models.sample import Sample, SampleSubmission from app.models.tolid_request import TolidRequest from app.schemas.tolid import TolidRequestReport @@ -48,71 +49,46 @@ def test_tolid_request_model_has_expected_indexes(): assert "idx_tolid_request_request_id" in indexes -def test_ensure_row_for_sample_updates_existing_row_instead_of_creating_duplicate(): +def test_get_by_specimen_accession_returns_virtual_not_requested_state_when_row_absent(): sample_id = uuid4() sample = Sample(id=sample_id, taxon_id=1729, kind="specimen", specimen_id="SPEC-1") - existing = TolidRequest( + organism = Organism(taxon_id=1729, scientific_name="New name") + submission = SampleSubmission( id=uuid4(), sample_id=sample_id, - tolid_external_id="SAMEA0001", - taxon_id=1729, - scientific_name="Old name", - status="not_requested", - ) - existing.sample = sample - db = _Session({Sample: [sample], TolidRequest: [existing]}) - - out = tolid_request_service.ensure_row_for_sample( - db, - sample_id=sample_id, - tolid_external_id="SAMEA9999", - scientific_name="New name", + status="accepted", + accession="ERS123456", + authority="ENA", + prepared_payload={}, + project_id=uuid4(), ) + sample.organism = organism + db = _Session({Sample: [sample], SampleSubmission: [submission], Organism: [organism]}) - assert out is existing - assert existing.tolid_external_id == "SAMEA9999" - assert existing.scientific_name == "New name" - assert len(db.data_map[TolidRequest]) == 1 + out = tolid_request_service.get_by_specimen_accession(db, "ERS123456") + assert out.sample_id == sample_id + assert out.specimen_id == "ERS123456" + assert out.status == "not_requested" + assert out.scientific_name == "New name" + assert out.kind == "specimen" -def test_ensure_row_for_sample_skips_non_specimen_samples(): - sample = Sample(id=uuid4(), taxon_id=1729, kind="derived", specimen_id="SPEC-1") - db = _Session({Sample: [sample]}) - out = tolid_request_service.ensure_row_for_sample( - db, - sample_id=sample.id, - tolid_external_id="SAMEA0001", - scientific_name="Species name", - ) - - assert out is None - assert TolidRequest not in db.data_map - - -def test_report_result_assigned_updates_sample_tolid(): +def test_report_result_creates_row_lazily_and_updates_sample_tolid(): sample = Sample(id=uuid4(), taxon_id=1729, kind="specimen", specimen_id="SPEC-1") + organism = Organism(taxon_id=1729, scientific_name="Species name") submission = SampleSubmission( id=uuid4(), sample_id=sample.id, status="accepted", - accession="SAMEA0001", + accession="ERS123456", authority="ENA", prepared_payload={}, project_id=uuid4(), submitted_at=datetime(2024, 1, 1, tzinfo=timezone.utc), ) - row = TolidRequest( - id=uuid4(), - sample_id=sample.id, - tolid_external_id="SAMEA0001", - taxon_id=1729, - scientific_name="Species name", - status="pending", - request_id="REQ-1", - ) - row.sample = sample - db = _Session({Sample: [sample], SampleSubmission: [submission], TolidRequest: [row]}) + sample.organism = organism + db = _Session({Sample: [sample], SampleSubmission: [submission], Organism: [organism]}) view = tolid_request_service.report_result( db, @@ -125,7 +101,10 @@ def test_report_result_assigned_updates_sample_tolid(): ), ) + row = db.data_map[TolidRequest][0] assert row.status == "assigned" assert row.tolid == "tol123" + assert row.tolid_external_id == "ERS123456" assert sample.tolid == "tol123" assert view.tolid == "tol123" + assert view.specimen_id == "ERS123456" From c38fba2eb974af6c8bd60bd73564445cfad56dad Mon Sep 17 00:00:00 2001 From: Emily Marshall Date: Thu, 18 Jun 2026 19:11:53 +1000 Subject: [PATCH 3/5] feat: update shape of body for bulk-insert specimen samples endpoint to match mapper output --- app/api/v1/endpoints/samples.py | 142 +++++++++++------- .../endpoints/test_bulk_import_samples.py | 111 ++++++++------ 2 files changed, 154 insertions(+), 99 deletions(-) diff --git a/app/api/v1/endpoints/samples.py b/app/api/v1/endpoints/samples.py index 52393fa..f6581ad 100644 --- a/app/api/v1/endpoints/samples.py +++ b/app/api/v1/endpoints/samples.py @@ -518,14 +518,14 @@ def _create_sample_with_submission( def bulk_import_specimen_samples( *, db: Session = Depends(get_db), - samples_data: Dict[str, Dict[str, Any]], + samples_data: Dict[str, Dict[str, Dict[str, Any]]], current_user: User = Depends(get_current_active_user), ) -> Any: """ Bulk import specimen samples (kind='specimen'). - Expected format: Dictionary keyed by sample_key (a concat of taxon_id and specimen_id). - Each sample must have taxon_id and specimen_id. + Expected format: Dictionary keyed by taxon_id, then specimen_id. + The nested keys supply taxon_id and specimen_id for each sample. Enforces uniqueness constraint: one specimen per (taxon_id, specimen_id). """ # Load the ENA-ATOL mapping file @@ -541,69 +541,97 @@ def bulk_import_specimen_samples( skipped_count = 0 errors = [] - for sample_key, sample_data in samples_data.items(): + for taxon_key, specimen_map in samples_data.items(): + if not isinstance(specimen_map, dict): + errors.append(f"{taxon_key}: Expected an object keyed by specimen_id") + skipped_count += 1 + continue + try: - # Get organism reference - taxon_id = sample_data.get("taxon_id") - if taxon_id is None: - errors.append(f"{sample_key}: Missing taxon_id") - skipped_count += 1 - continue + taxon_id = int(taxon_key) + except (TypeError, ValueError): + errors.append(f"{taxon_key}: Invalid taxon_id key") + skipped_count += len(specimen_map) or 1 + continue - organism = db.query(Organism).filter(Organism.taxon_id == int(taxon_id)).first() - if not organism: - errors.append(f"{sample_key}: Organism not found with taxon_id '{taxon_id}'") - skipped_count += 1 - continue + organism = db.query(Organism).filter(Organism.taxon_id == taxon_id).first() + if not organism: + errors.append(f"{taxon_key}: Organism not found with taxon_id '{taxon_id}'") + skipped_count += len(specimen_map) or 1 + continue - organism_taxon_id = _organism_taxon_id(organism) + organism_taxon_id = _organism_taxon_id(organism) - # Validate specimen_id is present - specimen_id = sample_data.get("specimen_id") - if not specimen_id: - errors.append(f"{sample_key}: specimen_id is required for specimen samples") - skipped_count += 1 - continue + for specimen_key, raw_sample_data in specimen_map.items(): + sample_key = f"{taxon_key}/{specimen_key}" + try: + if not isinstance(raw_sample_data, dict): + errors.append(f"{sample_key}: Expected sample payload object") + skipped_count += 1 + continue - # Check for duplicate specimen - existing_specimen = ( - db.query(Sample) - .filter( - Sample.taxon_id == organism_taxon_id, - Sample.specimen_id == specimen_id, - Sample.kind == SampleKind.SPECIMEN, - ) - .first() - ) - if existing_specimen: - errors.append( - f"{sample_key}: Specimen already exists for taxon_id '{organism_taxon_id}' " - f"and specimen_id '{specimen_id}'" + sample_data = dict(raw_sample_data) + specimen_id = specimen_key + if not specimen_id: + errors.append(f"{sample_key}: specimen_id key is required for specimen samples") + skipped_count += 1 + continue + + if sample_data.get("taxon_id") not in (None, organism_taxon_id): + errors.append( + f"{sample_key}: taxon_id in payload does not match outer key '{taxon_key}'" + ) + skipped_count += 1 + continue + + if sample_data.get("specimen_id") not in (None, specimen_id): + errors.append( + f"{sample_key}: specimen_id in payload does not match nested key '{specimen_id}'" + ) + skipped_count += 1 + continue + + sample_data["taxon_id"] = organism_taxon_id + sample_data["specimen_id"] = specimen_id + + existing_specimen = ( + db.query(Sample) + .filter( + Sample.taxon_id == organism_taxon_id, + Sample.specimen_id == specimen_id, + Sample.kind == SampleKind.SPECIMEN, + ) + .first() ) - skipped_count += 1 - continue - sample_data["organism_part"] = "WHOLE ORGANISM" + if existing_specimen: + errors.append( + f"{sample_key}: Specimen already exists for taxon_id '{organism_taxon_id}' " + f"and specimen_id '{specimen_id}'" + ) + skipped_count += 1 + continue - # Create specimen sample (bpa_sample_id is optional for specimens) - sample, sample_submission = _create_sample_with_submission( - db=db, - bpa_sample_id=sample_data.get("bpa_sample_id"), # Optional for specimens - sample_data=sample_data, - taxon_id=organism_taxon_id, - kind=SampleKind.SPECIMEN, - derived_from_sample_id=None, - ena_atol_map=ena_atol_map, - ) + sample_data["organism_part"] = "WHOLE ORGANISM" - db.add(sample) - db.add(sample_submission) - db.commit() - created_count += 1 + sample, sample_submission = _create_sample_with_submission( + db=db, + bpa_sample_id=sample_data.get("bpa_sample_id"), + sample_data=sample_data, + taxon_id=organism_taxon_id, + kind=SampleKind.SPECIMEN, + derived_from_sample_id=None, + ena_atol_map=ena_atol_map, + ) - except Exception as e: - errors.append(f"{sample_key}: {str(e)}") - db.rollback() - skipped_count += 1 + db.add(sample) + db.add(sample_submission) + db.commit() + created_count += 1 + + except Exception as e: + errors.append(f"{sample_key}: {str(e)}") + db.rollback() + skipped_count += 1 message = f"Specimen import complete. Created: {created_count}, Skipped: {skipped_count}" diff --git a/tests/unit/endpoints/test_bulk_import_samples.py b/tests/unit/endpoints/test_bulk_import_samples.py index 9397663..2732a90 100644 --- a/tests/unit/endpoints/test_bulk_import_samples.py +++ b/tests/unit/endpoints/test_bulk_import_samples.py @@ -132,12 +132,12 @@ def mock_create_sample( mock_open.return_value.__enter__.return_value.read.return_value = '{"sample": {}}' with patch("json.load", return_value={"sample": {}}): payload = { - "SPEC001_9606": { - "taxon_id": 9606, - "specimen_id": "SPEC001", - "lifestage": "adult", - "sex": "male", - "organism_part": "blood", + "9606": { + "SPEC001": { + "lifestage": "adult", + "sex": "male", + "organism_part": "blood", + } } } @@ -151,7 +151,7 @@ def mock_create_sample( def test_bulk_import_specimens_missing_taxon_id(): - """Test bulk import fails when taxon_id is missing.""" + """Test bulk import fails when the taxon_id key is invalid.""" client = TestClient(app) fake_session = FakeSession() @@ -161,7 +161,7 @@ def test_bulk_import_specimens_missing_taxon_id(): with patch("builtins.open", create=True): with patch("json.load", return_value={"sample": {}}): - payload = {"SPEC001_9606": {"specimen_id": "SPEC001", "lifestage": "adult"}} + payload = {"not-a-taxon": {"SPEC001": {"lifestage": "adult"}}} resp = client.post("/api/v1/samples/bulk-import-specimens", json=payload) @@ -170,11 +170,11 @@ def test_bulk_import_specimens_missing_taxon_id(): assert body["created_count"] == 0 assert body["skipped_count"] == 1 assert body["errors"] is not None - assert any("Missing taxon_id" in err for err in body["errors"]) + assert any("Invalid taxon_id key" in err for err in body["errors"]) def test_bulk_import_specimens_missing_specimen_id(): - """Test bulk import fails when specimen_id is missing.""" + """Test bulk import fails when specimen_id key is missing/empty.""" client = TestClient(app) fake_organism = SimpleNamespace(taxon_id=9606) @@ -185,7 +185,7 @@ def test_bulk_import_specimens_missing_specimen_id(): with patch("builtins.open", create=True): with patch("json.load", return_value={"sample": {}}): - payload = {"SPEC001_9606": {"taxon_id": 9606, "lifestage": "adult"}} + payload = {"9606": {"": {"lifestage": "adult"}}} resp = client.post("/api/v1/samples/bulk-import-specimens", json=payload) @@ -194,7 +194,7 @@ def test_bulk_import_specimens_missing_specimen_id(): assert body["created_count"] == 0 assert body["skipped_count"] == 1 assert body["errors"] is not None - assert any("specimen_id is required" in err for err in body["errors"]) + assert any("specimen_id key is required" in err for err in body["errors"]) def test_bulk_import_specimens_duplicate_specimen(monkeypatch): @@ -231,10 +231,10 @@ def query(self, model): with patch("builtins.open", create=True): with patch("json.load", return_value={"sample": {}}): payload = { - "SPEC001_9606": { - "taxon_id": 9606, - "specimen_id": "SPEC001", - "lifestage": "adult", + "9606": { + "SPEC001": { + "lifestage": "adult", + } } } @@ -260,10 +260,10 @@ def test_bulk_import_specimens_organism_not_found(): with patch("builtins.open", create=True): with patch("json.load", return_value={"sample": {}}): payload = { - "SPEC001_9606": { - "taxon_id": 9606, - "specimen_id": "SPEC001", - "lifestage": "adult", + "9606": { + "SPEC001": { + "lifestage": "adult", + } } } @@ -313,13 +313,13 @@ def mock_create_sample( with patch("builtins.open", create=True): with patch("json.load", return_value={"sample": {}}): payload = { - "SPEC001_9606": { - "taxon_id": 9606, - "specimen_id": "SPEC001", - "lifestage": "adult", - "sex": "male", - "organism_part": "blood", - # Note: no bpa_sample_id + "9606": { + "SPEC001": { + "lifestage": "adult", + "sex": "male", + "organism_part": "blood", + # Note: no bpa_sample_id + } } } @@ -606,7 +606,7 @@ def test_bulk_import_specimens_requires_curator_or_admin(): app.dependency_overrides[samples.get_current_active_user] = _override_user(["viewer"]) app.dependency_overrides[samples.get_db] = _override_db(FakeSession()) - payload = {"SPEC001_9606": {"specimen_id": "SPEC001"}} + payload = {"9606": {"SPEC001": {}}} # This will fail at the require_role check # Note: The actual behavior depends on require_role implementation @@ -682,20 +682,16 @@ def mock_create_sample( with patch("builtins.open", create=True): with patch("json.load", return_value={"sample": {}}): payload = { - "SPEC001_9606": { - "taxon_id": 9606, - "specimen_id": "SPEC001", - "lifestage": "adult", - }, - "SPEC002_9606": { - "taxon_id": 9606, - "specimen_id": "SPEC002", - "lifestage": "juvenile", - }, - "SPEC003_9606": { - "taxon_id": 9606, - "specimen_id": "SPEC003", - "lifestage": "larva", + "9606": { + "SPEC001": { + "lifestage": "adult", + }, + "SPEC002": { + "lifestage": "juvenile", + }, + "SPEC003": { + "lifestage": "larva", + }, }, } @@ -705,3 +701,34 @@ def mock_create_sample( body = resp.json() assert body["created_count"] == 3 assert body["skipped_count"] == 0 + + +def test_bulk_import_specimens_rejects_payload_key_mismatch(): + """Test nested keys are treated as source of truth.""" + client = TestClient(app) + + fake_organism = SimpleNamespace(taxon_id=9606) + fake_session = FakeSession(organisms={"default": fake_organism}, samples={"default": None}) + + app.dependency_overrides[samples.get_current_active_user] = _override_user(["admin"]) + app.dependency_overrides[samples.get_db] = _override_db(fake_session) + + with patch("builtins.open", create=True): + with patch("json.load", return_value={"sample": {}}): + payload = { + "9606": { + "SPEC001": { + "taxon_id": 1234, + "specimen_id": "DIFFERENT", + "lifestage": "adult", + } + } + } + + resp = client.post("/api/v1/samples/bulk-import-specimens", json=payload) + + assert resp.status_code == 200 + body = resp.json() + assert body["created_count"] == 0 + assert body["skipped_count"] == 1 + assert any("taxon_id in payload does not match outer key" in err for err in body["errors"]) From b6503ef592a031b9b9da9eca665b7be50a20ee27 Mon Sep 17 00:00:00 2001 From: Emily Marshall Date: Thu, 18 Jun 2026 21:32:42 +1000 Subject: [PATCH 4/5] feat: update organism.scientific_name after importing taxonomy_info --- app/services/organism_service.py | 78 +++++++++++ app/services/taxonomy_info_service.py | 10 +- .../services/test_taxonomy_info_service.py | 129 +++++++++++++++++- 3 files changed, 209 insertions(+), 8 deletions(-) diff --git a/app/services/organism_service.py b/app/services/organism_service.py index 91e618d..72fac6a 100644 --- a/app/services/organism_service.py +++ b/app/services/organism_service.py @@ -36,6 +36,83 @@ def sync_organism_scientific_name( ) +def organism_label_for_projects(organism: Organism) -> str: + return organism.scientific_name or organism.bpa_scientific_name or str(organism.taxon_id) + + +def build_project_metadata(*, organism_label: str, project_type: str) -> Dict[str, str]: + if project_type == "root": + return { + "alias": f"{organism_label} genome assembly and related data", + "title": f"{organism_label}", + "description": ( + f"Genome assemblies and related data for the organism {organism_label}, " + f"brokered on behalf of the Australian Tree of Life (AToL) project" + ), + } + if project_type == "genomic_data": + return { + "alias": f"Genomic data for {organism_label}", + "title": f"{organism_label} - genomic data", + "description": ( + f"Genomic data for the organism {organism_label}, brokered on behalf of the " + f"Australian Tree of Life (AToL) project" + ), + } + return {} + + +def sync_projects_for_organism(db: Session, organism: Organism) -> None: + """Refresh project labels and draft submission payloads after organism naming changes.""" + organism_label = organism_label_for_projects(organism) + projects = db.query(Project).filter(Project.taxon_id == organism.taxon_id).all() + project_submissions = db.query(ProjectSubmission).all() + + for project in projects: + metadata = build_project_metadata( + organism_label=organism_label, + project_type=project.project_type.value + if hasattr(project.project_type, "value") + else project.project_type, + ) + if not metadata: + continue + + project.alias = metadata["alias"] + project.title = metadata["title"] + project.description = metadata["description"] + db.add(project) + + for submission in project_submissions: + if submission.project_id != project.id: + continue + submission_status = ( + submission.status.value + if hasattr(submission.status, "value") + else submission.status + ) + if submission_status not in {"draft", "ready"}: + continue + + prepared_payload = dict(submission.prepared_payload or {}) + prepared_payload.update( + { + "taxon_id": project.taxon_id, + "project_type": project.project_type.value + if hasattr(project.project_type, "value") + else project.project_type, + "study_type": project.study_type, + "alias": project.alias, + "title": project.title, + "description": project.description, + "centre_name": project.centre_name, + "study_attributes": project.study_attributes, + } + ) + submission.prepared_payload = prepared_payload + db.add(submission) + + class OrganismService(BaseService[Organism, OrganismCreate, OrganismUpdate]): """Service for Organism operations.""" @@ -309,6 +386,7 @@ def update_organism( ), ) db.add(organism) + sync_projects_for_organism(db, organism) db.commit() db.refresh(organism) return organism diff --git a/app/services/taxonomy_info_service.py b/app/services/taxonomy_info_service.py index 1020781..b54115e 100644 --- a/app/services/taxonomy_info_service.py +++ b/app/services/taxonomy_info_service.py @@ -9,7 +9,10 @@ from app.schemas.bulk_import import BulkImportResponse from app.schemas.taxonomy_info import TaxonomyInfoCreate, TaxonomyInfoUpdate from app.services.ncbi_taxonomy_service import fetch_taxonomy_for_taxon_ids -from app.services.organism_service import sync_organism_scientific_name +from app.services.organism_service import ( + sync_organism_scientific_name, + sync_projects_for_organism, +) logger = logging.getLogger(__name__) @@ -69,6 +72,7 @@ def create(self, db: Session, *, ti_in: TaxonomyInfoCreate) -> TaxonomyInfo: organism, ncbi_scientific_name=getattr(ti, "ncbi_scientific_name", None), ) + sync_projects_for_organism(db, organism) db.commit() db.refresh(ti) return ti @@ -111,6 +115,7 @@ def populate_from_ncbi_lookup( organism, ncbi_scientific_name=ti.ncbi_scientific_name, ) + sync_projects_for_organism(db, organism) db.flush() logger.info( "NCBI taxonomy enrichment %s taxonomy_info for taxon_id=%s; applied_fields=%s", @@ -139,6 +144,7 @@ def update( organism, ncbi_scientific_name=ti.ncbi_scientific_name, ) + sync_projects_for_organism(db, organism) db.add(organism) db.add(ti) db.commit() @@ -152,6 +158,7 @@ def delete(self, db: Session, *, taxon_id: int) -> Optional[TaxonomyInfo]: organism = db.query(Organism).filter(Organism.taxon_id == taxon_id).first() if organism: sync_organism_scientific_name(organism, ncbi_scientific_name=None) + sync_projects_for_organism(db, organism) db.add(organism) db.delete(ti) db.commit() @@ -231,6 +238,7 @@ def _bulk_process_rows( organism, ncbi_scientific_name=ti.ncbi_scientific_name, ) + sync_projects_for_organism(db, organism) db.add(organism) db.commit() diff --git a/tests/unit/services/test_taxonomy_info_service.py b/tests/unit/services/test_taxonomy_info_service.py index 895d60e..9235cf1 100644 --- a/tests/unit/services/test_taxonomy_info_service.py +++ b/tests/unit/services/test_taxonomy_info_service.py @@ -1,6 +1,9 @@ +import uuid + import pytest from app.models.organism import Organism +from app.models.project import Project, ProjectSubmission from app.models.taxonomy_info import TaxonomyInfo from app.schemas.bulk_import import BulkTaxonomyInfoImport from app.schemas.taxonomy_info import TaxonomyInfoCreate @@ -11,21 +14,31 @@ class _Query: def __init__(self, session, model): self.session = session self.model = model - self._taxon_id = None + self._filters = {} def filter(self, *criteria, **_kwargs): for criterion in criteria: + left = getattr(criterion, "left", None) right = getattr(criterion, "right", None) + field_name = getattr(left, "name", None) value = getattr(right, "value", None) - if value is not None: - self._taxon_id = value + if field_name is not None and value is not None: + self._filters[field_name] = value return self - def first(self): + def all(self): store = self.session.data.get(self.model, {}) - if self._taxon_id is None: - return next(iter(store.values()), None) - return store.get(self._taxon_id) + values = list(store.values()) + if not self._filters: + return values + filtered = [] + for item in values: + if all(getattr(item, field, None) == value for field, value in self._filters.items()): + filtered.append(item) + return filtered + + def first(self): + return next(iter(self.all()), None) class _Session: @@ -42,6 +55,9 @@ def add(self, obj): if isinstance(obj, (Organism, TaxonomyInfo)): self.data.setdefault(type(obj), {}) self.data[type(obj)][obj.taxon_id] = obj + elif isinstance(obj, (Project, ProjectSubmission)): + self.data.setdefault(type(obj), {}) + self.data[type(obj)][obj.id] = obj def delete(self, obj): store = self.data.get(type(obj), {}) @@ -148,6 +164,105 @@ def test_create_taxonomy_info_fetches_ncbi_and_applies_payload(monkeypatch): assert db.refresh_count == 1 +def test_create_taxonomy_info_updates_existing_project_metadata(monkeypatch): + organism = Organism(taxon_id=5303, bpa_scientific_name="Agaricus") + root_project = Project( + id=uuid.uuid4(), + taxon_id=5303, + project_type="root", + study_type="Whole Genome Sequencing", + alias="Agaricus genome assembly and related data", + title="Agaricus", + description="Genome assemblies and related data for the organism Agaricus, brokered on behalf of the Australian Tree of Life (AToL) project", + centre_name="Australian Tree of Life (AToL)", + study_attributes=None, + status="draft", + authority="ENA", + ) + genomic_project = Project( + id=uuid.uuid4(), + taxon_id=5303, + project_type="genomic_data", + study_type="Whole Genome Sequencing", + alias="Genomic data for Agaricus", + title="Agaricus - genomic data", + description="Genomic data for the organism Agaricus, brokered on behalf of the Australian Tree of Life (AToL) project", + centre_name="Australian Tree of Life (AToL)", + study_attributes=None, + status="draft", + authority="ENA", + ) + draft_submission = ProjectSubmission( + id=uuid.uuid4(), + project_id=root_project.id, + status="draft", + prepared_payload={ + "alias": root_project.alias, + "title": root_project.title, + "description": root_project.description, + }, + ) + accepted_submission = ProjectSubmission( + id=uuid.uuid4(), + project_id=genomic_project.id, + status="accepted", + prepared_payload={"alias": "Accepted alias", "title": "Accepted title"}, + ) + db = _Session( + { + Organism: {5303: organism}, + TaxonomyInfo: {}, + Project: { + root_project.id: root_project, + genomic_project.id: genomic_project, + }, + ProjectSubmission: { + draft_submission.id: draft_submission, + accepted_submission.id: accepted_submission, + }, + } + ) + + monkeypatch.setattr( + ti_service_module, + "fetch_taxonomy_for_taxon_ids", + lambda taxa, batch_size=20: ( + { + 5303: { + "taxon_id": 5303, + "ncbi_taxon_id": 5303, + "ncbi_rank": "species", + "ncbi_scientific_name": "Agaricus test", + } + }, + [], + ), + ) + + ti_service_module.taxonomy_info_service.create( + db, + ti_in=TaxonomyInfoCreate( + taxon_id=5303, + genetic_code_id=11, + ), + ) + + assert organism.scientific_name == "Agaricus test" + assert root_project.alias == "Agaricus test genome assembly and related data" + assert root_project.title == "Agaricus test" + assert genomic_project.alias == "Genomic data for Agaricus test" + assert genomic_project.title == "Agaricus test - genomic data" + assert ( + draft_submission.prepared_payload["alias"] + == "Agaricus test genome assembly and related data" + ) + assert draft_submission.prepared_payload["title"] == "Agaricus test" + assert accepted_submission.prepared_payload == { + "alias": "Accepted alias", + "title": "Accepted title", + } + + def test_taxonomy_info_create_rejects_ncbi_fields(): with pytest.raises(Exception): TaxonomyInfoCreate( From 09e2cde9d8deca1b65bd0ec031e1ff5052c34448 Mon Sep 17 00:00:00 2001 From: Emily Marshall Date: Thu, 18 Jun 2026 21:34:01 +1000 Subject: [PATCH 5/5] style: linting --- app/models/__init__.py | 2 +- app/models/tolid_request.py | 8 ++++---- app/services/tolid_service.py | 14 ++++++++++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/models/__init__.py b/app/models/__init__.py index 0a223ef..b22e063 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -10,6 +10,6 @@ from app.models.read import Read from app.models.sample import Sample, SampleSubmission from app.models.taxonomy_info import TaxonomyInfo -from app.models.tolid_request import TolidRequest from app.models.token import RefreshToken +from app.models.tolid_request import TolidRequest from app.models.user import User diff --git a/app/models/tolid_request.py b/app/models/tolid_request.py index 89133a2..0c4c2be 100644 --- a/app/models/tolid_request.py +++ b/app/models/tolid_request.py @@ -14,11 +14,11 @@ class TolidRequest(Base): __tablename__ = "tolid_request" id = Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - sample_id = Column(UUID(as_uuid=True), ForeignKey("sample.id", ondelete="CASCADE"), nullable=False) - tolid_external_id = Column(Text, nullable=False) - taxon_id = Column( - Integer, ForeignKey("organism.taxon_id", ondelete="CASCADE"), nullable=False + sample_id = Column( + UUID(as_uuid=True), ForeignKey("sample.id", ondelete="CASCADE"), nullable=False ) + tolid_external_id = Column(Text, nullable=False) + taxon_id = Column(Integer, ForeignKey("organism.taxon_id", ondelete="CASCADE"), nullable=False) scientific_name = Column(Text, nullable=True) tolid = Column(Text, nullable=True) request_id = Column(Text, nullable=True) diff --git a/app/services/tolid_service.py b/app/services/tolid_service.py index 5f2712a..1d895db 100644 --- a/app/services/tolid_service.py +++ b/app/services/tolid_service.py @@ -2,8 +2,8 @@ from typing import Dict, Iterable, List, Optional from uuid import UUID -from fastapi.encoders import jsonable_encoder from fastapi import status +from fastapi.encoders import jsonable_encoder from sqlalchemy.orm import Session from app.core.errors import AppError @@ -42,7 +42,9 @@ def _get_scientific_name(self, db: Session, taxon_id: int) -> Optional[str]: return organism.scientific_name if organism else None def _find_row(self, db: Session, sample_id: UUID) -> Optional[TolidRequest]: - return next((row for row in db.query(TolidRequest).all() if row.sample_id == sample_id), None) + return next( + (row for row in db.query(TolidRequest).all() if row.sample_id == sample_id), None + ) def _fallback_external_id(self, db: Session, sample: Sample) -> Optional[str]: submissions = db.query(SampleSubmission).all() @@ -96,7 +98,9 @@ def _to_broker_view( taxon_id=sample.taxon_id, scientific_name=self._resolved_scientific_name(db=db, sample=sample, row=row), status=( - self._status_value(row) if row is not None else TolidRequestStatus.NOT_REQUESTED.value + self._status_value(row) + if row is not None + else TolidRequestStatus.NOT_REQUESTED.value ), request_id=(row.request_id if row is not None else None), tolid=(row.tolid if row is not None else None), @@ -158,7 +162,9 @@ def get_row_for_sample(self, db: Session, sample_id: UUID) -> TolidRequestBroker ) row = self._find_row(db, sample_id) - specimen_id = row.tolid_external_id if row is not None else self._fallback_external_id(db, sample) + specimen_id = ( + row.tolid_external_id if row is not None else self._fallback_external_id(db, sample) + ) if not specimen_id: raise AppError( status_code=status.HTTP_404_NOT_FOUND,