5 Commits

Author SHA1 Message Date
15a517e389 🐾 fix(api): cascade collection and item deletes too
Collection and Item DELETE — both the REST endpoints and the HTML
views — called bare .delete(), leaking each collection's Items and every
item's Chunks/Images/ImageEmbeddings. New delete_collection_cascade and
delete_item_cascade in the shared library_delete service now back all
four paths. No orphan-Concept GC in either (that invariant stays
library-delete-only, matching the existing supersede path); the ingest
supersede helper in tasks.py now delegates to delete_item_cascade
instead of duplicating its Cypher.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-03 13:12:25 -04:00
c040e6f3dd Merge pull request '🐾 fix(api): cascade plain library delete; case-insensitive name conflict' (#9) from fix/library-api-delete-and-name-case into main
All checks were successful
CVE Scan & Docker Build / security-scan (push) Successful in 4m30s
Build & Deploy Docs / build-and-deploy (push) Successful in 1m16s
CVE Scan & Docker Build / build-and-push (push) Successful in 2m46s
Reviewed-on: #9
2026-08-03 16:45:48 +00:00
f1639dbdec 🐾 fix(api): cascade plain library delete; case-insensitive name conflict
DELETE /library/api/libraries/{uid}/ called bare lib.delete(), orphaning
Collections/Items/Chunks/Images and skipping Concept GC — the only delete
path not using the shared cascade. It now delegates to
delete_library_cascade like the HTML and workspace delete paths.

Name-conflict checks on both create endpoints are now case-insensitive
via find_library_by_name_ci (parameterised Cypher toLower comparison —
not neomodel iexact, which embeds the value in a regex and breaks on
names like "C++ Notes"). The Neo4j unique index is case-sensitive, so
"amazon connect" previously coexisted silently with "Amazon Connect",
confusing every name-matching client (Spelunker matches names
case-insensitively). The 409 reports the existing spelling. The plain
create also gains a UniqueProperty catch for the pre-check/save race,
mirroring the workspace path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-03 12:41:55 -04:00
7237468e3c Merge pull request '🐾 fix(tasks): unsupported file types fail ingest terminally, no retries' (#8) from fix/unsupported-file-type-terminal into main
All checks were successful
CVE Scan & Docker Build / security-scan (push) Successful in 4m11s
Build & Deploy Docs / build-and-deploy (push) Successful in 1m15s
CVE Scan & Docker Build / build-and-push (push) Successful in 2m49s
Reviewed-on: #8
2026-08-03 16:28:58 +00:00
e1f128659e 🐾 fix(tasks): unsupported file types fail ingest terminally, no retries
Unsupported file types raised a bare ValueError from the parser, which
ingest_from_daedalus treated like any transient fault — ERROR logs and
pointless Celery retries for input that can never parse. A new
UnsupportedFileTypeError (ValueError subclass, so existing callers still
catch it) is classified in the task as a terminal client-data failure:
logged at WARNING, job marked failed with reason
"unsupported_file_type", never retried.

Recovered from uncommitted work predating PR #6 (which deliberately
excluded it); test fixes on top: IngestJob's pk is id not job_id, and
.run() needs push_request() since the task persists self.request.id
into the NOT NULL celery_task_id column.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-03 12:27:24 -04:00
11 changed files with 540 additions and 54 deletions

View File

@@ -85,26 +85,21 @@ an explicit `when: mnemosyne_first_deploy` flag.
```bash ```bash
# Apply Django ORM migrations (PostgreSQL schema) # Apply Django ORM migrations (PostgreSQL schema)
docker compose -f /srv/mnemosyne/docker-compose.yaml run --rm app migrate docker compose run --rm app migrate
# Create Neo4j vector + full-text indexes and load library-type defaults # Create Neo4j vector + full-text indexes and load library-type defaults
docker compose -f /srv/mnemosyne/docker-compose.yaml \ docker compose run --rm app setup
run --rm app setup
# Seed the MCPSigningKey used to sign long-lived Pallas team JWTs. # Seed the MCPSigningKey used to sign long-lived Pallas team JWTs.
# --retire-other deactivates any previously-active key. The hex # --retire-other deactivates any previously-active key. The hex
# emitted to stdout is persisted in Mnemosyne's database and is # emitted to stdout is persisted in Mnemosyne's database and is
# not re-injected from the vault — no operator action required # not re-injected from the vault — no operator action required
# beyond running this command once per fresh deployment. # beyond running this command once per fresh deployment.
docker compose -f /srv/mnemosyne/docker-compose.yaml \ docker compose run --rm app python manage.py seed_signing_key --kid daedalus-1 --retire-other
run --rm app \
python manage.py seed_signing_key --kid daedalus-1 --retire-other
# Create Django groups for SSO role mapping (View Only / Staff / SME / Admin). # Create Django groups for SSO role mapping (View Only / Staff / SME / Admin).
# Safe to re-run — idempotent. # Safe to re-run — idempotent.
docker compose -f /srv/mnemosyne/docker-compose.yaml \ docker compose run --rm app python manage.py create_sso_groups
run --rm app \
python manage.py create_sso_groups
``` ```
The `seed_signing_key` command prints the generated secret once to stdout — it The `seed_signing_key` command prints the generated secret once to stdout — it

View File

@@ -10,6 +10,7 @@ import os
from django.core.files.base import ContentFile from django.core.files.base import ContentFile
from django.core.files.storage import default_storage from django.core.files.storage import default_storage
from neomodel.exceptions import UniqueProperty
from rest_framework import status from rest_framework import status
from rest_framework.decorators import api_view, parser_classes, permission_classes from rest_framework.decorators import api_view, parser_classes, permission_classes
from rest_framework.parsers import FormParser, JSONParser, MultiPartParser from rest_framework.parsers import FormParser, JSONParser, MultiPartParser
@@ -17,6 +18,11 @@ from rest_framework.permissions import IsAuthenticated
from rest_framework.response import Response from rest_framework.response import Response
from library.content_types import get_library_type_config from library.content_types import get_library_type_config
from library.services.library_delete import (
delete_collection_cascade,
delete_item_cascade,
delete_library_cascade,
)
from mcp_server.drf_auth import request_token_label from mcp_server.drf_auth import request_token_label
from .serializers import ( from .serializers import (
@@ -51,7 +57,7 @@ def library_list_create(request):
a per-library ``item_count``. Off by default because the count is a a per-library ``item_count``. Off by default because the count is a
Cypher aggregate; on for the Daedalus-side registry poll. Cypher aggregate; on for the Daedalus-side registry poll.
""" """
from library.models import Library from library.models import Library, find_library_by_name_ci
if request.method == "GET": if request.method == "GET":
include_workspace = request.GET.get("include_workspace", "true").lower() != "false" include_workspace = request.GET.get("include_workspace", "true").lower() != "false"
@@ -85,13 +91,13 @@ def library_list_create(request):
serializer.is_valid(raise_exception=True) serializer.is_valid(raise_exception=True)
data = serializer.validated_data data = serializer.validated_data
# Library.name is globally unique; reject collisions with a clean 409 # Library names are unique. The Neo4j index is case-sensitive, but
# (uid + managed_by included so the caller can say who owns the name) # clients (Spelunker, humans) treat names case-insensitively, so the
# instead of letting the unique-index save raise a 500. # create-time check is case-insensitive too — otherwise "amazon connect"
try: # silently creates a near-duplicate of "Amazon Connect". Reject with a
existing = Library.nodes.get(name=data["name"]) # clean 409 (uid + managed_by included so the caller can say who owns
except Library.DoesNotExist: # the name) instead of letting the unique-index save raise a 500.
existing = None existing = find_library_by_name_ci(data["name"])
if existing is not None: if existing is not None:
logger.warning( logger.warning(
"library_create name_conflict name=%s existing_uid=%s caller=%s", "library_create name_conflict name=%s existing_uid=%s caller=%s",
@@ -99,7 +105,7 @@ def library_list_create(request):
) )
return Response( return Response(
{ {
"detail": f"A library named '{data['name']}' already exists.", "detail": f"A library named '{existing.name}' already exists.",
"code": "name_conflict", "code": "name_conflict",
"uid": existing.uid, "uid": existing.uid,
"managed_by": existing.managed_by_display or None, "managed_by": existing.managed_by_display or None,
@@ -127,7 +133,22 @@ def library_list_create(request):
data.get("llm_context_prompt") or defaults["llm_context_prompt"] data.get("llm_context_prompt") or defaults["llm_context_prompt"]
), ),
) )
try:
lib.save() lib.save()
except UniqueProperty:
# Race between the pre-check and save — an exact-name twin landed
# in between. Same 409 shape, minus the loser's uid/manager.
logger.warning(
"library_create name_conflict (save race) name=%s caller=%s",
data["name"], request.user.username,
)
return Response(
{
"detail": f"A library named '{data['name']}' already exists.",
"code": "name_conflict",
},
status=status.HTTP_409_CONFLICT,
)
return Response(LibrarySerializer(lib).data, status=status.HTTP_201_CREATED) return Response(LibrarySerializer(lib).data, status=status.HTTP_201_CREATED)
@@ -165,8 +186,16 @@ def library_detail(request, uid):
lib.save() lib.save()
return Response(LibrarySerializer(lib).data) return Response(LibrarySerializer(lib).data)
# DELETE # DELETE — use the shared cascade so child nodes (Collections/Items/
lib.delete() # Chunks/Images) and orphan Concepts are removed too; a bare
# lib.delete() would leak them all.
result = delete_library_cascade(lib)
logger.info(
"Library deleted via API library_uid=%s name=%s items=%d "
"orphans_deleted=%d caller=%s",
result["library_uid"], result["name"], result["item_count"],
result["orphans_deleted"], request.user.username,
)
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)
@@ -248,7 +277,13 @@ def collection_detail(request, uid):
col.save() col.save()
return Response(CollectionSerializer(col).data) return Response(CollectionSerializer(col).data)
col.delete() # DELETE — cascade Items/Chunks/Images too; a bare col.delete() leaks them.
result = delete_collection_cascade(col)
logger.info(
"Collection deleted via API collection_uid=%s name=%s items=%d caller=%s",
result["collection_uid"], result["name"], result["item_count"],
request.user.username,
)
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)
@@ -329,7 +364,13 @@ def item_detail(request, uid):
item.save() item.save()
return Response(ItemSerializer(item).data) return Response(ItemSerializer(item).data)
item.delete() # DELETE — cascade Chunks/Images/embeddings too; a bare item.delete()
# leaks them.
delete_item_cascade(item.uid)
logger.info(
"Item deleted via API item_uid=%s caller=%s",
item.uid, request.user.username,
)
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)

