feat(07-04): POST /classify re-queues Celery, removes synchronous classify (D-11)
- Replace synchronous await classifier.classify_document() with extract_and_classify.delay()
- Set doc.status='processing' and commit before dispatching Celery task
- Return {'document_id': str, 'status': 'processing'} instead of assigned topics
- Promote test_reclassify_requeues_celery from xfail stub to passing test
- Add test_reclassify_cross_user_returns_404 for IDOR security invariant (T-07-10)
This commit is contained in:
@@ -714,14 +714,17 @@ async def delete_document(
|
|||||||
async def classify_document(
|
async def classify_document(
|
||||||
request: Request,
|
request: Request,
|
||||||
doc_id: str,
|
doc_id: str,
|
||||||
body: dict = {},
|
|
||||||
session: AsyncSession = Depends(get_db),
|
session: AsyncSession = Depends(get_db),
|
||||||
current_user: User = Depends(get_regular_user),
|
current_user: User = Depends(get_regular_user),
|
||||||
):
|
):
|
||||||
"""Reclassify a document's topics on demand.
|
"""Re-queue a document for classification via Celery (D-11).
|
||||||
|
|
||||||
|
Sets doc.status='processing', commits, dispatches extract_and_classify.delay(),
|
||||||
|
and returns {'document_id': str, 'status': 'processing'}.
|
||||||
|
|
||||||
D-16: requires authenticated regular user. Asserts ownership — cross-user
|
D-16: requires authenticated regular user. Asserts ownership — cross-user
|
||||||
classify returns 404 (not 403) to avoid information leakage (T-03-11).
|
classify returns 404 (not 403) to avoid information leakage (T-03-11).
|
||||||
|
T-07-10: ownership enforced here; IDOR returns 404 per STATE.md policy.
|
||||||
"""
|
"""
|
||||||
request.state.current_user = current_user
|
request.state.current_user = current_user
|
||||||
try:
|
try:
|
||||||
@@ -733,13 +736,12 @@ async def classify_document(
|
|||||||
if doc is None or doc.user_id != current_user.id:
|
if doc is None or doc.user_id != current_user.id:
|
||||||
raise HTTPException(404, "Document not found")
|
raise HTTPException(404, "Document not found")
|
||||||
|
|
||||||
topic_names = body.get("topics") if body else None
|
doc.status = "processing"
|
||||||
try:
|
await session.commit()
|
||||||
topics = await classifier.classify_document(session, doc_id, topic_names)
|
|
||||||
except Exception as e:
|
|
||||||
raise HTTPException(500, f"Classification failed: {e}")
|
|
||||||
|
|
||||||
return {"topics": topics}
|
extract_and_classify.delay(str(doc.id))
|
||||||
|
|
||||||
|
return {"document_id": str(doc.id), "status": "processing"}
|
||||||
|
|
||||||
|
|
||||||
# ── Range header parsing helper ───────────────────────────────────────────────
|
# ── Range header parsing helper ───────────────────────────────────────────────
|
||||||
|
|||||||
@@ -926,10 +926,107 @@ async def test_delete_cloud_remove_only(async_client, auth_user, db_session):
|
|||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Phase 7 Wave 0 xfail stub — D-11 re-classify endpoint
|
# Phase 7 Plan 04 — D-11 re-classify endpoint (POST /{id}/classify → Celery)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.xfail(strict=False, reason="Wave 0 stub — promoted in Plan 07-04")
|
async def test_reclassify_requeues_celery(async_client, auth_user, db_session, monkeypatch):
|
||||||
async def test_reclassify_requeues_celery():
|
"""POST /api/documents/{id}/classify sets doc.status='processing', enqueues
|
||||||
pytest.xfail("not implemented yet — Plan 07-04")
|
Celery task via extract_and_classify.delay, and returns {'document_id': ...,
|
||||||
|
'status': 'processing'}.
|
||||||
|
|
||||||
|
D-11: reclassify endpoint triggers async processing instead of running
|
||||||
|
synchronously. The previous synchronous call to classifier.classify_document()
|
||||||
|
is replaced by extract_and_classify.delay(doc_id).
|
||||||
|
"""
|
||||||
|
import uuid as _uuid
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
from db.models import Document
|
||||||
|
|
||||||
|
# Patch Celery delay to avoid real broker
|
||||||
|
mock_delay = MagicMock()
|
||||||
|
monkeypatch.setattr("tasks.document_tasks.extract_and_classify.delay", mock_delay)
|
||||||
|
|
||||||
|
# Create a document owned by auth_user
|
||||||
|
doc_id = _uuid.uuid4()
|
||||||
|
doc = Document(
|
||||||
|
id=doc_id,
|
||||||
|
user_id=auth_user["user"].id,
|
||||||
|
filename="reclassify_test.txt",
|
||||||
|
content_type="text/plain",
|
||||||
|
size_bytes=100,
|
||||||
|
storage_backend="minio",
|
||||||
|
status="uploaded",
|
||||||
|
object_key=f"{auth_user['user'].id}/{doc_id}/{_uuid.uuid4()}.txt",
|
||||||
|
)
|
||||||
|
db_session.add(doc)
|
||||||
|
await db_session.commit()
|
||||||
|
|
||||||
|
resp = await async_client.post(
|
||||||
|
f"/api/documents/{doc_id}/classify",
|
||||||
|
headers=auth_user["headers"],
|
||||||
|
)
|
||||||
|
assert resp.status_code == 200, resp.text
|
||||||
|
data = resp.json()
|
||||||
|
assert data["status"] == "processing", f"Expected status=processing: {data}"
|
||||||
|
assert data["document_id"] == str(doc_id), f"Expected document_id={doc_id}: {data}"
|
||||||
|
|
||||||
|
# Celery task was dispatched once with the doc's string ID
|
||||||
|
mock_delay.assert_called_once_with(str(doc_id))
|
||||||
|
|
||||||
|
# DB row was updated to processing
|
||||||
|
await db_session.refresh(doc)
|
||||||
|
assert doc.status == "processing", f"Expected doc.status=processing, got {doc.status}"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_reclassify_cross_user_returns_404(async_client, auth_user, db_session):
|
||||||
|
"""POST /api/documents/{other_user_doc_id}/classify by a different user returns 404.
|
||||||
|
|
||||||
|
T-07-10 / D-16: cross-user reclassify attempt must return 404 (not 403) to
|
||||||
|
avoid information leakage. _get_owned_doc ownership check enforces this.
|
||||||
|
"""
|
||||||
|
import uuid as _uuid
|
||||||
|
from db.models import Document, User, Quota
|
||||||
|
from services.auth import hash_password, create_access_token
|
||||||
|
|
||||||
|
# Create a document owned by auth_user
|
||||||
|
doc_id = _uuid.uuid4()
|
||||||
|
doc = Document(
|
||||||
|
id=doc_id,
|
||||||
|
user_id=auth_user["user"].id,
|
||||||
|
filename="other_user_doc.txt",
|
||||||
|
content_type="text/plain",
|
||||||
|
size_bytes=100,
|
||||||
|
storage_backend="minio",
|
||||||
|
status="uploaded",
|
||||||
|
object_key=f"{auth_user['user'].id}/{doc_id}/{_uuid.uuid4()}.txt",
|
||||||
|
)
|
||||||
|
db_session.add(doc)
|
||||||
|
|
||||||
|
# Create User B
|
||||||
|
user_b_id = _uuid.uuid4()
|
||||||
|
user_b = User(
|
||||||
|
id=user_b_id,
|
||||||
|
handle=f"classify_idor_{user_b_id.hex[:8]}",
|
||||||
|
email=f"classify_idor_{user_b_id.hex[:8]}@example.com",
|
||||||
|
password_hash=hash_password("Testpassword123!"),
|
||||||
|
role="user",
|
||||||
|
is_active=True,
|
||||||
|
password_must_change=False,
|
||||||
|
)
|
||||||
|
quota_b = Quota(user_id=user_b_id, limit_bytes=104857600, used_bytes=0)
|
||||||
|
db_session.add(user_b)
|
||||||
|
db_session.add(quota_b)
|
||||||
|
await db_session.commit()
|
||||||
|
|
||||||
|
token_b = create_access_token(str(user_b_id), "user")
|
||||||
|
headers_b = {"Authorization": f"Bearer {token_b}"}
|
||||||
|
|
||||||
|
# User B attempts to reclassify User A's document — must get 404
|
||||||
|
resp = await async_client.post(
|
||||||
|
f"/api/documents/{doc_id}/classify",
|
||||||
|
headers=headers_b,
|
||||||
|
)
|
||||||
|
assert resp.status_code == 404, (
|
||||||
|
f"Expected 404 for cross-user reclassify, got {resp.status_code}: {resp.text}"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user