diff --git a/lib/galaxy/dependencies/__init__.py b/lib/galaxy/dependencies/__init__.py index 1c8b7d6bf16..1b0fbd611a8 100644 --- a/lib/galaxy/dependencies/__init__.py +++ b/lib/galaxy/dependencies/__init__.py @@ -258,12 +258,14 @@ class ConditionalDependencies: def check_fs_dropboxfs(self): return "dropbox" in self.file_sources - def check_fs_webdavfs(self): + def check_webdav4(self): return "webdav" in self.file_sources + def check_fs_webdavfs(self): + return self.check_webdav4() + def check_webdavclient3(self): - # fs.webdavfs dependency for which we need an unreleased version - return self.check_fs_webdavfs() + return self.check_webdav4() def check_fs_anvilfs(self): # pyfilesystem plugin access to terra on anvil diff --git a/lib/galaxy/dependencies/conditional-requirements.txt b/lib/galaxy/dependencies/conditional-requirements.txt index 9bba95f0d73..90128df6053 100644 --- a/lib/galaxy/dependencies/conditional-requirements.txt +++ b/lib/galaxy/dependencies/conditional-requirements.txt @@ -21,9 +21,7 @@ fastmcp>=2.13.0 redis>=5.3.0,<6 # For file sources plugins -fs.webdavfs>=0.4.2 # type: webdav -# webdavclient3 on the develop branch contains an important fix for username-based authentification containing the '@' symbol -webdavclient3 @ git+https://github.com/ezhov-evgeny/webdav-client-python-3@98c23d1abd15efc3db9cfc756429f00041578bc2 +webdav4[fsspec] # type: webdav fs.dropboxfs>=1.0.3 # type: dropbox fs.sshfs # type: ssh fs.anvilfs # type: anvil diff --git a/lib/galaxy/files/sources/webdav.py b/lib/galaxy/files/sources/webdav.py index e65193087a4..857a946f639 100644 --- a/lib/galaxy/files/sources/webdav.py +++ b/lib/galaxy/files/sources/webdav.py @@ -1,11 +1,6 @@ -try: - from webdavfs.webdavfs import WebDAVFS -except ImportError: - WebDAVFS = None - -import tempfile from typing import ( Annotated, + Any, Optional, Union, ) @@ -13,86 +8,153 @@ from typing import ( from pydantic import ( Field, field_validator, + model_validator, ) -from galaxy.files.models import ( - BaseFileSourceConfiguration, - BaseFileSourceTemplateConfiguration, - FilesSourceRuntimeContext, -) +from galaxy.files.models import FilesSourceRuntimeContext from galaxy.util.config_templates import TemplateExpansion -from ._pyfilesystem2 import PyFilesystem2FilesSource +from ._fsspec import ( + CacheOptionsDictType, + FsspecBaseFileSourceConfiguration, + FsspecBaseFileSourceTemplateConfiguration, + FsspecFilesSource, +) + +try: + from webdav4.fsspec import WebdavFileSystem +except ImportError: + WebdavFileSystem = None -class WebDavFileSourceTemplateConfiguration(BaseFileSourceTemplateConfiguration): +def _normalize_root(root: Optional[str]) -> Optional[str]: + if root is None: + return None + root = root.strip() + if not root or root == "/": + return None + return f"/{root.strip('/')}" + + +def _normalize_base_url(base_url: Optional[str]) -> Optional[str]: + if base_url is None: + return None + base_url = base_url.strip() + if not base_url: + return None + return base_url.rstrip("/") + + +def _compose_base_url(url: Optional[str], root: Optional[str]) -> Optional[str]: + if not url: + return None + url = url.rstrip("/") + normalized_root = _normalize_root(root) + if normalized_root: + return f"{url}{normalized_root}" + return url + + +class WebDavFileSourceTemplateConfiguration(FsspecBaseFileSourceTemplateConfiguration): url: Union[str, TemplateExpansion, None] = None root: Optional[Union[str, TemplateExpansion]] = None + base_url: Union[str, TemplateExpansion, None] = None login: Optional[Union[str, TemplateExpansion]] = None password: Optional[Union[str, TemplateExpansion]] = None temp_path: Optional[Union[str, TemplateExpansion]] = None use_temp_files: Union[bool, TemplateExpansion] = True + @model_validator(mode="before") + @classmethod + def normalize_endpoint(cls, data: Any) -> Any: + if not isinstance(data, dict): + return data + normalized = dict(data) + normalized["root"] = _normalize_root(normalized.get("root")) + normalized["base_url"] = _normalize_base_url( + normalized.get("base_url") or _compose_base_url(normalized.get("url"), normalized.get("root")) + ) + return normalized -class WebDavFileSourceConfiguration(BaseFileSourceConfiguration): - # Override url field to make it required for WebDAV - we keep a default but validate it's provided - url: Annotated[ + +class WebDavFileSourceConfiguration(FsspecBaseFileSourceConfiguration): + url: Optional[str] = None + root: Optional[str] = None + base_url: Annotated[ str, Field( - title="WebDAV URL", - description="The URL of the WebDAV server. This is required for WebDAV file sources.", + title="WebDAV base URL", + description="The fully-qualified WebDAV endpoint URL used to access this file source.", ), - ] = None # type: ignore[assignment] - root: Optional[str] = None + ] login: Optional[str] = None password: Optional[str] = None temp_path: Optional[str] = None - use_temp_files: bool = True # Default to True to avoid memory issues with large files. + use_temp_files: bool = True - @field_validator("url") + @model_validator(mode="before") @classmethod - def validate_url_required(cls, v): - if v is None or v == "": - raise ValueError("url is required for WebDAV file source") + def normalize_endpoint(cls, data: Any) -> Any: + if not isinstance(data, dict): + return data + normalized = dict(data) + normalized["root"] = _normalize_root(normalized.get("root")) + normalized["base_url"] = _normalize_base_url( + normalized.get("base_url") or _compose_base_url(normalized.get("url"), normalized.get("root")) + ) + return normalized + + @field_validator("base_url") + @classmethod + def validate_base_url_required(cls, v: str) -> str: + if not v: + raise ValueError("base_url is required for WebDAV file source") return v -class WebDavFilesSource(PyFilesystem2FilesSource[WebDavFileSourceTemplateConfiguration, WebDavFileSourceConfiguration]): +class WebDavFilesSource(FsspecFilesSource[WebDavFileSourceTemplateConfiguration, WebDavFileSourceConfiguration]): plugin_type = "webdav" - required_module = WebDAVFS - required_package = "fs.webdavfs" - allow_key_error_on_empty_directories = True + required_module = WebdavFileSystem + required_package = "webdav4" template_config_class = WebDavFileSourceTemplateConfiguration resolved_config_class = WebDavFileSourceConfiguration - def _open_fs(self, context: FilesSourceRuntimeContext[WebDavFileSourceConfiguration]): - if WebDAVFS is None: + def __init__(self, template_config: WebDavFileSourceTemplateConfiguration): + defaults: dict[str, Any] = {} + if ( + "use_temp_files" not in template_config.model_fields_set + and template_config.file_sources_config.webdav_use_temp_files is not None + ): + defaults["use_temp_files"] = template_config.file_sources_config.webdav_use_temp_files + if defaults: + template_config = self._apply_defaults_to_template(defaults, template_config) + super().__init__(template_config) + + def _open_fs( + self, + context: FilesSourceRuntimeContext[WebDavFileSourceConfiguration], + cache_options: CacheOptionsDictType, + ): + if WebdavFileSystem is None: raise self.required_package_exception config = context.config - file_sources_config = self._file_sources_config - use_temp_files = config.use_temp_files - if file_sources_config and file_sources_config.webdav_use_temp_files is not None: - use_temp_files = file_sources_config.webdav_use_temp_files + auth = (config.login, config.password) if config.login or config.password else None + return WebdavFileSystem(config.base_url, auth=auth, **cache_options) - if use_temp_files: - temp_path = config.temp_path - if temp_path is None and file_sources_config and file_sources_config.tmp_dir: - temp_path = file_sources_config.tmp_dir - if temp_path is None: - temp_path = tempfile.mkdtemp(prefix="webdav_") - config.temp_path = temp_path - config.use_temp_files = use_temp_files + def _to_filesystem_path(self, path: str, config: WebDavFileSourceConfiguration) -> str: + if path in ("", "/"): + return "" + return path.lstrip("/") - handle = WebDAVFS( - url=config.url, - root=config.root, - login=config.login, - password=config.password, - temp_path=config.temp_path, - use_temp_files=config.use_temp_files, - ) - return handle + def _adapt_entry_path(self, filesystem_path: str, config: WebDavFileSourceConfiguration) -> str: + if not filesystem_path or filesystem_path == "/": + return "/" + return filesystem_path if filesystem_path.startswith("/") else f"/{filesystem_path}" + + def _get_cache_options(self, config: WebDavFileSourceConfiguration) -> dict[str, Any]: + # webdav4 does not accept fsspec listing-cache constructor kwargs and forwards unexpected values to its client implementation. + return {} __all__ = ("WebDavFilesSource",) diff --git a/test/integration/test_webdav.py b/test/integration/test_webdav.py index 6f93147d3e3..ae3e594ee99 100644 --- a/test/integration/test_webdav.py +++ b/test/integration/test_webdav.py @@ -11,6 +11,8 @@ from galaxy_test.base import api_asserts from galaxy_test.base.populators import DatasetPopulator from galaxy_test.driver import integration_util +pytest.importorskip("webdav4.fsspec") + SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) FILE_SOURCES_JOB_CONF = os.path.join(SCRIPT_DIRECTORY, "file_sources_conf.yml") diff --git a/test/unit/files/test_webdav.py b/test/unit/files/test_webdav.py index c66df98435c..84a9ed176c9 100644 --- a/test/unit/files/test_webdav.py +++ b/test/unit/files/test_webdav.py @@ -18,6 +18,8 @@ from ._util import ( ) from .test_posix import _download_and_check_file +pytest.importorskip("webdav4.fsspec") + SCRIPT_DIRECTORY = os.path.abspath(os.path.dirname(__file__)) FILE_SOURCES_CONF = os.path.join(SCRIPT_DIRECTORY, "webdav_file_sources_conf.yml") FILE_SOURCES_CONF_NO_USE_TEMP_FILES = os.path.join(SCRIPT_DIRECTORY, "webdav_file_sources_without_use_temp_conf.yml") @@ -69,9 +71,15 @@ def test_sniff_to_tmp(): def test_serialization(): configs = [FILE_SOURCES_CONF_NO_USE_TEMP_FILES, FILE_SOURCES_CONF] for config in configs: + file_sources_o = configured_file_sources(config) + original = file_source_as_webdav(file_sources_o._file_sources[0]) + assert original._get_runtime_context().config.base_url == "http://127.0.0.1:7083" + # serialize the configured file sources and rematerialize them, # ensure they still function. This is needed for uploading files. - file_sources = serialize_and_recover(configured_file_sources(config)) + file_sources = serialize_and_recover(file_sources_o) + recovered = file_source_as_webdav(file_sources._file_sources[0]) + assert recovered._get_runtime_context().config.base_url == "http://127.0.0.1:7083" res = list_root(file_sources, "gxfiles://test1", recursive=True) assert find_file_a(res) @@ -114,9 +122,15 @@ def test_serialization_user(): file_sources_o = configured_file_sources(USER_FILE_SOURCES_CONF) user_context = user_context_fixture() + original = file_source_as_webdav(file_sources_o._file_sources[0]) + assert original._get_runtime_context(user_context=user_context).config.base_url == "http://127.0.0.1:7083" + res = list_root(file_sources_o, "gxfiles://test1", recursive=True, user_context=user_context) assert find_file_a(res) file_sources = serialize_and_recover(file_sources_o, user_context=user_context) + recovered = file_source_as_webdav(file_sources._file_sources[0]) + assert recovered._get_runtime_context().config.base_url == "http://127.0.0.1:7083" + res = list_root(file_sources, "gxfiles://test1", recursive=True, user_context=None) assert find_file_a(res)