View File

@@ -67,7 +67,7 @@ def workspace_create(request):
workspace (200) — not an error. The library_type is frozen at first workspace (200) — not an error. The library_type is frozen at first
create; subsequent calls are not allowed to change it. create; subsequent calls are not allowed to change it.
""" """
from library.models import Library from library.models import Library, find_library_by_name_ci
serializer = WorkspaceCreateSerializer(data=request.data) serializer = WorkspaceCreateSerializer(data=request.data)
serializer.is_valid(raise_exception=True) serializer.is_valid(raise_exception=True)
@@ -127,6 +127,28 @@ def workspace_create(request):
status=status.HTTP_200_OK, status=status.HTTP_200_OK,
) )
# New workspace: reject a name already taken by any other library,
# case-insensitively — the Neo4j index is case-sensitive, so without
# this check "amazon connect" would silently coexist with an existing
# "Amazon Connect" and confuse every name-matching client.
name_taken = find_library_by_name_ci(data["name"])
if name_taken is not None:
logger.warning(
"workspace_create name_conflict workspace_id=%s name=%s "
"existing_uid=%s",
data["workspace_id"], data["name"], name_taken.uid,
)
return Response(
{
"detail": (
f"A library named '{name_taken.name}' already exists in "
"Mnemosyne."
),
"code": "name_conflict",
},
status=status.HTTP_409_CONFLICT,
)
defaults = get_library_type_config(data["library_type"]) defaults = get_library_type_config(data["library_type"])
lib = Library( lib = Library(
name=data["name"], name=data["name"],

View File

@@ -63,6 +63,23 @@ def infer_legacy_manager(workspace_id):
return "Kairos" if workspace_id.startswith("kairos-mail-") else "Daedalus" return "Kairos" if workspace_id.startswith("kairos-mail-") else "Daedalus"
def find_library_by_name_ci(name):
"""Case-insensitively find a Library by name, or None.
Parameterised Cypher rather than neomodel's ``iexact``, which embeds
the value in a regex and so breaks on names containing regex
metacharacters (e.g. "C++ Notes").
"""
from neomodel import db
rows, _ = db.cypher_query(
"MATCH (l:Library) WHERE toLower(l.name) = toLower($name) "
"RETURN l LIMIT 1",
{"name": name},
)
return Library.inflate(rows[0][0]) if rows else None
class Library(StructuredNode): class Library(StructuredNode):
""" """
Top-level container representing a content library. Top-level container representing a content library.

View File

@@ -106,3 +106,99 @@ def delete_library_cascade(lib) -> dict:
"item_s3_keys": item_s3_keys, "item_s3_keys": item_s3_keys,
"orphans_deleted": orphans_deleted, "orphans_deleted": orphans_deleted,
} }
def delete_collection_cascade(col) -> dict:
"""Delete ``col`` and all content reachable and unique to it.
Removes the Collection's Items with their Chunks, Images, and
ImageEmbeddings, then the Collection itself. No orphan-Concept GC —
that invariant belongs to library-level deletes only (see
:func:`delete_library_cascade`); Concepts orphaned here are collected
on the next library delete.
:param col: A ``library.models.Collection`` node instance.
:returns: Dict with ``collection_uid``, ``name``, ``item_count``, and
``item_s3_keys`` (list of ``(uid, s3_key)`` for async S3 cleanup).
"""
collection_uid = col.uid
collection_name = col.name
s3_rows, _ = db.cypher_query(
"MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item) "
"RETURN i.uid, i.s3_key",
{"uid": collection_uid},
)
item_s3_keys = [(r[0], r[1]) for r in s3_rows if r[1]]
db.cypher_query(
"""
MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item)
-[:HAS_CHUNK]->(c:Chunk)
DETACH DELETE c
""",
{"uid": collection_uid},
)
db.cypher_query(
"""
MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item)
-[:HAS_IMAGE]->(img:Image)
OPTIONAL MATCH (img)-[:HAS_EMBEDDING]->(emb:ImageEmbedding)
DETACH DELETE img, emb
""",
{"uid": collection_uid},
)
db.cypher_query(
"""
MATCH (col:Collection {uid: $uid})-[:CONTAINS]->(i:Item)
DETACH DELETE i
""",
{"uid": collection_uid},
)
db.cypher_query(
"MATCH (col:Collection {uid: $uid}) DETACH DELETE col",
{"uid": collection_uid},
)
logger.info(
"Collection cascade-deleted collection_uid=%s name=%s items=%d",
collection_uid, collection_name, len(item_s3_keys),
)
return {
"collection_uid": collection_uid,
"name": collection_name,
"item_count": len(item_s3_keys),
"item_s3_keys": item_s3_keys,
}
def delete_item_cascade(item_uid: str) -> dict:
"""Delete the Item ``item_uid`` with its Chunks, Images, and embeddings.
Keyed on the uid (not a node instance) so the ingest supersede path in
``library.tasks`` can share it. No orphan-Concept GC — see
:func:`delete_collection_cascade`.
:returns: Dict with ``item_uid`` and ``s3_key`` (empty string when the
item had no stored file) for async S3 cleanup.
"""
s3_rows, _ = db.cypher_query(
"MATCH (i:Item {uid: $uid}) RETURN i.s3_key",
{"uid": item_uid},
)
s3_key = (s3_rows[0][0] or "") if s3_rows else ""
db.cypher_query(
"""
MATCH (i:Item {uid: $uid})
OPTIONAL MATCH (i)-[:HAS_CHUNK]->(c:Chunk)
OPTIONAL MATCH (i)-[:HAS_IMAGE]->(img:Image)
OPTIONAL MATCH (img)-[:HAS_EMBEDDING]->(emb:ImageEmbedding)
DETACH DELETE c, img, emb, i
""",
{"uid": item_uid},
)
logger.info("Item cascade-deleted item_uid=%s", item_uid)
return {"item_uid": item_uid, "s3_key": s3_key}

