diff --git a/src/datajoint/builtin_codecs/filepath.py b/src/datajoint/builtin_codecs/filepath.py index f20047963..9ba128c86 100644 --- a/src/datajoint/builtin_codecs/filepath.py +++ b/src/datajoint/builtin_codecs/filepath.py @@ -164,7 +164,7 @@ def encode( return { "path": path, - "store": store_name, + "store": config.resolve_store_name(store_name, use_filepath_default=True), "size": size, "is_dir": is_dir, "timestamp": datetime.now(timezone.utc).isoformat(), diff --git a/src/datajoint/builtin_codecs/npy.py b/src/datajoint/builtin_codecs/npy.py index a2bb7519a..c6eab2118 100644 --- a/src/datajoint/builtin_codecs/npy.py +++ b/src/datajoint/builtin_codecs/npy.py @@ -356,7 +356,7 @@ def encode( # Return metadata (includes numpy-specific shape/dtype) return { "path": path, - "store": store_name, + "store": config.resolve_store_name(store_name), "dtype": str(value.dtype), "shape": list(value.shape), } diff --git a/src/datajoint/builtin_codecs/object.py b/src/datajoint/builtin_codecs/object.py index 755688262..138f8a9c2 100644 --- a/src/datajoint/builtin_codecs/object.py +++ b/src/datajoint/builtin_codecs/object.py @@ -168,7 +168,7 @@ def encode( timestamp = datetime.now(timezone.utc) metadata = { "path": path, - "store": store_name, + "store": config.resolve_store_name(store_name), "size": size, "ext": ext, "is_dir": is_dir, diff --git a/src/datajoint/builtin_codecs/schema.py b/src/datajoint/builtin_codecs/schema.py index 7a55e68a8..87a8f6148 100644 --- a/src/datajoint/builtin_codecs/schema.py +++ b/src/datajoint/builtin_codecs/schema.py @@ -66,7 +66,7 @@ def encode(self, value, *, key=None, context=None, store_name=None): ) backend = self._get_backend(store_name, config=config) backend.put_buffer(serialize(value), path) - return {"path": path, "store": store_name, ...} + return {"path": path, "store": config.resolve_store_name(store_name), ...} def decode(self, stored, *, key=None, context=None): config = self._codec_config(key, context) diff --git a/src/datajoint/gc.py b/src/datajoint/gc.py index 68820ab1e..97f8c89d2 100644 --- a/src/datajoint/gc.py +++ b/src/datajoint/gc.py @@ -113,8 +113,14 @@ def __init__(self, *schemas: "Schema", store: str | None = None, config=None) -> if not schemas: raise DataJointError("At least one schema must be provided") self.schemas = schemas - self.store = store + # An empty name (the bare ``@`` form) means the default store, as None + # does. Kept as "", it would match no reference in _references, and every + # stored file would look orphaned. + self.store = store or None self.config = config if config is not None else schemas[0].connection._config + # References record the resolved store name. Metadata written before + # that may record an empty name or none; both mean the default store. + self._default_store = self.config.stores.get("default") # Resolve the store eagerly: validates it exists and pins the backend # and section prefixes for this collector's lifetime (one store only). self.backend = get_store_backend(store, config=self.config) @@ -174,7 +180,7 @@ def _references(self, heading_attr: str, verbose: bool) -> set[str]: try: for row in table.proj(attr_name).cursor(as_dict=True): for path, ref_store in attr.codec.referenced_paths(row[attr_name]): - if self.store is None or ref_store == self.store: + if self.store is None or (ref_store or self._default_store) == self.store: referenced.add(path) except Exception as e: logger.warning(f"Error scanning {table_name}.{attr_name}: {e}") diff --git a/src/datajoint/hash_registry.py b/src/datajoint/hash_registry.py index fb6f22818..7a14c76f7 100644 --- a/src/datajoint/hash_registry.py +++ b/src/datajoint/hash_registry.py @@ -250,7 +250,7 @@ def put_hash( "hash": content_hash, "path": path, # Always stored for retrieval "schema": schema_name, - "store": store_name, + "store": config.resolve_store_name(store_name), "size": len(data), } diff --git a/src/datajoint/settings.py b/src/datajoint/settings.py index 9a185f821..38339a1d9 100644 --- a/src/datajoint/settings.py +++ b/src/datajoint/settings.py @@ -447,6 +447,58 @@ def convert_path(cls, v: Any) -> Path | None: return None return Path(v) if not isinstance(v, Path) else v + def resolve_store_name(self, store: str | None = None, *, use_filepath_default: bool = False) -> str: + """ + Name the configured store that a store reference resolves to. + + An empty or missing name means the default store: ``stores.default``, + or ``stores.filepath_default`` for filepath references. Codecs record + the resolved name in their metadata, so a stored object stays tied to + the store it was written to even if the default is later re-pointed. + + Parameters + ---------- + store : str, optional + Store name as declared. ``""`` (the bare ``@`` form) and None both + mean the default store. + use_filepath_default : bool, optional + If True, resolve the default to stores.filepath_default instead of + stores.default. Default: False. + + Returns + ------- + str + Name of a key in ``stores``. + + Raises + ------ + DataJointError + If the default store is needed but not configured. + """ + if store: + return store + # An empty name is the bare ``@`` form (````, ````, ...), + # which the declaration keeps as "" so the codec can tell "store + # requested" from "no store"; it means the default store. + if use_filepath_default: + # Filepath references use separate default (not part of OAS) + if "filepath_default" not in self.stores: + raise DataJointError( + "stores.filepath_default is not configured. " + "Set stores.filepath_default or specify store explicitly with " + ) + store = self.stores["filepath_default"] + else: + # Integrated storage (hash, schema) uses stores.default + if "default" not in self.stores: + raise DataJointError("stores.default is not configured") + store = self.stores["default"] + + if not isinstance(store, str): + default_key = "filepath_default" if use_filepath_default else "default" + raise DataJointError(f"stores.{default_key} must be a string") + return store + def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool = False) -> dict[str, Any]: """ Get configuration for a storage store. @@ -470,25 +522,7 @@ def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool DataJointError If store is not configured or has invalid config. """ - # Handle default store - if store is None: - if use_filepath_default: - # Filepath references use separate default (not part of OAS) - if "filepath_default" not in self.stores: - raise DataJointError( - "stores.filepath_default is not configured. " - "Set stores.filepath_default or specify store explicitly with " - ) - store = self.stores["filepath_default"] - else: - # Integrated storage (hash, schema) uses stores.default - if "default" not in self.stores: - raise DataJointError("stores.default is not configured") - store = self.stores["default"] - - if not isinstance(store, str): - default_key = "filepath_default" if use_filepath_default else "default" - raise DataJointError(f"stores.{default_key} must be a string") + store = self.resolve_store_name(store, use_filepath_default=use_filepath_default) # Check store exists if store not in self.stores: diff --git a/tests/integration/test_gc.py b/tests/integration/test_gc.py index a9bd5a4a5..2a90d73a7 100644 --- a/tests/integration/test_gc.py +++ b/tests/integration/test_gc.py @@ -2,6 +2,7 @@ Tests for garbage collection (gc.py). """ +import json from contextlib import ExitStack from unittest.mock import MagicMock, patch @@ -402,6 +403,108 @@ def test_custom_codec_survives_collect(self, schema_custom): assert not (deleted_paths & stored_after), f"deleted row's orphan not reclaimed: {deleted_paths & stored_after}" +class GcBareNpyTest(dj.Manual): + definition = """ + rid : int32 + --- + waveform : + """ + + +class TestBareAtUsesDefaultStore: + """```` with no store name writes to, reads from, and is collected as stores.default.""" + + @pytest.fixture + def schema_bare(self, connection_test, prefix, mock_stores): + dj.config.stores["default"] = "local" + schema = dj.Schema( + f"{prefix}_test_gc_bare_at", + context={"GcBareNpyTest": GcBareNpyTest}, + connection=connection_test, + ) + schema(GcBareNpyTest) + yield schema + schema.drop() + + def test_insert_without_default_store_raises(self, connection_test, prefix, mock_stores): + """```` with no stores.default configured fails loudly on insert.""" + dj.config.stores.pop("default", None) + schema = dj.Schema( + f"{prefix}_test_gc_bare_at_no_default", + context={"GcBareNpyTest": GcBareNpyTest}, + connection=connection_test, + ) + try: + schema(GcBareNpyTest) + with pytest.raises(DataJointError, match="stores.default is not configured"): + GcBareNpyTest.insert1({"rid": 1, "waveform": np.arange(4, dtype="float32")}) + assert len(GcBareNpyTest()) == 0 + finally: + schema.drop() + + def test_roundtrip(self, schema_bare): + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + def test_collecting_the_default_store_by_name_keeps_bare_at_files(self, schema_bare): + """A collector bound to the default store by name must count bare-@ references.""" + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + path = (GcBareNpyTest & "rid=1").fetch1("waveform").path + + stats = _gc(schema_bare, store="local").collect(dry_run=False) + + assert stats["schema_paths_referenced"] >= 1 + assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"]) + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + def test_metadata_records_the_resolved_store(self, schema_bare): + """A bare-@ write records the store it went to, not the empty name.""" + GcBareNpyTest.insert1({"rid": 1, "waveform": np.arange(8, dtype="float32")}) + + (raw,) = (GcBareNpyTest & "rid=1").proj("waveform").cursor(as_dict=True) + metadata = raw["waveform"] + if isinstance(metadata, str): # MySQL returns JSON as text, PostgreSQL as a dict + metadata = json.loads(metadata) + + assert metadata["store"] == "local" + + def test_repointing_the_default_store_keeps_existing_files(self, schema_bare): + """Re-pointing stores.default leaves existing bare-@ objects readable and collected. + + References record "local", so after the default moves to "raw" they still read + from "local", and a collector bound to "local" still counts them. + """ + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + path = (GcBareNpyTest & "rid=1").fetch1("waveform").path + + dj.config.stores["default"] = "raw" + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + stats = _gc(schema_bare, store="local").collect(dry_run=False) + + assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"]) + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + def test_empty_store_name_is_the_default_collector(self, schema_bare): + """``store=""`` builds the same collector as no store, and deletes no live file.""" + waveform = np.arange(64, dtype="float32") + GcBareNpyTest.insert1({"rid": 1, "waveform": waveform}) + path = (GcBareNpyTest & "rid=1").fetch1("waveform").path + + collector = _gc(schema_bare, store="") + assert collector.store is None + + stats = collector.collect(dry_run=False) + + assert stats["schema_paths_referenced"] >= 1 + assert not any(orphan.endswith(path) for orphan in stats["orphaned_schema_paths"]) + assert np.array_equal(np.asarray((GcBareNpyTest & "rid=1").fetch1("waveform")), waveform) + + class TestDirectoryObjects: """Directory-valued objects (e.g. Zarr stores) — the referenced metadata path is a directory PREFIX whose stored form is many chunk files plus a diff --git a/tests/unit/test_settings.py b/tests/unit/test_settings.py index 22de19d14..a2fd26b3f 100644 --- a/tests/unit/test_settings.py +++ b/tests/unit/test_settings.py @@ -396,6 +396,73 @@ def test_get_store_spec_default_store(self): finally: dj.config.stores = original_stores + def test_get_store_spec_empty_name_is_default_store(self): + """An empty store name (the bare ``@`` in ````) resolves to stores.default.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores["default"] = "my_default" + dj.config.stores["my_default"] = { + "protocol": "file", + "location": "/tmp/default", + } + assert dj.config.get_store_spec("") == dj.config.get_store_spec(None) + assert dj.config.get_store_spec("")["location"] == "/tmp/default" + finally: + dj.config.stores = original_stores + + def test_get_store_spec_empty_name_without_default_raises(self): + """With no stores.default, the bare ``@`` form names no store: an error, not a guess.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores = {"main": {"protocol": "file", "location": "/tmp/main"}} + with pytest.raises(DataJointError, match="stores.default is not configured"): + dj.config.get_store_spec("") + finally: + dj.config.stores = original_stores + + def test_get_store_spec_empty_name_is_filepath_default(self): + """For filepath references, an empty store name resolves to stores.filepath_default.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores["filepath_default"] = "raw_data" + dj.config.stores["raw_data"] = { + "protocol": "file", + "location": "/data/acquisition", + } + spec = dj.config.get_store_spec("", use_filepath_default=True) + assert spec["location"] == "/data/acquisition" + finally: + dj.config.stores = original_stores + + def test_resolve_store_name(self): + """An empty or missing name resolves to the configured default; a name to itself.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores = { + "default": "main", + "filepath_default": "raw_data", + "main": {"protocol": "file", "location": "/tmp/main"}, + "raw_data": {"protocol": "file", "location": "/data/acquisition"}, + "archive": {"protocol": "file", "location": "/tmp/archive"}, + } + assert dj.config.resolve_store_name("") == "main" + assert dj.config.resolve_store_name(None) == "main" + assert dj.config.resolve_store_name("archive") == "archive" + assert dj.config.resolve_store_name("", use_filepath_default=True) == "raw_data" + assert dj.config.resolve_store_name("archive", use_filepath_default=True) == "archive" + finally: + dj.config.stores = original_stores + + def test_resolve_store_name_without_default_raises(self): + """The bare ``@`` form with no stores.default configured is an error.""" + original_stores = dj.config.stores.copy() + try: + dj.config.stores = {"main": {"protocol": "file", "location": "/tmp/main"}} + with pytest.raises(DataJointError, match="stores.default is not configured"): + dj.config.resolve_store_name("") + finally: + dj.config.stores = original_stores + def test_get_store_spec_no_default_configured(self): """Test error when stores.default is not configured.""" original_stores = dj.config.stores.copy()