From 8d8b3ec89219aeebd553ea930cf50791291d7d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20M=C3=BCller?= Date: Thu, 22 Oct 2026 09:41:47 +0200 Subject: [PATCH] fix: BagIt manifest incomplete for recently uploaded datasets - `bag_dataset` downloaded the condensed file for all resources ending with ".rtdc", but `write_manifest` only listed it for resources with mimetype "RT-DC", which DCOR sets in the background after the upload; use `is_dc_resource_dict` in both places and always list existing condensed files in the manifest - if DCOR did not yet compute the SHA256 sum of a resource, compute it from the downloaded (ETag-verified) file instead of writing "None" Co-Authored-By: Claude Opus 5.5 --- CHANGELOG | 3 +++ dcoraid/bagit/archive.py | 6 ++++-- dcoraid/bagit/manifest.py | 25 +++++++++++++-------- tests/test_bagit_archive.py | 43 +++++++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 663bf71..e132f0e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,9 @@ - fix: `dcoraid-upload-task` silenced exceptions by returning in `finally` - fix: BagIt archiving silently created bags for resources whose download or verification failed + - fix: BagIt archives of recently uploaded datasets could be invalid, + because the condensed file or the SHA256 sum of a resource was missing + in the manifest - fix: comparing the cached dataset list with a list of a different length returned True or raised an IndexError - fix: dataset search via the API returned more results than `limit` diff --git a/dcoraid/bagit/archive.py b/dcoraid/bagit/archive.py index ea24d44..1c237c1 100644 --- a/dcoraid/bagit/archive.py +++ b/dcoraid/bagit/archive.py @@ -14,6 +14,7 @@ from ..api import CKANAPI from ..api import errors as api_errors +from ..common import is_dc_resource_dict from ..dbmodel import APIInterrogator from ..download import DownloadJob from . import info, manifest @@ -243,8 +244,9 @@ def bag_dataset(api: CKANAPI, if abort_event is not None and abort_event.is_set(): return - # condensed resource - if res_dict["name"].endswith(".rtdc"): + # condensed resource (must match the criterion used in + # `manifest.write_manifest`) + if is_dc_resource_dict(res_dict): download_resource(api=api, bag_path=bag_path, res_dict=res_dict, diff --git a/dcoraid/bagit/manifest.py b/dcoraid/bagit/manifest.py index 7581955..c726cd9 100644 --- a/dcoraid/bagit/manifest.py +++ b/dcoraid/bagit/manifest.py @@ -1,6 +1,8 @@ import hashlib import warnings +from ..common import is_dc_resource_dict + class ManifestValidationError(BaseException): pass @@ -46,15 +48,20 @@ def write_manifest(bag_path, ds_dict): """ manifest = [] for res_dict in ds_dict["resources"]: - manifest.append(f"{res_dict['sha256']} data/{res_dict['name']}") - if res_dict.get("mimetype") == "RT-DC": - pp_res = bag_path / "data" / res_dict["name"] - pp_cond = pp_res.with_name(f"{pp_res.stem}_condensed.rtdc") - # compute that hash for the condensed file - if pp_cond.exists(): - manifest.append(f"{hash_file(pp_cond)} data/{pp_cond.name}") - else: - warnings.warn(f"No condensed file found for {pp_res}") + pp_res = bag_path / "data" / res_dict["name"] + # The SHA256 sum is computed by DCOR in the background after the + # upload. If it is not available yet, compute it from the + # downloaded file (which was verified via its ETag). + res_hash = res_dict.get("sha256") or hash_file(pp_res) + manifest.append(f"{res_hash} data/{pp_res.name}") + # The condensed file is downloaded for all DC resources (see + # `archive.bag_dataset`). Every file in the payload must be listed + # in the manifest, so we add it whenever it exists. + pp_cond = pp_res.with_name(f"{pp_res.stem}_condensed.rtdc") + if pp_cond.exists(): + manifest.append(f"{hash_file(pp_cond)} data/{pp_cond.name}") + elif is_dc_resource_dict(res_dict): + warnings.warn(f"No condensed file found for {pp_res}") # add dataset.json json_hash = hash_file(bag_path / "data" / "dataset.json") diff --git a/tests/test_bagit_archive.py b/tests/test_bagit_archive.py index 90ec7ea..cbd5f7e 100644 --- a/tests/test_bagit_archive.py +++ b/tests/test_bagit_archive.py @@ -1,3 +1,4 @@ +import hashlib from unittest import mock import warnings import uuid @@ -166,3 +167,45 @@ def task_verify_resource(self): assert len(temp_paths) == 2 for pp in temp_paths: assert not pp.exists() + + +def fake_download_resource(api, bag_path, res_dict, condensed, **kwargs): + """Write a dummy file instead of downloading the resource""" + dl_path = bag_path / "data" / res_dict["name"] + if condensed: + dl_path = dl_path.with_name(dl_path.stem + "_condensed.rtdc") + dl_path.write_bytes(f"content of {dl_path.name}".encode()) + + +@pytest.mark.parametrize("server_metadata", [True, False]) +def test_bag_dataset_fresh_metadata(tmp_path, server_metadata): + """Bags must be valid even if DCOR did not yet process the resources + + Right after an upload, DCOR might not yet have set the "mimetype" + and "sha256" of a resource (offline test). + """ + resources = [{"id": "res-1", "name": "data.rtdc"}, + {"id": "res-2", "name": "notes.txt"}] + if server_metadata: + for res_dict in resources: + content = f"content of {res_dict['name']}".encode() + res_dict["sha256"] = hashlib.sha256(content).hexdigest() + resources[0]["mimetype"] = "RT-DC" + ds_dict = {"id": "ds-1", + "name": "dataset-1", + "title": "Dataset 1", + "organization": {"name": "circle-1"}, + "resources": resources, + } + with mock.patch.object(archive, "download_resource", + fake_download_resource): + archive.bag_dataset(api=mock.MagicMock(), + ds_dict=ds_dict, + bag_path=tmp_path) + + payload = sorted(pp.name for pp in (tmp_path / "data").iterdir()) + assert payload == ["data.rtdc", "data_condensed.rtdc", "dataset.json", + "notes.txt"] + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + bagit.Bag(str(tmp_path)).validate()