Archived
test: verify search engine API acceptance
This commit is contained in:
1 parent
56805a97cf
commit
77fa48c7a2
21 files changed
+512
-109
No files matched your search
@@ -7,7 +7,7 @@ How `recoll-modern` is built today. Update in the same commit as any structural
|
||||
The Python service scaffold, SQLite schema, collection and membership repositories, document
|
||||
discovery/identity modules, durable job worker, engine adapter, authentication service, and initial
|
||||
HTTP shell, membership routes, job routes, scoped search, document retrieval, and Docker packaging are
|
||||
built. Final contract, recovery and security acceptance coverage remains. Recoll behaviour
|
||||
built and covered by host and container acceptance tests. Recoll behaviour
|
||||
below was verified against a real container.
|
||||
|
||||
The goal: a self-hosted service that wraps Recoll's indexing and full-text search behind (a) a modern
|
||||
@@ -36,6 +36,7 @@ web UI and (b) an MCP server, running in Docker, with several independent collec
|
||||
| `src/recoll_modern/api/` | HTTP auth, health, file browsing, collection/membership/rule/job resources, search, document retrieval, prefix/proxy handling and JSON errors |
|
||||
| `src/recoll_modern/domain/search.py` | Validates explicit scope and fields, projects current SQLite identities and membership onto engine-ranked hits |
|
||||
| `tests/` | Automated service tests |
|
||||
| `tests/contract/`, `tests/integration/`, `tests/security/` | Requirement matrix, real-container Recoll contract, lifecycle, and boundary checks |
|
||||
|
||||
Discovery produces content hashes and root-relative paths without writing to the document volume.
|
||||
The document repository records these in one transaction, chooses the smallest live path as canonical,
|
||||
@@ -123,6 +124,12 @@ absent; Recoll may generate additional support files there. Compose mounts docum
|
||||
keeps the SQLite/config and Xapian index volumes separate. Initial health can be 503 until an indexing
|
||||
job creates the first index.
|
||||
|
||||
The host Recoll contract tests skip when no container is running. Required-container mode executes the
|
||||
adapter inside Compose, avoiding a host Recoll dependency; the generated-corpus test proves that a
|
||||
read-only file can be indexed, searched in two collections, and retrieved as extracted text. The HTTP
|
||||
lifecycle test uses a deterministic engine to exercise rules, identity, projection, canonical-path
|
||||
promotion and interrupted-job recovery.
|
||||
|
||||
The resource groups are auth, health, read-only file browsing, collections and membership sources,
|
||||
directory rules, scoped search, document text/original retrieval, and indexing jobs. Search callers
|
||||
select returned fields; only `document_id` is unconditional, and inline full text is bounded. Every
|
||||
|
||||
@@ -316,3 +316,15 @@ replacement); run the service as root.
|
||||
**Consequences:** service-state backups include generated Recoll configuration, and config changes made
|
||||
by an operator persist across image upgrades. Template updates require an explicit migration if an
|
||||
existing config should adopt them.
|
||||
|
||||
---
|
||||
|
||||
## 2026-09-26 — Treat an empty Recoll document URL as a missing hit
|
||||
**Status:** accepted
|
||||
**Context:** the live Recoll 1.43 Python binding returns a `Doc` object with an empty `getbinurl()` for
|
||||
a UDI absent from the index; it does not return `None`. Passing that object to `rclextract.Extractor`
|
||||
raises an internal-file error.
|
||||
**Decision:** check the returned URL before extraction and return empty text for an absent UDI.
|
||||
**Alternatives:** catch the generic extraction error, which would hide genuine extraction failures.
|
||||
**Consequences:** exact-path text lookup follows the engine contract for missing documents while real
|
||||
extraction failures still surface as `engine_unavailable`.
|
||||
+5
-6
@@ -15,14 +15,13 @@ Where the workflow stands (`AGENTS.md` -> Workflow: brainstorm -> spec -> plan -
|
||||
selectable search fields, subpath hosting and the packaging seam for deferred desktop spec 005.
|
||||
- Plan `docs/plans/001-search-engine-api.md`: **approved 2026-09-18.** Its listed Python runtime and
|
||||
development dependencies are approved. Execution follows the plan task by task.
|
||||
- Plan Tasks 1–3 are complete and independently reviewed. Tasks 4–13 add discovery, content-hash
|
||||
- Plan Tasks 1–3 are complete and independently reviewed. Tasks 4–14 add discovery, content-hash
|
||||
identity, additive membership, durable jobs, the atomic membership map, the engine boundary,
|
||||
session authentication, the HTTP shell, membership/rule/job routes, scoped search and document
|
||||
retrieval and Docker packaging. Their checkboxes are current in the plan. The next task is final
|
||||
contract, recovery, security and acceptance coverage.
|
||||
retrieval, Docker packaging and acceptance coverage. Their checkboxes are current in the plan.
|
||||
Final code review and branch handoff remain.
|
||||
|
||||
The next action is Task 14, "Complete contract, recovery, security, and acceptance coverage." Do not
|
||||
repeat Tasks 1–13.
|
||||
The next action is Task 14 Step 6, "Request review before integration." Do not repeat Tasks 1–14.
|
||||
|
||||
Things a resuming agent will otherwise get wrong:
|
||||
|
||||
@@ -56,7 +55,7 @@ Things a resuming agent will otherwise get wrong:
|
||||
once per file per index run.
|
||||
- [ ] Re-measure indexing overhead against real documents (PDF, EPUB) rather than small text files —
|
||||
extraction cost should dwarf the helper, but that is an assumption until measured.
|
||||
- [ ] Promote a new canonical path and force a reindex when a canonical path disappears, or the
|
||||
- [x] Promote a new canonical path and force a reindex when a canonical path disappears, or the
|
||||
document silently leaves every collection it is in.
|
||||
- [x] Recover interrupted indexing jobs at startup; Task 6 requeues valid running jobs and fails
|
||||
unreadable requests.
|
||||
|
||||
@@ -980,11 +980,11 @@ git commit -m "feat: package Recoll service"
|
||||
- Consumes: all prior tasks
|
||||
- Produces: executable evidence for every spec acceptance criterion
|
||||
|
||||
- [ ] **Step 1: Add a requirement-to-test matrix**
|
||||
- [x] **Step 1: Add a requirement-to-test matrix**
|
||||
|
||||
At the top of `test_http_contract.py`, map `R1` through `R31` and `C1` through `C4` to named tests. Write missing tests before changing implementation. The end-to-end flow must: login under `/recoll`; create two collections; add explicit, snapshot, and persistent sources; exclude `.calibre`; index duplicate/same-name fixtures; search one and several collections; request projected fields and bounded text; fetch full text and a byte range; reconcile a moved/deleted canonical path; restart during a running job; and verify recovery.
|
||||
|
||||
- [ ] **Step 2: Prove each new test fails for its intended reason**
|
||||
- [x] **Step 2: Prove each new test fails for its intended reason**
|
||||
|
||||
Run each new node individually, for example:
|
||||
|
||||
@@ -995,11 +995,11 @@ uv run pytest tests/security/test_proxy_headers.py::test_direct_client_cannot_fo
|
||||
|
||||
Expected: each new test fails on the missing/incorrect behavior, not fixture setup.
|
||||
|
||||
- [ ] **Step 3: Apply minimal fixes and rerun focused tests**
|
||||
- [x] **Step 3: Apply minimal fixes and rerun focused tests**
|
||||
|
||||
For every failure, change only the responsible module, rerun the single red test to green, then its containing test file. Append deferred discoveries to `docs/TODO.md`; do not leave source TODO comments.
|
||||
|
||||
- [ ] **Step 4: Run the complete verification matrix**
|
||||
- [x] **Step 4: Run the complete verification matrix**
|
||||
|
||||
Run in this order:
|
||||
|
||||
@@ -1020,7 +1020,7 @@ git status --short
|
||||
|
||||
Expected: every command exits 0; host suite reports explicit skips only for Recoll-dependent tests when the container is down; required-container suite has no such skips; Git status contains only the intended final documentation/checkbox changes before the commit.
|
||||
|
||||
- [ ] **Step 5: Complete docs and commit**
|
||||
- [x] **Step 5: Complete docs and commit**
|
||||
|
||||
Check every acceptance box in the spec only when its named test passed in this session. Update the
|
||||
architecture map/data flow, append decisions that an implementer actually made, remove completed TODO
|
||||
|
||||
@@ -284,36 +284,36 @@ exists. Unexpected errors return a request ID rather than exception text or file
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] A collection can be created, renamed and deleted over the API; renaming enqueues no job (R1–R4).
|
||||
- [ ] Deleting without a matching confirmation value is refused, and no index is deleted or rebuilt (R4).
|
||||
- [ ] Two files in the same directory can belong to different collections, and each is found only by its
|
||||
- [x] A collection can be created, renamed and deleted over the API; renaming enqueues no job (R1–R4).
|
||||
- [x] Deleting without a matching confirmation value is refused, and no index is deleted or rebuilt (R4).
|
||||
- [x] Two files in the same directory can belong to different collections, and each is found only by its
|
||||
own collection's scope (C2, R6).
|
||||
- [ ] Two byte-identical files under different names are one document and appear once in a scoped
|
||||
- [x] Two byte-identical files under different names are one document and appear once in a scoped
|
||||
search (C3, R5, R7).
|
||||
- [ ] Two files sharing a name with different content are two documents and both are findable (C3).
|
||||
- [ ] Removing a canonical path promotes another path and the document remains findable (R8).
|
||||
- [ ] Snapshot directories add their current documents once; persistent directories reconcile at
|
||||
- [x] Two files sharing a name with different content are two documents and both are findable (C3).
|
||||
- [x] Removing a canonical path promotes another path and the document remains findable (R8).
|
||||
- [x] Snapshot directories add their current documents once; persistent directories reconcile at
|
||||
startup and on reindex, honoring scoped exact-directory exclusions (R10–R12).
|
||||
- [ ] A direct file addition remains a member despite a rule exclusion, and removing a rule never
|
||||
- [x] A direct file addition remains a member despite a rule exclusion, and removing a rule never
|
||||
removes membership contributed by another source (R11).
|
||||
- [ ] A strict job applies no membership changes after any discovery failure; the same best-effort job
|
||||
- [x] A strict job applies no membership changes after any discovery failure; the same best-effort job
|
||||
applies valid files and reports `completed_with_errors` (R18).
|
||||
- [ ] A search without `collections` is rejected with `scope_required` (C4, R19).
|
||||
- [ ] A search across two collections returns one ranked set, with each hit reporting its collections
|
||||
- [x] A search without `collections` is rejected with `scope_required` (C4, R19).
|
||||
- [x] A search across two collections returns one ranked set, with each hit reporting its collections
|
||||
(R20, R21).
|
||||
- [ ] Search field selection omits unrequested data; inline text is bounded and full authenticated text
|
||||
- [x] Search field selection omits unrequested data; inline text is bounded and full authenticated text
|
||||
and ranged original-file retrieval work (R21–R22).
|
||||
- [ ] A membership change is visible to search after its job completes, without the document being
|
||||
- [x] A membership change is visible to search after its job completes, without the document being
|
||||
written to (C1, R13, R14).
|
||||
- [ ] Indexing a document volume mounted `:ro` succeeds (C1).
|
||||
- [ ] A job interrupted by a restart is not left claiming to be in progress (R17).
|
||||
- [ ] An unauthenticated request to any endpoint except `/health` and `/auth/login` is refused (R24).
|
||||
- [ ] The service refuses to start with no user and no seeded credentials (R26).
|
||||
- [ ] Hosting under `/recoll` produces working routes, cookies, OpenAPI and document links, while
|
||||
- [x] Indexing a document volume mounted `:ro` succeeds (C1).
|
||||
- [x] A job interrupted by a restart is not left claiming to be in progress (R17).
|
||||
- [x] An unauthenticated request to any endpoint except `/health` and `/auth/login` is refused (R24).
|
||||
- [x] The service refuses to start with no user and no seeded credentials (R26).
|
||||
- [x] Hosting under `/recoll` produces working routes, cookies, OpenAPI and document links, while
|
||||
untrusted forwarded-prefix headers are ignored (R29).
|
||||
- [ ] The service uses non-default state paths and listen settings without code changes, contains no
|
||||
- [x] The service uses non-default state paths and listen settings without code changes, contains no
|
||||
runtime Docker invocation, and recovers durable work after a cleanly handled shutdown (R31).
|
||||
- [ ] `uv run pytest`, `uv run ruff check .` and `uv run mypy src` pass, and the suite skips rather than
|
||||
- [x] `uv run pytest`, `uv run ruff check .` and `uv run mypy src` pass, and the suite skips rather than
|
||||
fails with the container down.
|
||||
|
||||
## Known gaps
|
||||
|
||||
@@ -23,11 +23,11 @@ def _path(request: Request, document_hash: str) -> Path:
|
||||
if document is None:
|
||||
raise AppError("document_not_found", "Document was not found", 404)
|
||||
if document.canonical_path is None:
|
||||
raise AppError("document_missing", "Document is missing", 404)
|
||||
raise AppError("document_missing", "Document is missing", 410)
|
||||
root = request.app.state.path_policy.root
|
||||
path = (root / document.canonical_path).resolve()
|
||||
if not path.is_relative_to(root) or not path.is_file():
|
||||
raise AppError("document_missing", "Document is missing", 404)
|
||||
raise AppError("document_missing", "Document is missing", 410)
|
||||
return cast(Path, path)
|
||||
|
||||
|
||||
|
||||
@@ -16,6 +16,24 @@ ALLOWED_FIELDS = frozenset({
|
||||
DEFAULT_FIELDS = ALLOWED_FIELDS - {"text", "links"}
|
||||
|
||||
|
||||
def valid_query(query: str) -> bool:
|
||||
"""Keep user grouping entirely inside the engine's mandatory scope clause."""
|
||||
if not query.strip() or "\\" in query:
|
||||
return False
|
||||
depth = 0
|
||||
quoted = False
|
||||
for character in query:
|
||||
if character == '"':
|
||||
quoted = not quoted
|
||||
elif not quoted and character == "(":
|
||||
depth += 1
|
||||
elif not quoted and character == ")":
|
||||
depth -= 1
|
||||
if depth < 0:
|
||||
return False
|
||||
return depth == 0 and not quoted
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class SearchInput:
|
||||
query: str
|
||||
@@ -45,12 +63,12 @@ class SearchService:
|
||||
self._text_limit_max = text_limit_max
|
||||
|
||||
def search(self, search: SearchInput) -> SearchResult:
|
||||
if not search.query.strip():
|
||||
if not valid_query(search.query):
|
||||
raise AppError("invalid_query", "Query must not be empty", 400)
|
||||
if not search.collection_ids:
|
||||
raise AppError("scope_required", "At least one collection is required", 400)
|
||||
if search.fields is not None and set(search.fields) - ALLOWED_FIELDS:
|
||||
raise AppError("invalid_query", "Unknown search field", 400)
|
||||
raise AppError("invalid_field", "Unknown search field", 422)
|
||||
if not 1 <= search.limit <= 100 or search.offset < 0 or search.text_limit < 0:
|
||||
raise AppError("validation_failed", "Invalid search bounds", 422)
|
||||
fields = DEFAULT_FIELDS if search.fields is None else set(search.fields)
|
||||
@@ -61,6 +79,8 @@ class SearchService:
|
||||
))
|
||||
except EngineUnavailable:
|
||||
raise AppError("engine_unavailable", "Search engine is unavailable", 503) from None
|
||||
except ValueError:
|
||||
raise AppError("invalid_query", "Query is invalid", 400) from None
|
||||
results: list[dict[str, object]] = []
|
||||
for hit in page.hits:
|
||||
try:
|
||||
|
||||
@@ -8,6 +8,7 @@ from datetime import UTC, datetime
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from recoll_modern.domain.search import valid_query
|
||||
from recoll_modern.engine.protocol import (
|
||||
EngineHealth,
|
||||
EngineHit,
|
||||
@@ -46,7 +47,7 @@ class RecollEngine:
|
||||
return EngineHealth(True)
|
||||
|
||||
def search(self, request: SearchRequest) -> EnginePage:
|
||||
if not _safe_query(request.query):
|
||||
if not valid_query(request.query):
|
||||
raise ValueError("query syntax cannot escape collection scope")
|
||||
if not request.collection_ids or any(
|
||||
_COLLECTION_ID.fullmatch(collection_id) is None
|
||||
@@ -86,7 +87,7 @@ class RecollEngine:
|
||||
db = self._connect()
|
||||
try:
|
||||
doc = db.getDoc(fsudi.fs_udi(path))
|
||||
if doc is None:
|
||||
if doc is None or not doc.getbinurl():
|
||||
return ""
|
||||
extracted = rclextract.Extractor(doc).textextract(doc.ipath)
|
||||
return str(extracted.text)
|
||||
@@ -143,21 +144,3 @@ def _hit(doc: Any, query: Any, fetch_text: bool, rank: int) -> EngineHit:
|
||||
snippets=snippets,
|
||||
text=str(getattr(doc, "text", "")) if fetch_text else None,
|
||||
)
|
||||
|
||||
|
||||
def _safe_query(query: str) -> bool:
|
||||
"""Keep user grouping inside the enclosing scope expression."""
|
||||
if not query.strip() or "\\" in query:
|
||||
return False
|
||||
depth = 0
|
||||
quoted = False
|
||||
for character in query:
|
||||
if character == '"':
|
||||
quoted = not quoted
|
||||
elif not quoted and character == "(":
|
||||
depth += 1
|
||||
elif not quoted and character == ")":
|
||||
depth -= 1
|
||||
if depth < 0:
|
||||
return False
|
||||
return depth == 0 and not quoted
|
||||
@@ -1,47 +0,0 @@
|
||||
from collections.abc import Generator
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from recoll_modern.app import create_app
|
||||
from recoll_modern.config import Settings
|
||||
from recoll_modern.engine.fake import FakeEngine
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app_client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Generator[TestClient]:
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_USERNAME", "admin")
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_PASSWORD", "secret-password")
|
||||
(tmp_path / "docs").mkdir()
|
||||
settings = Settings.from_env({
|
||||
"RECOLL_MODERN_DOCUMENT_ROOT": str(tmp_path / "docs"),
|
||||
"RECOLL_MODERN_STATE_ROOT": str(tmp_path / "state"),
|
||||
"RECOLL_MODERN_RECOLL_CONFIG_DIR": str(tmp_path / "recoll"),
|
||||
})
|
||||
with TestClient(create_app(settings, FakeEngine([])), base_url="https://testserver") as client:
|
||||
yield client
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def prefixed_client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Generator[TestClient]:
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_USERNAME", "admin")
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_PASSWORD", "secret-password")
|
||||
(tmp_path / "docs").mkdir()
|
||||
settings = Settings.from_env({
|
||||
"RECOLL_MODERN_DOCUMENT_ROOT": str(tmp_path / "docs"),
|
||||
"RECOLL_MODERN_STATE_ROOT": str(tmp_path / "state"),
|
||||
"RECOLL_MODERN_RECOLL_CONFIG_DIR": str(tmp_path / "recoll"),
|
||||
"RECOLL_MODERN_ROOT_PATH": "/recoll",
|
||||
})
|
||||
with TestClient(create_app(settings, FakeEngine([])), base_url="https://testserver") as client:
|
||||
yield client
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def authenticated(app_client: TestClient) -> tuple[TestClient, str]:
|
||||
response = app_client.post(
|
||||
"/auth/login", json={"username": "admin", "password": "secret-password"}
|
||||
)
|
||||
assert response.status_code == 200
|
||||
return app_client, response.json()["csrf_token"]
|
||||
@@ -46,7 +46,7 @@ def test_unknown_and_missing_documents(
|
||||
digest = _document(client, tmp_path)
|
||||
(tmp_path / "docs" / 'quoted"name.txt').unlink()
|
||||
response = client.get(f"/documents/{digest}/file")
|
||||
assert response.status_code == 404
|
||||
assert response.status_code == 410
|
||||
assert response.json()["error"]["code"] == "document_missing"
|
||||
|
||||
|
||||
|
||||
@@ -60,8 +60,8 @@ def test_search_validates_fields_collections_and_pagination(
|
||||
for params, status, code in [
|
||||
({"q": " ", "collections": collection_id}, 400, "invalid_query"),
|
||||
({"q": "a", "collections": "c0000000000"}, 404, "collection_not_found"),
|
||||
({"q": "a", "collections": collection_id, "fields": "bad"}, 400,
|
||||
"invalid_query"),
|
||||
({"q": "a", "collections": collection_id, "fields": "bad"}, 422,
|
||||
"invalid_field"),
|
||||
({"q": "a", "collections": collection_id, "limit": 101}, 422,
|
||||
"validation_failed"),
|
||||
]:
|
||||
|
||||
+49
-1
@@ -1 +1,49 @@
|
||||
"""Shared pytest configuration."""
|
||||
"""Shared service fixtures for HTTP, contract and security tests."""
|
||||
|
||||
from collections.abc import Generator
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from recoll_modern.app import create_app
|
||||
from recoll_modern.config import Settings
|
||||
from recoll_modern.engine.fake import FakeEngine
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app_client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Generator[TestClient]:
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_USERNAME", "admin")
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_PASSWORD", "secret-password")
|
||||
(tmp_path / "docs").mkdir()
|
||||
settings = Settings.from_env({
|
||||
"RECOLL_MODERN_DOCUMENT_ROOT": str(tmp_path / "docs"),
|
||||
"RECOLL_MODERN_STATE_ROOT": str(tmp_path / "state"),
|
||||
"RECOLL_MODERN_RECOLL_CONFIG_DIR": str(tmp_path / "recoll"),
|
||||
})
|
||||
with TestClient(create_app(settings, FakeEngine([])), base_url="https://testserver") as client:
|
||||
yield client
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def prefixed_client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Generator[TestClient]:
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_USERNAME", "admin")
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_PASSWORD", "secret-password")
|
||||
(tmp_path / "docs").mkdir()
|
||||
settings = Settings.from_env({
|
||||
"RECOLL_MODERN_DOCUMENT_ROOT": str(tmp_path / "docs"),
|
||||
"RECOLL_MODERN_STATE_ROOT": str(tmp_path / "state"),
|
||||
"RECOLL_MODERN_RECOLL_CONFIG_DIR": str(tmp_path / "recoll"),
|
||||
"RECOLL_MODERN_ROOT_PATH": "/recoll",
|
||||
})
|
||||
with TestClient(create_app(settings, FakeEngine([])), base_url="https://testserver") as client:
|
||||
yield client
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def authenticated(app_client: TestClient) -> tuple[TestClient, str]:
|
||||
response = app_client.post(
|
||||
"/auth/login", json={"username": "admin", "password": "secret-password"}
|
||||
)
|
||||
assert response.status_code == 200
|
||||
return app_client, response.json()["csrf_token"]
|
||||
@@ -0,0 +1,87 @@
|
||||
"""Requirement-to-test matrix for spec 001.
|
||||
|
||||
R1–R4: api/test_collections.py, storage/test_collection_repository.py
|
||||
R5–R8: storage/test_documents.py, indexing/test_reconcile.py, integration/test_end_to_end.py
|
||||
R9–R12: api/test_memberships.py, indexing/test_discovery.py, indexing/test_paths.py
|
||||
R13–R14: helper/test_membership_helper.py, integration/test_read_only_corpus.py
|
||||
R15–R18: api/test_jobs.py, indexing/test_job_handlers.py, indexing/test_worker.py
|
||||
R19–R23: api/test_search.py, api/test_documents.py, domain/test_search.py
|
||||
R24–R26: api/test_auth.py, domain/test_auth.py, test_unauthenticated_routes
|
||||
R27–R30: api/test_health.py, api/test_files.py, api/test_root_path.py,
|
||||
security/test_proxy_headers.py, security/test_paths.py, security/test_downloads.py
|
||||
R31: unit/test_config.py, integration/test_container_smoke.py, integration/test_end_to_end.py
|
||||
C1: integration/test_read_only_corpus.py, helper/test_membership_helper.py
|
||||
C2–C3: storage/test_documents.py, integration/test_end_to_end.py
|
||||
C4: api/test_search.py, test_scoped_search_error_codes
|
||||
"""
|
||||
|
||||
import hashlib
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from recoll_modern.engine.fake import FakeRecord
|
||||
from recoll_modern.indexing.discovery import ObservedFile
|
||||
|
||||
|
||||
def test_unauthenticated_routes(app_client: TestClient) -> None:
|
||||
for path in (
|
||||
"/files", "/collections", "/jobs", "/search?q=book&collections=c0000000000",
|
||||
f"/documents/{'a' * 64}/text", f"/documents/{'a' * 64}/file",
|
||||
):
|
||||
response = app_client.get(path)
|
||||
assert response.status_code == 401, path
|
||||
assert response.json()["error"]["code"] == "unauthenticated", path
|
||||
|
||||
|
||||
def test_scoped_search_error_codes(authenticated: tuple[TestClient, str]) -> None:
|
||||
client, csrf = authenticated
|
||||
collection_id = client.post(
|
||||
"/collections", json={"name": "Books"}, headers={"X-CSRF-Token": csrf}
|
||||
).json()["id"]
|
||||
invalid_field = client.get("/search", params={
|
||||
"q": "book", "collections": collection_id, "fields": "unknown",
|
||||
})
|
||||
assert invalid_field.status_code == 422
|
||||
assert invalid_field.json()["error"]["code"] == "invalid_field"
|
||||
invalid_query = client.get("/search", params={
|
||||
"q": "book\\evil", "collections": collection_id,
|
||||
})
|
||||
assert invalid_query.status_code == 400
|
||||
assert invalid_query.json()["error"]["code"] == "invalid_query"
|
||||
|
||||
|
||||
def test_same_directory_membership_and_same_name_identity(
|
||||
authenticated: tuple[TestClient, str], tmp_path: Path
|
||||
) -> None:
|
||||
client, csrf = authenticated
|
||||
headers = {"X-CSRF-Token": csrf}
|
||||
first_id = client.post("/collections", json={"name": "First"}, headers=headers).json()["id"]
|
||||
second_id = client.post("/collections", json={"name": "Second"}, headers=headers).json()["id"]
|
||||
root = tmp_path / "docs"
|
||||
(root / "a").mkdir()
|
||||
(root / "b").mkdir()
|
||||
sources = [
|
||||
("a/same.txt", b"python first", first_id),
|
||||
("a/other.txt", b"python second", second_id),
|
||||
("b/same.txt", b"python third", second_id),
|
||||
]
|
||||
app = cast(FastAPI, client.app)
|
||||
engine_records = []
|
||||
for relative, content, collection_id in sources:
|
||||
path = root / relative
|
||||
path.write_bytes(content)
|
||||
digest = hashlib.sha256(content).hexdigest()
|
||||
app.state.documents.observe_files([ObservedFile(relative, digest, len(content))])
|
||||
app.state.memberships.add_direct(collection_id, digest)
|
||||
engine_records.append(FakeRecord(str(path), relative, content.decode(), (collection_id,)))
|
||||
app.state.engine.records = engine_records
|
||||
first = client.get("/search", params={"q": "python", "collections": first_id}).json()
|
||||
second = client.get("/search", params={"q": "python", "collections": second_id}).json()
|
||||
assert [hit["path"] for hit in first["hits"]] == ["a/same.txt"]
|
||||
assert {hit["path"] for hit in second["hits"]} == {"a/other.txt", "b/same.txt"}
|
||||
assert first["hits"][0]["document_id"] != next(
|
||||
hit["document_id"] for hit in second["hits"] if hit["path"] == "b/same.txt"
|
||||
)
|
||||
@@ -123,7 +123,9 @@ def test_extracted_text_uses_exact_path_udi(
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "recoll", recoll_package)
|
||||
db = SimpleNamespace(
|
||||
getDoc=lambda udi: SimpleNamespace(ipath="") if udi == "udi" else None,
|
||||
getDoc=lambda udi: SimpleNamespace(
|
||||
ipath="", getbinurl=lambda: path.encode()
|
||||
) if udi == "udi" else None,
|
||||
close=lambda: None,
|
||||
)
|
||||
monkeypatch.setattr(RecollEngine, "_connect", lambda self: db)
|
||||
|
||||
@@ -1,19 +1,92 @@
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from dataclasses import asdict
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
import pytest
|
||||
|
||||
from recoll_modern.engine.protocol import Engine, EngineHealth, EngineHit, EnginePage, SearchRequest
|
||||
from recoll_modern.engine.recoll import RecollEngine
|
||||
|
||||
from .engine_contract import EngineContract
|
||||
|
||||
_CONTAINER_SCRIPT = """
|
||||
import json, sys
|
||||
from dataclasses import asdict
|
||||
from pathlib import Path
|
||||
from recoll_modern.engine.protocol import SearchRequest
|
||||
from recoll_modern.engine.recoll import RecollEngine
|
||||
engine = RecollEngine(Path('/var/lib/recoll-modern/recoll-config'))
|
||||
method, payload = sys.argv[1], json.loads(sys.argv[2])
|
||||
if method == 'search':
|
||||
result = engine.search(SearchRequest(**payload))
|
||||
elif method == 'health':
|
||||
result = engine.health()
|
||||
elif method == 'extracted_text':
|
||||
result = engine.extracted_text(payload['path'])
|
||||
else:
|
||||
raise ValueError(method)
|
||||
print(json.dumps(asdict(result) if method != 'extracted_text' else result, default=str))
|
||||
"""
|
||||
|
||||
|
||||
class ContainerRecollEngine:
|
||||
def _call(self, method: str, payload: dict[str, object]) -> object:
|
||||
result = subprocess.run(
|
||||
["docker", "compose", "exec", "-T", "app", "python", "-c", _CONTAINER_SCRIPT,
|
||||
method, json.dumps(payload)],
|
||||
capture_output=True, text=True, check=False, timeout=30,
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
return json.loads(result.stdout)
|
||||
|
||||
def search(self, request: SearchRequest) -> EnginePage:
|
||||
value = cast(dict[str, object], self._call("search", asdict(request)))
|
||||
hits = []
|
||||
for raw in cast(list[dict[str, object]], value["hits"]):
|
||||
date = raw["date"]
|
||||
raw["date"] = datetime.fromisoformat(date) if isinstance(date, str) else None
|
||||
raw["collection_ids"] = tuple(cast(list[str], raw["collection_ids"]))
|
||||
raw["snippets"] = tuple(cast(list[str], raw["snippets"]))
|
||||
hits.append(EngineHit(**raw)) # type: ignore[arg-type]
|
||||
return EnginePage(
|
||||
tuple(hits), int(cast(int, value["total"])), bool(value["total_is_approximate"])
|
||||
)
|
||||
|
||||
def health(self) -> EngineHealth:
|
||||
value = cast(dict[str, object], self._call("health", {}))
|
||||
return EngineHealth(bool(value["available"]), cast(str | None, value["message"]))
|
||||
|
||||
def extracted_text(self, path: str) -> str:
|
||||
return cast(str, self._call("extracted_text", {"path": path}))
|
||||
|
||||
def force_reindex(self, paths: tuple[str, ...]) -> None:
|
||||
raise NotImplementedError
|
||||
|
||||
|
||||
class TestRecollEngine(EngineContract):
|
||||
@pytest.fixture(autouse=True)
|
||||
def available_engine(self) -> None:
|
||||
config = os.environ.get("RECOLL_MODERN_TEST_RECOLL_CONFIG_DIR")
|
||||
if config is None:
|
||||
pytest.skip("Recoll test container is not running")
|
||||
if os.environ.get("RECOLL_MODERN_REQUIRE_DOCKER") != "1":
|
||||
pytest.skip("Recoll test container is not running")
|
||||
corpus_file = os.environ.get("RECOLL_MODERN_TEST_CORPUS_FILE")
|
||||
if corpus_file is None:
|
||||
pytest.fail("RECOLL_MODERN_TEST_CORPUS_FILE is required for container contract")
|
||||
bootstrap = subprocess.run(
|
||||
["docker", "compose", "exec", "-T", "app", "recollindex", "-c",
|
||||
"/var/lib/recoll-modern/recoll-config", "-i", "-Z",
|
||||
f"/documents/{corpus_file}"],
|
||||
capture_output=True, text=True, check=False, timeout=30,
|
||||
)
|
||||
assert bootstrap.returncode == 0, bootstrap.stderr
|
||||
self.engine = cast(Engine, ContainerRecollEngine())
|
||||
assert self.engine.health().available
|
||||
return
|
||||
engine = RecollEngine(Path(config))
|
||||
if not engine.health().available:
|
||||
pytest.skip("Recoll test container is not running")
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
"""Exercise the complete HTTP lifecycle with durable state and a deterministic engine."""
|
||||
|
||||
import time
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from recoll_modern.app import create_app
|
||||
from recoll_modern.config import Settings
|
||||
from recoll_modern.engine.fake import FakeEngine, FakeRecord
|
||||
from recoll_modern.storage.database import Database
|
||||
from recoll_modern.storage.jobs import JobRepository
|
||||
|
||||
|
||||
def _wait(client: TestClient, job_id: str) -> dict[str, object]:
|
||||
deadline = time.monotonic() + 5
|
||||
while time.monotonic() < deadline:
|
||||
job = client.get(f"/recoll/jobs/{job_id}").json()["job"]
|
||||
if job["state"] not in ("queued", "running"):
|
||||
return cast(dict[str, object], job)
|
||||
time.sleep(0.01)
|
||||
raise AssertionError(f"job {job_id} did not finish")
|
||||
|
||||
|
||||
def test_complete_collection_lifecycle(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
root = tmp_path / "docs"
|
||||
(root / "a" / ".calibre").mkdir(parents=True)
|
||||
(root / "b").mkdir()
|
||||
(root / "a" / "shared.txt").write_text("python duplicate")
|
||||
(root / "b" / "shared.txt").write_text("python duplicate")
|
||||
(root / "a" / "other.txt").write_text("python separate")
|
||||
(root / "a" / ".calibre" / "hidden.txt").write_text("python hidden")
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_USERNAME", "admin")
|
||||
monkeypatch.setenv("RECOLL_MODERN_ADMIN_PASSWORD", "secret-password")
|
||||
settings = Settings.from_env({
|
||||
"RECOLL_MODERN_DOCUMENT_ROOT": str(root),
|
||||
"RECOLL_MODERN_STATE_ROOT": str(tmp_path / "state"),
|
||||
"RECOLL_MODERN_RECOLL_CONFIG_DIR": str(tmp_path / "config"),
|
||||
"RECOLL_MODERN_ROOT_PATH": "/recoll",
|
||||
"RECOLL_MODERN_INLINE_TEXT_LIMIT": "12",
|
||||
"RECOLL_MODERN_INLINE_TEXT_LIMIT_MAX": "12",
|
||||
})
|
||||
engine = FakeEngine([])
|
||||
with TestClient(create_app(settings, engine), base_url="https://testserver") as client:
|
||||
login = client.post("/recoll/auth/login", json={
|
||||
"username": "admin", "password": "secret-password",
|
||||
})
|
||||
assert login.status_code == 200
|
||||
assert "Path=/recoll" in login.headers["set-cookie"]
|
||||
headers = {"X-CSRF-Token": login.json()["csrf_token"]}
|
||||
first = client.post("/recoll/collections", json={"name": "First"}, headers=headers)
|
||||
second = client.post("/recoll/collections", json={"name": "Second"}, headers=headers)
|
||||
a, b = first.json()["id"], second.json()["id"]
|
||||
for collection_id, sources in (
|
||||
(a, [{"type": "directory", "path": "a", "mode": "persistent",
|
||||
"excluded_directories": ["a/.calibre"]}]),
|
||||
(b, [{"type": "directory", "path": "b", "mode": "snapshot"}]),
|
||||
):
|
||||
response = client.post(
|
||||
f"/recoll/collections/{collection_id}/memberships",
|
||||
headers=headers, json={"sources": sources},
|
||||
)
|
||||
assert _wait(client, response.json()["job"]["id"])["state"] == "completed"
|
||||
direct = client.post(
|
||||
f"/recoll/collections/{a}/memberships", headers=headers,
|
||||
json={"sources": [
|
||||
{"type": "file", "path": "a/other.txt"},
|
||||
{"type": "file", "path": "a/shared.txt"},
|
||||
]},
|
||||
)
|
||||
assert _wait(client, direct.json()["job"]["id"])["state"] == "completed"
|
||||
members_a = client.get(f"/recoll/collections/{a}/documents").json()["items"]
|
||||
members_b = client.get(f"/recoll/collections/{b}/documents").json()["items"]
|
||||
assert len(members_a) == 2 and len(members_b) == 1
|
||||
shared = next(item for item in members_a if item["path"] == "a/shared.txt")
|
||||
assert shared["document_id"] == members_b[0]["document_id"]
|
||||
assert all(item["path"] != "a/.calibre/hidden.txt" for item in members_a)
|
||||
engine.records = [
|
||||
FakeRecord(str(root / "a" / "shared.txt"), "Shared", "python duplicate", (a, b)),
|
||||
FakeRecord(str(root / "a" / "other.txt"), "Other", "python separate", (a,)),
|
||||
]
|
||||
result = client.get("/recoll/search", params=[
|
||||
("q", "python"), ("collections", a), ("collections", b),
|
||||
("fields", "text"), ("text_limit", "6"),
|
||||
])
|
||||
assert result.status_code == 200
|
||||
assert len(result.json()["hits"]) == 2
|
||||
assert result.json()["hits"][0]["text"] == "python"
|
||||
assert client.get(
|
||||
f"/recoll/documents/{shared['document_id']}/text"
|
||||
).json()["text"] == "python duplicate"
|
||||
partial = client.get(
|
||||
f"/recoll/documents/{shared['document_id']}/file", headers={"Range": "bytes=0-5"}
|
||||
)
|
||||
assert partial.status_code == 206 and partial.content == b"python"
|
||||
|
||||
(root / "a" / "shared.txt").unlink()
|
||||
refresh = client.post("/recoll/jobs/reindex", headers=headers, json={})
|
||||
assert _wait(client, refresh.json()["job"]["id"])["state"] == "completed"
|
||||
promoted = cast(FastAPI, client.app).state.documents.get(shared["document_id"])
|
||||
assert promoted.canonical_path == "b/shared.txt"
|
||||
engine.records = [
|
||||
FakeRecord(str(root / "b" / "shared.txt"), "Shared", "python duplicate", (a, b))
|
||||
]
|
||||
assert client.get("/recoll/search", params={
|
||||
"q": "python", "collections": a,
|
||||
}).json()["hits"][0]["document_id"] == shared["document_id"]
|
||||
|
||||
jobs = JobRepository(Database(settings.database_path))
|
||||
interrupted = jobs.enqueue("reindex", {"version": 1, "paths": [], "collections": []}, False)
|
||||
with Database(settings.database_path).transaction() as connection:
|
||||
connection.execute("UPDATE jobs SET state = 'running' WHERE id = ?", (interrupted.id,))
|
||||
with TestClient(create_app(settings, engine), base_url="https://testserver") as client:
|
||||
client.post("/recoll/auth/login", json={
|
||||
"username": "admin", "password": "secret-password",
|
||||
})
|
||||
assert _wait(client, interrupted.id)["state"] == "completed"
|
||||
@@ -58,3 +58,35 @@ def test_generated_corpus_indexes_and_searches(
|
||||
)
|
||||
assert result.status_code == 200, result.text
|
||||
assert result.json()["hits"], result.json()
|
||||
document_id = result.json()["hits"][0]["document_id"]
|
||||
second = client.post(
|
||||
"/collections", json={"name": f"Integration second {time.time_ns()}"},
|
||||
headers=headers,
|
||||
)
|
||||
assert second.status_code == 201, second.text
|
||||
second_id = second.json()["id"]
|
||||
second_job = client.post(
|
||||
f"/collections/{second_id}/memberships", headers=headers,
|
||||
json={"sources": [{"type": "file", "path": corpus_file}]},
|
||||
)
|
||||
assert second_job.status_code == 202, second_job.text
|
||||
second_job_id = second_job.json()["job"]["id"]
|
||||
deadline = time.monotonic() + 30
|
||||
while time.monotonic() < deadline:
|
||||
job = client.get(f"/jobs/{second_job_id}", headers=headers).json()["job"]
|
||||
if job["state"] not in ("queued", "running"):
|
||||
break
|
||||
time.sleep(0.1)
|
||||
assert job["state"] == "completed", job
|
||||
combined = client.get(
|
||||
"/search", params=[
|
||||
("q", "zebraplatypus"), ("collections", collection_id),
|
||||
("collections", second_id), ("fields", "collections"),
|
||||
], headers=headers,
|
||||
)
|
||||
assert combined.status_code == 200, combined.text
|
||||
assert len(combined.json()["hits"]) == 1
|
||||
assert set(combined.json()["hits"][0]["collections"]) >= {collection_id, second_id}
|
||||
extracted = client.get(f"/documents/{document_id}/text", headers=headers)
|
||||
assert extracted.status_code == 200, extracted.text
|
||||
assert "zebraplatypus" in extracted.json()["text"]
|
||||
@@ -0,0 +1 @@
|
||||
"""Boundary and adversarial request checks."""
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Missing documents use the spec's distinct gone response."""
|
||||
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from recoll_modern.indexing.discovery import ObservedFile
|
||||
|
||||
|
||||
def test_missing_document_is_gone(
|
||||
authenticated: tuple[TestClient, str], tmp_path: Path
|
||||
) -> None:
|
||||
client, _ = authenticated
|
||||
path = tmp_path / "docs" / "gone.txt"
|
||||
path.write_text("gone")
|
||||
digest = "f" * 64
|
||||
cast(FastAPI, client.app).state.documents.observe_files([
|
||||
ObservedFile("gone.txt", digest, 4)
|
||||
])
|
||||
path.unlink()
|
||||
response = client.get(f"/documents/{digest}/file")
|
||||
assert response.status_code == 410
|
||||
assert response.json()["error"]["code"] == "document_missing"
|
||||
@@ -0,0 +1,29 @@
|
||||
"""Containment is rechecked when serving a document."""
|
||||
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from recoll_modern.indexing.discovery import ObservedFile
|
||||
|
||||
|
||||
def test_swapped_symlink_cannot_escape_document_root(
|
||||
authenticated: tuple[TestClient, str], tmp_path: Path
|
||||
) -> None:
|
||||
client, _ = authenticated
|
||||
root = tmp_path / "docs"
|
||||
path = root / "file.txt"
|
||||
path.write_text("safe")
|
||||
digest = "a" * 64
|
||||
cast(FastAPI, client.app).state.documents.observe_files([
|
||||
ObservedFile("file.txt", digest, 4)
|
||||
])
|
||||
outside = tmp_path / "secret.txt"
|
||||
outside.write_text("secret")
|
||||
path.unlink()
|
||||
path.symlink_to(outside)
|
||||
response = client.get(f"/documents/{digest}/file")
|
||||
assert response.status_code == 410
|
||||
assert b"secret" not in response.content
|
||||
@@ -0,0 +1,12 @@
|
||||
"""Untrusted clients cannot replace the configured mount prefix."""
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
|
||||
def test_direct_client_cannot_forge_prefix(prefixed_client: TestClient) -> None:
|
||||
response = prefixed_client.get("/evil/health", headers={
|
||||
"X-Forwarded-Prefix": "/evil", "X-Forwarded-Host": "attacker.example",
|
||||
"X-Forwarded-Proto": "https",
|
||||
})
|
||||
assert response.status_code == 404
|
||||
assert response.json()["error"]["code"] == "path_not_found"
|
||||
Reference in new issue
Block a user