test(points): cover the read paths and the query service
Why:
- The isolation and pagination guarantees are the ones that fail silently, so
they need tests that would actually notice.
Changes:
- API tests over real Postgres and Qdrant together, because the invariant worth
testing spans both: the tenant Postgres derived is the only one Qdrant is ever
queried with.
- Pagination holds when a point is inserted behind the cursor mid-listing --
the defect an offset cursor would have.
- A route-order guard, since /{point_id} declared first turns count into a 422
and nothing else in the suite would catch it.
- Unit tests for the query service against the fake, including the Arabic to
Persian letterform fold and raising rather than returning None.
Impact:
- Suite goes to 325 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
395
tests/integration/qdrant/test_points_api.py
Normal file
395
tests/integration/qdrant/test_points_api.py
Normal file
@@ -0,0 +1,395 @@
|
||||
"""`/v1/points` read paths over HTTP (ADR-0002, ADR-0008).
|
||||
|
||||
Needs both containers: Postgres authenticates the key and derives the tenant,
|
||||
Qdrant holds the points. Testing them together is the point — the invariant this
|
||||
file exists for is that the tenant Postgres derived is the only one Qdrant is
|
||||
ever queried with.
|
||||
|
||||
Everything below goes through the real app: real routing, real `require_scope`,
|
||||
real error envelope. Only the session factory is overridden, so a route that
|
||||
forgot its scope or read a tenant from the query string fails here.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from collections.abc import AsyncIterator
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from asgi_lifespan import LifespanManager
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
from qdrant_client import AsyncQdrantClient
|
||||
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
|
||||
|
||||
from src.bootstrap.dependencies import get_sessionmaker
|
||||
from src.config import QdrantSettings, Settings
|
||||
from src.infrastructure.qdrant.collection import ensure_chunks_collection
|
||||
from src.infrastructure.qdrant.points import QdrantPointStorage
|
||||
from src.main import create_app
|
||||
from tests.support.factories import create_api_key, create_tenant
|
||||
from tests.support.point_contract import SeedSpec, chunk_point_for, point_id_for
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.qdrant,
|
||||
pytest.mark.postgres,
|
||||
pytest.mark.asyncio(loop_scope="session"),
|
||||
]
|
||||
|
||||
FILE_ID = uuid.UUID("aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
|
||||
OTHER_FILE_ID = uuid.UUID("bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def api_settings(settings: Settings, qdrant_settings: QdrantSettings) -> Settings:
|
||||
"""The unit-test settings with Qdrant pointed at the real container.
|
||||
|
||||
Everything else keeps its closed-port default, so this exercises the point
|
||||
paths without the lifespan reaching a colleague's Ollama box or OpenAI.
|
||||
"""
|
||||
return settings.model_copy(update={"qdrant": qdrant_settings})
|
||||
|
||||
|
||||
@pytest_asyncio.fixture(loop_scope="session")
|
||||
async def api_client(
|
||||
api_settings: Settings, db_sessionmaker: async_sessionmaker[AsyncSession]
|
||||
) -> AsyncIterator[AsyncClient]:
|
||||
app = create_app(api_settings)
|
||||
app.dependency_overrides[get_sessionmaker] = lambda: db_sessionmaker
|
||||
async with (
|
||||
LifespanManager(app) as manager,
|
||||
AsyncClient(transport=ASGITransport(app=manager.app), base_url="http://test") as client,
|
||||
):
|
||||
yield client
|
||||
|
||||
|
||||
def _auth(token: str) -> dict[str, str]:
|
||||
return {"Authorization": f"Bearer {token}"}
|
||||
|
||||
|
||||
async def _seed(
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
specs: tuple[SeedSpec, ...],
|
||||
) -> None:
|
||||
await ensure_chunks_collection(qdrant_client, collection=qdrant_settings.collection)
|
||||
storage = QdrantPointStorage(qdrant_client, collection=qdrant_settings.collection)
|
||||
await storage.upsert_points([chunk_point_for(spec) for spec in specs])
|
||||
|
||||
|
||||
async def _tenant_with_key(
|
||||
db_session: AsyncSession, *, scopes: list[str] | None = None
|
||||
) -> tuple[uuid.UUID, str]:
|
||||
tenant = await create_tenant(db_session)
|
||||
_, token = await create_api_key(
|
||||
db_session, tenant=tenant, scopes=scopes if scopes is not None else ["points:read"]
|
||||
)
|
||||
await db_session.commit()
|
||||
return tenant.id, token
|
||||
|
||||
|
||||
async def test_list_points_returns_a_files_points_in_order_id_order(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
await _seed(
|
||||
qdrant_client,
|
||||
qdrant_settings,
|
||||
(
|
||||
SeedSpec(tenant_id, FILE_ID, 0, "first", 1.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 2, "third", 3.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 1, "second", 2.0),
|
||||
),
|
||||
)
|
||||
|
||||
response = await api_client.get(
|
||||
"/v1/points", params={"file_id": str(FILE_ID)}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert [point["content"] for point in response.json()["points"]] == [
|
||||
"first",
|
||||
"second",
|
||||
"third",
|
||||
]
|
||||
|
||||
|
||||
async def test_list_points_omits_the_tenant_id_from_the_response(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
"""Echoing `tenant_id` back invites clients to start sending it."""
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
await _seed(qdrant_client, qdrant_settings, (SeedSpec(tenant_id, FILE_ID, 0, "only", 1.0),))
|
||||
|
||||
response = await api_client.get(
|
||||
"/v1/points", params={"file_id": str(FILE_ID)}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert "tenant_id" not in response.json()["points"][0]
|
||||
|
||||
|
||||
async def test_get_point_returns_404_for_another_tenants_point(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
"""`404`, not `403`: the API must not confirm the id exists (ADR-0016)."""
|
||||
owner_id, _ = await _tenant_with_key(db_session)
|
||||
_, intruder_token = await _tenant_with_key(db_session)
|
||||
await _seed(qdrant_client, qdrant_settings, (SeedSpec(owner_id, FILE_ID, 0, "secret", 1.0),))
|
||||
|
||||
response = await api_client.get(
|
||||
f"/v1/points/{point_id_for(FILE_ID, 0)}", headers=_auth(intruder_token)
|
||||
)
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json()["error"]["code"] == "not_found"
|
||||
|
||||
|
||||
async def test_list_points_returns_nothing_for_another_tenants_file(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
owner_id, _ = await _tenant_with_key(db_session)
|
||||
_, intruder_token = await _tenant_with_key(db_session)
|
||||
await _seed(qdrant_client, qdrant_settings, (SeedSpec(owner_id, FILE_ID, 0, "secret", 1.0),))
|
||||
|
||||
response = await api_client.get(
|
||||
"/v1/points", params={"file_id": str(FILE_ID)}, headers=_auth(intruder_token)
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["points"] == []
|
||||
|
||||
|
||||
async def test_list_points_excludes_inactive_points_unless_asked(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
await _seed(
|
||||
qdrant_client,
|
||||
qdrant_settings,
|
||||
(
|
||||
SeedSpec(tenant_id, FILE_ID, 0, "live", 1.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 1, "deleted", 2.0, is_active=False),
|
||||
),
|
||||
)
|
||||
|
||||
default = await api_client.get(
|
||||
"/v1/points", params={"file_id": str(FILE_ID)}, headers=_auth(token)
|
||||
)
|
||||
opted_in = await api_client.get(
|
||||
"/v1/points",
|
||||
params={"file_id": str(FILE_ID), "include_inactive": True},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert [point["content"] for point in default.json()["points"]] == ["live"]
|
||||
assert [point["content"] for point in opted_in.json()["points"]] == ["live", "deleted"]
|
||||
|
||||
|
||||
async def test_list_points_pagination_does_not_skip_or_repeat_under_a_concurrent_insert(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
"""The defect an offset cursor would have.
|
||||
|
||||
Between page one and page two, a point is inserted *behind* the cursor
|
||||
(`order_id` 0.5, ahead of everything already returned). An offset-based
|
||||
cursor would shift every later row down by one and hand back `b` a second
|
||||
time. A value-based cursor is unaffected: the reader has passed 2.0 and the
|
||||
new row is behind it.
|
||||
"""
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
await _seed(
|
||||
qdrant_client,
|
||||
qdrant_settings,
|
||||
(
|
||||
SeedSpec(tenant_id, FILE_ID, 0, "a", 1.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 1, "b", 2.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 2, "c", 3.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 3, "d", 4.0),
|
||||
),
|
||||
)
|
||||
|
||||
first = await api_client.get(
|
||||
"/v1/points",
|
||||
params={"file_id": str(FILE_ID), "limit": 2},
|
||||
headers=_auth(token),
|
||||
)
|
||||
await _seed(qdrant_client, qdrant_settings, (SeedSpec(tenant_id, FILE_ID, 9, "inserted", 0.5),))
|
||||
second = await api_client.get(
|
||||
"/v1/points",
|
||||
params={
|
||||
"file_id": str(FILE_ID),
|
||||
"limit": 2,
|
||||
"cursor": first.json()["next_cursor"],
|
||||
},
|
||||
headers=_auth(token),
|
||||
)
|
||||
|
||||
assert [point["content"] for point in first.json()["points"]] == ["a", "b"]
|
||||
assert [point["content"] for point in second.json()["points"]] == ["c", "d"]
|
||||
|
||||
|
||||
async def test_count_points_respects_tenant_and_domain_filters(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
other_id, _ = await _tenant_with_key(db_session)
|
||||
await _seed(
|
||||
qdrant_client,
|
||||
qdrant_settings,
|
||||
(
|
||||
SeedSpec(tenant_id, FILE_ID, 0, "one", 1.0),
|
||||
SeedSpec(tenant_id, OTHER_FILE_ID, 0, "two", 1.0, domain="ops"),
|
||||
SeedSpec(other_id, FILE_ID, 5, "theirs", 1.0),
|
||||
),
|
||||
)
|
||||
|
||||
everything = await api_client.get("/v1/points/count", headers=_auth(token))
|
||||
scoped = await api_client.get(
|
||||
"/v1/points/count", params={"domain": "ops"}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert everything.json()["count"] == 2
|
||||
assert scoped.json()["count"] == 1
|
||||
|
||||
|
||||
async def test_search_points_matches_content_within_the_tenant_only(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
other_id, _ = await _tenant_with_key(db_session)
|
||||
await _seed(
|
||||
qdrant_client,
|
||||
qdrant_settings,
|
||||
(
|
||||
SeedSpec(tenant_id, FILE_ID, 0, "annual leave policy", 1.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 1, "expense policy", 2.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 2, "office address", 3.0),
|
||||
SeedSpec(other_id, OTHER_FILE_ID, 0, "their leave policy", 1.0),
|
||||
),
|
||||
)
|
||||
|
||||
response = await api_client.get(
|
||||
"/v1/points/search", params={"q": "policy"}, headers=_auth(token)
|
||||
)
|
||||
|
||||
body = response.json()
|
||||
assert body["query"] == "policy"
|
||||
assert {point["content"] for point in body["points"]} == {
|
||||
"annual leave policy",
|
||||
"expense policy",
|
||||
}
|
||||
|
||||
|
||||
async def test_search_points_folds_arabic_letterforms_in_the_query(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
"""A query typed on an Arabic keyboard must match Persian-folded content.
|
||||
|
||||
Ingestion folds U+064A/U+0643 to U+06CC/U+06A9 before storing, so an
|
||||
unfolded query would match nothing — an empty result rather than an error,
|
||||
which is exactly the kind of silent failure nobody reports as a bug.
|
||||
Codepoints, not literals: the two forms are visually identical here.
|
||||
"""
|
||||
persian = "مدیریت" # stored, Persian yeh
|
||||
arabic = "مديريت" # queried, Arabic yeh
|
||||
assert persian != arabic
|
||||
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
await _seed(qdrant_client, qdrant_settings, (SeedSpec(tenant_id, FILE_ID, 0, persian, 1.0),))
|
||||
|
||||
response = await api_client.get("/v1/points/search", params={"q": arabic}, headers=_auth(token))
|
||||
|
||||
assert [point["content"] for point in response.json()["points"]] == [persian]
|
||||
|
||||
|
||||
async def test_point_read_routes_require_the_points_read_scope(
|
||||
api_client: AsyncClient, db_session: AsyncSession
|
||||
) -> None:
|
||||
"""An upload key must not double as a way to read every chunk."""
|
||||
_, token = await _tenant_with_key(db_session, scopes=["files:write"])
|
||||
|
||||
response = await api_client.get(
|
||||
"/v1/points", params={"file_id": str(FILE_ID)}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert response.status_code == 403
|
||||
assert response.json()["error"]["code"] == "missing_scope"
|
||||
|
||||
|
||||
async def test_count_and_search_paths_are_not_parsed_as_point_ids(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
"""Route order regression guard.
|
||||
|
||||
`/{point_id}` declared before `/count` would make this `422` — Pydantic
|
||||
failing to read "count" as a UUID — rather than a count. The bug is one
|
||||
tidy-up reorder away, and nothing else in the suite would catch it.
|
||||
|
||||
Seeds nothing but still bootstraps the collection: querying a collection
|
||||
that does not exist is a `500`, which `/readyz` is responsible for
|
||||
preventing, and which would mask the `422` this test is watching for.
|
||||
"""
|
||||
_, token = await _tenant_with_key(db_session)
|
||||
await _seed(qdrant_client, qdrant_settings, ())
|
||||
|
||||
count = await api_client.get("/v1/points/count", headers=_auth(token))
|
||||
search = await api_client.get(
|
||||
"/v1/points/search", params={"q": "anything"}, headers=_auth(token)
|
||||
)
|
||||
|
||||
assert count.status_code == 200
|
||||
assert search.status_code == 200
|
||||
|
||||
|
||||
async def test_file_points_route_lists_the_same_points_as_the_query_form(
|
||||
api_client: AsyncClient,
|
||||
db_session: AsyncSession,
|
||||
qdrant_client: AsyncQdrantClient,
|
||||
qdrant_settings: QdrantSettings,
|
||||
) -> None:
|
||||
tenant_id, token = await _tenant_with_key(db_session)
|
||||
await _seed(
|
||||
qdrant_client,
|
||||
qdrant_settings,
|
||||
(
|
||||
SeedSpec(tenant_id, FILE_ID, 0, "first", 1.0),
|
||||
SeedSpec(tenant_id, FILE_ID, 1, "second", 2.0),
|
||||
),
|
||||
)
|
||||
|
||||
by_query = await api_client.get(
|
||||
"/v1/points", params={"file_id": str(FILE_ID)}, headers=_auth(token)
|
||||
)
|
||||
by_path = await api_client.get(f"/v1/files/{FILE_ID}/points", headers=_auth(token))
|
||||
|
||||
assert by_path.status_code == 200
|
||||
assert by_path.json() == by_query.json()
|
||||
103
tests/unit/application/points/test_queries.py
Normal file
103
tests/unit/application/points/test_queries.py
Normal file
@@ -0,0 +1,103 @@
|
||||
"""`application/points/queries.py` against the fake repository (ADR-0016).
|
||||
|
||||
The HTTP-level behaviour is covered in `tests/integration/qdrant/`. What is
|
||||
worth isolating here is the small amount of logic the service actually owns —
|
||||
raising instead of returning `None`, and folding a query's Arabic letterforms —
|
||||
because both are cheap to test directly and expensive to notice when broken.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
|
||||
import pytest
|
||||
|
||||
from src.application.points.point import PointNotFoundError
|
||||
from src.application.points.queries import (
|
||||
count_points,
|
||||
get_point,
|
||||
list_file_points,
|
||||
search_points,
|
||||
)
|
||||
from tests.fakes import FakePointRepository
|
||||
from tests.support.point_contract import SeedSpec, build_point, point_id_for
|
||||
|
||||
pytestmark = [pytest.mark.unit, pytest.mark.asyncio]
|
||||
|
||||
TENANT = uuid.UUID("11111111-1111-4111-8111-111111111111")
|
||||
OTHER_TENANT = uuid.UUID("22222222-2222-4222-8222-222222222222")
|
||||
FILE = uuid.UUID("33333333-3333-4333-8333-333333333333")
|
||||
|
||||
|
||||
def _repository(*specs: SeedSpec) -> FakePointRepository:
|
||||
repository = FakePointRepository()
|
||||
for spec in specs:
|
||||
repository.add(build_point(spec))
|
||||
return repository
|
||||
|
||||
|
||||
async def test_get_point_returns_the_point_for_its_owner() -> None:
|
||||
repository = _repository(SeedSpec(TENANT, FILE, 0, "hello", 1.0))
|
||||
|
||||
point = await get_point(repository, tenant_id=TENANT, point_id=point_id_for(FILE, 0))
|
||||
|
||||
assert point.content == "hello"
|
||||
|
||||
|
||||
async def test_get_point_raises_for_another_tenants_point() -> None:
|
||||
"""Raising, not returning `None`, so a route cannot serve `200 null`."""
|
||||
repository = _repository(SeedSpec(TENANT, FILE, 0, "hello", 1.0))
|
||||
|
||||
with pytest.raises(PointNotFoundError):
|
||||
await get_point(repository, tenant_id=OTHER_TENANT, point_id=point_id_for(FILE, 0))
|
||||
|
||||
|
||||
async def test_get_point_raises_for_an_unknown_point() -> None:
|
||||
repository = _repository()
|
||||
|
||||
with pytest.raises(PointNotFoundError):
|
||||
await get_point(repository, tenant_id=TENANT, point_id=uuid.uuid4())
|
||||
|
||||
|
||||
async def test_search_points_folds_arabic_letterforms_before_matching() -> None:
|
||||
"""The query is normalized the same way ingestion normalized the content.
|
||||
|
||||
Stored content carries Persian yeh (U+06CC) because `normalize_persian_text`
|
||||
folded it at ingest. A query typed with Arabic yeh (U+064A) is a different
|
||||
codepoint and would match nothing — silently, with no error to notice.
|
||||
"""
|
||||
persian = "مدیریت"
|
||||
arabic = "مديريت"
|
||||
assert persian != arabic
|
||||
|
||||
repository = _repository(SeedSpec(TENANT, FILE, 0, persian, 1.0))
|
||||
|
||||
found = await search_points(repository, tenant_id=TENANT, query=arabic, limit=10)
|
||||
|
||||
assert [point.content for point in found.points] == [persian]
|
||||
|
||||
|
||||
async def test_search_points_does_not_reach_another_tenant() -> None:
|
||||
repository = _repository(SeedSpec(OTHER_TENANT, FILE, 0, "secret", 1.0))
|
||||
|
||||
found = await search_points(repository, tenant_id=TENANT, query="secret", limit=10)
|
||||
|
||||
assert found.points == ()
|
||||
|
||||
|
||||
async def test_list_file_points_returns_an_empty_page_for_a_foreign_file() -> None:
|
||||
"""Indistinguishable from an unknown file — the same non-disclosure rule."""
|
||||
repository = _repository(SeedSpec(OTHER_TENANT, FILE, 0, "secret", 1.0))
|
||||
|
||||
page = await list_file_points(repository, tenant_id=TENANT, file_id=FILE, limit=10)
|
||||
|
||||
assert page.points == ()
|
||||
assert page.next_cursor is None
|
||||
|
||||
|
||||
async def test_count_points_excludes_inactive_unless_asked() -> None:
|
||||
repository = _repository(
|
||||
SeedSpec(TENANT, FILE, 0, "live", 1.0),
|
||||
SeedSpec(TENANT, FILE, 1, "gone", 2.0, is_active=False),
|
||||
)
|
||||
|
||||
assert await count_points(repository, tenant_id=TENANT) == 1
|
||||
assert await count_points(repository, tenant_id=TENANT, include_inactive=True) == 2
|
||||
Reference in New Issue
Block a user