From 63c0cd3c279625854fb49676c9297df69906a15d Mon Sep 17 00:00:00 2001 From: arthurmccray Date: Wed, 16 Sep 2026 15:17:01 -0700 Subject: [PATCH] allowing for files to be selected from within .zip on zenodo --- CONTRIBUTING.md | 4 + docs/source/_build_docs.py | 6 +- docs/source/contributing.rst | 41 ++++- emdatabase/_archive.py | 156 ++++++++++++++++++ emdatabase/catalogue.py | 3 + emdatabase/data/__init__.pyi | 17 +- emdatabase/downloadable_dataset.py | 32 +++- emdatabase/index/TEMPLATE.yaml | 13 ++ .../index/TwistedBilayerWSe2Themis.yaml | 54 ++++++ emdatabase/index/json-schema.json | 36 ++++ emdatabase/metadata.py | 30 ++++ emdatabase/new_dataset.py | 15 ++ emdatabase/tests/conftest.py | 21 +++ emdatabase/tests/test_archive.py | 155 +++++++++++++++++ emdatabase/tests/test_fill_download_fields.py | 25 +++ emdatabase/tests/test_load_data.py | 35 +++- emdatabase/tests/test_metadata.py | 9 + emdatabase/tests/test_new_dataset.py | 20 +++ 18 files changed, 658 insertions(+), 14 deletions(-) create mode 100644 emdatabase/_archive.py create mode 100644 emdatabase/index/TwistedBilayerWSe2Themis.yaml create mode 100644 emdatabase/tests/test_archive.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 23f8d84..750645c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,4 +21,8 @@ Neither route needs the checksum or the size: an entry missing either one has th downloaded on GitHub and the fields filled in for it, unless the pull request comes from a fork, whose branch cannot be pushed to. +A dataset that is one file inside a zip on a record nobody can re-publish is written by +hand instead, with an `archive` block naming the zip and the member inside it. +`download()` then fetches only that member, and never the whole archive. + Full instructions, including what to do by hand and what CI checks: . diff --git a/docs/source/_build_docs.py b/docs/source/_build_docs.py index dcdb5c4..bae0c99 100644 --- a/docs/source/_build_docs.py +++ b/docs/source/_build_docs.py @@ -88,7 +88,11 @@ link.href = pin.url; link.target = "_blank"; link.rel = "noopener"; - link.textContent = `⤓ Download ${pin.file}`; + // An archive entry's only link is the zip the file lives inside, so the + // label names the archive rather than a file the link does not serve. + link.textContent = item.archive + ? `⤓ Download ${pin.url.split("/").pop()} (archive holding ${item.archive})` + : `⤓ Download ${pin.file}`; const wrap = el("div", "emdb-dl-link"); wrap.appendChild(link); detailsEl.appendChild(wrap); diff --git a/docs/source/contributing.rst b/docs/source/contributing.rst index 6c31963..b1ef0f4 100644 --- a/docs/source/contributing.rst +++ b/docs/source/contributing.rst @@ -82,6 +82,38 @@ on its format and whether it is required, and fill it in. Then check it: That prints one line per problem and exits non-zero, or prints ``valid``. :func:`emdatabase.metadata.validate_file` is the same check from Python. +A file inside an archive +------------------------ + +Some data worth shipping is one file inside a multi-gigabyte zip on a record +nobody can re-publish, where downloading all of it to get one file is not +reasonable. Such an entry adds an ``archive`` block naming the zip and the +member inside it: + +.. code-block:: yaml + + archive: + url: https://zenodo.org/records/0000000/files/Figures.zip + member: Figure_01/Panel_a/scan_x128_y128.raw + +``download()`` then fetches only that member, over HTTP range requests: a few +requests read the zip's directory, and the member costs its own compressed bytes +rather than the whole archive. What comes back is a path, as for any other entry. + +The entry's own ``file``, ``checksum`` and ``size_bytes`` go on describing the +member, which is the file you end up with; ``checksum`` and ``size_bytes`` +inside the block describe the archive instead, and are optional. Give ``member`` +as the complete path inside the zip - one file name can appear in several of its +directories, so a basename alone is ambiguous. + +These entries are written by hand. The two fields CI otherwise fills in would be +taken from the archive rather than from the member, so a pull request leaving +them blank is refused rather than guessed at. ``download_url``, and the download +link on the docs site, point at the archive: the member has no link of its own. + +A host that ignores ``Range`` and answers with the whole archive fails loudly, +naming the host, rather than quietly pulling gigabytes. + Contributing model weights -------------------------- @@ -151,13 +183,18 @@ a dataset with one, fail as well. files. It downloads the file behind each changed entry that is missing its ``checksum`` or ``size_bytes`` - a weights family's ``latest`` and each dated version on their own links - fills the fields in and pushes the result back to -the branch. A fork's branch cannot be pushed to, so a pull request from one +the branch. An entry naming an ``archive`` is refused instead: its ``checksum`` +and ``size_bytes`` describe the member inside the zip, and the only thing there +is to download is the whole archive, so those two are filled in by hand. A +fork's branch cannot be pushed to, so a pull request from one fails instead and prints the values to paste in. An entry coming in through the issue form is filled in the same way before its pull request is opened, so it arrives complete. ``check_sources.yml`` runs weekly and asks each source server whether the file -is still there and still the size the entry claims. +is still there and still the size the entry claims. For an entry fetched out of +an archive it reads the archive's directory too, over a few range requests, and +checks the member is still there at the size the entry declares. ``check_weights.yml`` runs weekly as well, and what it does depends on where the family is hosted. diff --git a/emdatabase/_archive.py b/emdatabase/_archive.py new file mode 100644 index 0000000..1508a62 --- /dev/null +++ b/emdatabase/_archive.py @@ -0,0 +1,156 @@ +"""Fetching one file out of a zip on someone else's server. + +Some data worth shipping is a single member of a multi-gigabyte archive on a +record that cannot be re-published, where downloading all of it to get one file +is not reasonable. A zip's directory sits at its end and names the byte range of +every member, so with HTTP range requests the whole archive never has to move: +three requests find the directory, and the member costs its own stored bytes. + +:class:`_HTTPRangeFile` is the seekable file ``zipfile`` reads that through, and +:class:`ArchiveMemberDownloader` is the pooch downloader +:meth:`~emdatabase.downloadable_dataset.DownloadableDataset._retrieve` hands to +:func:`pooch.retrieve` in place of :class:`pooch.HTTPDownloader` when an entry +names an ``archive``. +""" + +from __future__ import annotations + +import io +import urllib.parse +import urllib.request +import zipfile +from typing import Any + +from emdatabase.downloadable_dataset import USER_AGENT, Progress + +# The zip directory is read in many small seeks, so an unbuffered reader would +# cost hundreds of requests before a single byte of the member moved. +_BUFFER_SIZE = 1 << 20 + + +class ArchiveError(Exception): + """The host will not serve the archive in a way one member can be read out of. + + Deliberately not an :class:`OSError`: ``zipfile`` turns any ``OSError`` + raised while it is reading the directory into + ``BadZipFile("File is not a zip file")``, which would replace the real + explanation with a wrong one. + """ + + +def _host(url: str) -> str: + return urllib.parse.urlsplit(url).netloc + + +class _HTTPRangeFile(io.RawIOBase): + """A seekable, read-only file over HTTP range requests. + + ``zipfile`` only seeks and reads, so ranges stand in for a local copy. + """ + + def __init__(self, url: str, timeout: float = 120) -> None: + self.url = url + self.timeout = timeout + self.pos = 0 + request = urllib.request.Request(url, method="HEAD", headers={"User-Agent": USER_AGENT}) + with urllib.request.urlopen(request, timeout=timeout) as response: + declared = response.headers["Content-Length"] + if declared is None: + raise ArchiveError( + f"{_host(url)} did not say how big {url} is, so the end of the archive - " + "where a zip keeps its directory - cannot be found." + ) + self.size = int(declared) + + def readable(self) -> bool: + return True + + def seekable(self) -> bool: + return True + + def tell(self) -> int: + return self.pos + + def seek(self, offset: int, whence: int = io.SEEK_SET) -> int: + if whence == io.SEEK_SET: + self.pos = offset + elif whence == io.SEEK_CUR: + self.pos += offset + else: + self.pos = self.size + offset + return self.pos + + def readinto(self, buffer) -> int: # pyright: ignore[reportMissingParameterType] + if self.pos >= self.size: + return 0 + end = min(self.pos + len(buffer), self.size) - 1 + request = urllib.request.Request( + self.url, + headers={"User-Agent": USER_AGENT, "Range": f"bytes={self.pos}-{end}"}, + ) + with urllib.request.urlopen(request, timeout=self.timeout) as response: + # A host that does not do ranges answers 200 with the whole body. + # Reading it would quietly pull the entire archive, which is the one + # thing this exists to avoid, so it is an error rather than a + # fallback. + if response.status != 206: + raise ArchiveError( + f"{_host(self.url)} ignored a Range request and answered " + f"{response.status}, so fetching one member would mean downloading " + f"all {self.size} bytes of {self.url}." + ) + data = response.read() + buffer[: len(data)] = data + self.pos += len(data) + return len(data) + + +class ArchiveMemberDownloader: + """A pooch downloader that pulls one member out of a remote zip. + + pooch calls a downloader as ``(url, output_file, pooch)`` from inside + ``pooch.core.stream_download``, which streams to a temporary file, checks it + against the entry's ``checksum`` and only then renames it into place, + deleting the temporary file on any failure. So this only has to produce the + member's bytes and drive ``progressbar`` the way :class:`pooch.HTTPDownloader` + does: ``total`` once before streaming, ``update(n)`` per chunk, then + ``reset()``, ``update(total)``, ``close()``. The widgets' cancel works by + raising from ``update``, which aborts the stream and takes the temporary + file with it. + """ + + def __init__( + self, + member: str, + progressbar: Progress | bool = False, + chunk_size: int = 4096, + ) -> None: + self.member = member + # `True` means "build your own bar", which pooch's HTTPDownloader does + # and this does not: _retrieve has already swapped it for a Progress. + self.progressbar = None if isinstance(progressbar, bool) else progressbar + self.chunk_size = chunk_size + + def __call__(self, url: str, output_file: str, _pooch: Any = None) -> None: + bar = self.progressbar + with io.BufferedReader(_HTTPRangeFile(url), buffer_size=_BUFFER_SIZE) as stream: # pyright: ignore[reportArgumentType] + with zipfile.ZipFile(stream) as archive: + try: + info = archive.getinfo(self.member) + except KeyError: + raise KeyError( + f"{url} holds no member {self.member!r}. Name the complete path " + "inside the archive: one file name can appear in several of its " + "directories." + ) from None + if bar: + bar.total = info.file_size + with archive.open(info) as member, open(output_file, "wb") as out: + while chunk := member.read(self.chunk_size): + out.write(chunk) + if bar: + bar.update(len(chunk)) + if bar: + bar.reset() + bar.update(info.file_size) + bar.close() diff --git a/emdatabase/catalogue.py b/emdatabase/catalogue.py index b411305..a5bf394 100644 --- a/emdatabase/catalogue.py +++ b/emdatabase/catalogue.py @@ -113,6 +113,9 @@ def entry(name: str, ds: DownloadableDataset) -> dict: "source": md.source, "file": md.file, "url": ds.download_url, + # For an entry fetched out of a zip, `url` is the archive rather than + # the file; this is the member inside it, and "" for everything else. + "archive": md.archive.member if md.archive else "", "latest_checksum": ds.checksum or "", "versions": _versions(ds), "model_class": md.model.class_ if md.model else "", diff --git a/emdatabase/data/__init__.pyi b/emdatabase/data/__init__.pyi index b647562..396aae7 100644 --- a/emdatabase/data/__init__.pyi +++ b/emdatabase/data/__init__.pyi @@ -326,6 +326,21 @@ class TutorialUNet(DownloadableDataset): Model weights hosted at https://drive.google.com; see ``.versions`` for the dated snapshots. + """ + ... + +class TwistedBilayerWSe2Themis(DownloadableDataset): + """ + TwistedBilayerWSe2Themis + + Electron ptychography of a twisted bilayer WSe2, acquired at 80 kV on an uncorrected Thermo Fisher Themis with an EMPAD. From the record for "Achieving sub-0.5-Angstrom resolution ptychography in an uncorrected electron microscope", which reaches 0.44 Angstrom without an aberration corrector (Science 384, adl2029). The file is one member of the record's 1.9 GB Fig_01.zip and is fetched out of it directly, without downloading the archive. It is a headerless raw: a 128 x 128 scan of 128 x 130 little-endian float32 frames, each frame the 128 x 128 detector followed by two rows of EMPAD metadata. Read it with numpy.fromfile(path, dtype=" _Resolved: A dataset is a single pinned file and takes no version. A weights entry is a family: with no version it is the ``latest`` link, which is not pinned, and with one it is that dated snapshot, which is. + + A dataset naming an ``archive`` resolves to that archive's link and the + member inside it, since the archive is the only thing there is a link to. """ md = self.metadata if md.kind != "weights": if version is not None: raise ValueError(f"{type(self).__name__} is a dataset and has no versions") return _Resolved( - url=md.url or f"{md.source}/{md.file}", + url=md.archive.url if md.archive else (md.url or f"{md.source}/{md.file}"), checksum=md.checksum, size_bytes=md.size_bytes, file=md.file, pinned=True, + member=md.archive.member if md.archive else None, ) if version is None: if md.latest is None: @@ -399,6 +408,9 @@ def download_url(self) -> str: link. Where it is not - a Google Drive link, or anything else with a query string - the entry gives the whole link as ``url`` instead, and a weights entry gives one link per version. + + For an entry whose file lives inside an archive this is the archive: + the member has no link of its own. """ return self._resolve(None).url @@ -595,11 +607,19 @@ def _retrieve( if shared is not None: return shared destination = self._resolve_destination(destination) - downloader = pooch.HTTPDownloader( - progressbar=progressbar, # pyright: ignore[reportArgumentType] - chunk_size=chunk_size, - headers={"User-Agent": USER_AGENT}, - ) + downloader: Any + if resolved.member is None: + downloader = pooch.HTTPDownloader( + progressbar=progressbar, # pyright: ignore[reportArgumentType] + chunk_size=chunk_size, + headers={"User-Agent": USER_AGENT}, + ) + else: + # Imported here, not at the top, because _archive imports this + # module for USER_AGENT and the progress protocol. + from emdatabase._archive import ArchiveMemberDownloader + + downloader = ArchiveMemberDownloader(resolved.member, progressbar, chunk_size) try: if refresh: # pooch keeps a file whose hash it was not given anything to diff --git a/emdatabase/index/TEMPLATE.yaml b/emdatabase/index/TEMPLATE.yaml index 83bf698..f4292d9 100644 --- a/emdatabase/index/TEMPLATE.yaml +++ b/emdatabase/index/TEMPLATE.yaml @@ -23,6 +23,19 @@ MyDatasetName: file: MyDatasetName.zspy # The file's Content-Length in bytes; the weekly CI check compares it to the server. size_bytes: 1000000 + # Only for a file that lives inside a zip on a record you cannot re-publish, + # where downloading the whole archive to get one file is not reasonable. + # `download()` fetches just this member, with HTTP range requests, and hands + # back a path like any other entry. `url`, `checksum` and `size_bytes` here + # describe the archive and are what `download_url` points at; `file`, + # `checksum` and `size_bytes` above go on describing the member, the file you + # end up with. Give the complete path inside the zip: the same file name can + # appear in several of its directories. Not allowed on a `kind: weights` entry. + # archive: + # url: https://zenodo.org/records/0000000/files/Figures.zip + # member: Figure_01/Panel_a/scan_x128_y128.raw + # checksum: md5:0123456789abcdef0123456789abcdef + # size_bytes: 2000000 # Who made the detector. See vendors.yaml for the names already in use. detector_manufacturer: Direct Electron # The detector model. diff --git a/emdatabase/index/TwistedBilayerWSe2Themis.yaml b/emdatabase/index/TwistedBilayerWSe2Themis.yaml new file mode 100644 index 0000000..7f62bc9 --- /dev/null +++ b/emdatabase/index/TwistedBilayerWSe2Themis.yaml @@ -0,0 +1,54 @@ +# $schema: ./json-schema.json +TwistedBilayerWSe2Themis: + description: >- + Electron ptychography of a twisted bilayer WSe2, acquired at 80 kV on an uncorrected + Thermo Fisher Themis with an EMPAD. From the record for "Achieving sub-0.5-Angstrom + resolution ptychography in an uncorrected electron microscope", which reaches 0.44 Angstrom + without an aberration corrector (Science 384, adl2029). The file is one member of the + record's 1.9 GB Fig_01.zip and is fetched out of it directly, without downloading the + archive. It is a headerless raw: a 128 x 128 scan of 128 x 130 little-endian float32 + frames, each frame the 128 x 128 detector followed by two rows of EMPAD metadata. Read it + with numpy.fromfile(path, dtype=" str: value = ", ".join(value) elif entry.name == "model": value = " · ".join(p for p in (value.class_, value.framework, value.quantem) if p) + elif entry.name == "archive": + value = f"{value.member} in {value.url}" elif entry.name == "latest": value = " · ".join(p for p in (value.checksum, value.url) if p) elif entry.name == "versions": diff --git a/emdatabase/new_dataset.py b/emdatabase/new_dataset.py index a8f99f4..d6b265e 100644 --- a/emdatabase/new_dataset.py +++ b/emdatabase/new_dataset.py @@ -62,6 +62,7 @@ "checksum", "file", "size_bytes", + "archive", "detector_manufacturer", "detector", "microscope_vendor", @@ -332,6 +333,10 @@ def fill_download_fields(document: dict[str, Any]) -> list[str]: the top level, so ``latest`` and each dated version is followed on its own link. An entry that already has both is not downloaded. + An entry naming an ``archive`` is refused rather than filled: its two fields + describe one member inside the zip, and the only thing there is to download + is the whole archive. + The document is filled in place and nothing is written - the caller decides where the result goes. The lines returned say what was filled, and are empty when nothing was. @@ -347,6 +352,16 @@ def fill_download_fields(document: dict[str, Any]) -> list[str]: for label, pin in pins: if pin: lines += _fill_pin(label, pin, pin.get("url", "")) + elif entry.get("archive"): + # The entry's checksum and size_bytes describe the member inside the + # archive. Following the link here would hash the whole zip and + # write a value that is wrong in a way nothing downstream notices. + if not (entry.get("checksum") and entry.get("size_bytes")): + raise ValueError( + f"{name}: checksum and size_bytes describe the member inside the " + "archive, which is not something this can download. Fill them in " + "by hand." + ) else: url = entry.get("url") or f"{entry.get('source', '')}/{entry.get('file', '')}" lines += _fill_pin(name, entry, url) diff --git a/emdatabase/tests/conftest.py b/emdatabase/tests/conftest.py index d4ac4c8..072544a 100644 --- a/emdatabase/tests/conftest.py +++ b/emdatabase/tests/conftest.py @@ -77,6 +77,11 @@ class _Handler(http.server.SimpleHTTPRequestHandler): both do. ``/uc?export=download&id=`` is the Google Drive shape: a link that names no file, redirecting to bytes that name themselves in a ``Content-Disposition`` header. + + A plain path answers a ``Range`` header with ``206`` and that slice, which is + what reading one member out of a remote zip needs. ``/attached/`` keeps + ignoring ``Range`` and answering ``200``, standing in for a host that does + not do ranges at all. """ def send_head(self): @@ -101,8 +106,24 @@ def send_head(self): self.send_header("Content-Disposition", f'attachment; filename="{name}"') self.end_headers() return io.BytesIO(body) + if "Range" in self.headers: + return self._send_range() return super().send_head() + def _send_range(self): + """``206`` with the requested slice, for ``bytes=-[]``.""" + body = Path(self.translate_path(self.path)).read_bytes() + first, _, last = self.headers["Range"].partition("=")[2].partition("-") + start = int(first) + end = int(last) if last else len(body) - 1 + chunk = body[start : end + 1] + self.send_response(206) + self.send_header("Content-Type", "application/octet-stream") + self.send_header("Content-Length", str(len(chunk))) + self.send_header("Content-Range", f"bytes {start}-{end}/{len(body)}") + self.end_headers() + return io.BytesIO(chunk) + def log_message(self, format, *args): pass diff --git a/emdatabase/tests/test_archive.py b/emdatabase/tests/test_archive.py new file mode 100644 index 0000000..a26f39c --- /dev/null +++ b/emdatabase/tests/test_archive.py @@ -0,0 +1,155 @@ +"""Tests for fetching one member out of a remote zip. + +``conftest``'s server answers ``Range`` with ``206``, so all of this runs +offline against a zip built here rather than the multi-gigabyte archives the +feature exists for. Its ``/attached/`` path still ignores ``Range`` and answers +``200``, which is the host that does not do ranges at all. + +pooch owns the temporary file, the checksum check and the rename, so the tests +that assert nothing is left behind are asserting that this downloader lets pooch +do that job rather than writing the destination itself. +""" + +import hashlib +import io +import zipfile +from pathlib import Path + +import pytest + +from emdatabase import config +from emdatabase._archive import ArchiveError +from emdatabase.downloadable_dataset import DownloadableDataset +from emdatabase.widget import DownloadCancelled + +MEMBER = "Fig_01/Panel_g-h_Themis/scan.raw" +CONTENT = b"detector frames, allegedly" * 4000 +# The same file name in another directory of the same archive: naming the member +# by its basename alone would be ambiguous, which is why entries give the path. +DECOY = "Fig_01/Panel_c-d_Talos/scan.raw" +DECOY_CONTENT = b"the wrong panel entirely" * 4000 + + +def _zip_bytes() -> bytes: + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w", zipfile.ZIP_DEFLATED) as archive: + archive.writestr(DECOY, DECOY_CONTENT) + archive.writestr(MEMBER, CONTENT) + return buffer.getvalue() + + +@pytest.fixture +def archived(http_server): + """Build a dataset whose file is one member of a zip served from localhost.""" + base, directory = http_server + (directory / "Fig_01.zip").write_bytes(_zip_bytes()) + + def make(**overrides): + archive = {"url": f"{base}/Fig_01.zip", "member": MEMBER, **overrides.pop("archive", {})} + spec = { + "description": "One headerless raw file inside a zip.", + "source": base, + "file": "scan.raw", + "checksum": f"md5:{hashlib.md5(CONTENT).hexdigest()}", + "size_bytes": len(CONTENT), + "archive": archive, + **overrides, + } + return DownloadableDataset(**spec) + + return make + + +@pytest.fixture +def dest(tmp_path): + """A download directory of its own. + + ``tmp_path`` itself is not empty - the config fixture and ``http_server`` + both keep directories there - so "nothing was left behind" has to be asked + of a directory only the download writes to. + """ + path = tmp_path / "downloads" + path.mkdir() + return path + + +class _Recorder: + """The progress protocol, remembering what it was driven with.""" + + def __init__(self): + self.total = 0 + self.done = 0 + self.closed = False + + def update(self, n): + self.done += n + + def reset(self): + self.done = 0 + + def close(self): + self.closed = True + + +def test_the_member_comes_out_byte_for_byte(archived, dest): + """Including that it is the named member, not the one with the same basename.""" + path = archived().download(destination=dest, progressbar=False, background=False) + assert Path(path).read_bytes() == CONTENT + + +def test_download_url_is_the_archive(archived): + """The member has no link of its own; the archive is the only one there is.""" + assert archived().download_url.endswith("/Fig_01.zip") + + +def test_a_wrong_checksum_fails_and_leaves_no_file(archived, dest): + ds = archived(checksum="md5:" + "0" * 32) + with pytest.raises(ValueError): + ds.download(destination=dest, progressbar=False, background=False) + assert list(dest.iterdir()) == [] # not the file, and not a temporary either + + +def test_the_progress_object_is_driven_the_way_pooch_drives_it(archived, dest): + bar = _Recorder() + archived().download(destination=dest, progressbar=bar, background=False) + assert bar.total == len(CONTENT) + assert bar.done == len(CONTENT) # reset() then one final update, so it reaches 100% + assert bar.closed + + +def test_a_cancel_raised_from_update_aborts_and_leaves_no_file(archived, dest): + """How the widgets cancel a download: the exception comes back out of update.""" + + class Cancelling(_Recorder): + def update(self, n): + raise DownloadCancelled("stop") + + with pytest.raises(DownloadCancelled): + archived().download(destination=dest, progressbar=Cancelling(), background=False) + assert list(dest.iterdir()) == [] + + +def test_a_member_the_archive_does_not_hold_names_it(archived, dest): + ds = archived(archive={"member": "Fig_01/Panel_g-h_Themis/missing.raw"}) + with pytest.raises(KeyError, match="missing.raw"): + ds.download(destination=dest, progressbar=False, background=False) + + +def test_a_host_that_ignores_range_fails_loudly(archived, dest, http_server): + """Answering 200 to a range request means the whole archive; refuse it.""" + base, _ = http_server + ds = archived(archive={"url": f"{base}/attached/Fig_01.zip"}) + with pytest.raises(ArchiveError, match="Range"): + ds.download(destination=dest, progressbar=False, background=False) + assert list(dest.iterdir()) == [] + + +def test_filepath_and_delete_behave_as_for_any_other_dataset(archived, dest): + config.set({"locations": {"personal": str(dest)}}) + + ds = archived() + assert ds.filepath() is None + ds.download(progressbar=False, background=False) + assert ds.filepath() == dest / "scan.raw" + assert ds.delete() is True + assert ds.filepath() is None diff --git a/emdatabase/tests/test_fill_download_fields.py b/emdatabase/tests/test_fill_download_fields.py index 23b41a7..0f43397 100644 --- a/emdatabase/tests/test_fill_download_fields.py +++ b/emdatabase/tests/test_fill_download_fields.py @@ -24,6 +24,8 @@ FILE = "MyData.zspy" CONTENT = b"a small 4D-STEM dataset, allegedly" * 100 MD5 = f"md5:{hashlib.md5(CONTENT).hexdigest()}" +# Deliberately unreachable: an archive entry must never be downloaded here. +ARCHIVE = {"url": "https://example.invalid/Figures.zip", "member": "Fig/Panel/scan.raw"} @pytest.fixture(scope="module") @@ -85,6 +87,29 @@ def test_a_blank_entry_is_filled_in_and_written(script, index, tmp_path): assert MD5 in summary and str(path) in summary +def test_an_archive_entry_with_blank_fields_is_refused(script, index, tmp_path): + """Following the link would hash the whole zip, not the member inside it.""" + _, directory, write = index + path = write(archive=ARCHIVE) + before = path.read_text(encoding="utf-8") + + code, summary = _run(script, directory, tmp_path) + assert code == 1 + assert "by hand" in summary + assert path.read_text(encoding="utf-8") == before # nothing guessed, nothing written + + +def test_a_complete_archive_entry_is_not_downloaded(script, index, tmp_path): + """The archive URL does not resolve, so getting here at all means it was left alone.""" + _, directory, write = index + path = write(checksum=MD5, size_bytes=len(CONTENT), archive=ARCHIVE) + before = path.read_text(encoding="utf-8") + + code, _ = _run(script, directory, tmp_path) + assert code == 0 + assert path.read_text(encoding="utf-8") == before + + def test_a_complete_index_is_not_rewritten(script, index, tmp_path): _, directory, write = index path = write(checksum=MD5, size_bytes=len(CONTENT)) diff --git a/emdatabase/tests/test_load_data.py b/emdatabase/tests/test_load_data.py index 14e9e47..8694eba 100644 --- a/emdatabase/tests/test_load_data.py +++ b/emdatabase/tests/test_load_data.py @@ -10,16 +10,19 @@ them. """ +import io import os import threading import time import urllib.error import urllib.request +import zipfile from pathlib import Path import pytest import emdatabase.data as data +from emdatabase._archive import _HTTPRangeFile from emdatabase.data import MgONanoCrystals, NiEBSDLarge from emdatabase.downloadable_dataset import ( _PENDING, @@ -67,6 +70,17 @@ def _head(url, timeout=60): return urllib.request.urlopen(request, timeout=timeout) +def _archive_member(url, member): + """The directory entry for one member of a remote zip. + + A few small range requests rather than the whole archive, which is the + reason an entry names a member in the first place. + """ + with io.BufferedReader(_HTTPRangeFile(url), buffer_size=1 << 20) as stream: + with zipfile.ZipFile(stream) as archive: + return archive.getinfo(member) + + @pytest.mark.network @pytest.mark.parametrize( ("name", "version"), @@ -82,7 +96,8 @@ def test_source_url_resolves(name, version): of the push matrix meant 120 of them per push. The weekly check_sources workflow runs it instead. """ - resolved = getattr(data, name)()._resolve(version) + dataset = getattr(data, name)() + resolved = dataset._resolve(version) url = resolved.url try: response = _head(url) @@ -95,10 +110,22 @@ def test_source_url_resolves(name, version): # source file being replaced, which is otherwise invisible until someone's # checksum fails. A weights family's `latest` link is meant to serve new # bytes, so only a pinned link is held to its declared size. + # An archive entry's link is the zip, so the length to compare is the + # archive's; the entry's own size_bytes describes the member inside it. + archive = dataset.metadata.archive + declared = archive.size_bytes if archive else resolved.size_bytes length = response.headers.get("Content-Length") - if resolved.pinned and length is not None and resolved.size_bytes is not None: - assert int(length) == resolved.size_bytes, ( - f"{name}: {url} is {int(length)} bytes, but the YAML declares {resolved.size_bytes}" + if resolved.pinned and length is not None and declared is not None: + assert int(length) == declared, ( + f"{name}: {url} is {int(length)} bytes, but the YAML declares {declared}" + ) + if archive is not None: + # What rots for an archive entry is the member being renamed or moved + # inside a zip whose own size never changes. + info = _archive_member(url, archive.member) + assert info.file_size == resolved.size_bytes, ( + f"{name}: {archive.member} is {info.file_size} bytes inside {url}, " + f"but the YAML declares {resolved.size_bytes}" ) diff --git a/emdatabase/tests/test_metadata.py b/emdatabase/tests/test_metadata.py index 88cb1d8..f178177 100644 --- a/emdatabase/tests/test_metadata.py +++ b/emdatabase/tests/test_metadata.py @@ -13,6 +13,7 @@ from emdatabase.metadata import ( TEMPLATE_PATH, + ArchiveMember, Author, DatasetMetadata, WeightsVersion, @@ -109,6 +110,14 @@ def test_weights_file_schema_and_dataclass_agree(): assert ENTRY_SCHEMA["properties"]["latest"] == {"$ref": "#/$defs/weightsFile"} +def test_archive_member_schema_and_dataclass_agree(): + """The archive a member is fetched out of is the same four fields, in two files.""" + archive = SCHEMA["$defs"]["archiveMember"] + assert list(archive["properties"]) == [f.name for f in dataclasses.fields(ArchiveMember)] + assert archive["required"] == ["url", "member"] + assert ENTRY_SCHEMA["properties"]["archive"] == {"$ref": "#/$defs/archiveMember"} + + @pytest.mark.parametrize( ("file", "expected"), [("w.pt", "w_260902.pt"), ("weights", "weights_260902"), ("a.tar.gz", "a.tar_260902.gz")], diff --git a/emdatabase/tests/test_new_dataset.py b/emdatabase/tests/test_new_dataset.py index cc2e884..b80362f 100644 --- a/emdatabase/tests/test_new_dataset.py +++ b/emdatabase/tests/test_new_dataset.py @@ -16,6 +16,8 @@ from emdatabase.metadata import validate_file from emdatabase.new_dataset import ( + FIELD_ORDER, + build_document, default_name, fill_download_fields, main, @@ -469,6 +471,24 @@ def test_keep_leaves_the_temporary_download(server, tmp_path, monkeypatch): assert kept[0].read_bytes() == CONTENT +def test_build_document_keeps_the_archive_block(): + """A key missing from FIELD_ORDER is dropped, silently, from every entry the + CLI, the issue route and both CI scripts rewrite.""" + entry = { + "description": "One headerless raw file inside a zip.", + "source": "https://zenodo.org/records/1/files", + "file": "scan.raw", + "archive": { + "url": "https://zenodo.org/records/1/files/Fig_01.zip", + "member": "Fig_01/Panel/scan.raw", + }, + } + written = build_document("Archived", entry)["Archived"] + + assert written["archive"] == entry["archive"] + assert list(written) == [key for key in FIELD_ORDER if key in written] + + def test_write_document_matches_the_hand_written_style(tmp_path): long_text = "word " * 30 url = "https://example.org/" + "x" * 90