Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 24 additions & 17 deletions src/datajoint/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,27 @@ def is_url(path: str) -> bool:
return path.lower().startswith(URL_PROTOCOLS)


def _path_to_file_url(resolved_path: Path | PurePosixPath) -> str:
"""
Convert an already-resolved absolute path to a ``file://`` URL.

Uses ``as_posix()`` so the same logic handles both POSIX paths (which
already start with ``/``) and Windows paths (``C:/...``, no leading
slash) without OS-specific branching.

Parameters
----------
resolved_path : Path or PurePosixPath
Absolute, already-resolved path.

Returns
-------
str
``file://`` URL.
"""
return f"file:///{resolved_path.as_posix().lstrip('/')}"


def normalize_to_url(path: str) -> str:
"""
Normalize a path to URL form.
Expand Down Expand Up @@ -72,15 +93,7 @@ def normalize_to_url(path: str) -> str:
"""
if is_url(path):
return path
# Convert local path to file:// URL
# Ensure absolute path and proper format
abs_path = str(Path(path).resolve())
# Handle Windows paths (C:\...) vs Unix paths (/...)
if abs_path.startswith("/"):
return f"file://{abs_path}"
else:
# Windows: file:///C:/path
return f"file:///{abs_path.replace(chr(92), '/')}"
return _path_to_file_url(Path(path).resolve())


def parse_url(url: str) -> tuple[str, str]:
Expand Down Expand Up @@ -418,7 +431,7 @@ def _full_path(self, path: str | PurePosixPath) -> str:
elif self.protocol == "file":
location = self.spec.get("location", "")
if location:
return str(Path(location) / path)
return (Path(location) / path).as_posix()
return path
else:
return self._require_adapter().full_path(self.spec, path)
Expand Down Expand Up @@ -453,13 +466,7 @@ def get_url(self, path: str | PurePosixPath) -> str:
full_path = self._full_path(path)

if self.protocol == "file":
# Ensure absolute path for file:// URL
abs_path = str(Path(full_path).resolve())
if abs_path.startswith("/"):
return f"file://{abs_path}"
else:
# Windows path
return f"file:///{abs_path.replace(chr(92), '/')}"
return _path_to_file_url(Path(full_path).resolve())
elif self.protocol == "s3":
return f"s3://{full_path}"
elif self.protocol == "gcs":
Expand Down
35 changes: 35 additions & 0 deletions tests/unit/test_gc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
"""Tests for garbage collection module."""

from pathlib import PurePosixPath, PureWindowsPath
from unittest.mock import MagicMock

import pytest

from datajoint import storage
from datajoint.gc import GarbageCollector
from datajoint.storage import StorageBackend


@pytest.mark.parametrize(
("path_cls", "location"),
[
pytest.param(PureWindowsPath, "data\\blobs", id="windows"),
pytest.param(PurePosixPath, "data/blobs", id="posix"),
],
)
def test_delete_schema_path_prunes_parent_dir(monkeypatch, path_cls, location):
"""Pruning's `rsplit("/", 1)` needs forward slashes; `windows` param catches
a regression to `str(Path(...))`, `posix` pins the already-correct case."""
monkeypatch.setattr(storage, "Path", path_cls)
backend = StorageBackend.__new__(StorageBackend)
backend.spec = {"protocol": "file", "location": location}
backend.protocol = "file"
backend._fs = MagicMock()
backend.fs.exists.return_value = True
# Ensure rmdir called only once: non-empty dir stops the walk one level up
backend.fs.ls.side_effect = lambda p: [] if p == "data/blobs/schema/ab/cd" else ["sibling"]
collector = GarbageCollector.__new__(GarbageCollector)
collector.backend = backend
assert collector.delete_schema_path("schema/ab/cd/hash123") is True
backend.fs.rm.assert_called_once_with("data/blobs/schema/ab/cd/hash123")
backend.fs.rmdir.assert_called_once_with("data/blobs/schema/ab/cd")
27 changes: 26 additions & 1 deletion tests/unit/test_storage_adapter.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
"""Tests for the StorageAdapter plugin system."""

import re
from pathlib import PureWindowsPath

import pytest

import datajoint as dj
from datajoint import storage
from datajoint.errors import DataJointError
from datajoint.storage import StorageBackend
from datajoint.storage_adapter import (
_COMMON_STORE_KEYS,
StorageAdapter,
_adapter_registry,
_COMMON_STORE_KEYS,
get_storage_adapter,
)

Expand Down Expand Up @@ -193,6 +197,27 @@ def test_unsupported_protocol_get_url_raises(self):
with pytest.raises(DataJointError, match="Unsupported storage protocol"):
backend.get_url("schema/file.dat")

def test_file_protocol_full_path_uses_forward_slashes(self, monkeypatch):
"""`_full_path` must return forward slashes to match fsspec's walk()
output (gc.py relies on this for string-prefix stripping)."""
# monkeypatch to PureWindowsPath so that the test is platform-independent
monkeypatch.setattr(storage, "Path", PureWindowsPath)
backend = StorageBackend.__new__(StorageBackend)
backend.spec = {"protocol": "file", "location": "data\\blobs"}
backend.protocol = "file"
result = backend._full_path("schema/ab/cd/hash123")
assert result == "data/blobs/schema/ab/cd/hash123"

def test_file_protocol_get_url_no_backslash(self, tmp_path):
"""`get_url` must produce a valid file:// URL (forward slashes only)
on whatever OS the test runs on, including Windows."""
backend = StorageBackend.__new__(StorageBackend)
backend.spec = {"protocol": "file", "location": str(tmp_path)}
backend.protocol = "file"
result = backend.get_url("schema/ab/cd/hash123")
# exactly 3 slashes, no backslash and disregard tmp_path
assert re.fullmatch(r"file:///[^/\\][^\\]*/schema/ab/cd/hash123", result)


class TestGetStoreSpecPluginDelegation:
"""Tests for plugin protocol handling in Config.get_store_spec()."""
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/test_storage_urls.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
"""Unit tests for storage URL functions."""

from pathlib import PurePosixPath, PureWindowsPath

import pytest

from datajoint.storage import (
URL_PROTOCOLS,
_path_to_file_url,
is_url,
normalize_to_url,
parse_url,
Expand Down Expand Up @@ -79,6 +82,18 @@ def test_relative_path_becomes_absolute(self):
assert "/" in url[7:] # After "file://"


class TestPathToFileUrl:
"""Test _path_to_file_url function (platform-independent via Pure*Path)."""

def test_posix_path(self):
url = _path_to_file_url(PurePosixPath("/data/file.dat"))
assert url == "file:///data/file.dat"

def test_windows_path_no_backslash_in_result(self):
url = _path_to_file_url(PureWindowsPath("C:\\data\\file.dat"))
assert url == "file:///C:/data/file.dat"


class TestParseUrl:
"""Test parse_url function."""

Expand Down
Loading