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" },