View File

@@ -22,6 +22,17 @@ from .text_utils import remove_excessive_whitespace, sanitize_text
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class UnsupportedFileTypeError(ValueError):
"""Raised when a file's type cannot be parsed.
Deterministic and input-driven — re-parsing identical bytes can never
succeed — so ingest must treat it as a terminal failure and never retry.
Subclasses ``ValueError`` so existing ``except ValueError`` callers still
catch it.
"""
# File extensions supported by PyMuPDF # File extensions supported by PyMuPDF
PYMUPDF_EXTENSIONS = { PYMUPDF_EXTENSIONS = {
"pdf", "epub", "xps", "mobi", "fb2", "cbz", "svg", "pdf", "epub", "xps", "mobi", "fb2", "cbz", "svg",
@@ -88,7 +99,7 @@ class DocumentParser:
:param file_path: Path to the document file. :param file_path: Path to the document file.
:param file_type: File extension (without dot), e.g. 'pdf', 'epub'. :param file_type: File extension (without dot), e.g. 'pdf', 'epub'.
:returns: ParseResult with text blocks, images, and metadata. :returns: ParseResult with text blocks, images, and metadata.
:raises ValueError: If the file type is not supported. :raises UnsupportedFileTypeError: If the file type is not supported.
""" """
file_type = file_type.lower().lstrip(".") file_type = file_type.lower().lstrip(".")
@@ -117,7 +128,7 @@ class DocumentParser:
if file_type in ("html", "htm"): if file_type in ("html", "htm"):
return self._parse_with_pymupdf(file_path, file_type) return self._parse_with_pymupdf(file_path, file_type)
raise ValueError( raise UnsupportedFileTypeError(
f"Unsupported file type '{file_type}'. " f"Unsupported file type '{file_type}'. "
f"Supported: {sorted(PYMUPDF_EXTENSIONS | PLAINTEXT_EXTENSIONS | IMAGE_EXTENSIONS)}" f"Supported: {sorted(PYMUPDF_EXTENSIONS | PLAINTEXT_EXTENSIONS | IMAGE_EXTENSIONS)}"
) )

View File

@@ -347,6 +347,7 @@ def ingest_from_daedalus(self, job_id: str):
from datetime import datetime, timezone from datetime import datetime, timezone
from library.models import IngestJob, Item, Library from library.models import IngestJob, Item, Library
from library.services.parsers import UnsupportedFileTypeError
from library.services.source_s3 import ( from library.services.source_s3 import (
copy_into_mnemosyne, copy_into_mnemosyne,
fetch_from_source, fetch_from_source,
@@ -465,6 +466,24 @@ def ingest_from_daedalus(self, job_id: str):
**result, **result,
} }
except UnsupportedFileTypeError as exc:
# Deterministic, input-driven — re-parsing identical bytes can never
# succeed. Terminal client-data failure, never retried, and logged at
# WARNING (not ERROR) because an unparseable input is not a server fault.
logger.warning(
"Task ingest_from_daedalus rejected job_id=%s: %s", job_id, exc,
)
job.status = "failed"
job.error = str(exc)
job.completed_at = datetime.now(timezone.utc)
job.save(update_fields=["status", "error", "completed_at"])
return {
"success": False,
"job_id": job_id,
"error": str(exc),
"reason": "unsupported_file_type",
}
except Exception as exc: except Exception as exc:
logger.error( logger.error(
"Task ingest_from_daedalus failed job_id=%s: %s", "Task ingest_from_daedalus failed job_id=%s: %s",
@@ -484,16 +503,9 @@ def ingest_from_daedalus(self, job_id: str):
def _delete_item_and_chunks(item_uid: str): def _delete_item_and_chunks(item_uid: str):
"""Delete an Item, its chunks, and its images. Concept GC is workspace-delete only.""" """Delete an Item, its chunks, and its images. Concept GC is workspace-delete only."""
db.cypher_query( from library.services.library_delete import delete_item_cascade
"""
MATCH (i:Item {uid: $uid}) delete_item_cascade(item_uid)
OPTIONAL MATCH (i)-[:HAS_CHUNK]->(c:Chunk)
OPTIONAL MATCH (i)-[:HAS_IMAGE]->(img:Image)
OPTIONAL MATCH (img)-[:HAS_EMBEDDING]->(emb:ImageEmbedding)
DETACH DELETE c, img, emb, i
""",
{"uid": item_uid},
)
def _resolve_or_create_default_collection(lib, collection_uid: str = ""): def _resolve_or_create_default_collection(lib, collection_uid: str = ""):

View File

@@ -0,0 +1,148 @@
"""Tests for the plain library REST endpoints beyond create.
Currently covers the DELETE cascades: library, collection, and item DELETE
endpoints must go through the shared ``library_delete`` service functions —
bare ``.delete()`` calls leak child nodes (Collections, Items, Chunks,
Images) and, for libraries, skip orphan-Concept GC. Neo4j is stubbed via
``sys.modules``, same style as ``test_managed_by.py``.
"""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
from django.contrib.auth import get_user_model
from django.test import TestCase
from rest_framework.test import APIClient
User = get_user_model()
def _fake_node_cls(instance):
"""A neomodel-class stand-in whose ``nodes.get`` returns ``instance``.
``instance=None`` makes ``nodes.get`` raise the class's DoesNotExist.
"""
fake_nodes = MagicMock()
class DoesNotExist(Exception):
pass
if instance is None:
fake_nodes.get.side_effect = DoesNotExist()
else:
fake_nodes.get.return_value = instance
return SimpleNamespace(nodes=fake_nodes, DoesNotExist=DoesNotExist)
class LibraryApiDeleteTests(TestCase):
"""DELETE on the plain library endpoint cascades."""
def setUp(self):
self.user = User.objects.create_user(username="op", password="pw")
self.client = APIClient()
self.client.force_authenticate(user=self.user)
def _fake_models_module(self, lib):
return SimpleNamespace(Library=_fake_node_cls(lib))
def test_delete_uses_shared_cascade(self):
lib = SimpleNamespace(uid="lib-1", name="Docs")
cascade_result = {
"library_uid": "lib-1",
"name": "Docs",
"item_count": 3,
"item_s3_keys": [],
"orphans_deleted": 1,
}
with patch.dict(
"sys.modules", {"library.models": self._fake_models_module(lib)}
), patch(
"library.api.views.delete_library_cascade",
return_value=cascade_result,
) as mock_cascade:
response = self.client.delete("/library/api/libraries/lib-1/")
self.assertEqual(response.status_code, 204)
mock_cascade.assert_called_once_with(lib)
def test_delete_missing_library_returns_404(self):
with patch.dict(
"sys.modules", {"library.models": self._fake_models_module(None)}
), patch("library.api.views.delete_library_cascade") as mock_cascade:
response = self.client.delete("/library/api/libraries/nope/")
self.assertEqual(response.status_code, 404)
mock_cascade.assert_not_called()
class CollectionApiDeleteTests(TestCase):
"""DELETE on the collection endpoint cascades Items/Chunks/Images."""
def setUp(self):
self.user = User.objects.create_user(username="op", password="pw")
self.client = APIClient()
self.client.force_authenticate(user=self.user)
def test_delete_uses_shared_cascade(self):
col = SimpleNamespace(uid="col-1", name="Default")
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Collection=_fake_node_cls(col))},
), patch(
"library.api.views.delete_collection_cascade",
return_value={
"collection_uid": "col-1",
"name": "Default",
"item_count": 2,
"item_s3_keys": [],
},
) as mock_cascade:
response = self.client.delete("/library/api/collections/col-1/")
self.assertEqual(response.status_code, 204)
mock_cascade.assert_called_once_with(col)
def test_delete_missing_collection_returns_404(self):
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Collection=_fake_node_cls(None))},
), patch("library.api.views.delete_collection_cascade") as mock_cascade:
response = self.client.delete("/library/api/collections/nope/")
self.assertEqual(response.status_code, 404)
mock_cascade.assert_not_called()
class ItemApiDeleteTests(TestCase):
"""DELETE on the item endpoint cascades Chunks/Images/embeddings."""
def setUp(self):
self.user = User.objects.create_user(username="op", password="pw")
self.client = APIClient()
self.client.force_authenticate(user=self.user)
def test_delete_uses_shared_cascade(self):
item = SimpleNamespace(uid="item-1", title="Doc")
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Item=_fake_node_cls(item))},
), patch(
"library.api.views.delete_item_cascade",
return_value={"item_uid": "item-1", "s3_key": ""},
) as mock_cascade:
response = self.client.delete("/library/api/items/item-1/")
self.assertEqual(response.status_code, 204)
mock_cascade.assert_called_once_with("item-1")
def test_delete_missing_item_returns_404(self):
with patch.dict(
"sys.modules",
{"library.models": SimpleNamespace(Item=_fake_node_cls(None))},
), patch("library.api.views.delete_item_cascade") as mock_cascade:
response = self.client.delete("/library/api/items/nope/")
self.assertEqual(response.status_code, 404)
mock_cascade.assert_not_called()

