fdc32d431d
5 plans across 5 sequential waves covering: Alembic migration 0003 (null-user cleanup, NOT NULL constraint, quota reconciliation), presigned MinIO PUT upload flow with atomic quota enforcement, auth guards on all document/topic endpoints, flat-file settings retirement + per-user AI classification, and frontend quota bar with 3-step XHR upload progress. Verification passed across all 12 dimensions. All 8 phase requirements covered (STORE-03/04/05/06, SEC-04, DOC-03/04/05). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
298 lines
24 KiB
Markdown
298 lines
24 KiB
Markdown
---
|
|
phase: 03-document-migration-multi-user-isolation
|
|
plan: 01
|
|
type: execute
|
|
wave: 1
|
|
depends_on: []
|
|
files_modified:
|
|
- backend/tests/test_quota.py
|
|
- backend/tests/test_alembic.py
|
|
- backend/tests/test_classifier.py
|
|
- backend/tests/test_documents.py
|
|
- backend/tests/test_topics.py
|
|
- backend/tests/test_settings.py
|
|
- backend/tests/conftest.py
|
|
- backend/migrations/versions/0003_multi_user_isolation.py
|
|
autonomous: true
|
|
requirements:
|
|
- STORE-03
|
|
- STORE-04
|
|
- STORE-05
|
|
- STORE-06
|
|
- SEC-04
|
|
- DOC-03
|
|
- DOC-04
|
|
- DOC-05
|
|
|
|
must_haves:
|
|
truths:
|
|
- "All xfail(strict=False) test stubs for Phase 3 requirements exist and are collectable by pytest"
|
|
- "Alembic migration 0003 deletes every documents row where user_id IS NULL and removes the corresponding MinIO objects before any schema change"
|
|
- "After migration 0003 runs, documents.user_id is NOT NULL"
|
|
- "After migration 0003 runs, the topics table is empty (all rows deleted) and ix_topics_user_id exists"
|
|
- "After migration 0003 runs, every quotas.used_bytes equals SUM(documents.size_bytes) for that user (or 0 if no documents)"
|
|
- "auth_user and admin_user pytest fixtures issue JWTs that get_current_user accepts and that resolve to active users with Quota rows"
|
|
artifacts:
|
|
- path: "backend/tests/test_quota.py"
|
|
provides: "Wave 0 xfail stubs for STORE-03 / STORE-05 / STORE-06 / SC2"
|
|
contains: "test_concurrent_quota_race"
|
|
- path: "backend/tests/test_classifier.py"
|
|
provides: "Wave 0 xfail stubs for DOC-03 / DOC-05 per-user provider resolution"
|
|
contains: "test_per_user_provider"
|
|
- path: "backend/tests/conftest.py"
|
|
provides: "auth_user, admin_user, mock_minio_presigned, mock_minio_stat fixtures"
|
|
exports:
|
|
- "auth_user"
|
|
- "admin_user"
|
|
- "mock_minio_presigned"
|
|
- "mock_minio_stat"
|
|
- path: "backend/migrations/versions/0003_multi_user_isolation.py"
|
|
provides: "Null-user document cleanup + topic cleanup + NOT NULL constraint + quota reconciliation + ix_topics_user_id"
|
|
contains: "revision = \"0003\""
|
|
key_links:
|
|
- from: "backend/migrations/versions/0003_multi_user_isolation.py"
|
|
to: "MinIO (synchronous SDK)"
|
|
via: "from minio import Minio; client.remove_object(bucket, key) for each null-user object_key"
|
|
pattern: "remove_object"
|
|
- from: "backend/tests/conftest.py"
|
|
to: "backend/services/auth.py"
|
|
via: "create_access_token + User row + Quota row for fixture users"
|
|
pattern: "create_access_token"
|
|
---
|
|
|
|
<objective>
|
|
Lay the Wave 0 foundation for Phase 3. Two outputs:
|
|
|
|
1. Test scaffolds (xfail stubs) covering every requirement in the VALIDATION.md Per-Task Verification Map plus the shared fixtures (`auth_user`, `admin_user`, MinIO mocks) every later plan needs.
|
|
2. Alembic migration `0003_multi_user_isolation.py` that performs the null-user document cleanup (D-01/D-02), all-topics cleanup (D-10), `documents.user_id` NOT NULL constraint, quota reconciliation (D-03), and the `ix_topics_user_id` index.
|
|
|
|
Purpose: Wave 0 tests must exist before any later plan can run them. The migration is the precondition for adding `get_current_user` to `/api/documents/*` — once `documents.user_id` is NOT NULL, every Document has an owner and ownership assertions become meaningful.
|
|
Output: 6 test files (5 stubs + 1 fixture file), 1 Alembic migration file.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@/Users/nik/.claude/get-shit-done/workflows/execute-plan.md
|
|
@/Users/nik/.claude/get-shit-done/templates/summary.md
|
|
</execution_context>
|
|
|
|
<context>
|
|
@.planning/ROADMAP.md
|
|
@.planning/STATE.md
|
|
@.planning/phases/03-document-migration-multi-user-isolation/03-CONTEXT.md
|
|
@.planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md
|
|
@.planning/phases/03-document-migration-multi-user-isolation/03-PATTERNS.md
|
|
@.planning/phases/03-document-migration-multi-user-isolation/03-VALIDATION.md
|
|
@CLAUDE.md
|
|
|
|
@backend/migrations/versions/0002_add_backup_codes_and_password_must_change.py
|
|
@backend/db/models.py
|
|
@backend/tests/conftest.py
|
|
@backend/tests/test_alembic.py
|
|
@backend/services/auth.py
|
|
|
|
<interfaces>
|
|
<!-- Contracts the executor needs without re-reading the codebase. -->
|
|
|
|
From backend/db/models.py (Phase 1):
|
|
```python
|
|
class Document(Base):
|
|
__tablename__ = "documents"
|
|
id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), primary_key=True)
|
|
user_id: Mapped[Optional[uuid.UUID]] # nullable in Phase 1 (D-03) — Phase 3 will flip to NOT NULL
|
|
object_key: Mapped[str]
|
|
size_bytes: Mapped[int]
|
|
status: Mapped[str] # 'pending' | 'uploaded' | 'classified' | 'classification_failed'
|
|
created_at: Mapped[datetime]
|
|
|
|
class Quota(Base):
|
|
__tablename__ = "quotas"
|
|
user_id: Mapped[uuid.UUID] # PK; FK -> users.id
|
|
limit_bytes: Mapped[int] # default 104857600 (100 MB)
|
|
used_bytes: Mapped[int]
|
|
|
|
class Topic(Base):
|
|
__tablename__ = "topics"
|
|
id: Mapped[uuid.UUID]
|
|
user_id: Mapped[Optional[uuid.UUID]] # NULL = system topic, UUID = per-user
|
|
name: Mapped[str]
|
|
# UniqueConstraint("user_id", "name", name="uq_topics_user_name")
|
|
```
|
|
|
|
From backend/services/auth.py (Phase 2):
|
|
```python
|
|
def hash_password(password: str) -> str
|
|
def create_access_token(user_id: str, role: str) -> str
|
|
```
|
|
|
|
From backend/tests/conftest.py (existing):
|
|
```python
|
|
@pytest_asyncio.fixture
|
|
async def db_session() -> AsyncSession # in-memory SQLite
|
|
|
|
@pytest_asyncio.fixture
|
|
async def async_client(db_session) -> AsyncClient # ASGI httpx client with get_db override
|
|
```
|
|
|
|
Migration 0002 reference style (revision/down_revision header + numbered # ── 1. comments + op.execute("GRANT ...") + downgrade()).
|
|
</interfaces>
|
|
</context>
|
|
|
|
<threat_model>
|
|
## Trust Boundaries
|
|
|
|
| Boundary | Description |
|
|
|----------|-------------|
|
|
| migration runtime → MinIO | DDL transaction holds DB rows; MinIO deletes happen outside any DB transaction |
|
|
| test fixtures → backend code | Fixtures fabricate JWTs that hit every guarded endpoint — must not leak across tests |
|
|
|
|
## STRIDE Threat Register
|
|
|
|
| Threat ID | Category | Component | Disposition | Mitigation Plan |
|
|
|-----------|----------|-----------|-------------|-----------------|
|
|
| T-03-01 | Tampering | Alembic migration 0003 upgrade() | mitigate | Collect object_keys via SELECT before DELETE; wrap MinIO remove_object in try/except so MinIO partial failure cannot leave DB+MinIO in worse state than orphans (RESEARCH.md Finding 1 + Risk 3) |
|
|
| T-03-02 | Denial of Service | Alembic migration when MinIO unreachable | accept | Migration is run only by docuvault_migrate after docker-compose health checks per RESEARCH.md Risk 3; retry on next deploy. Document in upgrade() docstring. |
|
|
| T-03-03 | Information Disclosure | xfail test fixtures (auth_user JWT) | mitigate | Fixture-issued JWTs use short-lived test secret_key; fixtures clean up via async_client teardown; never log token values |
|
|
| T-03-SC | Tampering | pip installs | mitigate | No new package-manager installs in this plan — pytest, pytest-asyncio, alembic, sqlalchemy, minio already present in requirements.txt from Phase 1/2 |
|
|
</threat_model>
|
|
|
|
<tasks>
|
|
|
|
<task type="auto">
|
|
<name>Task 1: Create Wave 0 test scaffolds and shared fixtures</name>
|
|
<files>backend/tests/test_quota.py, backend/tests/test_alembic.py, backend/tests/test_classifier.py, backend/tests/test_documents.py, backend/tests/test_topics.py, backend/tests/test_settings.py, backend/tests/conftest.py</files>
|
|
<read_first>
|
|
- backend/tests/conftest.py — current async_client + db_session fixture definitions; must not break existing fixtures
|
|
- backend/tests/test_alembic.py — existing xfail style for migration tests (revision 0001/0002)
|
|
- backend/tests/test_documents.py — existing async test shape used by later plans
|
|
- backend/tests/test_topics.py — existing test shape
|
|
- backend/tests/test_settings.py — existing tests (will become obsolete after D-12, but kept here for the stub `test_settings_endpoint_removed`)
|
|
- backend/tests/test_classifier.py — existing mock-provider tests; new tests live in same file
|
|
- backend/services/auth.py — `hash_password`, `create_access_token` signatures used by new fixtures
|
|
- backend/db/models.py — `User`, `Quota`, `Document`, `Topic` columns referenced by fixtures
|
|
- .planning/phases/03-document-migration-multi-user-isolation/03-VALIDATION.md — Per-Task Verification Map names every test stub required
|
|
- .planning/phases/03-document-migration-multi-user-isolation/03-CONTEXT.md — D-01..D-17 referenced by stub docstrings
|
|
</read_first>
|
|
<action>
|
|
Add new pytest fixtures to `backend/tests/conftest.py` (append, do not rewrite existing fixtures). Each fixture is `@pytest_asyncio.fixture`:
|
|
|
|
1. `auth_user(db_session)` — creates a User row (role="user", is_active=True, password_must_change=False, password_hash via `services.auth.hash_password("Testpassword123!")`), creates a Quota row (limit_bytes=104857600, used_bytes=0), issues an access token via `services.auth.create_access_token(str(user.id), "user")`, returns dict `{"user": user, "token": token, "headers": {"Authorization": f"Bearer {token}"}}`. Persist with `db_session.commit()`.
|
|
2. `admin_user(db_session)` — same as above but `role="admin"` and token created with `"admin"`. Returns identical dict shape.
|
|
3. `mock_minio_presigned(monkeypatch)` — uses `monkeypatch.setattr` to replace `storage.minio_backend.MinIOBackend.generate_presigned_put_url` (method-level patch with `AsyncMock`) returning the literal string `"http://localhost:9000/docuvault/test-presigned-url"`. The fixture yields the AsyncMock so tests can assert call counts/args.
|
|
4. `mock_minio_stat(monkeypatch)` — same pattern for `MinIOBackend.stat_object`, AsyncMock returning a configurable integer (default 1024). Yields the mock for per-test customization (e.g. `mock_minio_stat.return_value = 50_000_000`).
|
|
|
|
The patched methods do not exist yet (added in Plan 03-02) — using `monkeypatch.setattr` with `raising=False` ensures the patch installs before the attribute exists. Document this with an inline comment referencing Plan 03-02.
|
|
|
|
Create the following xfail test stub files. Every test is marked `@pytest.mark.xfail(strict=False, reason="implemented in plan 03-XX")` and asserts a single placeholder (`assert True` or `pytest.skip("scaffold")`). Stub file headers explain which requirement(s) they cover and which plan implements them.
|
|
|
|
`backend/tests/test_quota.py` (NEW) — async tests under Plan 03-02:
|
|
- `test_quota_increment_atomic(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-03; after one /confirm with stat=50_000_000, GET /api/auth/me/quota returns used_bytes == 50_000_000
|
|
- `test_concurrent_quota_race(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-03 SC2; calls asyncio.gather on two /confirm POSTs targeting two documents totaling 110 MB against a 100 MB quota; asserts statuses sorted == [200, 413]
|
|
- `test_quota_exceeded_response(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-05; /confirm returns 413 and body shape `{"detail": {"used_bytes", "limit_bytes", "rejected_bytes"}}`
|
|
- `test_delete_decrements_quota(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — STORE-06; upload + confirm then DELETE; GET /api/auth/me/quota returns used_bytes == 0
|
|
|
|
`backend/tests/test_alembic.py` (APPEND to existing) — new sync test `test_migration_0003`:
|
|
- Wraps Phase 1 fixture setup; runs `alembic upgrade head` against tmp SQLite; pre-seeds a `documents` row with `user_id=None` AND a row with `user_id=<some uuid>` via raw INSERT before running upgrade head past 0002, then runs `alembic upgrade head` to apply 0003 and asserts: the null-user row is gone, the populated-user row remains, PRAGMA table_info shows `documents.user_id` notnull=1, all `topics` rows are gone, `ix_topics_user_id` is in `sqlite_master`, and `quotas.used_bytes` for the populated user equals SUM(size_bytes). Mark `@pytest.mark.xfail(strict=False, reason="implemented in plan 03-01 migration step")`.
|
|
|
|
`backend/tests/test_classifier.py` (APPEND to existing) — new async tests under Plan 03-04:
|
|
- `test_per_user_provider(db_session)` — DOC-03; with `user.ai_provider="openai"`, `user.ai_model="gpt-4o"`, the classifier.classify_document call resolves `_settings["active_provider"] == "openai"` (use AsyncMock for `ai.get_provider` and assert call args)
|
|
- `test_celery_task_uses_user_provider(db_session)` — DOC-05; calling `_run(document_id)` with a Document owned by `user.ai_provider="anthropic"` calls classifier with `ai_provider="anthropic"`
|
|
- `test_default_provider_fallback(db_session)` — D-15; when `user.ai_provider is None`, classifier receives `config.settings.default_ai_provider`
|
|
|
|
`backend/tests/test_documents.py` (APPEND to existing) — new async tests under Plans 03-02 / 03-03:
|
|
- `test_upload_url_endpoint(async_client, auth_user, mock_minio_presigned)` — D-05; POST /api/documents/upload-url returns `{"upload_url": "...", "document_id": "..."}` and creates a Document row with status="pending"
|
|
- `test_confirm_endpoint(async_client, auth_user, mock_minio_presigned, mock_minio_stat)` — D-05; POST /api/documents/{id}/confirm calls stat_object once, updates Document.size_bytes from stat return, sets status="uploaded"
|
|
- `test_get_quota(async_client, auth_user)` — STORE-04; GET /api/auth/me/quota returns `{"used_bytes": 0, "limit_bytes": 104857600}`
|
|
- `test_cross_user_access_404(async_client, auth_user, db_session)` — SEC-04; user A creates a Document, user B's request for `GET /api/documents/{A_doc_id}` returns 404
|
|
- `test_admin_cannot_access_documents(async_client, admin_user)` — SEC-04 SC4; GET /api/documents using admin_user.headers returns 403
|
|
- `test_documents_require_auth(async_client)` — anonymous GET /api/documents returns 401 or 403
|
|
|
|
`backend/tests/test_topics.py` (APPEND to existing) — new async tests under Plan 03-03:
|
|
- `test_topic_namespace(async_client, auth_user, db_session)` — DOC-04; seed one system topic (user_id=NULL), one auth_user-owned topic, one different-user topic; GET /api/topics returns only the first two
|
|
- `test_admin_create_system_topic(async_client, admin_user)` — D-09; POST /api/admin/topics returns 201 and creates a Topic with user_id=NULL
|
|
- `test_regular_user_cannot_create_system_topic(async_client, auth_user)` — D-09; POST /api/admin/topics with auth_user.headers returns 403
|
|
- `test_topics_require_auth(async_client)` — D-17; anonymous GET /api/topics returns 401 or 403
|
|
|
|
`backend/tests/test_settings.py` (APPEND new test, leave existing) — under Plan 03-04:
|
|
- `test_settings_endpoint_removed(async_client)` — D-12; GET /api/settings returns 404
|
|
|
|
Every NEW test must include `@pytest.mark.xfail(strict=False, reason="implemented in plan 03-XX")` with the XX matching its target plan. None of the new tests should pass today (xfail strict=False keeps unexpected passes from breaking CI per Phase 1 D-Wave0).
|
|
</action>
|
|
<verify>
|
|
<automated>cd backend && pytest tests/test_quota.py tests/test_alembic.py::test_migration_0003 tests/test_classifier.py::test_per_user_provider tests/test_classifier.py::test_celery_task_uses_user_provider tests/test_classifier.py::test_default_provider_fallback tests/test_documents.py::test_upload_url_endpoint tests/test_documents.py::test_confirm_endpoint tests/test_documents.py::test_get_quota tests/test_documents.py::test_cross_user_access_404 tests/test_documents.py::test_admin_cannot_access_documents tests/test_documents.py::test_documents_require_auth tests/test_topics.py::test_topic_namespace tests/test_topics.py::test_admin_create_system_topic tests/test_topics.py::test_regular_user_cannot_create_system_topic tests/test_topics.py::test_topics_require_auth tests/test_settings.py::test_settings_endpoint_removed --collect-only -q</automated>
|
|
</verify>
|
|
<done>
|
|
All 16 new test IDs are collected by pytest with no collection errors. `grep -c "xfail" backend/tests/test_quota.py` returns >= 4. `grep -c "xfail" backend/tests/test_documents.py` >= 6. `grep -c "xfail" backend/tests/test_topics.py` >= 4. `grep -c "auth_user" backend/tests/conftest.py` returns >= 1.
|
|
</done>
|
|
</task>
|
|
|
|
<task type="auto" tdd="true">
|
|
<name>Task 2: Write Alembic migration 0003 (null-user cleanup + NOT NULL + topic cleanup + quota reconcile + index)</name>
|
|
<files>backend/migrations/versions/0003_multi_user_isolation.py</files>
|
|
<read_first>
|
|
- backend/migrations/versions/0002_add_backup_codes_and_password_must_change.py — header style, numbered comments, GRANT pattern, downgrade structure
|
|
- backend/migrations/versions/0001_initial_schema.py — column types, table names, baseline schema
|
|
- backend/db/models.py — Document, Quota, Topic columns + Index/UniqueConstraint definitions
|
|
- backend/config.py — `settings.minio_endpoint`, `settings.minio_access_key`, `settings.minio_secret_key`, `settings.minio_bucket` — but DO NOT import settings inside the migration (env vars only, see action)
|
|
- .planning/phases/03-document-migration-multi-user-isolation/03-RESEARCH.md — Finding 1 (sync MinIO SDK inside upgrade), Finding 4 (quota reconciliation SQL), Risk 3 (idempotency, MinIO unreachability)
|
|
- backend/tests/test_alembic.py — `test_migration_0003` (xfail stub from Task 1) defines the expected post-state
|
|
</read_first>
|
|
<behavior>
|
|
- Migration revision = "0003", down_revision = "0002", branch_labels = None, depends_on = None
|
|
- upgrade() must, in order:
|
|
1. SELECT id, object_key FROM documents WHERE user_id IS NULL → list of (id, object_key)
|
|
2. DELETE FROM document_topics WHERE document_id IN (those ids) [cascade safety]
|
|
3. DELETE FROM documents WHERE user_id IS NULL
|
|
4. For each collected object_key: call synchronous `minio.Minio(...).remove_object(bucket, key)` wrapped in try/except (log via op-style `op.execute("-- comment")` skipped; use `print()` to stderr or silent pass per RESEARCH.md Finding 1)
|
|
5. DELETE FROM topics (all rows, per D-10)
|
|
6. op.alter_column("documents", "user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=False)
|
|
7. op.create_index("ix_topics_user_id", "topics", ["user_id"])
|
|
8. op.execute(text("UPDATE quotas SET used_bytes = (SELECT COALESCE(SUM(size_bytes), 0) FROM documents WHERE documents.user_id = quotas.user_id)"))
|
|
- downgrade() must:
|
|
1. op.drop_index("ix_topics_user_id", table_name="topics")
|
|
2. op.alter_column("documents", "user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=True)
|
|
3. Docstring states that deleted rows + MinIO objects are NOT restorable (one-way data cleanup per D-01)
|
|
- MinIO connection details read from os.environ ONLY (MINIO_ENDPOINT, MINIO_ACCESS_KEY, MINIO_SECRET_KEY, MINIO_BUCKET) — never `from config import settings` (circular import + Alembic side effects)
|
|
- test_migration_0003 (Task 1) must transition from xfail → pass once this task lands (remove or keep the xfail marker; if strict=False it XPASSes silently)
|
|
</behavior>
|
|
<action>
|
|
Create `backend/migrations/versions/0003_multi_user_isolation.py` following the exact header style of `0002_add_backup_codes_and_password_must_change.py`:
|
|
|
|
Header: revision = "0003", down_revision = "0002", branch_labels = None, depends_on = None. Docstring lists the 8 numbered steps and references D-01, D-02, D-03, D-10. Imports: `import os; from __future__ import annotations; import sqlalchemy as sa; from sqlalchemy import text; from sqlalchemy.dialects import postgresql; from alembic import op`. Conditionally `from minio import Minio` inside upgrade() (deferred import — minio not needed in downgrade or for test envs that skip MinIO).
|
|
|
|
upgrade() body uses numbered section comments matching the 0002 style (`# ── 1. Collect null-user object keys ─────`). Section 1 uses `bind = op.get_bind(); result = bind.execute(text("SELECT id, object_key FROM documents WHERE user_id IS NULL"))` and materializes `null_user_objects = [(row[0], row[1]) for row in result]`. Section 2 uses `op.execute(text("DELETE FROM document_topics WHERE document_id IN (SELECT id FROM documents WHERE user_id IS NULL)"))`. Section 3 uses `op.execute(text("DELETE FROM documents WHERE user_id IS NULL"))`. Section 4 instantiates `Minio(os.environ.get("MINIO_ENDPOINT", "minio:9000"), access_key=os.environ.get("MINIO_ACCESS_KEY", ""), secret_key=os.environ.get("MINIO_SECRET_KEY", ""), secure=False)` and loops over `null_user_objects`, calling `client.remove_object(bucket, key)` inside `try/except Exception: pass` (skip silently — orphans are harmless per Finding 1). Bucket name from `os.environ.get("MINIO_BUCKET", "docuvault")`. Section 5 uses `op.execute(text("DELETE FROM topics"))`. Section 6 uses `op.alter_column("documents", "user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=False)`. Section 7 uses `op.create_index("ix_topics_user_id", "topics", ["user_id"])`. Section 8 uses the quota reconciliation `UPDATE quotas SET used_bytes = (SELECT COALESCE(SUM(size_bytes), 0) FROM documents WHERE documents.user_id = quotas.user_id)` via `op.execute(text(...))`.
|
|
|
|
SQLite test compatibility (Task 1's `test_migration_0003` runs against SQLite): wrap the MinIO step in `if os.environ.get("MINIO_ENDPOINT"): ...` so the test env without MinIO does not import minio at migration time. Also wrap `op.alter_column` with `with op.batch_alter_table("documents") as batch_op: batch_op.alter_column("user_id", existing_type=sa.dialects.postgresql.UUID(as_uuid=True), nullable=False)` — batch mode is required for SQLite ALTER COLUMN, and is a no-op pass-through on PostgreSQL. Section 7's `op.create_index` works on both dialects without batch mode.
|
|
|
|
downgrade() body: section 1 `op.drop_index("ix_topics_user_id", table_name="topics")`; section 2 `with op.batch_alter_table("documents") as batch_op: batch_op.alter_column("user_id", existing_type=postgresql.UUID(as_uuid=True), nullable=True)`. Docstring on downgrade() states "WARNING: deleted null-user document rows and their MinIO objects are NOT restored — Phase 3 cleanup is one-way per CONTEXT.md D-01." NO GRANT statements are needed (no new tables per RESEARCH.md Finding 1).
|
|
|
|
Do NOT introduce `pytest.mark.xfail` removal in `test_migration_0003`; leave xfail(strict=False) so it XPASSes silently (consistent with Wave 0 D-Wave0 from Phase 1).
|
|
</action>
|
|
<verify>
|
|
<automated>cd backend && pytest tests/test_alembic.py::test_migration_0003 -x -q 2>&1 | tail -20 && grep -c "DELETE FROM topics" backend/migrations/versions/0003_multi_user_isolation.py && grep -c "ix_topics_user_id" backend/migrations/versions/0003_multi_user_isolation.py && grep -c "SUM(size_bytes)" backend/migrations/versions/0003_multi_user_isolation.py && grep -c "nullable=False" backend/migrations/versions/0003_multi_user_isolation.py</automated>
|
|
</verify>
|
|
<done>
|
|
`test_migration_0003` either passes or xpasses (no FAILED). The migration file contains: `revision = "0003"`, `down_revision = "0002"`, `DELETE FROM topics`, `ix_topics_user_id`, `SUM(size_bytes)`, `nullable=False`, and a `Minio` import inside upgrade(). `grep -c "revision = \"0003\"" backend/migrations/versions/0003_multi_user_isolation.py` returns 1.
|
|
</done>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<verification>
|
|
- All 16 new test IDs collect cleanly: `cd backend && pytest tests/ --collect-only -q 2>&1 | grep -E "(test_quota|test_migration_0003|test_per_user_provider|test_upload_url|test_confirm|test_get_quota|test_cross_user|test_admin_cannot|test_topic_namespace|test_settings_endpoint_removed)" | wc -l` >= 16
|
|
- Migration applies cleanly against a pre-seeded SQLite DB (covered by `test_migration_0003`)
|
|
- Existing tests still pass: `cd backend && pytest tests/test_documents.py tests/test_topics.py tests/test_settings.py -x -q --ignore-glob="*test_quota*"` reports no regressions in previously-green tests
|
|
</verification>
|
|
|
|
<success_criteria>
|
|
- 6 test files exist with the 16 new test IDs from VALIDATION.md
|
|
- `backend/tests/conftest.py` exports `auth_user`, `admin_user`, `mock_minio_presigned`, `mock_minio_stat`
|
|
- `backend/migrations/versions/0003_multi_user_isolation.py` exists with revision "0003", down_revision "0002"
|
|
- Migration upgrade() deletes null-user documents (DB + MinIO), deletes all topics, alters documents.user_id to NOT NULL, creates ix_topics_user_id, reconciles quotas.used_bytes
|
|
- Migration downgrade() reverses schema-level changes only (documented in docstring)
|
|
- All Wave 0 stubs are xfail(strict=False) — none of them pass/fail unexpectedly
|
|
</success_criteria>
|
|
|
|
<output>
|
|
Create `.planning/phases/03-document-migration-multi-user-isolation/03-01-SUMMARY.md` when done — list every new test ID and the migration revision identifier; flag the SQLite-vs-PostgreSQL alter_column batch-mode choice as a key downstream constraint.
|
|
</output>
|