diff --git a/CHANGELOG.md b/CHANGELOG.md index fa343c1..97ea198 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,34 @@ ## Unreleased +## v5.10.9 (2026-08-08) + +Three findings reported from the openwebindex.eu dashboard, measured against a running 5.10.8 server. + +### Bug Fixes + +- **http**: `GET /remote/ls` discarded the descriptive record it already held. Each row was hand-built from eleven fixed keys while `ds.metadata` carried 38–43, including `creators`, `descriptions`, `publisher`, `publicationYear`, `rightsList` and `relatedIdentifiers`. The data was retrieved, carried onto the `Dataset`, and dropped one function before the response. See the new `?include=metadata` below. +- **core**: `DatasetMetadata` did not behave as the mapping it advertises. `keys()` listed six fields (`startDate`, `endDate`, `contributor`, `lastChanged`, `rightsIdentifier`, `rightsList`) that `__getitem__` refused with `KeyError`, so `dict(md)`, `{**md}` and `md.items()` all failed on exactly the records that legitimately lack a field — a corpus has no `startDate`. `.get()` already returned `None` for those. Advertised-but-absent keys now return `None`, agreeing with `.get()`; an unadvertised key still raises. **This was reachable from owilix itself** — `cli/_common/output.py` calls `dict(d.metadata)`. +- **core**: `RightsListMetadata.short_repr()` indexed `r['rightsIdentifier']`, which DataCite does not require. The resulting `KeyError` was swallowed by `.get()`, so **`metadata.get("rightsList")` returned `None` for exactly the datasets that publish a real licence** — the curated corpora. It now falls back `rightsIdentifier` → `rights` → `rightsUri`, and tolerates a malformed entry. + +### Features + +- **http**: `GET /remote/ls?include=metadata` (also on `/local/ls`) attaches the full descriptive record with **native values**, not display renderings — all eight creators with ORCIDs and affiliations rather than `"Lukas Gienapp+7"`, the untruncated description, the licence with its URI. DataCite fields are nested under a `datacite` key. Opt-in, because an unfiltered listing is ~1,900 rows. + + Absent fields are **omitted, never null-filled**, so "not published" stays distinguishable from "not carried". Nothing keys on a fixed key set or count — one deployment carries six distinct shapes of 33–43 keys — and a record whose metadata cannot be rendered reports `metadata_error` while the rest of the page still answers. **No re-pull or migration required.** + +- **http**: `GET /remote/ls` and `/local/ls` now declare response models (`DatasetListResponse`, `DatasetRecord`, `IncompleteSource`). Previously 0 of 46 200-responses declared a schema, which is why dropping thirty metadata fields was invisible to the OpenAPI, to generated clients and to schema-level tests. Field validators coerce rather than reject, so declaring types cannot make a listing fail on one odd record. + +### Renames + +- **http**: `require_auth` → `require_upstream_session`, with the old name kept as an alias so existing imports and `dependency_overrides` continue to work. The old name promised caller authentication it never performed: it checks whether *the server* holds a LEXIS credential and never inspects the request. The 401 body keeps `error: "auth_required"` for compatibility and gains a `message` saying so. `docs/source/http.md` now states plainly that **anything reaching the port can drive every route**, including `POST /remote/remove` and `/local/rm`. + +### Known limits, unchanged + +- `/health` is a literal and cannot fail; there is no readiness probe that goes unhealthy when the remotes cannot be listed. +- `GET /remote/ls` pagination re-walks the backend per page and nothing caches it, so paging is O(pages x full listing). + + ## v5.10.8 (2026-08-07) Closes the last two open items in the summarize/integrity plan. diff --git a/Readme.md b/Readme.md index 1d3a19d..9c4cdc1 100644 --- a/Readme.md +++ b/Readme.md @@ -1,6 +1,6 @@ # OWILIX - Open Web Index CLI -[![Version](https://img.shields.io/badge/version-5.10.8-blue.svg)](https://openwebsearcheu-public.pages.it4i.eu/owi-cli/) +[![Version](https://img.shields.io/badge/version-5.10.9-blue.svg)](https://openwebsearcheu-public.pages.it4i.eu/owi-cli/) [![Python](https://img.shields.io/badge/python-3.11+-green.svg)](https://www.python.org/) [![License](https://img.shields.io/badge/license-Apache_2.0-orange.svg)](http://www.apache.org/licenses/LICENSE-2.0) diff --git a/docs/changes.md b/docs/changes.md index a901260..cbcfaa0 100644 --- a/docs/changes.md +++ b/docs/changes.md @@ -24,6 +24,28 @@ Brief description of what was accomplished. ## Unreleased +### main @ v5.10.9 - 2026-08-08 + +#### The descriptive record, and two accessors that lied about it + +**Summary**: Asks 1, 2 and 3a/3b from the openwebindex.eu dashboard. Each was reproduced against live data before being changed. + +**Changes**: +- `?include=metadata` on `/remote/ls` and `/local/ls`, sourced from `as_json_dict()`. Verified against the real `German Commons` record: 8 creators with ORCIDs where `get()` said `Lukas Gienapp+7`, a 1,385-character description where `get()` truncated, and a licence where `get()` returned `None`. Swept `as_json_dict()` across all **1,864 LEXIS records: zero failures**, across all six shape variants. +- `DatasetMetadata.__getitem__` now agrees with `.get()` for advertised-but-absent keys, so `dict(md)` works. Six keys raised before. +- `RightsListMetadata.short_repr()` no longer assumes `rightsIdentifier`. This is why `rightsList` was missing from the flat projection: the `KeyError` was swallowed by `.get()`. +- Response models declared for the listing endpoints, with coercing validators so typing cannot turn one odd sidecar into a 500. +- `require_auth` renamed to `require_upstream_session`, alias kept, wire error code kept, docs made explicit about the absence of caller authentication. + +**Two mistakes caught during the work, both worth recording**: +- Declaring the response model initially emitted `metadata: null` and `metadata_error: null` on every row — precisely the null-filling the report asked us to avoid. Fixed with `response_model_exclude_unset=True`, verified by diffing the default response. +- Two workflow tests written yesterday pinned a fixture date and queried a relative `days=7` window. They passed on the day written and failed the next morning, looking like a regression in unrelated code. Fixtures are now relative to now. + +**Breaking Changes**: None. `include` is opt-in, the default listing is byte-identical, `require_auth` still resolves, and the 401 code is unchanged. + +**Not addressed** (reported, deferred by the reporter): `/health` cannot fail and there is no readiness probe; `/remote/ls` pagination re-walks the backend per page with no cache. + + ### main @ v5.10.8 - 2026-08-07 #### The last two plan items: --since, and reading verification back diff --git a/docs/source/http.md b/docs/source/http.md index 801b52c..a20ab06 100644 --- a/docs/source/http.md +++ b/docs/source/http.md @@ -5,9 +5,22 @@ service demos. It is a preview API in `v5.5.0`: the endpoint surface is useful, but the server should be treated as a local service unless an external access-control layer is placed in front of it. -Do not expose `owilix http serve` on a public or shared network as-is. The -current built-in auth check verifies that the server process has OWILIX -credentials available; it does not authenticate each HTTP caller. +**There is no caller authentication.** The dependency guarding 25 routes — +`require_upstream_session`, formerly named `require_auth` — checks only whether +*the server* holds a usable LEXIS credential. It never inspects the request. +There are no `securitySchemes` in the OpenAPI document and no operation declares +`security`. + +Two consequences worth stating plainly: + +- A `401` means **the server is logged out**, not *you are not allowed*. +- **Anything that can reach the port can drive every route**, including + `POST /remote/remove`, `POST /remote/push` and `POST /local/rm`, under the + server's single shared identity. + +Do not expose `owilix http serve` on a public or shared network as-is. Run it on +localhost, or behind a reverse proxy, tunnel or gateway that authenticates +callers. ## Install @@ -113,6 +126,9 @@ Query endpoints: - `POST /query/slice` - `POST /query/stream` +`GET /remote/ls` and `GET /local/ls` accept `?include=metadata` — see +[Full descriptive metadata](#full-descriptive-metadata). + Derived artifact endpoints: - `GET /remote/catalog/summary` — declared vs observed across a scope @@ -196,6 +212,63 @@ owilix http serve For local-only testing, `OWILIX_HTTP_S3_NO_AUTH=true` disables S3 facade credential checks. +## Full descriptive metadata + +`GET /remote/ls` returns eleven flat keys per dataset. Those come from +`metadata.get()`, which returns a **display rendering, not the stored value**: + +| field | flat key gives | actual value | +| --- | --- | --- | +| `creators` | `"Lukas Gienapp+7"` | 8 author objects, with ORCIDs and affiliations | +| `descriptions` | ellipsis-truncated | full text | +| `publisher` | `{"name": "CORAL Project"}` | same | +| `rightsList` | *(absent)* | licence with `rights` and `rightsUri` | + +The `+7` means "and seven more" — a page cannot recover eight authors from it. +For anything that needs the record rather than a table row, ask for it: + +```bash +curl "http://127.0.0.1:8080/remote/ls?specifier=lexis/collectionName=corpora&include=metadata" +``` + +**Shape.** `metadata` carries the record as stored, with DataCite fields nested +under a `datacite` key: + +```jsonc +{ + "id": "…", "title": "German Commons", "collectionName": "corpora", + "metadata": { + "datacite": { + "creators": [{"name": "Lukas Gienapp", "givenName": "Lukas", + "nameIdentifiers": [{"nameIdentifier": "0000-0001-…", + "nameIdentifierScheme": "ORCID"}], + "affiliation": [{"name": "University of Kassel"}]}, …], + "publisher": {"name": "CORAL Project"}, + "publicationYear": 2025, + "rightsList": [{"rights": "Open Data Commons Attribution License v1.0", + "rightsUri": "https://opendatacommons.org/licenses/by/1-0/"}], + "relatedIdentifiers": [{"relatedIdentifier": "https://huggingface.co/…", + "relatedIdentifierType": "URL"}] + }, + "schema": "http://datacite.org/schema/kernel-4" + } +} +``` + +**Why opt-in.** An unfiltered listing is ~1,900 records and most callers want the +flat keys. `include` takes a comma-separated list; unknown values are ignored +rather than rejected. + +**Absent fields are omitted, never null-filled.** Mirrors carry several metadata +shapes — one deployment has six distinct key sets between 33 and 43 keys, with +`relatedIdentifiers` on only 65% of datasets. Nothing keys on a fixed key set or +count, `schema` is read as the version marker where present and tolerated where +absent, and **a record whose metadata cannot be rendered reports +`metadata_error` while the rest of the page still answers** — one malformed +sidecar does not fail the listing. + +No re-pull or migration is required: old sidecars work as they are. + ## Derived Artifacts What `summarize`, `verify` and `reindex` write beside the data, readable without diff --git a/owilix/_version.py b/owilix/_version.py index 1a54dfd..5571a43 100644 --- a/owilix/_version.py +++ b/owilix/_version.py @@ -1,3 +1,3 @@ # Version is set here and imported elsewhere -__version__ = "5.10.8" -__version_tuple__ = (5, 10, 8) +__version__ = "5.10.9" +__version_tuple__ = (5, 10, 9) diff --git a/owilix/core/models/dataset.py b/owilix/core/models/dataset.py index 10c6f2e..e774962 100644 --- a/owilix/core/models/dataset.py +++ b/owilix/core/models/dataset.py @@ -1047,10 +1047,26 @@ class RightsListMetadata(MetadataField): return {} def short_repr(self): - if self.rightsList: - rights = ";".join([r['rightsIdentifier'] for r in self.rightsList]) - return rights - return '' + # Not every licence carries a rightsIdentifier. DataCite requires only + # `rights`; an entry may have `rights` + `rightsUri` and no identifier, + # which is what the curated corpora actually publish (e.g. "Open Data + # Commons Attribution License v1.0"). + # + # Indexing r['rightsIdentifier'] raised KeyError for those, and because + # `.get()` swallows KeyError, `metadata.get("rightsList")` returned + # **None** -- the licence was silently dropped for exactly the datasets + # that have a real one. Fall back through identifier -> rights -> uri. + if not self.rightsList: + return '' + labels = [] + for entry in self.rightsList: + if not isinstance(entry, dict): + labels.append(str(entry)) + continue + label = entry.get('rightsIdentifier') or entry.get('rights') or entry.get('rightsUri') + if label: + labels.append(str(label)) + return ";".join(labels) @staticmethod def get_default(): @@ -1476,6 +1492,18 @@ class DatasetMetadata: else: return _md else: + # `keys()` advertises the mapped fields whose mapping is flagged + # public, whether or not this record carries one. Raising here for + # those broke the mapping contract -- `k in md.keys()` did not imply + # `md[k]` works -- so `dict(md)`, `{**md}` and `md.items()` all + # failed on exactly the records that legitimately lack a field (a + # corpus has no startDate). `.get()` already returned None for them. + # + # Advertised-but-absent now returns None, agreeing with `.get()`. + # A key nobody advertises still raises, which is what a mapping + # should do. + if key in self._key_mapping and self._key_mapping[key][3]: + return None raise KeyError(key) def __setitem__(self, key, value): diff --git a/owilix/http_api/server.py b/owilix/http_api/server.py index 22b6756..ee0d063 100644 --- a/owilix/http_api/server.py +++ b/owilix/http_api/server.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import os import json import shutil @@ -18,10 +19,12 @@ from owilix.core.utils import split_query_access from owilix.http_api.auth import DeviceAuthManager from owilix.http_api.jobs import JobManager +logger = logging.getLogger("owilix") + try: from fastapi import Depends, FastAPI, HTTPException, Query from fastapi.responses import JSONResponse - from pydantic import BaseModel, Field + from pydantic import BaseModel, Field, field_validator except ImportError as exc: # pragma: no cover - exercised by CLI import path raise RuntimeError("Install owilix with the 'http' extra to use the HTTP server.") from exc @@ -254,6 +257,98 @@ class PluginRunRequest(BaseModel): kwargs: dict[str, Any] = Field(default_factory=dict) + +class DatasetRecord(BaseModel): + """One row of a dataset listing. + + The flat fields are a projection for tables. `metadata` carries the record + itself and is present only with `?include=metadata`; see `_dataset_record` + for why the two differ. + """ + + model_config = {"extra": "allow"} + + id: str | None = None + title: str | None = None + collectionName: str | None = None + dataCenter: str | None = None + zone: str | None = None + access: str | None = None + startDate: str | None = None + endDate: str | None = None + size: int | None = None + fileCount: int | None = None + objectCount: int | None = None + path: str | None = None + metadata: dict[str, Any] | None = Field( + default=None, + description="Full descriptive record, native values. DataCite fields are " + "nested under 'datacite'. Fields absent from the sidecar are " + "omitted rather than emitted as null, so 'not published' stays " + "distinguishable from 'not carried'. Only with ?include=metadata.", + ) + metadata_error: str | None = Field( + default=None, + description="Set when this record's metadata could not be rendered. The " + "listing still returns; one malformed sidecar does not fail it.", + ) + + @field_validator("id", "title", "collectionName", "dataCenter", "zone", "access", + "startDate", "endDate", "path", mode="before") + @classmethod + def _as_text(cls, value): + """Coerce to text rather than reject. + + Declaring types must not make the listing more fragile than it was. A + sidecar carrying an unexpected type in one of these fields would + otherwise fail response validation and turn the whole page into a 500 -- + one malformed record taking out the listing, which is the failure this + API has spent several releases removing. + """ + if value is None or isinstance(value, str): + return value + return str(value) + + @field_validator("size", "fileCount", "objectCount", mode="before") + @classmethod + def _as_number(cls, value): + """Coerce to int where possible, otherwise None. Never raise.""" + if value is None or isinstance(value, int): + return value + try: + return int(value) + except (TypeError, ValueError): + return None + + +class IncompleteSource(BaseModel): + """Why part of a listing is missing.""" + + source: str + reason: str = Field(description="auth | unreachable | timeout | error | partial_read") + message: str = "" + + +class DatasetListResponse(BaseModel): + """A page of datasets, and whether it is the whole answer.""" + + model_config = {"extra": "allow"} + + count: int + total: int + offset: int + limit: int | None = None + has_more: bool + datasets: list[DatasetRecord] + partial: bool = Field( + default=False, + description="True when at least one source could not be listed in full. " + "An empty result with partial=true is 'broken', not 'empty'.", + ) + failed: list[str] = Field(default_factory=list) + incomplete: list[IncompleteSource] = Field(default_factory=list) + + def _manager() -> OWIlixManager: owi_path = OWILIXEnv.values.owi_path config_path = os.getenv("OWILIX_CONFIG") or os.path.join(owi_path, "owilix.cfg") @@ -277,12 +372,57 @@ def get_job_manager() -> JobManager: return JobManager() -def require_auth(auth: DeviceAuthManager = Depends(get_auth_manager)) -> None: +def require_upstream_session(auth: DeviceAuthManager = Depends(get_auth_manager)) -> None: + """Require that **the server** holds a usable upstream (LEXIS) credential. + + This is not caller authentication and never has been. It inspects the + server's own refresh token and never looks at the request: there is no + caller identity, no API key, no `securitySchemes`. A 401 from here means + *the server is logged out*, not *you are not allowed*. + + Consequently **anything that can reach the port can drive every route**, + including `POST /remote/remove`, `/remote/push` and `/local/rm`, under one + shared identity. Run the server on localhost or behind an access-control + layer; see `docs/source/http.md`. + + Renamed from `require_auth` in v5.10.9 because the old name promised + something it did not do. The old name remains as an alias so existing + imports and `dependency_overrides` keep working. + """ if not auth.has_refresh_token(): - raise HTTPException(status_code=401, detail={"error": "auth_required"}) + raise HTTPException( + status_code=401, + # The code stays `auth_required`: it is on the wire and callers + # branch on it. The clarification goes in `message`, where adding it + # breaks nobody. + detail={ + "error": "auth_required", + "message": "The server has no usable LEXIS credential. This is not a " + "statement about the caller: this API does not authenticate callers.", + }, + ) + + +#: Backwards-compatible alias. Kept so that existing imports and FastAPI +#: `dependency_overrides` continue to resolve; prefer the explicit name. +require_auth = require_upstream_session -def _dataset_record(ds: Any) -> dict[str, Any]: +def _dataset_record(ds: Any, include_metadata: bool = False) -> dict[str, Any]: + """One listing row. + + The eleven flat keys are a *projection*: `metadata.get()` returns a display + rendering, not the stored value -- `creators` comes back as + `'Lukas Gienapp+7'`, where the `+7` is seven authors a caller cannot + recover. Those keys are fine for a table and wrong for anything that needs + the record itself. + + `include_metadata` attaches the full descriptive record from + `as_json_dict()`, which returns native values: all eight creators with their + ORCIDs, the licence with its URI, untruncated descriptions. It is opt-in + because the unfiltered listing is ~1,900 rows and most callers want the + flat keys only. + """ metadata = ds.metadata record = { "id": metadata.get("internalID") or metadata.get("id"), @@ -300,9 +440,27 @@ def _dataset_record(ds: Any) -> dict[str, Any]: path = getattr(ds, "path", None) if path is not None: record["path"] = path + + if include_metadata: + # One malformed sidecar must not fail the listing -- the mirror holds + # six distinct metadata shapes with 33-43 keys, and other installations + # have mirrors nobody will ever migrate. A record that cannot be + # rendered says so and the rest of the page still answers. + try: + record["metadata"] = ds.metadata.as_json_dict() + except Exception as e: # pragma: no cover - defensive + logger.warning(f"Could not render metadata for {record.get('id')}: {e}") + record["metadata_error"] = str(e) return record +def _parse_include(include: str | None) -> set[str]: + """Parse `?include=a,b`. Unknown values are ignored rather than rejected.""" + if not include: + return set() + return {part.strip() for part in str(include).split(",") if part.strip()} + + def _page( records: list[dict[str, Any]], offset: int, @@ -720,13 +878,19 @@ def create_app() -> FastAPI: raise HTTPException(status_code=404, detail={"error": "unknown_job"}) return job.public_dict() - @app.get("/local/ls") + @app.get("/local/ls", response_model_exclude_unset=True) def local_ls( specifier: str = Query("all"), offset: int = Query(0, ge=0), limit: int | None = Query(None, ge=1), + include: str | None = Query( + None, + description="Comma-separated extras. 'metadata' attaches the full " + "descriptive record (DataCite fields nested under 'datacite') " + "with native values rather than display renderings.", + ), manager: OWIlixManager = Depends(get_manager), - ) -> dict[str, Any]: + ) -> DatasetListResponse: spec = manager.parse_specifier(specifier) access, query = split_query_access(spec.get("query")) datasets = manager.local.list( @@ -735,7 +899,8 @@ def create_app() -> FastAPI: duration=spec.get("duration") or 0, query=query, ) - records = [_dataset_record(ds) for ds in datasets] + records = [_dataset_record(ds, include_metadata="metadata" in _parse_include(include)) + for ds in datasets] return _page(records, offset=offset, limit=limit, listing=datasets) @app.post("/local/free") @@ -1209,13 +1374,20 @@ def create_app() -> FastAPI: kwargs=payload.kwargs, ) - @app.get("/remote/ls", dependencies=[Depends(require_auth)]) + @app.get("/remote/ls", dependencies=[Depends(require_auth)], + response_model_exclude_unset=True) def remote_ls( specifier: str = Query("all"), offset: int = Query(0, ge=0), limit: int | None = Query(None, ge=1), + include: str | None = Query( + None, + description="Comma-separated extras. 'metadata' attaches the full " + "descriptive record (DataCite fields nested under 'datacite') " + "with native values rather than display renderings.", + ), manager: OWIlixManager = Depends(get_manager), - ) -> dict[str, Any]: + ) -> DatasetListResponse: spec = manager.parse_specifier(specifier) access, query = split_query_access(spec.get("query")) datasets = manager.remote_data.list( @@ -1225,7 +1397,8 @@ def create_app() -> FastAPI: duration=spec.get("duration") or 0, query=query, ) - records = [_dataset_record(ds) for ds in datasets] + records = [_dataset_record(ds, include_metadata="metadata" in _parse_include(include)) + for ds in datasets] return _page(records, offset=offset, limit=limit, listing=datasets) @app.post("/remote/search", dependencies=[Depends(require_auth)]) diff --git a/pyproject.toml b/pyproject.toml index 9b52945..78c2eec 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "hatchling.build" [project] name = "owilix" -version = "5.10.8" +version = "5.10.9" description = "OWILIX - the Command Line Interface for slicing and consuming the Open Web Index. " readme = "Readme.md" license = { text = "MIT" } diff --git a/tests/owilix/core/models/__init__.py b/tests/owilix/core/models/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/owilix/core/models/test_metadata_mapping.py b/tests/owilix/core/models/test_metadata_mapping.py new file mode 100644 index 0000000..a64071d --- /dev/null +++ b/tests/owilix/core/models/test_metadata_mapping.py @@ -0,0 +1,107 @@ +"""DatasetMetadata must behave as the mapping it advertises. + +`keys()` listed fields that `__getitem__` refused, so `dict(md)`, `{**md}` and +`md.items()` all raised on exactly the records that legitimately lack a field -- +a corpus has no startDate. `.get()` already returned None for those, so the two +accessors disagreed. + +Reported from the openwebindex.eu dashboard, whose obvious implementation of +"give me the whole record" is `dict(ds.metadata)` -- and owilix's own +`cli/_common/output.py` does the same, so this was reachable from the CLI too. +""" +import pytest + +from owilix.core.models.dataset import DatasetMetadata + + +def _md(**overrides): + base = {"id": "ds-1", "title": "German Commons", + "collectionName": "corpora", "access": "public"} + base.update(overrides) + return DatasetMetadata(base) + + +class TestMappingContract: + def test_every_advertised_key_resolves(self): + """`k in md.keys()` must imply `md[k]` works.""" + md = _md() + unresolvable = [] + for key in md.keys(): + try: + md[key] + except KeyError: + unresolvable.append(key) + assert unresolvable == [] + + def test_dict_round_trip(self): + md = _md() + assert isinstance(dict(md), dict) + assert isinstance({**md}, dict) + assert isinstance(list(md.items()), list) + + def test_getitem_agrees_with_get_for_an_absent_known_field(self): + """A corpus has no startDate; absence is correct, disagreement was not.""" + md = _md() + assert md.get("startDate") is None + assert md["startDate"] is None + + def test_an_unknown_key_still_raises(self): + """The fix must not turn the mapping into a defaultdict.""" + with pytest.raises(KeyError): + _md()["no_such_field_anywhere"] + + def test_a_present_field_is_unaffected(self): + assert _md()["title"] == "German Commons" + + +class TestRightsShortRepr: + """DataCite requires `rights`; `rightsIdentifier` is optional. + + Tested against `RightsListMetadata` directly: constructing a whole + `DatasetMetadata` from a bare dict runs `fill_defaults`, which injects the + OWIL licence and would mask what is being checked. + """ + + def _rights(self, entries): + from owilix.core.models.dataset import RightsListMetadata + + field = RightsListMetadata() + field.rightsList = entries + return field + + def test_a_licence_without_an_identifier_is_not_dropped(self): + """This raised KeyError before, so the licence vanished. + + `short_repr` indexed `r['rightsIdentifier']`, and because `.get()` + swallows KeyError, `metadata.get("rightsList")` answered **None** for + exactly the datasets that publish a real licence. + """ + field = self._rights([{"rights": "Open Data Commons Attribution License v1.0", + "rightsUri": "https://opendatacommons.org/licenses/by/1-0/"}]) + + assert field.short_repr() == "Open Data Commons Attribution License v1.0" + + def test_an_identifier_is_preferred_when_present(self): + field = self._rights([{"rights": "Open Web Index License V1.0", + "rightsIdentifier": "OWIL V1.0"}]) + + assert field.short_repr() == "OWIL V1.0" + + def test_a_uri_only_licence_falls_back_to_the_uri(self): + field = self._rights([{"rightsUri": "https://example.test/licence"}]) + + assert field.short_repr() == "https://example.test/licence" + + def test_several_licences_are_joined(self): + field = self._rights([{"rightsIdentifier": "A"}, {"rights": "B"}]) + + assert field.short_repr() == "A;B" + + def test_no_licence_is_empty_not_an_error(self): + assert self._rights([]).short_repr() == "" + + def test_a_malformed_entry_does_not_raise(self): + """One odd record must not take out a listing.""" + field = self._rights(["not-a-dict", {"rights": "B"}]) + + assert field.short_repr() == "not-a-dict;B" diff --git a/tests/owilix/core/tasks/test_workflow.py b/tests/owilix/core/tasks/test_workflow.py index c12db16..0107ff0 100644 --- a/tests/owilix/core/tasks/test_workflow.py +++ b/tests/owilix/core/tasks/test_workflow.py @@ -18,16 +18,26 @@ from owilix.core.tasks import workflow as wf from owilix.core.types import ErrorType, ExitCode +#: Relative to now, deliberately. +#: +#: These fixtures were originally pinned to fixed dates and queried with +#: `days=7`. That passes on the day it is written and fails once the fixture +#: falls outside the window -- a test that expires overnight while looking +#: unrelated to whatever change is in flight. +_RECENT = (datetime.now(timezone.utc) - timedelta(hours=6)).isoformat() +_RECENT_END = (datetime.now(timezone.utc) - timedelta(hours=5)).isoformat() + + def _execution(**overrides): base = { "dag_id": "openwebsearch_OpenWebIndex_Main_V2_IT4I", - "dag_run_id": "scheduled__2026-08-01T01:00:00+00:00", + "dag_run_id": "scheduled__run", "task_id": "count_warc_files", "operator": "_ShortCircuitDecoratedOperator", "state": "success", - "start_date": "2026-08-01T01:00:04.115311+00:00", - "end_date": "2026-08-01T01:00:05.410557+00:00", - "execution_date": "2026-07-31T01:00:00+00:00", + "start_date": _RECENT, + "end_date": _RECENT_END, + "execution_date": _RECENT, "duration": 1.295246, "try_number": 1, "max_tries": 0, diff --git a/tests/owilix/http/test_routes.py b/tests/owilix/http/test_routes.py index b4112e9..04ab9a9 100644 --- a/tests/owilix/http/test_routes.py +++ b/tests/owilix/http/test_routes.py @@ -436,7 +436,10 @@ def test_protected_endpoint_requires_auth(): response = client.get("/remote/ls") assert response.status_code == 401 - assert response.json()["detail"] == {"error": "auth_required"} + detail = response.json()["detail"] + assert detail["error"] == "auth_required" + # The code is unchanged for compatibility; the message says what a 401 means. + assert "does not authenticate callers" in detail["message"] def test_remote_ls_endpoint_with_fake_manager(): @@ -856,3 +859,156 @@ class TestCollectionVerificationEndpoint: assert body["partial"] is True assert body["failed"] == ["owi-up"] + + +class TestIncludeMetadata: + """`/remote/ls?include=metadata` must return values, not display renderings. + + The eleven flat keys come from `metadata.get()`, which renders for display: + `creators` becomes `'Lukas Gienapp+7'`, where the `+7` is seven authors a + page cannot recover. Adding `md.get("creators")` would have looked like a + fix and shipped lossy data. + """ + + def _dataset(self, native=None, raises=False): + from unittest.mock import MagicMock + + meta = {"internalID": "ds-1", "id": "ds-1", "title": "German Commons", + "collectionName": "corpora", "creators": "Lukas Gienapp+7"} + dataset = MagicMock() + dataset.metadata.get.side_effect = lambda k, d=None: meta.get(k, d) + if raises: + dataset.metadata.as_json_dict.side_effect = ValueError("malformed sidecar") + else: + dataset.metadata.as_json_dict.return_value = native or {} + dataset.path = "/store/public/corpora/ds-1" + dataset.dataCenter = "lexis" + dataset.zone = "IT4ILexisV2" + dataset.access = "public" + return dataset + + def _client(self, datasets): + from owilix.core.listing import DatasetListing + + manager = FakeManager() + manager.remote_data = SimpleNamespace(list=lambda *a, **k: DatasetListing(datasets)) + manager.parse_specifier = lambda s: { + "data_center": None, "query": {"access": "public"}, "day": None, "duration": 0 + } + return make_client(manager=manager) + + def _native(self): + return { + "datacite": { + "creators": [{"name": f"Author {i}"} for i in range(8)], + "publisher": {"name": "CORAL Project"}, + "publicationYear": 2025, + "rightsList": [{"rights": "Open Data Commons Attribution License v1.0", + "rightsUri": "https://opendatacommons.org/licenses/by/1-0/"}], + }, + "schema": "http://datacite.org/schema/kernel-4", + } + + def test_metadata_is_absent_by_default(self): + """Opt-in: the unfiltered listing is ~1,900 rows.""" + body = self._client([self._dataset(self._native())]).get("/remote/ls").json() + + assert "metadata" not in body["datasets"][0] + + def test_no_null_filled_fields_by_default(self): + """'not published' must stay distinguishable from 'not carried'.""" + record = self._client([self._dataset(self._native())]).get("/remote/ls").json()["datasets"][0] + + assert "metadata" not in record + assert "metadata_error" not in record + + def test_include_metadata_returns_native_values(self): + body = self._client([self._dataset(self._native())]).get( + "/remote/ls?include=metadata").json() + + datacite = body["datasets"][0]["metadata"]["datacite"] + assert len(datacite["creators"]) == 8, "all eight authors, not 'Author 0+7'" + assert datacite["publisher"] == {"name": "CORAL Project"} + assert datacite["rightsList"][0]["rightsUri"].startswith("https://") + + def test_the_flat_keys_are_unchanged_when_metadata_is_included(self): + record = self._client([self._dataset(self._native())]).get( + "/remote/ls?include=metadata").json()["datasets"][0] + + assert record["title"] == "German Commons" + assert record["collectionName"] == "corpora" + + def test_absent_fields_are_omitted_rather_than_nulled(self): + """Six metadata shapes exist across a mirror; do not key on a fixed set.""" + sparse = {"datacite": {"titles": [{"title": "Minimal"}]}} + record = self._client([self._dataset(sparse)]).get( + "/remote/ls?include=metadata").json()["datasets"][0] + + assert record["metadata"]["datacite"] == {"titles": [{"title": "Minimal"}]} + assert "creators" not in record["metadata"]["datacite"] + + def test_one_malformed_record_does_not_fail_the_listing(self): + good = self._dataset(self._native()) + bad = self._dataset(raises=True) + body = self._client([good, bad]).get("/remote/ls?include=metadata").json() + + assert body["total"] == 2 + errored = [d for d in body["datasets"] if "metadata_error" in d] + assert len(errored) == 1 and "malformed" in errored[0]["metadata_error"] + + def test_an_unknown_include_value_is_ignored_not_rejected(self): + response = self._client([self._dataset(self._native())]).get("/remote/ls?include=nonsense") + + assert response.status_code == 200 + assert "metadata" not in response.json()["datasets"][0] + + def test_include_accepts_a_comma_separated_list(self): + body = self._client([self._dataset(self._native())]).get( + "/remote/ls?include=foo,metadata").json() + + assert "metadata" in body["datasets"][0] + + +class TestDeclaredResponseModels: + """3a: a hand-built dict made dropping thirty fields invisible.""" + + def test_the_listing_declares_a_schema(self): + spec = server.create_app().openapi() + schema = spec["paths"]["/remote/ls"]["get"]["responses"]["200"]["content"]["application/json"]["schema"] + + assert schema == {"$ref": "#/components/schemas/DatasetListResponse"} + + def test_the_record_schema_names_metadata(self): + spec = server.create_app().openapi() + record = spec["components"]["schemas"]["DatasetRecord"]["properties"] + + assert "metadata" in record + assert "title" in record and "fileCount" in record + + def test_the_partial_contract_is_declared(self): + spec = server.create_app().openapi() + listing = spec["components"]["schemas"]["DatasetListResponse"]["properties"] + + assert "partial" in listing and "failed" in listing + + +class TestUpstreamSessionNaming: + """3b: the old name promised caller authentication it never performed.""" + + def test_the_alias_still_resolves(self): + from owilix.http_api.server import require_auth, require_upstream_session + + assert require_auth is require_upstream_session + + def test_the_401_says_it_is_about_the_server_not_the_caller(self): + from owilix.http_api.server import get_auth_manager + + app = server.create_app() + app.dependency_overrides[get_auth_manager] = lambda: SimpleNamespace( + has_refresh_token=lambda: False + ) + response = TestClient(app).get("/remote/ls") + + assert response.status_code == 401 + assert response.json()["detail"]["error"] == "auth_required" + assert "does not authenticate callers" in response.json()["detail"]["message"] diff --git a/uv.lock b/uv.lock index 0288875..3ead7ac 100644 --- a/uv.lock +++ b/uv.lock @@ -1102,7 +1102,7 @@ wheels = [ [[package]] name = "owilix" -version = "5.10.8" +version = "5.10.9" source = { editable = "." } dependencies = [ { name = "ciff-toolkit" },