From 786c45b7e2ee36696aab33a0c449c34b20893dc9 Mon Sep 17 00:00:00 2001 From: mgrani Date: Fri, 7 Aug 2026 20:57:28 +0200 Subject: [PATCH] feat(remote): a rebuildable collection index, and remote reindex M4. `_index.parquet` per collection plus `owi remote reindex`, shipped as one change because an index without a rebuild path is a liability rather than a cache. A listing goes from one GET per dataset to one LIST plus one range read -- on a 585-dataset collection, 585 requests replaced by 2. Most of the work is in the ways it refuses to be trusted, because this is the milestone that puts a derived artifact in the read path, and that is the exact place this codebase has repeatedly turned a failure into a confident wrong answer. - The sidecars stay authoritative. The index is derived from them and always rebuildable from them; if they disagree the sidecars win. Deleting every _index.parquet costs speed and nothing else. - A stale index is never served. The index stores a fingerprint over every sidecar's name, size and mtime, recomputed on read from one cheap ls. Chosen over a bare object count because it also catches a resized or re-uploaded sidecar at identical cost -- the listing already returns the details. - Absent, unreadable, stale, or written by a newer owilix: all four walk, and log why. read_index raises IndexUnusable rather than returning None so the one correct response is impossible to forget at the call site. - One unreadable sidecar does not cost the index, matching the tolerance list() gained in 5.9.0 rather than reintroducing the bug it fixed. - The fingerprint excludes _index.parquet itself; an index that invalidated itself by existing would never be usable. Parquet rather than SQLite: duckdb and pyarrow are already dependencies and DuckDB queries Parquet in place over HTTP range requests with predicate pushdown. SQLite over an object store has no locking, needs the whole file, and rewrites all of it per update -- on a medium with no byte-range writes, two writers silently lose each other's work. Flattened columns carry the predicates; metadata_json carries the sidecar, so a dataset from the index is identical to one from disk. Verified end to end rather than by unit test alone: walk and index return the same results; adding a dataset without reindexing gives 4 not 3; a corrupted index gives 4; a deleted index gives 4. Known limit, documented: the fingerprint cannot see an edit preserving both size and mtime. That is why reindex exists and why the sidecars stay authoritative. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017phbSd6D8u4iEQsCAPEw6s --- CHANGELOG.md | 29 ++++ Readme.md | 2 +- docs/changes.md | 21 +++ docs/source/details/verify.md | 63 +++++++++ owilix/_version.py | 4 +- owilix/cli/remote.py | 43 ++++++ owilix/core/index.py | 237 ++++++++++++++++++++++++++++++++ owilix/core/repository/file.py | 24 ++++ owilix/core/tasks/verify.py | 86 ++++++++++++ pyproject.toml | 2 +- tests/owilix/core/test_index.py | 232 +++++++++++++++++++++++++++++++ uv.lock | 2 +- 12 files changed, 740 insertions(+), 5 deletions(-) create mode 100644 owilix/core/index.py create mode 100644 tests/owilix/core/test_index.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a4cbbf..bd2c642 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,35 @@ ## Unreleased +## v5.10.4 (2026-08-07) + +M4 of the summarize/integrity plan: a derived, rebuildable per-collection index. This is the milestone that puts an artifact in the read path, so most of the work is in the ways it refuses to be trusted. + +### Features + +- **remote**: New `owi remote reindex ` writes `_index.parquet` into each collection directory, built from the dataset sidecars. +- **remote**: `remote ls` reads the index when one is present and current, turning a listing from **one GET per dataset** into **one LIST plus one range read**. On a 585-dataset collection that is 585 requests replaced by 2. +- **remote**: The index carries flattened columns (`id`, `title`, `collectionName`, `dataCenter`, `access`, `zone`, dates, `fileCount`, `objectCount`, `size`) for DuckDB predicate pushdown, plus `metadata_json` holding the whole sidecar — so a dataset read from the index is identical to one read from disk. The flattened columns are for querying; content always comes from `metadata_json`. + +### Safety + +- **The sidecars stay authoritative.** The index is a cache, always rebuildable from them, and if the two disagree the sidecars win. Deleting every `_index.parquet` costs speed and nothing else. +- **A stale index is never served.** The index stores a fingerprint of the collection — every sidecar's name, size and mtime — recomputed on read from one cheap `ls`. Any mismatch falls back to the walk and logs why. +- **Four failure paths, one behaviour: walk.** Absent, unreadable/corrupt, stale, or written by a newer owilix than this one. An index in the read path that can answer *less* than the source is the same failure as a listing reporting `total: 0` for a full bucket — self-inflicted this time, which is why the fallback is blunt. +- **One unreadable sidecar does not cost the index**, matching `list()`'s tolerance rather than reintroducing the bug it fixed. +- The fingerprint deliberately excludes `_index.parquet` itself; an index that invalidated itself by existing would never be usable. + +### Notes + +- What the fingerprint cannot detect is an edit preserving both size and mtime. That is why `reindex` exists and why the sidecars remain authoritative — run it after anything that rewrites metadata in place. +- `use_index: false` in a repository's options forces the walk. +- Parquet rather than SQLite: owilix already depends on `duckdb` and `pyarrow`, and DuckDB queries Parquet in place over HTTP range requests. SQLite over an object store has no locking, needs the whole file, and rewrites all of it per update. + +### Tests + +- 16 tests, weighted toward refusal rather than speed: index and walk return the same datasets; a stale index does not hide an added dataset, resurrect a removed one, or survive a changed sidecar; corrupt, absent and unknown-format indexes all fall back. + + ## v5.10.3 (2026-08-07) M6 tiers 2 and 3 of the summarize/integrity plan: `owi remote verify`. diff --git a/Readme.md b/Readme.md index fd90a4f..960d094 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.3-blue.svg)](https://openwebsearcheu-public.pages.it4i.eu/owi-cli/) +[![Version](https://img.shields.io/badge/version-5.10.4-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 531f0e6..14c5181 100644 --- a/docs/changes.md +++ b/docs/changes.md @@ -24,6 +24,27 @@ Brief description of what was accomplished. ## Unreleased +### main @ v5.10.4 - 2026-08-07 + +#### A collection index that refuses to be trusted when it should not be + +**Summary**: M4. `_index.parquet` per collection plus `remote reindex`, shipped together because an index without a rebuild path is a liability rather than a cache. + +**Changes**: +- New `owilix/core/index.py`: fingerprinting, build, read, and an `IndexUnusable` exception whose only correct handling is to walk — raising rather than returning `None` makes that impossible to forget at the call site. +- `FileBasedRepository.list()` consults the index per collection and falls through to the existing walk on any failure. +- `remote reindex` walks the sidecars once and writes the index; `owi remote ls` needs no flag. +- Staleness is a fingerprint over each sidecar's name, size and mtime, recomputed from one `ls`. Chosen over a bare object count because it also catches a resized or re-uploaded sidecar at identical cost, since the listing already returns the details. + +**Verified end to end on the fixture**: walk and index return identical results; adding a dataset without reindexing yields 4 not 3; a corrupted index yields 4; a deleted index yields 4. All four paths agree with the source. + +**Breaking Changes**: None. No index means the previous behaviour exactly. + +**Second customer**: M6 tier 3 currently re-reads every byte each pass because it has no way to know what changed. The index is what an incremental checksum mode would build on — that is now the obvious next step for it. + +**Still open from the plan**: M5 (HTTP exposure of the derived artifacts), and incremental checksums on top of this index. + + ### main @ v5.10.3 - 2026-08-07 #### Verification: presence, readability, checksums — and what a hash cannot tell you diff --git a/docs/source/details/verify.md b/docs/source/details/verify.md index 650e7d9..3896ded 100644 --- a/docs/source/details/verify.md +++ b/docs/source/details/verify.md @@ -156,3 +156,66 @@ zone rejecting the credential — the result carries `partial: true` and `failed_sources`. **A verification of an incomplete listing is not a verification of the mirror.** Do not read "0 problems" from a run that could not see everything. + +--- + +# The collection index + +`owi remote reindex` writes `_index.parquet` into each collection directory. +`remote ls` reads it when one is present and current, turning a listing from +**one GET per dataset** into **one LIST plus one range read**. + +```bash +owi remote reindex owi-up # after a pull or sync that changed a collection +owi remote ls owi-up # uses the index automatically when it is current +``` + +## It is a cache, and it behaves like one + +**The sidecars stay authoritative.** The index is derived from them and always +rebuildable from them. If the two ever disagree, the sidecars win. Something +that can be rebuilt can never be the thing that loses data. + +**Deleting every `_index.parquet` costs speed and nothing else.** Listings go +back to walking, exactly as before it existed. + +**A stale index is never served.** The index stores a fingerprint of the +collection — every sidecar's name, size and mtime. One cheap `ls` recomputes it +on read, and any mismatch falls back to the walk. Four failure paths, one +behaviour: + +| the index is… | what `ls` does | +| --- | --- | +| absent | walks | +| unreadable or corrupt | walks, logs why | +| stale (a sidecar added, removed, resized or re-uploaded) | walks, logs why | +| written by a newer owilix than this one | walks | + +That is deliberately blunt. An index in the read path that can answer *less* +than the source is the same failure as a listing that reports `total: 0` for a +full bucket, and it would be self-inflicted. + +**What the fingerprint cannot see:** an edit that preserves both size and mtime. +That is why `reindex` exists and why the sidecars remain authoritative — run it +after anything that rewrites metadata in place. + +## Why Parquet, not SQLite + +owilix already depends on `duckdb` and `pyarrow`. DuckDB queries Parquet **in +place over HTTP range requests** with predicate pushdown, so filtering on +`collectionName` does not download the index. SQLite over an object store has no +locking, requires fetching the whole file, and rewrites all of it on every +update — on a store with no byte-range writes, two writers silently lose each +other's work. + +The index carries flattened columns (`id`, `title`, `collectionName`, +`dataCenter`, `access`, `zone`, dates, `fileCount`, `objectCount`, `size`) for +querying, plus `metadata_json` holding the whole sidecar. **The flattened columns +are for predicates; the content always comes from `metadata_json`**, so a dataset +read from the index is identical to one read from disk. + +## Disabling it + +Pass `use_index: false` in a repository's options to always walk. Useful when +diagnosing whether a discrepancy comes from the index or the store — though the +fallback rules above mean the answer is almost always "the store". diff --git a/owilix/_version.py b/owilix/_version.py index 84e3953..84d9cdb 100644 --- a/owilix/_version.py +++ b/owilix/_version.py @@ -1,3 +1,3 @@ # Version is set here and imported elsewhere -__version__ = "5.10.3" -__version_tuple__ = (5, 10, 3) +__version__ = "5.10.4" +__version_tuple__ = (5, 10, 4) diff --git a/owilix/cli/remote.py b/owilix/cli/remote.py index d871488..7fa4b7a 100644 --- a/owilix/cli/remote.py +++ b/owilix/cli/remote.py @@ -1059,3 +1059,46 @@ def verify( f"[yellow]This verification is incomplete: {', '.join(data['failed_sources'])} " "could not be listed in full.[/yellow]" ) + + +@app.command("reindex") +def reindex( + ctx: typer.Context, + specifier: str = typer.Argument("all", help="Dataset specifier selecting the repositories"), + as_json: bool = typer.Option(False, "--json", help="Emit JSON"), +): + """Rebuild the per-collection listing index from the dataset sidecars. + + `remote ls` reads `_index.parquet` when one is present and current, turning + a listing from one GET per dataset into one LIST plus one range read. + + The index is a cache. The sidecars stay authoritative, the index is always + rebuildable from them, and any index that is absent, unreadable, stale or of + an unknown format is ignored in favour of walking -- so deleting every + `_index.parquet` costs speed and nothing else. + + Run it after a pull or sync that changed a collection. + + Examples: + owi remote reindex owi-up + owi remote reindex all --json + """ + import json as _json + + from owilix.core.tasks.verify import remote_reindex + + cli_ctx: CLIContext = ctx.obj + result = remote_reindex(cli_ctx.owi, specifier, console=cli_ctx.console) + + if as_json: + cli_ctx.console.print_json(_json.dumps(result.object, default=str)) + else: + cli_ctx.console.print( + f"[green]{result.object['collections']} collection(s), " + f"{result.object['datasets']} dataset(s) indexed.[/green]" + ) + for failure in result.object["failures"]: + cli_ctx.console.print(f"[red] failed: {failure}[/red]") + + if not result.success: + raise typer.Exit(code=result.exit_code or 1) diff --git a/owilix/core/index.py b/owilix/core/index.py new file mode 100644 index 0000000..9ff3583 --- /dev/null +++ b/owilix/core/index.py @@ -0,0 +1,237 @@ +"""A derived, rebuildable index over a collection's dataset sidecars. + +Listing a collection costs one GET per dataset -- 585 on a dev mirror, and it is +what made a single unreadable sidecar able to take out a whole listing. This +replaces that with **one LIST plus one range read**. + +Three properties matter more than the speed, because this puts a derived +artifact in the read path and that is the exact place this codebase has +repeatedly turned a failure into a confident wrong answer: + +1. **The sidecars stay authoritative.** The index is a cache. It is rebuilt from + the sidecars by `remote reindex`, and if the two ever disagree the sidecars + win. Something that can be rebuilt can never be the thing that loses data. + +2. **Staleness is detected, not assumed.** The index carries a fingerprint of + the collection directory -- every sidecar's name, size and mtime. One cheap + `ls` recomputes it. Any mismatch falls back to the walk **and says so**. + Never serve a stale index silently; that is the empty-listing bug in a new + place, and it would be self-inflicted. + +3. **Absent, unreadable or stale all mean "walk".** An old mirror, a + hand-assembled one, and one whose index write failed must all keep working + exactly as before. + +Parquet rather than SQLite: owilix already depends on `duckdb` and `pyarrow`, +and DuckDB queries Parquet in place over HTTP range requests with predicate +pushdown, so a `collectionName` filter need not download the index. SQLite over +an object store has no locking and requires fetching the whole file, and every +update rewrites all of it. +""" +from __future__ import annotations + +import hashlib +import json +import logging +import os +from datetime import datetime, timezone +from typing import Any, Dict, List, Optional, Tuple + +logger = logging.getLogger("owilix") + +__all__ = [ + "INDEX_FILENAME", + "INDEX_FORMAT_VERSION", + "collection_fingerprint", + "build_index_table", + "write_index", + "read_index", + "IndexUnusable", +] + +INDEX_FILENAME = "_index.parquet" + +#: Bumped when the schema changes in a way an older reader cannot handle. A +#: reader that sees a version it does not know falls back to the walk rather +#: than guessing. +INDEX_FORMAT_VERSION = 1 + +#: Flattened for predicate pushdown. `metadata_json` carries the whole sidecar, +#: so a Dataset rebuilt from a row is identical to one read from disk -- the +#: flattened columns are for querying, never the source of truth for content. +_FLAT_COLUMNS = ( + "id", "title", "collectionName", "dataCenter", "access", "zone", + "startDate", "endDate", "fileCount", "objectCount", "size", +) + + +class IndexUnusable(Exception): + """The index cannot be trusted for this read. Always recoverable by walking.""" + + +def _entries_for_fingerprint(fs, collection_dir: str) -> List[Tuple[str, Any, Any]]: + """Name, size and mtime of every sidecar in the collection. + + The index itself is excluded -- writing it changes the directory, and an + index that invalidated itself on write would never be usable. + """ + out = [] + for entry in fs.ls(collection_dir, detail=True): + name = entry["name"] if isinstance(entry, dict) else entry + base = os.path.basename(name.rstrip("/")) + if base == INDEX_FILENAME or not name.endswith(".json"): + continue + info = entry if isinstance(entry, dict) else {} + mtime = info.get("mtime") or info.get("LastModified") or info.get("last_modified") + out.append((base, info.get("size"), str(mtime) if mtime is not None else None)) + return sorted(out) + + +def collection_fingerprint(fs, collection_dir: str) -> Tuple[str, int]: + """A cheap signature of the collection's sidecars: (digest, count). + + One `ls` -- no per-dataset GETs. Detects an added, removed, renamed, resized + or re-uploaded sidecar. It cannot detect an edit that preserves size *and* + mtime, which is why `remote reindex` exists and why the sidecars remain + authoritative. + """ + entries = _entries_for_fingerprint(fs, collection_dir) + hasher = hashlib.sha256() + for name, size, mtime in entries: + hasher.update(f"{name}\0{size}\0{mtime}\n".encode()) + return hasher.hexdigest(), len(entries) + + +def _row_from_metadata(metadata: Dict[str, Any], dataset_dir: str) -> Dict[str, Any]: + row: Dict[str, Any] = {"dataset_dir": dataset_dir, "metadata_json": json.dumps(metadata)} + for column in _FLAT_COLUMNS: + value = metadata.get(column) + if column in ("fileCount", "objectCount", "size"): + try: + value = None if value is None else int(value) + except (TypeError, ValueError): + value = None + elif value is not None: + value = str(value) + row[column] = value + return row + + +def build_index_table(rows: List[Dict[str, Any]], fingerprint: str, count: int): + """Build the Arrow table, with the staleness marker in its metadata.""" + import pyarrow as pa + + import owilix + + columns = ["dataset_dir", "metadata_json", *_FLAT_COLUMNS] + data = {c: [r.get(c) for r in rows] for c in columns} + schema = pa.schema([ + pa.field(c, pa.int64() if c in ("fileCount", "objectCount", "size") else pa.string()) + for c in columns + ]) + table = pa.table(data, schema=schema) + return table.replace_schema_metadata({ + b"owilix_index_version": str(INDEX_FORMAT_VERSION).encode(), + b"owilix_version": owilix.__version__.encode(), + b"indexed_at": datetime.now(timezone.utc).isoformat().encode(), + b"fingerprint": fingerprint.encode(), + b"sidecar_count": str(count).encode(), + }) + + +def write_index(repository, collection_dir: str, rows: List[Dict[str, Any]]) -> Dict[str, Any]: + """Write `_index.parquet` for one collection. Returns a small report.""" + import pyarrow.parquet as pq + + fs = repository.fs + fingerprint, count = collection_fingerprint(fs, collection_dir) + table = build_index_table(rows, fingerprint, count) + target = os.path.join(collection_dir.rstrip("/"), INDEX_FILENAME) + + with fs.open(target, "wb") as handle: + pq.write_table(table, handle, compression="snappy") + + return {"path": target, "datasets": len(rows), "sidecars": count, "fingerprint": fingerprint} + + +def read_index(repository, collection_dir: str, access: str) -> List[Any]: + """Return Datasets from the index, or raise `IndexUnusable`. + + Raising rather than returning None on every failure path is deliberate: the + caller has exactly one correct response -- walk -- and an exception makes + that impossible to forget. + """ + import pyarrow.parquet as pq + + from owilix.core.models.dataset import Dataset + + fs = repository.fs + target = os.path.join(collection_dir.rstrip("/"), INDEX_FILENAME) + + if not fs.exists(target): + raise IndexUnusable("no index") + + try: + info = fs.info(target) + size = info.get("size") if isinstance(info, dict) else None + with fs.open(target, "rb", **({"size": size} if size is not None else {})) as handle: + table = pq.read_table(handle) + except Exception as e: + raise IndexUnusable(f"index unreadable: {e}") from e + + metadata = table.schema.metadata or {} + version = metadata.get(b"owilix_index_version") + if version is None or int(version) != INDEX_FORMAT_VERSION: + raise IndexUnusable(f"index format {version!r} is not {INDEX_FORMAT_VERSION}") + + stored_fingerprint = (metadata.get(b"fingerprint") or b"").decode() + current_fingerprint, current_count = collection_fingerprint(fs, collection_dir) + if stored_fingerprint != current_fingerprint: + stored_count = (metadata.get(b"sidecar_count") or b"?").decode() + raise IndexUnusable( + f"index is stale: {stored_count} sidecar(s) when written, {current_count} now " + "(or one changed)" + ) + + datasets = [] + for row in table.to_pylist(): + try: + data = json.loads(row["metadata_json"]) + except (ValueError, TypeError) as e: + raise IndexUnusable(f"index row is not readable: {e}") from e + data["access"] = access + data.pop("path", None) + datasets.append(Dataset(repository=repository, path=row["dataset_dir"], **data)) + return datasets + + +def index_rows_from_sidecars(repository, collection_dir: str, access: str): + """Read every sidecar once, yielding index rows plus any read failures. + + This is the expensive walk the index exists to avoid -- run by `reindex`, + not by `list`. + """ + from owilix.core.utils import check_path_for_uuid_filename + + fs = repository.fs + rows: List[Dict[str, Any]] = [] + skipped: List[str] = [] + + for entry in fs.ls(collection_dir, detail=True): + path = entry["name"] if isinstance(entry, dict) else entry + if not path.endswith(".json"): + continue + dataset_dir = path[: -len(".json")] + if not check_path_for_uuid_filename(os.path.basename(dataset_dir)): + continue + size = entry.get("size") if isinstance(entry, dict) else None + try: + metadata = repository.load_metadata(path, size=size) + except Exception as e: + # Same tolerance as list(): one bad sidecar must not cost the index. + logger.error(f"Skipping unreadable sidecar {path} while indexing: {e}") + skipped.append(path) + continue + rows.append(_row_from_metadata(metadata, dataset_dir)) + + return rows, skipped diff --git a/owilix/core/repository/file.py b/owilix/core/repository/file.py index e8d445c..b3aebdb 100644 --- a/owilix/core/repository/file.py +++ b/owilix/core/repository/file.py @@ -114,6 +114,9 @@ class FileBasedRepository(AbstractRepository): self._filesystem, self.path, self._filesystem_async = FileBasedRepository.get_fsspec_from_config(path, protocol, **kwargs) self._repo_name = repo_name if repo_name else "unnamed_repo" self._collection_name_in_path = kwargs.get("collections_in_path", True) + # Opt-out rather than opt-in: an index is only consulted when one exists, + # and every failure falls back to the walk, so the safe default is on. + self._use_index = kwargs.get("use_index", True) self._retry_count = kwargs.get("retry_count", 3) self._chunk_size = kwargs.get("chunk_size", 5 * 1024 * 1024) self._tags = tags if tags is not None else ["main"] @@ -272,6 +275,27 @@ class FileBasedRepository(AbstractRepository): for subdir in dirs: if not self.fs.exists(subdir): continue + + # One LIST plus one range read, when a usable index is present. + # + # Every failure path -- absent, unreadable, stale, wrong format -- + # falls through to the walk below and logs why. A derived artifact + # in the read path must never be able to answer *less* than the + # source; serving a stale index quietly would be the empty-listing + # bug again, self-inflicted this time. + if self._use_index: + try: + from owilix.core.index import read_index + + listing.extend(read_index(self, subdir, access)) + continue + except Exception as e: + from owilix.core.index import IndexUnusable + + if isinstance(e, IndexUnusable): + logger.debug(f"Walking {subdir}: {e}") + else: + logger.warning(f"Walking {subdir}: unexpected index failure: {e}") # detail=True so the size is known before opening; see load_metadata. try: entries = self.fs.ls(subdir, detail=True) diff --git a/owilix/core/tasks/verify.py b/owilix/core/tasks/verify.py index 7212527..23b8a98 100644 --- a/owilix/core/tasks/verify.py +++ b/owilix/core/tasks/verify.py @@ -382,3 +382,89 @@ def remote_verify( + (f", {skipped} skipped" if skipped else ""), command="remote verify", ) + + +def remote_reindex( + manager, + specifier: str, + *, + console=None, +) -> CommandResult: + """Rebuild `_index.parquet` for every collection the specifier touches. + + The index is a cache over the sidecars, which stay authoritative. Rebuilding + is always safe: if the index can be rebuilt, it can never be the thing that + loses data. + """ + import os + + from owilix.core.index import INDEX_FILENAME, index_rows_from_sidecars, write_index + + spec = manager.parse_specifier(specifier) + repositories = manager.remote_data.get_repos(spec.get("data_center")) + if not repositories: + return CommandResult( + success=False, msg=f"No repository matched specifier '{specifier}'", + error_type=ErrorType.DATA, exit_code=ExitCode.DATA_ERROR, command="remote reindex", + ) + + written, failures = [], [] + for name, repository in repositories.items(): + if not hasattr(repository, "_get_collection_paths"): + # LEXIS is catalogue-backed; there is nothing here to index. + logger.debug(f"Skipping {name}: not a file-based repository") + continue + for access in ("public", "project", "user"): + try: + roots = [p for p in repository._get_collection_paths(access) if repository.fs.exists(p)] + except Exception as e: + failures.append({"repository": name, "access": access, "error": str(e)}) + continue + collections = [] + if getattr(repository, "_collection_name_in_path", False): + for root in roots: + try: + collections += [ + e["name"] for e in repository.fs.listdir(root) if e["type"] == "directory" + ] + except Exception as e: + failures.append({"repository": name, "access": access, "error": str(e)}) + else: + collections = roots + + for collection_dir in collections: + try: + rows, skipped = index_rows_from_sidecars(repository, collection_dir, access) + report = write_index(repository, collection_dir, rows) + report.update({ + "repository": name, "access": access, + "collection": os.path.basename(collection_dir.rstrip("/")), + "skipped_sidecars": len(skipped), + }) + written.append(report) + if console: + note = f", {len(skipped)} sidecar(s) unreadable" if skipped else "" + console.print( + f" {name}/{access}/{report['collection']}: " + f"{report['datasets']} dataset(s) indexed{note}" + ) + except Exception as e: + logger.exception(f"Could not index {collection_dir}") + failures.append({"repository": name, "access": access, + "collection": collection_dir, "error": str(e)}) + + return CommandResult( + success=not failures, + object={ + "indexed": written, + "collections": len(written), + "datasets": sum(r["datasets"] for r in written), + "failures": failures, + "index_filename": INDEX_FILENAME, + }, + msg=f"{len(written)} collection(s) indexed" + + (f", {len(failures)} failed" if failures else ""), + error_type=ErrorType.DATA if failures else "", + exit_code=ExitCode.DATA_ERROR if failures else ExitCode.SUCCESS, + command="remote reindex", + ) diff --git a/pyproject.toml b/pyproject.toml index 6cf6614..908d5a5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "hatchling.build" [project] name = "owilix" -version = "5.10.3" +version = "5.10.4" 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/test_index.py b/tests/owilix/core/test_index.py new file mode 100644 index 0000000..e261cf7 --- /dev/null +++ b/tests/owilix/core/test_index.py @@ -0,0 +1,232 @@ +"""M4 -- the per-collection index, and the ways it must refuse to be trusted. + +This puts a derived artifact in the read path, which is the exact place this +codebase has repeatedly turned a failure into a confident wrong answer. So the +tests that matter most are not "the index is fast" but **"the index never +answers less than the walk"**: absent, unreadable, stale, or of an unknown +format must all produce the same datasets as reading the sidecars directly. +""" +import json +import os +import uuid + +import pytest + +from owilix.core.index import ( + INDEX_FILENAME, + INDEX_FORMAT_VERSION, + IndexUnusable, + collection_fingerprint, + index_rows_from_sidecars, + read_index, + write_index, +) +from owilix.core.repository.file import FileBasedRepository + + +def _repo(tmp_path, **kwargs): + return FileBasedRepository( + manager=None, + path=os.path.join(str(tmp_path), "{access}"), + repo_name="owi-up", + protocol="file", + collections_in_path=True, + **kwargs, + ) + + +def _write_dataset(tmp_path, collection="main", ds_id=None, access="public"): + ds_id = ds_id or str(uuid.uuid4()) + base = tmp_path / access / collection + base.mkdir(parents=True, exist_ok=True) + (base / ds_id).mkdir(exist_ok=True) + (base / f"{ds_id}.json").write_text( + json.dumps({"id": ds_id, "internalID": ds_id, "collectionName": collection, + "title": f"Dataset {ds_id[:8]}", "fileCount": 3, "size": 1024}), + encoding="utf-8", + ) + return ds_id + + +def _collection_dir(tmp_path, collection="main", access="public"): + return str(tmp_path / access / collection) + + +def _build(tmp_path, repo, collection="main", access="public"): + directory = _collection_dir(tmp_path, collection, access) + rows, skipped = index_rows_from_sidecars(repo, directory, access) + return write_index(repo, directory, rows), skipped + + +class TestIndexMatchesTheWalk: + def test_same_datasets_from_index_and_walk(self, tmp_path): + ids = {_write_dataset(tmp_path) for _ in range(4)} + repo = _repo(tmp_path) + + walked = {d.metadata.get("id") for d in _repo(tmp_path, use_index=False).list("public")} + _build(tmp_path, repo) + indexed = {d.metadata.get("id") for d in repo.list("public")} + + assert walked == indexed == ids + + def test_metadata_survives_the_round_trip(self, tmp_path): + """The flattened columns are for querying; content comes from the sidecar.""" + ds_id = _write_dataset(tmp_path) + repo = _repo(tmp_path) + _build(tmp_path, repo) + + dataset = repo.list("public")[0] + + assert dataset.metadata.get("id") == ds_id + assert dataset.metadata.get("fileCount") == 3 + assert dataset.metadata.get("title").startswith("Dataset") + + def test_filters_still_apply(self, tmp_path): + _write_dataset(tmp_path, collection="main") + _write_dataset(tmp_path, collection="other") + repo = _repo(tmp_path) + _build(tmp_path, repo, collection="main") + _build(tmp_path, repo, collection="other") + + assert len(repo.list("public", query={"collectionName": "main"})) == 1 + + +class TestItRefusesToBeTrustedWhenItShould: + """Every one of these must fall back to the walk, never under-report.""" + + def test_absent(self, tmp_path): + _write_dataset(tmp_path) + repo = _repo(tmp_path) + + assert len(repo.list("public")) == 1 + + def test_stale_after_a_dataset_is_added(self, tmp_path): + """The failure that would silently lose data.""" + _write_dataset(tmp_path) + repo = _repo(tmp_path) + _build(tmp_path, repo) + + _write_dataset(tmp_path) # index not rebuilt + + assert len(repo.list("public")) == 2, "a stale index must not hide the new dataset" + + def test_stale_after_a_dataset_is_removed(self, tmp_path): + first = _write_dataset(tmp_path) + _write_dataset(tmp_path) + repo = _repo(tmp_path) + _build(tmp_path, repo) + + (tmp_path / "public" / "main" / f"{first}.json").unlink() + + assert len(repo.list("public")) == 1, "a stale index must not resurrect a removed dataset" + + def test_stale_after_a_sidecar_changes_size(self, tmp_path): + ds_id = _write_dataset(tmp_path) + repo = _repo(tmp_path) + _build(tmp_path, repo) + directory = _collection_dir(tmp_path) + before, _ = collection_fingerprint(repo.fs, directory) + + (tmp_path / "public" / "main" / f"{ds_id}.json").write_text( + json.dumps({"id": ds_id, "internalID": ds_id, "collectionName": "main", + "title": "Renamed and longer than it was before"}), + encoding="utf-8", + ) + + after, _ = collection_fingerprint(repo.fs, directory) + assert before != after + with pytest.raises(IndexUnusable, match="stale"): + read_index(repo, directory, "public") + + def test_unreadable(self, tmp_path): + _write_dataset(tmp_path) + repo = _repo(tmp_path) + _build(tmp_path, repo) + + (tmp_path / "public" / "main" / INDEX_FILENAME).write_bytes(b"not parquet") + + assert len(repo.list("public")) == 1 + + def test_an_unknown_format_version(self, tmp_path, monkeypatch): + _write_dataset(tmp_path) + repo = _repo(tmp_path) + _build(tmp_path, repo) + + import owilix.core.index as index_module + + monkeypatch.setattr(index_module, "INDEX_FORMAT_VERSION", INDEX_FORMAT_VERSION + 1) + + with pytest.raises(IndexUnusable, match="format"): + read_index(repo, _collection_dir(tmp_path), "public") + + def test_use_index_false_always_walks(self, tmp_path): + _write_dataset(tmp_path) + built = _repo(tmp_path) + _build(tmp_path, built) + _write_dataset(tmp_path) + + assert len(_repo(tmp_path, use_index=False).list("public")) == 2 + + +class TestFingerprint: + def test_it_ignores_the_index_itself(self, tmp_path): + """An index that invalidated itself by existing would never be usable.""" + _write_dataset(tmp_path) + repo = _repo(tmp_path) + directory = _collection_dir(tmp_path) + before, count_before = collection_fingerprint(repo.fs, directory) + + _build(tmp_path, repo) + + after, count_after = collection_fingerprint(repo.fs, directory) + assert (before, count_before) == (after, count_after) + + def test_it_counts_only_sidecars(self, tmp_path): + _write_dataset(tmp_path) + _write_dataset(tmp_path) + repo = _repo(tmp_path) + + _, count = collection_fingerprint(repo.fs, _collection_dir(tmp_path)) + + assert count == 2 + + def test_it_is_stable_across_calls(self, tmp_path): + _write_dataset(tmp_path) + repo = _repo(tmp_path) + directory = _collection_dir(tmp_path) + + assert collection_fingerprint(repo.fs, directory) == collection_fingerprint(repo.fs, directory) + + +class TestBuilding: + def test_one_unreadable_sidecar_does_not_cost_the_index(self, tmp_path): + """Same tolerance as list(): the index inherits it rather than reintroducing the bug.""" + _write_dataset(tmp_path) + _write_dataset(tmp_path) + bad = tmp_path / "public" / "main" / "broken.json" + bad.write_text("{not json", encoding="utf-8") + repo = _repo(tmp_path) + + report, skipped = _build(tmp_path, repo) + + assert report["datasets"] == 2 + assert len(skipped) == 1 + + def test_an_empty_collection_indexes_to_nothing(self, tmp_path): + (tmp_path / "public" / "main").mkdir(parents=True) + repo = _repo(tmp_path) + + report, _ = _build(tmp_path, repo) + + assert report["datasets"] == 0 + assert repo.list("public") == [] + + def test_rebuilding_makes_a_stale_index_current(self, tmp_path): + _write_dataset(tmp_path) + repo = _repo(tmp_path) + _build(tmp_path, repo) + _write_dataset(tmp_path) + + _build(tmp_path, repo) + + assert len(read_index(repo, _collection_dir(tmp_path), "public")) == 2 diff --git a/uv.lock b/uv.lock index e273b02..42ed0d3 100644 --- a/uv.lock +++ b/uv.lock @@ -1102,7 +1102,7 @@ wheels = [ [[package]] name = "owilix" -version = "5.10.3" +version = "5.10.4" source = { editable = "." } dependencies = [ { name = "ciff-toolkit" }, -- 2.51.2