From 648923052fdf2e56d6395399a0bb70fce4603001 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 24 Apr 2024 16:43:34 +0200 Subject: [PATCH] Refactoring + stronger type annotations in file sources --- lib/galaxy/files/__init__.py | 61 ++++++++--- lib/galaxy/files/sources/__init__.py | 113 ++++++++++++++++----- lib/galaxy/files/sources/_pyfilesystem2.py | 36 +++++-- lib/galaxy/files/sources/_rdm.py | 4 +- lib/galaxy/files/sources/base64.py | 15 ++- lib/galaxy/files/sources/drs.py | 19 +++- lib/galaxy/files/sources/ftp.py | 16 ++- lib/galaxy/files/sources/http.py | 15 ++- lib/galaxy/files/sources/invenio.py | 12 ++- lib/galaxy/files/sources/posix.py | 34 +++++-- lib/galaxy/files/sources/s3fs.py | 34 +++++-- lib/galaxy/files/sources/util.py | 4 +- lib/galaxy/job_execution/setup.py | 6 +- lib/galaxy/jobs/__init__.py | 4 +- lib/galaxy/managers/remote_files.py | 8 +- lib/galaxy/model/store/__init__.py | 6 +- lib/galaxy/tools/parameters/basic.py | 4 +- test/unit/files/_util.py | 46 +++++++-- 18 files changed, 327 insertions(+), 110 deletions(-) diff --git a/lib/galaxy/files/__init__.py b/lib/galaxy/files/__init__.py index 3ddf2a35b1da..cf82df4f2f4f 100644 --- a/lib/galaxy/files/__init__.py +++ b/lib/galaxy/files/__init__.py @@ -3,10 +3,12 @@ from collections import defaultdict from typing import ( Any, + Callable, Dict, List, NamedTuple, Optional, + Protocol, Set, ) @@ -127,7 +129,7 @@ def get_file_source_path(self, uri): path = file_source.to_relative_path(uri) return FileSourcePath(file_source, path) - def validate_uri_root(self, uri: str, user_context: "ProvidesUserFileSourcesUserContext"): + def validate_uri_root(self, uri: str, user_context: "FileSourcesUserContext"): # validate a URI against Galaxy's configuration, environment, and the current # user. Throw appropriate exception if there is a problem with the files source # referenced by the URI. @@ -170,7 +172,7 @@ def looks_like_uri(self, path_or_uri): def plugins_to_dict( self, for_serialization: bool = False, - user_context: Optional["FileSourceDictifiable"] = None, + user_context: "OptionalUserContext" = None, browsable_only: Optional[bool] = False, include_kind: Optional[Set[PluginKind]] = None, exclude_kind: Optional[Set[PluginKind]] = None, @@ -189,9 +191,7 @@ def plugins_to_dict( rval.append(el) return rval - def to_dict( - self, for_serialization: bool = False, user_context: Optional["FileSourceDictifiable"] = None - ) -> Dict[str, Any]: + def to_dict(self, for_serialization: bool = False, user_context: "OptionalUserContext" = None) -> Dict[str, Any]: return { "file_sources": self.plugins_to_dict(for_serialization=for_serialization, user_context=user_context), "config": self._file_sources_config.to_dict(), @@ -284,30 +284,59 @@ def from_dict(as_dict): ) -class FileSourceDictifiable(Dictifiable): +class DictifiableFilesSourceContext(Protocol): + @property + def role_names(self) -> Set[str]: ... + + @property + def group_names(self) -> Set[str]: ... + + @property + def file_sources(self) -> ConfiguredFileSources: ... + + def to_dict( + self, view: str = "collection", value_mapper: Optional[Dict[str, Callable]] = None + ) -> Dict[str, Any]: ... + + +class FileSourceDictifiable(Dictifiable, DictifiableFilesSourceContext): dict_collection_visible_keys = ("email", "username", "ftp_dir", "preferences", "is_admin") - def to_dict(self, view="collection", value_mapper=None): + def to_dict(self, view="collection", value_mapper: Optional[Dict[str, Callable]] = None) -> Dict[str, Any]: rval = super().to_dict(view=view, value_mapper=value_mapper) rval["role_names"] = list(self.role_names) rval["group_names"] = list(self.group_names) return rval + +class FileSourcesUserContext(DictifiableFilesSourceContext, Protocol): + @property - def role_names(self) -> Set[str]: - raise NotImplementedError + def email(self) -> str: ... @property - def group_names(self) -> Set[str]: - raise NotImplementedError + def username(self) -> str: ... @property - def file_sources(self): - """Return other filesources available in the system, for chained filesource resolution""" - raise NotImplementedError + def ftp_dir(self) -> str: ... + + @property + def preferences(self) -> Dict[str, Any]: ... + + @property + def is_admin(self) -> bool: ... + + @property + def user_vault(self) -> Dict[str, Any]: ... + + @property + def app_vault(self) -> Dict[str, Any]: ... + + +OptionalUserContext = Optional[FileSourcesUserContext] -class ProvidesUserFileSourcesUserContext(FileSourceDictifiable): +class ProvidesFileSourcesUserContext(FileSourcesUserContext, FileSourceDictifiable): """Implement a FileSourcesUserContext from a Galaxy ProvidesUserContext (e.g. trans).""" def __init__(self, trans, **kwargs): @@ -366,7 +395,7 @@ def file_sources(self): return self.trans.app.file_sources -class DictFileSourcesUserContext(FileSourceDictifiable): +class DictFileSourcesUserContext(FileSourcesUserContext, FileSourceDictifiable): def __init__(self, **kwd): self._kwd = kwd diff --git a/lib/galaxy/files/sources/__init__.py b/lib/galaxy/files/sources/__init__.py index 80745cd4d41a..504ab7b5be6d 100644 --- a/lib/galaxy/files/sources/__init__.py +++ b/lib/galaxy/files/sources/__init__.py @@ -9,9 +9,11 @@ from typing import ( Any, ClassVar, + List, Optional, Set, TYPE_CHECKING, + Union, ) from typing_extensions import ( @@ -34,7 +36,11 @@ DEFAULT_WRITABLE = False if TYPE_CHECKING: - from galaxy.files import ConfiguredFileSourcesConfig + from galaxy.files import ( + ConfiguredFileSourcesConfig, + FileSourcesUserContext, + OptionalUserContext, + ) class PluginKind(str, Enum): @@ -143,6 +149,9 @@ class RemoteFile(RemoteEntry, TFileClass): ctime: str +AnyRemoteEntry = Union[RemoteDirectory, RemoteFile] + + class SingleFileSource(metaclass=abc.ABCMeta): """ Represents a protocol handler for a single remote file that can be read by or written to by Galaxy. @@ -163,12 +172,16 @@ def get_writable(self) -> bool: """Return a boolean indicating whether this target is writable.""" @abc.abstractmethod - def user_has_access(self, user_context) -> bool: + def user_has_access(self, user_context: "OptionalUserContext") -> bool: """Return a boolean indicating whether the user can access the FileSource.""" @abc.abstractmethod def realize_to( - self, source_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + source_path: str, + native_path: str, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, ): """Realize source path (relative to uri root) to local file system path. @@ -177,14 +190,18 @@ def realize_to( :param native_path: local path to write to. e.g. `/tmp/myfile.txt` :type native_path: str :param user_context: A user context , defaults to None - :type user_context: FileSourceDictifiable, optional + :type user_context: OptionalUserContext, optional :param opts: A set of options to exercise additional control over the realize_to method. Filesource specific, defaults to None :type opts: Optional[FilesSourceOptions], optional """ @abc.abstractmethod def write_from( - self, target_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + target_path: str, + native_path: str, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, ): """Write file at native path to target_path (relative to uri root). @@ -235,7 +252,7 @@ def to_relative_path(self, url: str) -> str: returned unchanged.""" @abc.abstractmethod - def to_dict(self, for_serialization=False, user_context=None) -> FilesSourceProperties: + def to_dict(self, for_serialization=False, user_context: "OptionalUserContext" = None) -> FilesSourceProperties: """Return a dictified representation of this FileSource instance. If ``user_context`` is supplied, properties should be written so user @@ -257,7 +274,13 @@ def get_uri_root(self) -> str: """Return a prefix for the root (e.g. gxfiles://prefix/).""" @abc.abstractmethod - def list(self, path="/", recursive=False, user_context=None, opts: Optional[FilesSourceOptions] = None) -> dict: + def list( + self, + path="/", + recursive=False, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, + ) -> List[AnyRemoteEntry]: """Return dictionary of 'Directory's and 'File's.""" @@ -290,7 +313,7 @@ def get_scheme(self) -> str: def get_writable(self) -> bool: return self.writable - def user_has_access(self, user_context) -> bool: + def user_has_access(self, user_context: "OptionalUserContext") -> bool: if user_context is None and self.user_context_required: return False return ( @@ -318,7 +341,7 @@ def score_url_match(self, url: str) -> int: root = self.get_uri_root() return len(root) if root in url else 0 - def uri_from_path(self, path) -> str: + def uri_from_path(self, path: str) -> str: uri_root = self.get_uri_root() return uri_join(uri_root, path) @@ -338,7 +361,7 @@ def _parse_common_config_opts(self, kwd: FilesSourceProperties): kwd.pop("browsable", None) return kwd - def to_dict(self, for_serialization=False, user_context=None) -> FilesSourceProperties: + def to_dict(self, for_serialization=False, user_context: "OptionalUserContext" = None) -> FilesSourceProperties: rval: FilesSourceProperties = { "id": self.id, "type": self.plugin_type, @@ -364,27 +387,45 @@ def to_dict_time(self, ctime): return ctime.strftime("%m/%d/%Y %I:%M:%S %p") @abc.abstractmethod - def _serialization_props(self, user_context=None) -> FilesSourceProperties: + def _serialization_props(self, user_context: "OptionalUserContext" = None) -> FilesSourceProperties: """Serialize properties needed to recover plugin configuration. Used in to_dict method if for_serialization is True. """ - def list(self, path="/", recursive=False, user_context=None, opts: Optional[FilesSourceOptions] = None): + def list( + self, + path="/", + recursive=False, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, + ) -> List[AnyRemoteEntry]: self._check_user_access(user_context) return self._list(path, recursive, user_context, opts) - def _list(self, path="/", recursive=False, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _list( + self, + path="/", + recursive=False, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, + ): pass def create_entry( - self, entry_data: EntryData, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + entry_data: EntryData, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, ) -> Entry: self._ensure_writeable() self._check_user_access(user_context) return self._create_entry(entry_data, user_context, opts) def _create_entry( - self, entry_data: EntryData, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + entry_data: EntryData, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, ) -> Entry: """Create a new entry (directory) in the file source. @@ -393,21 +434,45 @@ def _create_entry( """ raise NotImplementedError() - def write_from(self, target_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def write_from( + self, + target_path: str, + native_path: str, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, + ): self._ensure_writeable() self._check_user_access(user_context) self._write_from(target_path, native_path, user_context=user_context, opts=opts) @abc.abstractmethod - def _write_from(self, target_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _write_from( + self, + target_path: str, + native_path: str, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, + ): pass - def realize_to(self, source_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def realize_to( + self, + source_path: str, + native_path: str, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, + ): self._check_user_access(user_context) self._realize_to(source_path, native_path, user_context, opts=opts) @abc.abstractmethod - def _realize_to(self, source_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _realize_to( + self, + source_path: str, + native_path: str, + user_context: "OptionalUserContext" = None, + opts: Optional[FilesSourceOptions] = None, + ): pass def _ensure_writeable(self): @@ -423,7 +488,7 @@ def _check_user_access(self, user_context): if user_context is not None and not self.user_has_access(user_context): raise ItemAccessibilityException(f"User {user_context.username} has no access to file source.") - def _evaluate_prop(self, prop_val: Any, user_context): + def _evaluate_prop(self, prop_val: Any, user_context: "OptionalUserContext"): rval = prop_val if isinstance(prop_val, str) and "$" in prop_val: template_context = dict( @@ -439,12 +504,12 @@ def _evaluate_prop(self, prop_val: Any, user_context): return rval - def _user_has_required_roles(self, user_context) -> bool: + def _user_has_required_roles(self, user_context: "FileSourcesUserContext") -> bool: if self.requires_roles: return self._evaluate_security_rules(self.requires_roles, user_context.role_names) return True - def _user_has_required_groups(self, user_context) -> bool: + def _user_has_required_groups(self, user_context: "FileSourcesUserContext") -> bool: if self.requires_groups: return self._evaluate_security_rules(self.requires_groups, user_context.group_names) return True @@ -467,7 +532,7 @@ def _get_error_msg_for(rule_name: str) -> str: raise ConfigurationError(_get_error_msg_for("requires_groups")) -def uri_join(*args): +def uri_join(*args: str) -> str: # url_join doesn't work with non-standard scheme if "://" in (arg0 := args[0]): scheme, path = arg0.split("://", 1) @@ -477,6 +542,6 @@ def uri_join(*args): return rval -def slash_join(*args): +def slash_join(*args: str) -> str: # https://codereview.stackexchange.com/questions/175421/joining-strings-to-form-a-url return "/".join(arg.strip("/") for arg in args) diff --git a/lib/galaxy/files/sources/_pyfilesystem2.py b/lib/galaxy/files/sources/_pyfilesystem2.py index efdb5ac79ca8..671753368aa3 100644 --- a/lib/galaxy/files/sources/_pyfilesystem2.py +++ b/lib/galaxy/files/sources/_pyfilesystem2.py @@ -3,9 +3,7 @@ import logging import os from typing import ( - Any, ClassVar, - Dict, List, Optional, Type, @@ -20,7 +18,9 @@ AuthenticationRequired, MessageException, ) +from galaxy.files import OptionalUserContext from . import ( + AnyRemoteEntry, BaseFilesSource, FilesSourceOptions, FilesSourceProperties, @@ -42,15 +42,21 @@ def __init__(self, **kwd: Unpack[FilesSourceProperties]): self._props = props @abc.abstractmethod - def _open_fs(self, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _open_fs(self, user_context: OptionalUserContext = None, opts: Optional[FilesSourceOptions] = None): """Subclasses must instantiate a PyFilesystem2 handle for this file system.""" - def _list(self, path="/", recursive=False, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _list( + self, + path="/", + recursive=False, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ) -> List[AnyRemoteEntry]: """Return dictionary of 'Directory's and 'File's.""" try: with self._open_fs(user_context=user_context, opts=opts) as h: if recursive: - res: List[Dict[str, Any]] = [] + res: List[AnyRemoteEntry] = [] for p, dirs, files in h.walk(path): to_dict = functools.partial(self._resource_info_to_dict, p) res.extend(map(to_dict, dirs)) @@ -67,11 +73,23 @@ def _list(self, path="/", recursive=False, user_context=None, opts: Optional[Fil except fs.errors.FSError as e: raise MessageException(f"Problem listing file source path {path}. Reason: {e}") from e - def _realize_to(self, source_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _realize_to( + self, + source_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ): with open(native_path, "wb") as write_file: self._open_fs(user_context=user_context, opts=opts).download(source_path, write_file) - def _write_from(self, target_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _write_from( + self, + target_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ): with open(native_path, "rb") as read_file: openfs = self._open_fs(user_context=user_context, opts=opts) dirname = fs.path.dirname(target_path) @@ -79,7 +97,7 @@ def _write_from(self, target_path, native_path, user_context=None, opts: Optiona openfs.makedirs(dirname) openfs.upload(target_path, read_file) - def _resource_info_to_dict(self, dir_path, resource_info): + def _resource_info_to_dict(self, dir_path, resource_info) -> AnyRemoteEntry: name = resource_info.name path = os.path.join(dir_path, name) uri = self.uri_from_path(path) @@ -96,7 +114,7 @@ def _resource_info_to_dict(self, dir_path, resource_info): "path": path, } - def _serialization_props(self, user_context=None): + def _serialization_props(self, user_context: OptionalUserContext = None): effective_props = {} for key, val in self._props.items(): effective_props[key] = self._evaluate_prop(val, user_context=user_context) diff --git a/lib/galaxy/files/sources/_rdm.py b/lib/galaxy/files/sources/_rdm.py index 8cd6d8523e26..3c3400c13f8d 100644 --- a/lib/galaxy/files/sources/_rdm.py +++ b/lib/galaxy/files/sources/_rdm.py @@ -8,7 +8,7 @@ from typing_extensions import Unpack from galaxy.exceptions import AuthenticationRequired -from galaxy.files import ProvidesUserFileSourcesUserContext +from galaxy.files import OptionalUserContext from galaxy.files.sources import ( BaseFilesSource, FilesSourceProperties, @@ -19,8 +19,6 @@ log = logging.getLogger(__name__) -OptionalUserContext = Optional[ProvidesUserFileSourcesUserContext] - class RDMFilesSourceProperties(FilesSourceProperties): url: str diff --git a/lib/galaxy/files/sources/base64.py b/lib/galaxy/files/sources/base64.py index 98f48ef287ed..968ae499ac5f 100644 --- a/lib/galaxy/files/sources/base64.py +++ b/lib/galaxy/files/sources/base64.py @@ -4,6 +4,7 @@ from typing_extensions import Unpack +from galaxy.files import OptionalUserContext from . import ( BaseFilesSource, FilesSourceOptions, @@ -30,14 +31,22 @@ def __init__(self, **kwd: Unpack[FilesSourceProperties]): self._props = props def _realize_to( - self, source_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + source_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): with open(native_path, "wb") as temp: temp.write(base64.b64decode(source_path[len("base64://") :])) temp.flush() def _write_from( - self, target_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + target_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): raise NotImplementedError() @@ -47,7 +56,7 @@ def score_url_match(self, url: str): else: return 0 - def _serialization_props(self, user_context=None): + def _serialization_props(self, user_context: OptionalUserContext = None): effective_props = {} for key, val in self._props.items(): effective_props[key] = self._evaluate_prop(val, user_context=user_context) diff --git a/lib/galaxy/files/sources/drs.py b/lib/galaxy/files/sources/drs.py index 590bfd4666b3..00f1a3643385 100644 --- a/lib/galaxy/files/sources/drs.py +++ b/lib/galaxy/files/sources/drs.py @@ -8,6 +8,7 @@ from typing_extensions import Unpack +from galaxy.files import OptionalUserContext from . import ( BaseFilesSource, FilesSourceOptions, @@ -48,7 +49,13 @@ def __init__(self, **kwd: Unpack[FilesSourceProperties]): def _allowlist(self): return self._file_sources_config.fetch_url_allowlist - def _realize_to(self, source_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _realize_to( + self, + source_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ): props = self._serialization_props(user_context) headers = props.pop("http_headers", {}) or {} fetch_drs_to_file( @@ -60,7 +67,13 @@ def _realize_to(self, source_path, native_path, user_context=None, opts: Optiona force_http=self._force_http, ) - def _write_from(self, target_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _write_from( + self, + target_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ): raise NotImplementedError() def score_url_match(self, url: str): @@ -69,7 +82,7 @@ def score_url_match(self, url: str): else: return 0 - def _serialization_props(self, user_context=None) -> DRSFilesSourceProperties: + def _serialization_props(self, user_context: OptionalUserContext = None) -> DRSFilesSourceProperties: effective_props = {} for key, val in self._props.items(): effective_props[key] = self._evaluate_prop(val, user_context=user_context) diff --git a/lib/galaxy/files/sources/ftp.py b/lib/galaxy/files/sources/ftp.py index f2877f648392..5df4066e615d 100644 --- a/lib/galaxy/files/sources/ftp.py +++ b/lib/galaxy/files/sources/ftp.py @@ -1,5 +1,7 @@ import urllib.parse +from galaxy.files import OptionalUserContext + try: from fs.ftpfs import FTPFS except ImportError: @@ -30,14 +32,18 @@ class FtpFilesSource(PyFilesystem2FilesSource): required_module = FTPFS required_package = "fs.ftpfs" - def _open_fs(self, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _open_fs(self, user_context: OptionalUserContext = None, opts: Optional[FilesSourceOptions] = None): props = self._serialization_props(user_context) extra_props: FTPFilesSourceProperties = cast(FTPFilesSourceProperties, opts.extra_props or {} if opts else {}) handle = FTPFS(**{**props, **extra_props}) return handle def _realize_to( - self, source_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + source_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): extra_props: FTPFilesSourceProperties if opts and opts.extra_props: @@ -49,7 +55,11 @@ def _realize_to( super()._realize_to(path, native_path, user_context=user_context, opts=opts) def _write_from( - self, target_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + target_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): extra_props: FTPFilesSourceProperties if opts and opts.extra_props: diff --git a/lib/galaxy/files/sources/http.py b/lib/galaxy/files/sources/http.py index 0e538baec1dc..ed4ae1ea8d33 100644 --- a/lib/galaxy/files/sources/http.py +++ b/lib/galaxy/files/sources/http.py @@ -10,6 +10,7 @@ from typing_extensions import Unpack +from galaxy.files import OptionalUserContext from galaxy.files.uris import validate_non_local from galaxy.util import ( DEFAULT_SOCKET_TIMEOUT, @@ -56,7 +57,11 @@ def _allowlist(self): return self._file_sources_config.fetch_url_allowlist def _realize_to( - self, source_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + source_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): props = self._serialization_props(user_context) extra_props: HTTPFilesSourceProperties = cast(HTTPFilesSourceProperties, opts.extra_props or {} if opts else {}) @@ -73,11 +78,15 @@ def _realize_to( ) def _write_from( - self, target_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + target_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): raise NotImplementedError() - def _serialization_props(self, user_context=None) -> HTTPFilesSourceProperties: + def _serialization_props(self, user_context: OptionalUserContext = None) -> HTTPFilesSourceProperties: effective_props = {} for key, val in self._props.items(): effective_props[key] = self._evaluate_prop(val, user_context=user_context) diff --git a/lib/galaxy/files/sources/invenio.py b/lib/galaxy/files/sources/invenio.py index 8b237960878c..8406e4630694 100644 --- a/lib/galaxy/files/sources/invenio.py +++ b/lib/galaxy/files/sources/invenio.py @@ -4,6 +4,7 @@ import urllib.request from typing import ( Any, + cast, Dict, List, Optional, @@ -17,7 +18,9 @@ Unpack, ) +from galaxy.files import OptionalUserContext from galaxy.files.sources import ( + AnyRemoteEntry, DEFAULT_SCHEME, Entry, EntryData, @@ -26,7 +29,6 @@ RemoteFile, ) from galaxy.files.sources._rdm import ( - OptionalUserContext, RDMFilesSource, RDMFilesSourceProperties, RDMRepositoryInteractor, @@ -145,13 +147,15 @@ def _list( recursive=True, user_context: OptionalUserContext = None, opts: Optional[FilesSourceOptions] = None, - ): + ) -> List[AnyRemoteEntry]: writeable = opts and opts.writeable or False is_root_path = path == "/" if is_root_path: - return self.repository.get_records(writeable, user_context) + records = self.repository.get_records(writeable, user_context) + return cast(List[AnyRemoteEntry], records) record_id = self.get_record_id_from_path(path) - return self.repository.get_files_in_record(record_id, writeable, user_context) + files = self.repository.get_files_in_record(record_id, writeable, user_context) + return cast(List[AnyRemoteEntry], files) def _create_entry( self, diff --git a/lib/galaxy/files/sources/posix.py b/lib/galaxy/files/sources/posix.py index a844cf68b553..331dc7860671 100644 --- a/lib/galaxy/files/sources/posix.py +++ b/lib/galaxy/files/sources/posix.py @@ -2,8 +2,6 @@ import os import shutil from typing import ( - Any, - Dict, List, Optional, ) @@ -11,12 +9,14 @@ from typing_extensions import Unpack from galaxy import exceptions +from galaxy.files import OptionalUserContext from galaxy.util.path import ( safe_contains, safe_path, safe_walk, ) from . import ( + AnyRemoteEntry, BaseFilesSource, FilesSourceOptions, FilesSourceProperties, @@ -53,14 +53,20 @@ def __init__(self, **kwd: Unpack[PosixFilesSourceProperties]): self.delete_on_realize = props.get("delete_on_realize", DEFAULT_DELETE_ON_REALIZE) self.allow_subdir_creation = props.get("allow_subdir_creation", DEFAULT_ALLOW_SUBDIR_CREATION) - def _list(self, path="/", recursive=True, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _list( + self, + path="/", + recursive=True, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ) -> List[AnyRemoteEntry]: if not self.root: raise exceptions.ItemAccessibilityException("Listing files at file:// URLs has been disabled.") dir_path = self._to_native_path(path, user_context=user_context) if not self._safe_directory(dir_path): raise exceptions.ObjectNotFound(f"The specified directory does not exist [{dir_path}].") if recursive: - res: List[Dict[str, Any]] = [] + res: List[AnyRemoteEntry] = [] effective_root = self._effective_root(user_context) for p, dirs, files in safe_walk(dir_path, allowlist=self._allowlist): rel_dir = os.path.relpath(p, effective_root) @@ -74,7 +80,11 @@ def _list(self, path="/", recursive=True, user_context=None, opts: Optional[File return list(map(to_dict, res)) def _realize_to( - self, source_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + source_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): if not self.root and (not user_context or not user_context.is_admin): raise exceptions.ItemAccessibilityException("Writing to file:// URLs has been disabled.") @@ -94,7 +104,11 @@ def _realize_to( shutil.move(source_native_path, native_path) def _write_from( - self, target_path: str, native_path: str, user_context=None, opts: Optional[FilesSourceOptions] = None + self, + target_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, ): effective_root = self._effective_root(user_context) target_native_path = self._to_native_path(target_path, user_context=user_context) @@ -118,16 +132,16 @@ def _write_from( shutil.copyfile(native_path, target_native_path_part) os.rename(target_native_path_part, target_native_path) - def _to_native_path(self, source_path: str, user_context=None): + def _to_native_path(self, source_path: str, user_context: OptionalUserContext = None): source_path = os.path.normpath(source_path) if source_path.startswith("/"): source_path = source_path[1:] return os.path.join(self._effective_root(user_context), source_path) - def _effective_root(self, user_context=None): + def _effective_root(self, user_context: OptionalUserContext = None): return self._evaluate_prop(self.root or "/", user_context=user_context) - def _resource_info_to_dict(self, dir: str, name: str, user_context=None): + def _resource_info_to_dict(self, dir: str, name: str, user_context: OptionalUserContext = None) -> AnyRemoteEntry: rel_path = os.path.normpath(os.path.join(dir, name)) full_path = self._to_native_path(rel_path, user_context=user_context) uri = self.uri_from_path(rel_path) @@ -155,7 +169,7 @@ def _safe_directory(self, directory): return False return True - def _serialization_props(self, user_context=None) -> PosixFilesSourceProperties: + def _serialization_props(self, user_context: OptionalUserContext = None) -> PosixFilesSourceProperties: return { # abspath needed because will be used by external Python from # a job working directory diff --git a/lib/galaxy/files/sources/s3fs.py b/lib/galaxy/files/sources/s3fs.py index 687eb1881ea6..d9954ea20b32 100644 --- a/lib/galaxy/files/sources/s3fs.py +++ b/lib/galaxy/files/sources/s3fs.py @@ -2,16 +2,16 @@ import logging import os from typing import ( - Any, cast, - Dict, List, Optional, ) from typing_extensions import Unpack +from galaxy.files import OptionalUserContext from . import ( + AnyRemoteEntry, FilesSourceOptions, FilesSourceProperties, ) @@ -55,13 +55,19 @@ def __init__(self, **kwd: Unpack[S3FsFilesSourceProperties]): if self._endpoint_url: self._props.update({"client_kwargs": {"endpoint_url": self._endpoint_url}}) - def _list(self, path="/", recursive=True, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _list( + self, + path="/", + recursive=True, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ) -> List[AnyRemoteEntry]: _props = self._serialization_props(user_context) # we need to pop the 'bucket' here, because the argument is not recognised in a downstream function _bucket_name = _props.pop("bucket", "") fs = self._open_fs(props=_props, opts=opts) if recursive: - res: List[Dict[str, Any]] = [] + res: List[AnyRemoteEntry] = [] bucket_path = self._bucket_path(_bucket_name, path) for p, dirs, files in fs.walk(bucket_path, detail=True): to_dict = functools.partial(self._resource_info_to_dict, p) @@ -74,14 +80,26 @@ def _list(self, path="/", recursive=True, user_context=None, opts: Optional[File to_dict = functools.partial(self._resource_info_to_dict, path) return list(map(to_dict, res)) - def _realize_to(self, source_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _realize_to( + self, + source_path: str, + native_path: str, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ): _props = self._serialization_props(user_context) _bucket_name = _props.pop("bucket", "") fs = self._open_fs(props=_props, opts=opts) bucket_path = self._bucket_path(_bucket_name, source_path) fs.download(bucket_path, native_path) - def _write_from(self, target_path, native_path, user_context=None, opts: Optional[FilesSourceOptions] = None): + def _write_from( + self, + target_path, + native_path, + user_context: OptionalUserContext = None, + opts: Optional[FilesSourceOptions] = None, + ): _props = self._serialization_props(user_context) _bucket_name = _props.pop("bucket", "") fs = self._open_fs(props=_props, opts=opts) @@ -100,7 +118,7 @@ def _open_fs(self, props: FilesSourceProperties, opts: Optional[FilesSourceOptio fs = s3fs.S3FileSystem(**{**props, **extra_props}) return fs - def _resource_info_to_dict(self, dir_path: str, resource_info): + def _resource_info_to_dict(self, dir_path: str, resource_info) -> AnyRemoteEntry: name = os.path.basename(resource_info["name"]) path = os.path.join(dir_path, name) uri = self.uri_from_path(path) @@ -117,7 +135,7 @@ def _resource_info_to_dict(self, dir_path: str, resource_info): "path": path, } - def _serialization_props(self, user_context=None) -> S3FsFilesSourceProperties: + def _serialization_props(self, user_context: OptionalUserContext = None) -> S3FsFilesSourceProperties: effective_props = {} for key, val in self._props.items(): effective_props[key] = self._evaluate_prop(val, user_context=user_context) diff --git a/lib/galaxy/files/sources/util.py b/lib/galaxy/files/sources/util.py index 0616910b76ff..b43a05e303de 100644 --- a/lib/galaxy/files/sources/util.py +++ b/lib/galaxy/files/sources/util.py @@ -12,7 +12,7 @@ from galaxy import exceptions from galaxy.files import ( ConfiguredFileSources, - FileSourceDictifiable, + FileSourcesUserContext, ) from galaxy.files.sources import FilesSourceOptions from galaxy.files.sources.http import HTTPFilesSourceProperties @@ -79,7 +79,7 @@ def _get_access_info(obj_url: str, access_method: dict, headers=None) -> Tuple[s def fetch_drs_to_file( drs_uri: str, target_path: TargetPathT, - user_context: FileSourceDictifiable, + user_context: Optional[FileSourcesUserContext], force_http=False, retry_options: Optional[RetryOptions] = None, headers: Optional[dict] = None, diff --git a/lib/galaxy/job_execution/setup.py b/lib/galaxy/job_execution/setup.py index 934dd528e74a..4d2f54842c48 100644 --- a/lib/galaxy/job_execution/setup.py +++ b/lib/galaxy/job_execution/setup.py @@ -17,7 +17,7 @@ from galaxy.files import ( ConfiguredFileSources, DictFileSourcesUserContext, - ProvidesUserFileSourcesUserContext, + FileSourcesUserContext, ) from galaxy.job_execution.datasets import ( DatasetPath, @@ -109,13 +109,13 @@ def __init__( check_job_script_integrity_count: int, check_job_script_integrity_sleep: float, file_sources_dict: Dict[str, Any], - user_context: Union[ProvidesUserFileSourcesUserContext, Dict["str", Any]], + user_context: Union[FileSourcesUserContext, Dict[str, Any]], tool_source: Optional[str] = None, tool_source_class: Optional["str"] = "XmlToolSource", tool_dir: Optional[str] = None, is_task: bool = False, ): - user_context_instance: Union[ProvidesUserFileSourcesUserContext, DictFileSourcesUserContext] + user_context_instance: FileSourcesUserContext self.file_sources_dict = file_sources_dict if isinstance(user_context, dict): user_context_instance = DictFileSourcesUserContext(**user_context, file_sources=self.file_sources) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 5ba851359c92..302e53c1c3b4 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -38,7 +38,7 @@ ObjectInvalid, ObjectNotFound, ) -from galaxy.files import ProvidesUserFileSourcesUserContext +from galaxy.files import ProvidesFileSourcesUserContext from galaxy.job_execution.actions.post import ActionBox from galaxy.job_execution.compute_environment import SharedComputeEnvironment from galaxy.job_execution.output_collect import ( @@ -1060,7 +1060,7 @@ def job_io(self): if self._job_io is None: job = self.get_job() work_request = WorkRequestContext(self.app, user=job.user, galaxy_session=job.galaxy_session) - user_context = ProvidesUserFileSourcesUserContext(work_request) + user_context = ProvidesFileSourcesUserContext(work_request) tool_source = self.tool and self.tool.tool_source.to_string() self._job_io = JobIO( sa_session=self.sa_session, diff --git a/lib/galaxy/managers/remote_files.py b/lib/galaxy/managers/remote_files.py index 2be2274dbc70..4abccb8d9229 100644 --- a/lib/galaxy/managers/remote_files.py +++ b/lib/galaxy/managers/remote_files.py @@ -10,7 +10,7 @@ from galaxy.files import ( ConfiguredFileSources, FileSourcePath, - ProvidesUserFileSourcesUserContext, + ProvidesFileSourcesUserContext, ) from galaxy.files.sources import ( FilesSourceOptions, @@ -53,7 +53,7 @@ def index( ) -> AnyRemoteFilesListResponse: """Returns a list of remote files available to the user.""" - user_file_source_context = ProvidesUserFileSourcesUserContext(user_ctx) + user_file_source_context = ProvidesFileSourcesUserContext(user_ctx) default_recursive = False default_format = RemoteFilesFormat.uri @@ -143,7 +143,7 @@ def get_files_source_plugins( exclude_kind: Optional[Set[PluginKind]] = None, ): """Display plugin information for each of the gxfiles:// URI targets available.""" - user_file_source_context = ProvidesUserFileSourcesUserContext(user_context) + user_file_source_context = ProvidesFileSourcesUserContext(user_context) browsable_only = True if browsable_only is None else browsable_only plugins_dict = self._file_sources.plugins_to_dict( user_context=user_file_source_context, @@ -160,7 +160,7 @@ def _file_sources(self) -> ConfiguredFileSources: def create_entry(self, user_ctx: ProvidesUserContext, entry_data: CreateEntryPayload) -> CreatedEntryResponse: """Create an entry (directory or record) in a remote files location.""" target = entry_data.target - user_file_source_context = ProvidesUserFileSourcesUserContext(user_ctx) + user_file_source_context = ProvidesFileSourcesUserContext(user_ctx) self._file_sources.validate_uri_root(target, user_context=user_file_source_context) file_source_path = self._file_sources.get_file_source_path(target) file_source = file_source_path.file_source diff --git a/lib/galaxy/model/store/__init__.py b/lib/galaxy/model/store/__init__.py index dbca1f250f33..7033a91ab176 100644 --- a/lib/galaxy/model/store/__init__.py +++ b/lib/galaxy/model/store/__init__.py @@ -57,7 +57,7 @@ ) from galaxy.files import ( ConfiguredFileSources, - ProvidesUserFileSourcesUserContext, + ProvidesFileSourcesUserContext, ) from galaxy.files.uris import stream_url_to_file from galaxy.model.base import ( @@ -1902,7 +1902,7 @@ def __init__( sessionless = True security = IdEncodingHelper(id_secret="randomdoesntmatter") - self.user_context = ProvidesUserFileSourcesUserContext(user_context) + self.user_context = ProvidesFileSourcesUserContext(user_context) self.file_sources = file_sources self.serialize_jobs = serialize_jobs self.sessionless = sessionless @@ -3033,7 +3033,7 @@ def source_to_import_store( if source_uri.startswith("file://"): source_uri = source_uri[len("file://") :] if "://" in source_uri: - user_context = ProvidesUserFileSourcesUserContext(user_context) + user_context = ProvidesFileSourcesUserContext(user_context) source_uri = stream_url_to_file( source_uri, app.file_sources, prefix="gx_import_model_store", user_context=user_context ) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 0ec2a659e86c..698e97cd8ee4 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -25,7 +25,7 @@ from webob.compat import cgi_FieldStorage from galaxy import util -from galaxy.files import ProvidesUserFileSourcesUserContext +from galaxy.files import ProvidesFileSourcesUserContext from galaxy.managers.dbkeys import read_dbnames from galaxy.model import ( cached_id, @@ -2633,7 +2633,7 @@ def validate(self, value, trans=None): file_source = file_source_path.file_source if file_source is None: raise ParameterValueError(f"'{value}' is not a valid file source uri.", self.name) - user_context = ProvidesUserFileSourcesUserContext(trans) + user_context = ProvidesFileSourcesUserContext(trans) user_has_access = file_source.user_has_access(user_context) if not user_has_access: raise ParameterValueError(f"The user cannot access {value}.", self.name) diff --git a/test/unit/files/_util.py b/test/unit/files/_util.py index 866552146e95..96422d858d87 100644 --- a/test/unit/files/_util.py +++ b/test/unit/files/_util.py @@ -7,13 +7,14 @@ ConfiguredFileSources, ConfiguredFileSourcesConfig, DictFileSourcesUserContext, + OptionalUserContext, ) TEST_USERNAME = "alice" TEST_EMAIL = "alice@galaxyproject.org" -def serialize_and_recover(file_sources_o, user_context=None): +def serialize_and_recover(file_sources_o: ConfiguredFileSources, user_context: OptionalUserContext = None): as_dict = file_sources_o.to_dict(for_serialization=True, user_context=user_context) file_sources = ConfiguredFileSources.from_dict(as_dict) return file_sources @@ -33,14 +34,24 @@ def find(dir_list, class_=None, name=None): return None -def list_root(file_sources, uri, recursive, user_context=None): +def list_root( + file_sources: ConfiguredFileSources, + uri: str, + recursive: bool, + user_context: OptionalUserContext = None, +): file_source_pair = file_sources.get_file_source_path(uri) file_source = file_source_pair.file_source res = file_source.list("/", recursive=recursive, user_context=user_context) return res -def list_dir(file_sources, uri, recursive, user_context=None): +def list_dir( + file_sources: ConfiguredFileSources, + uri: str, + recursive: bool, + user_context: OptionalUserContext = None, +): file_source_pair = file_sources.get_file_source_path(uri) file_source = file_source_pair.file_source print(file_source_pair.path) @@ -82,7 +93,9 @@ def user_context_fixture(user_ftp_dir=None, role_names=None, group_names=None, i return user_context -def realize_to_temp_file(file_sources, uri, user_context=None): +def realize_to_temp_file( + file_sources: ConfiguredFileSources, uri: str, user_context: OptionalUserContext = None +) -> str: file_source_path = file_sources.get_file_source_path(uri) with tempfile.NamedTemporaryFile(mode="r") as temp: file_source_path.file_source.realize_to(file_source_path.path, temp.name, user_context=user_context) @@ -91,7 +104,12 @@ def realize_to_temp_file(file_sources, uri, user_context=None): return realized_contents -def assert_realizes_as(file_sources, uri, expected, user_context=None): +def assert_realizes_as( + file_sources: ConfiguredFileSources, + uri: str, + expected: str, + user_context: OptionalUserContext = None, +): realized_contents = realize_to_temp_file(file_sources, uri, user_context=user_context) if realized_contents != expected: raise AssertionError( @@ -99,7 +117,12 @@ def assert_realizes_as(file_sources, uri, expected, user_context=None): ) -def assert_realizes_contains(file_sources, uri, expected, user_context=None): +def assert_realizes_contains( + file_sources: ConfiguredFileSources, + uri: str, + expected: str, + user_context: OptionalUserContext = None, +): realized_contents = realize_to_temp_file(file_sources, uri, user_context=user_context) if expected not in realized_contents: raise AssertionError( @@ -107,7 +130,9 @@ def assert_realizes_contains(file_sources, uri, expected, user_context=None): ) -def assert_realizes_throws_exception(file_sources, uri, user_context=None) -> Exception: +def assert_realizes_throws_exception( + file_sources: ConfiguredFileSources, uri: str, user_context: OptionalUserContext = None +) -> Exception: exception = None try: realize_to_temp_file(file_sources, uri, user_context=user_context) @@ -117,7 +142,12 @@ def assert_realizes_throws_exception(file_sources, uri, user_context=None) -> Ex return exception -def write_from(file_sources, uri, content, user_context=None): +def write_from( + file_sources: ConfiguredFileSources, + uri: str, + content: str, + user_context: OptionalUserContext = None, +): file_source_path = file_sources.get_file_source_path(uri) with tempfile.NamedTemporaryFile(mode="w") as f: f.write(content)