View File

@@ -70,8 +70,9 @@ class _FakeLibrary:
class DoesNotExist(Exception): class DoesNotExist(Exception):
pass pass
existing = None # what nodes.get(name=...) returns existing = None # what find_library_by_name_ci returns
instances = [] # constructor kwargs, in order instances = [] # constructor kwargs, in order
save_raises = None # exception instance save() should raise, if any
def __init__(self, **kwargs): def __init__(self, **kwargs):
type(self).instances.append(kwargs) type(self).instances.append(kwargs)
@@ -81,16 +82,20 @@ class _FakeLibrary:
self.created_at = None self.created_at = None
def save(self): def save(self):
if type(self).save_raises is not None:
raise type(self).save_raises
return self return self
class _Nodes:
@staticmethod def _fake_find_ci(name):
def get(**kwargs): _FakeLibrary.ci_queries.append(name)
if _FakeLibrary.existing is None:
raise _FakeLibrary.DoesNotExist()
return _FakeLibrary.existing return _FakeLibrary.existing
nodes = _Nodes()
def _fake_models_module():
return SimpleNamespace(
Library=_FakeLibrary, find_library_by_name_ci=_fake_find_ci
)
class LibraryCreateStampingTests(TestCase): class LibraryCreateStampingTests(TestCase):
@@ -101,16 +106,15 @@ class LibraryCreateStampingTests(TestCase):
self.client = APIClient() self.client = APIClient()
_FakeLibrary.existing = None _FakeLibrary.existing = None
_FakeLibrary.instances = [] _FakeLibrary.instances = []
_FakeLibrary.save_raises = None
_FakeLibrary.ci_queries = []
def _post(self, token=None): def _post(self, token=None, name="Docs"):
self.client.force_authenticate(user=self.user, token=token) self.client.force_authenticate(user=self.user, token=token)
with patch.dict( with patch.dict("sys.modules", {"library.models": _fake_models_module()}):
"sys.modules",
{"library.models": SimpleNamespace(Library=_FakeLibrary)},
):
return self.client.post( return self.client.post(
"/library/api/libraries/", "/library/api/libraries/",
{"name": "Docs", "library_type": "technical"}, {"name": name, "library_type": "technical"},
format="json", format="json",
) )
@@ -129,7 +133,7 @@ class LibraryCreateStampingTests(TestCase):
def test_duplicate_name_returns_409_name_conflict(self): def test_duplicate_name_returns_409_name_conflict(self):
_FakeLibrary.existing = SimpleNamespace( _FakeLibrary.existing = SimpleNamespace(
uid="lib-old", managed_by_display="Daedalus" uid="lib-old", name="Docs", managed_by_display="Daedalus"
) )
response = self._post(token=UserToken(name="Spelunker")) response = self._post(token=UserToken(name="Spelunker"))
@@ -141,15 +145,39 @@ class LibraryCreateStampingTests(TestCase):
self.assertEqual(body["managed_by"], "Daedalus") self.assertEqual(body["managed_by"], "Daedalus")
self.assertEqual(_FakeLibrary.instances, []) self.assertEqual(_FakeLibrary.instances, [])
def test_duplicate_check_is_case_insensitive(self):
"""A case-variant name 409s and reports the existing spelling."""
_FakeLibrary.existing = SimpleNamespace(
uid="lib-old", name="Amazon Connect", managed_by_display="Spelunker"
)
response = self._post(token=None, name="amazon connect")
self.assertEqual(response.status_code, 409)
# The lookup received the posted name (case-folding happens in
# Cypher), and the response names the existing spelling.
self.assertEqual(_FakeLibrary.ci_queries, ["amazon connect"])
self.assertIn("Amazon Connect", response.json()["detail"])
self.assertEqual(_FakeLibrary.instances, [])
def test_duplicate_of_unmanaged_reports_null_manager(self): def test_duplicate_of_unmanaged_reports_null_manager(self):
_FakeLibrary.existing = SimpleNamespace( _FakeLibrary.existing = SimpleNamespace(
uid="lib-old", managed_by_display="" uid="lib-old", name="Docs", managed_by_display=""
) )
response = self._post(token=None) response = self._post(token=None)
self.assertEqual(response.status_code, 409) self.assertEqual(response.status_code, 409)
self.assertIsNone(response.json()["managed_by"]) self.assertIsNone(response.json()["managed_by"])
def test_save_race_returns_409_not_500(self):
"""An exact-name twin landing between pre-check and save 409s."""
from neomodel.exceptions import UniqueProperty
_FakeLibrary.save_raises = UniqueProperty("name")
response = self._post(token=None)
self.assertEqual(response.status_code, 409)
self.assertEqual(response.json()["code"], "name_conflict")
class WorkspaceStatusSerializerManagedByTests(TestCase): class WorkspaceStatusSerializerManagedByTests(TestCase):
"""The workspace status payload carries ``managed_by`` (nullable).""" """The workspace status payload carries ``managed_by`` (nullable)."""

View File

@@ -67,6 +67,114 @@ class ReembedItemTaskTests(TestCase):
mock_pipeline.reprocess_item.assert_called_once() mock_pipeline.reprocess_item.assert_called_once()
@override_settings(CELERY_TASK_ALWAYS_EAGER=True)
class IngestFromDaedalusFailureClassificationTests(TestCase):
"""A deterministic parse failure is terminal; a transient error retries.
Neo4j and S3 are mocked at the task's boundaries so the test exercises the
exception-classification branch (parsers.UnsupportedFileTypeError vs. any
other Exception) without a live graph or object store.
"""
def setUp(self):
from library.tasks import ingest_from_daedalus
# Calling .run() bypasses Celery's request setup, but the task
# persists self.request.id into the NOT NULL celery_task_id column
# and reads self.request.retries — push a real request context.
ingest_from_daedalus.push_request(id="test-task-id", retries=0)
self.addCleanup(ingest_from_daedalus.pop_request)
def _make_job(self):
from library.models import IngestJob
return IngestJob.objects.create(
id="job_test_unsupported",
library_uid="lib-uid-123",
source="daedalus",
s3_key="incoming/bad.drawio",
file_type="vnd.jgraph.mxfile",
title="bad.drawio",
content_hash="abc123",
)
def _patched_boundaries(self, pipeline_side_effect):
"""Patch every boundary the task hits before the pipeline runs.
Returns a context-manager list; the pipeline's ``process_item`` is set
to raise ``pipeline_side_effect``.
"""
from library.services.parsers import UnsupportedFileTypeError # noqa: F401
patchers = [
patch("library.tasks.db"),
patch("library.models.Library"),
patch("library.models.Item"),
patch("library.services.source_s3.fetch_from_source", return_value=b"data"),
patch("library.services.source_s3.copy_into_mnemosyne"),
patch("library.tasks._resolve_or_create_default_collection"),
patch("library.services.pipeline.EmbeddingPipeline"),
]
mocks = [p.start() for p in patchers]
self.addCleanup(lambda: [p.stop() for p in patchers])
# db.cypher_query returns (rows, meta); no prior item to supersede.
mocks[0].cypher_query.return_value = ([], None)
# Library.nodes.get returns a stand-in library node.
mocks[1].nodes.get.return_value = MagicMock(uid="lib-uid-123")
# Item() instances carry a uid used for the S3 key.
item_instance = MagicMock(uid="item-uid-abc")
mocks[2].return_value = item_instance
# The pipeline raises the classification-relevant error.
pipeline_instance = MagicMock()
pipeline_instance.process_item.side_effect = pipeline_side_effect
mocks[6].return_value = pipeline_instance
return pipeline_instance
def test_unsupported_file_type_is_terminal_and_not_retried(self):
from library.models import IngestJob
from library.services.parsers import UnsupportedFileTypeError
from library.tasks import ingest_from_daedalus
job = self._make_job()
self._patched_boundaries(
UnsupportedFileTypeError("Unsupported file type 'vnd.jgraph.mxfile'.")
)
with patch.object(ingest_from_daedalus, "retry") as mock_retry:
result = ingest_from_daedalus.run(job.id)
# Never retried.
mock_retry.assert_not_called()
# Terminal failure with a machine-readable reason.
self.assertFalse(result["success"])
self.assertEqual(result["reason"], "unsupported_file_type")
job.refresh_from_db()
self.assertEqual(job.status, "failed")
self.assertEqual(job.retry_count, 0)
self.assertIsNotNone(job.completed_at)
self.assertIn("Unsupported file type", job.error)
def test_transient_error_takes_the_retry_path(self):
from library.tasks import ingest_from_daedalus
job = self._make_job()
self._patched_boundaries(ConnectionError("S3 hiccup"))
# self.retry raises Retry in real Celery; simulate that so the task
# doesn't fall through to the terminal branch.
from celery.exceptions import Retry
with patch.object(ingest_from_daedalus, "retry", side_effect=Retry()) as mock_retry:
with self.assertRaises(Retry):
ingest_from_daedalus.run(job.id)
mock_retry.assert_called_once()
job.refresh_from_db()
self.assertEqual(job.retry_count, 1)
class ResolveUserTests(TestCase): class ResolveUserTests(TestCase):
"""Tests for the _resolve_user helper.""" """Tests for the _resolve_user helper."""

View File

@@ -460,7 +460,11 @@ def collection_delete(request, uid):
if request.method == "POST": if request.method == "POST":
name = col.name name = col.name
col.delete() # Shared cascade so Items/Chunks/Images go too — a bare
# col.delete() would leak them.
from .services.library_delete import delete_collection_cascade
delete_collection_cascade(col)
messages.success(request, f'Collection "{name}" deleted.') messages.success(request, f'Collection "{name}" deleted.')
return redirect("library:library-list") return redirect("library:library-list")
return render( return render(
@@ -648,7 +652,11 @@ def item_delete(request, uid):
if request.method == "POST": if request.method == "POST":
title = item.title title = item.title
item.delete() # Shared cascade so Chunks/Images/embeddings go too — a bare
# item.delete() would leak them.
from .services.library_delete import delete_item_cascade
delete_item_cascade(item.uid)
messages.success(request, f'Item "{title}" deleted.') messages.success(request, f'Item "{title}" deleted.')
return redirect("library:library-list") return redirect("library:library-list")
return render(request, "library/item_confirm_delete.html", {"item": item}) return render(request, "library/item_confirm_delete.html", {"item": item})