fix: clear the type checker and linter findings

flake8 reported 49 findings and pyright 10, and two of them were real
bugs rather than matters of style. The rest were unused imports and
four functions over the complexity limit.

- Unlock a DOI folder with the id the DOI response carries, rather
  than an attribute the model does not have, which raised
  `AttributeError` on every locked DOI folder.
- Size the `ls` Size column from the sub-folder on the row rather than
  from its parent, which pushed the later columns out of line.
- Share the path resolution and the destination checks between `cp`
  and `mv`, and split the recursive download and the `ls` row
  printing, bringing all four functions under the complexity limit.
- Accept a client built without a connection, which `config` and
  `version` rely on, and report the reason if one is then asked for.
- Declare the config protocol's constructor for the type checker
  alone, so the protocol keeps its guard against being instantiated.
- Remove 41 unused imports, and let flake8 accept black's spacing.
- Point pyright at the project's own environment, without which it
  resolved no dependency and reported 124 findings that were not real.
- Cover the two fixes and the shared `cp`/`mv` paths with new tests.
This commit is contained in:
2026-09-04 18:23:17 +09:00
parent 914dd729aa
commit 4c9954c1fd
17 changed files with 475 additions and 208 deletions
+94 -80
View File
@@ -1,21 +1,40 @@
import os import os
from typing import Any from dataclasses import dataclass
from unicodedata import normalize from unicodedata import normalize
from mdrsclient.api import DoiApi, FilesApi, FoldersApi, LaboratoriesApi, UsersApi from mdrsclient.api import FilesApi, FoldersApi
from mdrsclient.cache import CacheInterface from mdrsclient.cache import CacheInterface
from mdrsclient.config import ConfigInterface from mdrsclient.config import ConfigInterface
from mdrsclient.connection import MDRSConnection from mdrsclient.connection import MDRSConnection
from mdrsclient.exceptions import IllegalArgumentException, MDRSException, UnauthorizedException, UnexpectedException from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models import File, Folder, Laboratory, Token, User from mdrsclient.models import File, Folder, FolderSimple, Laboratory
from mdrsclient.models.file import find_file from mdrsclient.models.file import find_file
from mdrsclient.services import MdrsService from mdrsclient.services import MdrsService
@dataclass(frozen=True)
class TransferEndpoints:
"""Both ends of a `cp` or `mv`, resolved: the parents, their contents, and the names."""
laboratory: Laboratory
s_path: str
s_basename: str
s_parent_folder: Folder
s_parent_files: list[File]
d_path: str
d_basename: str
d_parent_folder: Folder
d_parent_files: list[File]
def is_same_place(self) -> bool:
"""Whether the destination names what the source already is, leaving nothing to do."""
return self.s_parent_folder.id == self.d_parent_folder.id and self.s_basename == self.d_basename
class MdrsClient(MdrsService): class MdrsClient(MdrsService):
"""Service layer client for MDRS.""" """Service layer client for MDRS."""
def __init__(self, connection: MDRSConnection, config_class: type[ConfigInterface] | None = None): def __init__(self, connection: MDRSConnection | None, config_class: type[ConfigInterface] | None = None):
super().__init__(connection, config_class) super().__init__(connection, config_class)
@classmethod @classmethod
@@ -63,7 +82,8 @@ class MdrsClient(MdrsService):
files = self.find_files(folder.id) files = self.find_files(folder.id)
return folder, files return folder, files
def cp(self, src_path: str, dest_path: str, is_recursive: bool = False) -> None: def __resolve_transfer(self, src_path: str, dest_path: str) -> TransferEndpoints:
"""Resolve both ends of a transfer, refusing one that crosses a remote or a laboratory."""
s_remote, s_laboratory_name, s_path = self.parse_remote_host_with_path(src_path) s_remote, s_laboratory_name, s_path = self.parse_remote_host_with_path(src_path)
d_remote, d_laboratory_name, d_path = self.parse_remote_host_with_path(dest_path) d_remote, d_laboratory_name, d_path = self.parse_remote_host_with_path(dest_path)
if s_remote != d_remote: if s_remote != d_remote:
@@ -71,8 +91,9 @@ class MdrsClient(MdrsService):
if s_laboratory_name != d_laboratory_name: if s_laboratory_name != d_laboratory_name:
raise IllegalArgumentException("Laboratory mismatched.") raise IllegalArgumentException("Laboratory mismatched.")
s_path = s_path.rstrip("/") s_path = s_path.rstrip("/")
s_dirname = os.path.dirname(s_path)
s_basename = os.path.basename(s_path) s_basename = os.path.basename(s_path)
# A destination ending in a separator names a folder to put the source into, and
# keeps the source's own name.
if d_path.endswith("/"): if d_path.endswith("/"):
d_dirname = d_path d_dirname = d_path
d_basename = s_basename d_basename = s_basename
@@ -80,84 +101,77 @@ class MdrsClient(MdrsService):
d_dirname = os.path.dirname(d_path) d_dirname = os.path.dirname(d_path)
d_basename = os.path.basename(d_path) d_basename = os.path.basename(d_path)
laboratory = self.find_laboratory(s_laboratory_name) laboratory = self.find_laboratory(s_laboratory_name)
s_parent_folder = self.find_folder(laboratory, s_dirname) s_parent_folder = self.find_folder(laboratory, os.path.dirname(s_path))
s_parent_files = self.find_files(s_parent_folder.id)
d_parent_folder = self.find_folder(laboratory, d_dirname) d_parent_folder = self.find_folder(laboratory, d_dirname)
d_parent_files = self.find_files(d_parent_folder.id) return TransferEndpoints(
s_file = find_file(s_parent_files, s_basename) laboratory=laboratory,
s_path=s_path,
s_basename=s_basename,
s_parent_folder=s_parent_folder,
s_parent_files=self.find_files(s_parent_folder.id),
d_path=d_path,
d_basename=d_basename,
d_parent_folder=d_parent_folder,
d_parent_files=self.find_files(d_parent_folder.id),
)
@staticmethod
def __check_file_destination(ends: TransferEndpoints) -> None:
"""Refuse a destination that a file cannot take the place of."""
if find_file(ends.d_parent_files, ends.d_basename) is not None:
raise IllegalArgumentException(f"File `{ends.d_basename}` already exists.")
if ends.d_parent_folder.find_sub_folder(ends.d_basename) is not None:
raise IllegalArgumentException(
f"Cannot overwrite non-folder `{ends.d_basename}` with folder `{ends.d_path}`."
)
@staticmethod
def __check_folder_destination(ends: TransferEndpoints, s_folder: FolderSimple) -> None:
"""Refuse a destination that a folder cannot take the place of."""
if find_file(ends.d_parent_files, ends.d_basename) is not None:
raise IllegalArgumentException(
f"Cannot overwrite non-folder `{ends.d_basename}` with folder `{ends.s_path}`."
)
d_folder = ends.d_parent_folder.find_sub_folder(ends.d_basename)
if d_folder is not None:
if d_folder.id == s_folder.id:
raise IllegalArgumentException(f"`{ends.s_path}` and `{ends.s_path}` are the same folder.")
raise IllegalArgumentException(f"Cannot move `{ends.s_path}` to `{ends.d_path}`: Folder not empty.")
@staticmethod
def __find_source_folder(ends: TransferEndpoints) -> FolderSimple:
s_folder = ends.s_parent_folder.find_sub_folder(ends.s_basename)
if s_folder is None:
raise IllegalArgumentException(f"File or folder `{ends.s_basename}` not found.")
return s_folder
def cp(self, src_path: str, dest_path: str, is_recursive: bool = False) -> None:
ends = self.__resolve_transfer(src_path, dest_path)
s_file = find_file(ends.s_parent_files, ends.s_basename)
if s_file is not None: if s_file is not None:
d_file = find_file(d_parent_files, d_basename) self.__check_file_destination(ends)
if d_file is not None: if not ends.is_same_place():
raise IllegalArgumentException(f"File `{d_basename}` already exists.") FilesApi(self.connection).copy(s_file, ends.d_parent_folder.id, normalize("NFC", ends.d_basename))
d_sub_folder = d_parent_folder.find_sub_folder(d_basename) return
if d_sub_folder is not None: s_folder = self.__find_source_folder(ends)
raise IllegalArgumentException(f"Cannot overwrite non-folder `{d_basename}` with folder `{d_path}`.") if not is_recursive:
file_api = FilesApi(self.connection) raise IllegalArgumentException(f"Cannot copy `{ends.s_path}`: Is a folder.")
if s_parent_folder.id != d_parent_folder.id or d_basename != s_basename: self.__check_folder_destination(ends, s_folder)
file_api.copy(s_file, d_parent_folder.id, normalize("NFC", d_basename)) if not ends.is_same_place():
else: FoldersApi(self.connection).copy(s_folder, ends.d_parent_folder.id, normalize("NFC", ends.d_basename))
s_folder = s_parent_folder.find_sub_folder(s_basename)
if s_folder is None:
raise IllegalArgumentException(f"File or folder `{s_basename}` not found.")
if not is_recursive:
raise IllegalArgumentException(f"Cannot copy `{s_path}`: Is a folder.")
if find_file(d_parent_files, d_basename) is not None:
raise IllegalArgumentException(f"Cannot overwrite non-folder `{d_basename}` with folder `{s_path}`.")
d_folder = d_parent_folder.find_sub_folder(d_basename)
if d_folder is not None:
if d_folder.id == s_folder.id:
raise IllegalArgumentException(f"`{s_path}` and `{s_path}` are the same folder.")
raise IllegalArgumentException(f"Cannot move `{s_path}` to `{d_path}`: Folder not empty.")
folder_api = FoldersApi(self.connection)
if s_parent_folder.id != d_parent_folder.id or s_basename != d_basename:
folder_api.copy(s_folder, d_parent_folder.id, normalize("NFC", d_basename))
def mv(self, src_path: str, dest_path: str) -> None: def mv(self, src_path: str, dest_path: str) -> None:
s_remote, s_laboratory_name, s_path = self.parse_remote_host_with_path(src_path) ends = self.__resolve_transfer(src_path, dest_path)
d_remote, d_laboratory_name, d_path = self.parse_remote_host_with_path(dest_path) s_file = find_file(ends.s_parent_files, ends.s_basename)
if s_remote != d_remote:
raise IllegalArgumentException("Remote host mismatched.")
if s_laboratory_name != d_laboratory_name:
raise IllegalArgumentException("Laboratory mismatched.")
s_path = s_path.rstrip("/")
s_dirname = os.path.dirname(s_path)
s_basename = os.path.basename(s_path)
if d_path.endswith("/"):
d_dirname = d_path
d_basename = s_basename
else:
d_dirname = os.path.dirname(d_path)
d_basename = os.path.basename(d_path)
laboratory = self.find_laboratory(s_laboratory_name)
s_parent_folder = self.find_folder(laboratory, s_dirname)
s_parent_files = self.find_files(s_parent_folder.id)
d_parent_folder = self.find_folder(laboratory, d_dirname)
d_parent_files = self.find_files(d_parent_folder.id)
s_file = find_file(s_parent_files, s_basename)
if s_file is not None: if s_file is not None:
d_file = find_file(d_parent_files, d_basename) self.__check_file_destination(ends)
if d_file is not None: if not ends.is_same_place():
raise IllegalArgumentException(f"File `{d_basename}` already exists.") FilesApi(self.connection).move(s_file, ends.d_parent_folder.id, normalize("NFC", ends.d_basename))
d_sub_folder = d_parent_folder.find_sub_folder(d_basename) return
if d_sub_folder is not None: s_folder = self.__find_source_folder(ends)
raise IllegalArgumentException(f"Cannot overwrite non-folder `{d_basename}` with folder `{d_path}`.") self.__check_folder_destination(ends, s_folder)
file_api = FilesApi(self.connection) if not ends.is_same_place():
if s_parent_folder.id != d_parent_folder.id or d_basename != s_basename: FoldersApi(self.connection).move(s_folder, ends.d_parent_folder.id, normalize("NFC", ends.d_basename))
file_api.move(s_file, d_parent_folder.id, normalize("NFC", d_basename))
else:
s_folder = s_parent_folder.find_sub_folder(s_basename)
if s_folder is None:
raise IllegalArgumentException(f"File or folder `{s_basename}` not found.")
if find_file(d_parent_files, d_basename) is not None:
raise IllegalArgumentException(f"Cannot overwrite non-folder `{d_basename}` with folder `{s_path}`.")
d_folder = d_parent_folder.find_sub_folder(d_basename)
if d_folder is not None:
if d_folder.id == s_folder.id:
raise IllegalArgumentException(f"`{s_path}` and `{s_path}` are the same folder.")
raise IllegalArgumentException(f"Cannot move `{s_path}` to `{d_path}`: Folder not empty.")
folder_api = FoldersApi(self.connection)
if s_parent_folder.id != d_parent_folder.id or d_basename != s_basename:
folder_api.move(s_folder, d_parent_folder.id, normalize("NFC", d_basename))
def chacl( def chacl(
self, remote_path: str, access_level: int, is_recursive: bool = False, password: str | None = None self, remote_path: str, access_level: int, is_recursive: bool = False, password: str | None = None
-1
View File
@@ -1,7 +1,6 @@
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from mdrsclient.api import FoldersApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
from mdrsclient.exceptions import IllegalArgumentException from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models import FolderAccessLevel from mdrsclient.models import FolderAccessLevel
-5
View File
@@ -1,12 +1,7 @@
import os
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from unicodedata import normalize
from mdrsclient.api import FilesApi, FoldersApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models.file import find_file
class CpCommand(BaseCommand): class CpCommand(BaseCommand):
-4
View File
@@ -1,12 +1,8 @@
import json import json
import os
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from mdrsclient.api import FilesApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models.file import find_file
class FileMetadataCommand(BaseCommand): class FileMetadataCommand(BaseCommand):
-1
View File
@@ -1,7 +1,6 @@
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from mdrsclient.api import LaboratoriesApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
+38 -28
View File
@@ -1,10 +1,10 @@
import json import json
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any, Final
from pydantic.dataclasses import dataclass from pydantic.dataclasses import dataclass
from mdrsclient.api import FilesApi, FoldersApi from mdrsclient.api import FoldersApi
from mdrsclient.client import MdrsClient from mdrsclient.client import MdrsClient
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
from mdrsclient.config import build_download_url from mdrsclient.config import build_download_url
@@ -95,25 +95,24 @@ class LsCommand(BaseCommand):
def _ls_json(cls, context: LsCommandContext, folder: Folder, files: list[File]) -> None: def _ls_json(cls, context: LsCommandContext, folder: Folder, files: list[File]) -> None:
print(json.dumps(cls._folder2dict(context, folder, files), ensure_ascii=False)) print(json.dumps(cls._folder2dict(context, folder, files), ensure_ascii=False))
LABELS: Final[dict[str, str]] = {
"type": "Type",
"acl": "Access",
"laboratory": "Laboratory",
"size": "Size",
"date": "Date",
"name": "Name",
}
@classmethod @classmethod
def _ls_plain(cls, context: LsCommandContext, folder: Folder, files: list[File]) -> None: def _column_widths(cls, context: LsCommandContext, folder: Folder, files: list[File]) -> dict[str, int]:
label = { """Width of each column: the widest of its heading and everything printed under it."""
"type": "Type", length = {key: len(label) if not context.is_quiet else 0 for key, label in cls.LABELS.items()}
"acl": "Access",
"laboratory": "Laboratory",
"size": "Size",
"date": "Date",
"name": "Name",
}
length: dict[str, int] = {}
for key in label.keys():
length[key] = len(label[key]) if not context.is_quiet else 0
for sub_folder in folder.sub_folders: for sub_folder in folder.sub_folders:
sub_laboratory = context.client.connection.laboratories.find_by_id(sub_folder.laboratory_id) sub_laboratory_name = cls._laboratory_name(context, sub_folder.laboratory_id)
sub_laboratory_name = sub_laboratory.name if sub_laboratory is not None else "(invalid)"
length["acl"] = max(length["acl"], len(sub_folder.access_level_name)) length["acl"] = max(length["acl"], len(sub_folder.access_level_name))
length["laboratory"] = max(length["laboratory"], len(sub_laboratory_name)) length["laboratory"] = max(length["laboratory"], len(sub_laboratory_name))
length["size"] = max(length["size"], len(str(folder.size))) length["size"] = max(length["size"], len(str(sub_folder.size)))
length["date"] = max(length["date"], len(sub_folder.updated_at_name)) length["date"] = max(length["date"], len(sub_folder.updated_at_name))
length["name"] = max(length["name"], len(sub_folder.name)) length["name"] = max(length["name"], len(sub_folder.name))
for file in files: for file in files:
@@ -122,6 +121,27 @@ class LsCommand(BaseCommand):
length["name"] = max(length["name"], len(file.name)) length["name"] = max(length["name"], len(file.name))
length["acl"] = max(length["acl"], len(folder.access_level_name)) length["acl"] = max(length["acl"], len(folder.access_level_name))
length["laboratory"] = max(length["laboratory"], len(context.laboratory.name)) length["laboratory"] = max(length["laboratory"], len(context.laboratory.name))
return length
@classmethod
def _ls_plain_children(cls, context: LsCommandContext, folder: Folder) -> None:
"""List each sub-folder in turn, passing over the ones the caller cannot open."""
print("")
folder_api = FoldersApi(context.client.connection)
for sub_folder in sorted(folder.sub_folders, key=lambda x: x.name):
try:
if sub_folder.lock:
folder_api.auth(sub_folder.id, context.password)
sub_detail = folder_api.retrieve(sub_folder.id)
sub_files = context.client.find_files(sub_folder.id)
except UnauthorizedException:
continue
cls._ls_plain(context, sub_detail, sub_files)
@classmethod
def _ls_plain(cls, context: LsCommandContext, folder: Folder, files: list[File]) -> None:
label = cls.LABELS
length = cls._column_widths(context, folder, files)
header = ( header = (
f"{label['type']:{length['type']}}\t{label['acl']:{length['acl']}}\t" f"{label['type']:{length['type']}}\t{label['acl']:{length['acl']}}\t"
f"{label['laboratory']:{length['laboratory']}}\t{label['size']:{length['size']}}\t" f"{label['laboratory']:{length['laboratory']}}\t{label['size']:{length['size']}}\t"
@@ -152,17 +172,7 @@ class LsCommand(BaseCommand):
) )
if context.is_recursive: if context.is_recursive:
print("") cls._ls_plain_children(context, folder)
for sub_folder in sorted(folder.sub_folders, key=lambda x: x.name):
folder_api = FoldersApi(context.client.connection)
try:
if sub_folder.lock:
folder_api.auth(sub_folder.id, context.password)
folder = folder_api.retrieve(sub_folder.id)
files = context.client.find_files(sub_folder.id)
cls._ls_plain(context, folder, files)
except UnauthorizedException:
pass
@classmethod @classmethod
def _folder2dict( def _folder2dict(
-1
View File
@@ -2,7 +2,6 @@ import json
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from mdrsclient.api import FoldersApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
-5
View File
@@ -1,12 +1,7 @@
import os
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from unicodedata import normalize
from mdrsclient.api import FoldersApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models.file import find_file
class MkdirCommand(BaseCommand): class MkdirCommand(BaseCommand):
-5
View File
@@ -1,12 +1,7 @@
import os
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from unicodedata import normalize
from mdrsclient.api import FilesApi, FoldersApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models.file import find_file
class MvCommand(BaseCommand): class MvCommand(BaseCommand):
-4
View File
@@ -1,11 +1,7 @@
import os
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from mdrsclient.api import FilesApi, FoldersApi
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models.file import find_file
class RmCommand(BaseCommand): class RmCommand(BaseCommand):
-1
View File
@@ -1,7 +1,6 @@
from argparse import Namespace from argparse import Namespace
from typing import Any from typing import Any
from mdrsclient.__version__ import __version__
from mdrsclient.commands.base import BaseCommand from mdrsclient.commands.base import BaseCommand
+9 -1
View File
@@ -1,7 +1,7 @@
import configparser import configparser
import os import os
import threading import threading
from typing import Final, Protocol, runtime_checkable from typing import TYPE_CHECKING, Final, Protocol, runtime_checkable
import validators import validators
@@ -39,6 +39,14 @@ def build_download_url(base_url: str | None, path: str) -> str:
class ConfigInterface(Protocol): class ConfigInterface(Protocol):
remote: str remote: str
if TYPE_CHECKING:
# Declared for the type checker, because the class itself is passed around and
# called: without this it describes something that takes no arguments to build.
# Kept out of the running class, because a protocol that defines `__init__` loses
# the guard that stops it being instantiated, and hands that empty `__init__` to
# any implementation that does not write its own.
def __init__(self, remote: str) -> None: ...
def list(self) -> list[tuple[str, str]]: ... def list(self) -> list[tuple[str, str]]: ...
@property @property
def url(self) -> str | None: ... def url(self) -> str | None: ...
+22 -4
View File
@@ -1,6 +1,5 @@
import os import os
import re import re
from typing import Any
from unicodedata import normalize from unicodedata import normalize
from mdrsclient.api import DoiApi, FilesApi, FoldersApi, LaboratoriesApi, UsersApi from mdrsclient.api import DoiApi, FilesApi, FoldersApi, LaboratoriesApi, UsersApi
@@ -20,11 +19,30 @@ from mdrsclient.utils import page_num_from_url
class MdrsService: class MdrsService:
config_class: type[ConfigInterface] = ConfigFile config_class: type[ConfigInterface] = ConfigFile
def __init__(self, connection: MDRSConnection, config_class: type[ConfigInterface] | None = None): def __init__(self, connection: MDRSConnection | None, config_class: type[ConfigInterface] | None = None):
self.connection = connection self.__connection = connection
if config_class is not None: if config_class is not None:
self.config_class = config_class self.config_class = config_class
@property
def connection(self) -> MDRSConnection:
"""
The connection every remote operation goes through.
Optional to supply, because `config` and `version` do their work without a remote
and are reached through the same client. Asking for it when none was given is a
mistake in the caller, and says so rather than failing later on `None`.
"""
if self.__connection is None:
raise MissingConfigurationException("This operation requires a remote host.")
return self.__connection
@connection.setter
def connection(self, connection: MDRSConnection | None) -> None:
# Assignable as it always was: this is a property to check for absence on read,
# not to make the connection fixed once the client is built.
self.__connection = connection
@classmethod @classmethod
def create_connection( def create_connection(
cls, remote: str, cache: CacheInterface | None = None, config: ConfigInterface | None = None cls, remote: str, cache: CacheInterface | None = None, config: ConfigInterface | None = None
@@ -182,7 +200,7 @@ class MdrsService:
if folder.lock: if folder.lock:
if password is None: if password is None:
raise UnauthorizedException(f"Folder for DOI `{doi_clean}` is locked.") raise UnauthorizedException(f"Folder for DOI `{doi_clean}` is locked.")
folder_api.auth(doi_resp.folder.id, password) folder_api.auth(doi_resp.folder_id, password)
lab_api = LaboratoriesApi(self.connection) lab_api = LaboratoriesApi(self.connection)
labs = lab_api.list() labs = lab_api.list()
+83 -68
View File
@@ -154,86 +154,101 @@ class Downloader:
) -> bool: ) -> bool:
"""Fetch what the remote path names, and report whether every file arrived.""" """Fetch what the remote path names, and report whether every file arrived."""
excludes_clean = excludes or [] excludes_clean = excludes or []
# Detect DOI path: "remote:10.xxxx/prefix.ID[/optional/sub/path]" l_dirname = os.path.realpath(local_path)
if not os.path.isdir(l_dirname):
raise IllegalArgumentException(f"Local directory `{local_path}` not found.")
# "remote:10.xxxx/prefix.ID[/optional/sub/path]" names a published dataset rather
# than a path within a laboratory, and is resolved through the DOI instead.
path_component = remote_path.split(":", 1)[1] if ":" in remote_path else "" path_component = remote_path.split(":", 1)[1] if ":" in remote_path else ""
if self.client.is_doi(path_component): if self.client.is_doi(path_component):
remote, doi, subpath = self.client.parse_doi_remote_host(remote_path) return self.__download_doi(
remote_path, l_dirname, is_recursive, is_skip_if_exists, password, excludes_clean
l_dirname = os.path.realpath(local_path)
if not os.path.isdir(l_dirname):
raise IllegalArgumentException(f"Local directory `{local_path}` not found.")
doi_folder, laboratory = self.client.find_folder_by_doi(doi, password)
subpath_clean = subpath.rstrip("/")
if not subpath_clean:
folder = doi_folder
is_folder = True
else:
r_dirname = os.path.dirname(subpath_clean)
r_basename = os.path.basename(subpath_clean)
abs_path = doi_folder.path.rstrip("/") + r_dirname
r_parent_folder = self.client.find_folder(laboratory, abs_path, password)
r_parent_files = self.client.find_files(r_parent_folder.id)
file = find_file(r_parent_files, r_basename)
if file is not None:
if self.__check_excludes(excludes_clean, laboratory, r_parent_folder, file):
return True
context = DownloadContext(is_skip_if_exists, [])
l_path = os.path.join(l_dirname, r_basename)
context.files.append(DownloadFileInfo(file, l_path))
return self.__multiple_download(context)
else:
folder_simple = r_parent_folder.find_sub_folder(r_basename)
if folder_simple is None:
raise IllegalArgumentException(f"File or folder `{subpath_clean}` not found.")
folder = FoldersApi(self.client.connection).retrieve(folder_simple.id)
is_folder = True
# For a DOI target the whole folder is the download target.
if not is_recursive:
# Non-recursive: download only the files at the top level of the DOI folder.
files = self.client.find_files(folder.id)
context = DownloadContext(is_skip_if_exists, [])
for file in files:
if self.__check_excludes(excludes_clean, laboratory, folder, file):
continue
l_path = os.path.join(l_dirname, file.name)
context.files.append(DownloadFileInfo(file, l_path))
return self.__multiple_download(context)
folder_api = FoldersApi(self.client.connection)
return self.__multiple_download_pickup_recursive_files(
folder_api, laboratory, folder.id, l_dirname, excludes_clean, is_skip_if_exists
) )
remote, laboratory_name, r_path = self.client.parse_remote_host_with_path(remote_path) remote, laboratory_name, r_path = self.client.parse_remote_host_with_path(remote_path)
r_path = r_path.rstrip("/") r_path = r_path.rstrip("/")
r_dirname = os.path.dirname(r_path)
r_basename = os.path.basename(r_path) r_basename = os.path.basename(r_path)
l_dirname = os.path.realpath(local_path)
if not os.path.isdir(l_dirname):
raise IllegalArgumentException(f"Local directory `{local_path}` not found.")
laboratory = self.client.find_laboratory(laboratory_name) laboratory = self.client.find_laboratory(laboratory_name)
r_parent_folder = self.client.find_folder(laboratory, r_dirname, password) r_parent_folder = self.client.find_folder(laboratory, os.path.dirname(r_path), password)
r_parent_files = self.client.find_files(r_parent_folder.id) r_parent_files = self.client.find_files(r_parent_folder.id)
file = find_file(r_parent_files, r_basename) file = find_file(r_parent_files, r_basename)
if file is not None: if file is not None:
if self.__check_excludes(excludes_clean, laboratory, r_parent_folder, file): return self.__download_one(
return True excludes_clean, laboratory, r_parent_folder, file, l_dirname, r_basename, is_skip_if_exists
context = DownloadContext(is_skip_if_exists, [])
l_path = os.path.join(l_dirname, r_basename)
context.files.append(DownloadFileInfo(file, l_path))
return self.__multiple_download(context)
else:
folder = r_parent_folder.find_sub_folder(r_basename)
if folder is None:
raise IllegalArgumentException(f"File or folder `{r_path}` not found.")
if not is_recursive:
raise IllegalArgumentException(f"Cannot download `{r_path}`: Is a folder.")
folder_api = FoldersApi(self.client.connection)
return self.__multiple_download_pickup_recursive_files(
folder_api, laboratory, folder.id, l_dirname, excludes_clean, is_skip_if_exists
) )
folder = r_parent_folder.find_sub_folder(r_basename)
if folder is None:
raise IllegalArgumentException(f"File or folder `{r_path}` not found.")
if not is_recursive:
raise IllegalArgumentException(f"Cannot download `{r_path}`: Is a folder.")
return self.__multiple_download_pickup_recursive_files(
FoldersApi(self.client.connection), laboratory, folder.id, l_dirname, excludes_clean, is_skip_if_exists
)
def __download_doi(
self,
remote_path: str,
l_dirname: str,
is_recursive: bool,
is_skip_if_exists: bool,
password: str | None,
excludes: list[str],
) -> bool:
"""Fetch what a DOI names: the dataset's folder, or something inside it."""
remote, doi, subpath = self.client.parse_doi_remote_host(remote_path)
doi_folder, laboratory = self.client.find_folder_by_doi(doi, password)
subpath_clean = subpath.rstrip("/")
if not subpath_clean:
folder = doi_folder
else:
r_basename = os.path.basename(subpath_clean)
abs_path = doi_folder.path.rstrip("/") + os.path.dirname(subpath_clean)
r_parent_folder = self.client.find_folder(laboratory, abs_path, password)
file = find_file(self.client.find_files(r_parent_folder.id), r_basename)
if file is not None:
return self.__download_one(
excludes, laboratory, r_parent_folder, file, l_dirname, r_basename, is_skip_if_exists
)
folder_simple = r_parent_folder.find_sub_folder(r_basename)
if folder_simple is None:
raise IllegalArgumentException(f"File or folder `{subpath_clean}` not found.")
folder = FoldersApi(self.client.connection).retrieve(folder_simple.id)
if is_recursive:
return self.__multiple_download_pickup_recursive_files(
FoldersApi(self.client.connection), laboratory, folder.id, l_dirname, excludes, is_skip_if_exists
)
# Without -r the dataset's own files are fetched, and its sub-folders are not.
context = DownloadContext(is_skip_if_exists, [])
for file in self.client.find_files(folder.id):
if self.__check_excludes(excludes, laboratory, folder, file):
continue
context.files.append(DownloadFileInfo(file, os.path.join(l_dirname, file.name)))
return self.__multiple_download(context)
def __download_one(
self,
excludes: list[str],
laboratory: Laboratory,
folder: Folder,
file: File,
l_dirname: str,
local_name: str,
is_skip_if_exists: bool,
) -> bool:
"""
Fetch a single named file into the local directory.
Saved under the name the caller asked for rather than the one the server holds:
the two are matched case-insensitively, so they need not be spelled alike.
"""
if self.__check_excludes(excludes, laboratory, folder, file):
return True
context = DownloadContext(is_skip_if_exists, [])
context.files.append(DownloadFileInfo(file, os.path.join(l_dirname, local_name)))
return self.__multiple_download(context)
def __multiple_download_pickup_recursive_files( def __multiple_download_pickup_recursive_files(
self, self,
+9
View File
@@ -68,6 +68,9 @@ exclude = '''
exclude = ".git, .venv, __pycache__, dist" exclude = ".git, .venv, __pycache__, dist"
max-complexity = 10 max-complexity = 10
max-line-length = 120 max-line-length = 120
# E203 flags the space black puts before a slice colon. Black formats this project, so
# its output is the standard and flake8 has to accept it.
extend-ignore = ["E203"]
[tool.isort] [tool.isort]
profile = "black" profile = "black"
@@ -77,6 +80,12 @@ line_length = 120
[tool.pyright] [tool.pyright]
typeCheckingMode = "basic" typeCheckingMode = "basic"
# This project keeps its environment in `./.venv`. Named explicitly because without it
# pyright falls back to the system interpreter, cannot resolve pydantic, and reports every
# model field as an unknown argument. An environment kept elsewhere gets that same fallback
# and can override it with a local `pyrightconfig.json` or `pyright --pythonpath`.
venvPath = "."
venv = ".venv"
exclude = ["**/__pycache__", "**/.*", "dist"] exclude = ["**/__pycache__", "**/.*", "dist"]
#reportUnknownMemberType = "warning" #reportUnknownMemberType = "warning"
#reportUnknownVariableType = "warning" #reportUnknownVariableType = "warning"
+165
View File
@@ -0,0 +1,165 @@
import unittest
from unittest.mock import MagicMock, patch
from mdrsclient.client import MdrsClient
from mdrsclient.exceptions import IllegalArgumentException
from mdrsclient.models import File, Folder, FolderSimple, Laboratories, Laboratory
TIMESTAMP = "2026-01-01T00:00:00+09:00"
LABORATORY = Laboratory(id=1, name="mylab", pi_name="PI", full_name="My Laboratory")
def make_file(id: str, name: str) -> File:
return File(
id=id,
name=name,
type="text/plain",
size=1,
thumbnail=None,
description="",
metadata={},
download_url=f"v3/files/{id}/download/",
created_at=TIMESTAMP,
updated_at=TIMESTAMP,
)
def make_sub_folder(id: str, name: str) -> FolderSimple:
return FolderSimple(
id=id,
pid=None,
name=name,
access_level=1,
lock=False,
size=0,
laboratory_id=1,
description="",
created_at=TIMESTAMP,
updated_at=TIMESTAMP,
restrict_opened_at=None,
)
def make_folder(id: str, name: str, path: str, sub_folders: list[FolderSimple] | None = None) -> Folder:
return Folder(
id=id,
pid=None,
name=name,
access_level=1,
lock=False,
size=0,
laboratory_id=1,
description="",
created_at=TIMESTAMP,
updated_at=TIMESTAMP,
restrict_opened_at=None,
metadata=[],
sub_folders=sub_folders if sub_folders is not None else [],
path=path,
)
class TestLsColumnWidths(unittest.TestCase):
"""Each column is as wide as the widest thing printed under it."""
def test_the_size_column_fits_a_sub_folder_larger_than_its_parent(self):
from unittest.mock import MagicMock as _MagicMock
from mdrsclient.commands.ls import LsCommand, LsCommandContext
laboratories = Laboratories()
laboratories.append(LABORATORY)
connection = _MagicMock()
connection.laboratories = laboratories
client = MdrsClient(connection)
big = make_sub_folder("s1", "big")
object.__setattr__(big, "size", 123456789)
parent = make_folder("f1", "root", "/root/", [big])
context = LsCommandContext("remote:/mylab", client, LABORATORY, "", False, False, False)
widths = LsCommand._column_widths(context, parent, [])
self.assertEqual(widths["size"], len("123456789"))
class TestCopyAndMove(unittest.TestCase):
"""`cp` and `mv` resolve both ends the same way and differ only in what they call."""
def make_client(self, folders_by_path: dict, files_by_folder: dict) -> MdrsClient:
client = MdrsClient(MagicMock())
client.find_laboratory = MagicMock(return_value=LABORATORY)
client.find_folder = MagicMock(side_effect=lambda lab, path, password=None: folders_by_path[path])
client.find_files = MagicMock(side_effect=lambda folder_id: files_by_folder.get(folder_id, []))
return client
def make_tree(self, source_has: str) -> tuple[MdrsClient, dict, dict]:
sub = make_sub_folder("fsub", "sub")
folder_a = make_folder("fa", "a", "/a/", [sub] if source_has == "folder" else [])
folder_b = make_folder("fb", "b", "/b/")
folders = {"/a": folder_a, "/b": folder_b, "/b/": folder_b}
files = {"fa": [make_file("x1", "data.txt")] if source_has == "file" else [], "fb": []}
return self.make_client(folders, files), folders, files
def test_a_file_is_copied_into_the_destination_folder(self):
client, _, files = self.make_tree("file")
with patch("mdrsclient.client.FilesApi") as files_api:
client.cp("myremote:/mylab/a/data.txt", "myremote:/mylab/b/data.txt")
files_api.return_value.copy.assert_called_once_with(files["fa"][0], "fb", "data.txt")
def test_a_file_is_moved_into_the_destination_folder(self):
client, _, files = self.make_tree("file")
with patch("mdrsclient.client.FilesApi") as files_api:
client.mv("myremote:/mylab/a/data.txt", "myremote:/mylab/b/data.txt")
files_api.return_value.move.assert_called_once_with(files["fa"][0], "fb", "data.txt")
def test_a_trailing_separator_keeps_the_source_name(self):
client, _, files = self.make_tree("file")
with patch("mdrsclient.client.FilesApi") as files_api:
client.mv("myremote:/mylab/a/data.txt", "myremote:/mylab/b/")
files_api.return_value.move.assert_called_once_with(files["fa"][0], "fb", "data.txt")
def test_copying_a_folder_needs_the_recursive_flag(self):
client, _, _ = self.make_tree("folder")
with patch("mdrsclient.client.FoldersApi") as folders_api:
with self.assertRaises(IllegalArgumentException) as caught:
client.cp("myremote:/mylab/a/sub", "myremote:/mylab/b/sub")
self.assertIn("Is a folder", str(caught.exception))
folders_api.return_value.copy.assert_not_called()
def test_a_folder_is_copied_when_recursive(self):
client, folders, _ = self.make_tree("folder")
with patch("mdrsclient.client.FoldersApi") as folders_api:
client.cp("myremote:/mylab/a/sub", "myremote:/mylab/b/sub", is_recursive=True)
folders_api.return_value.copy.assert_called_once_with(folders["/a"].sub_folders[0], "fb", "sub")
def test_a_folder_is_moved_without_the_recursive_flag(self):
client, folders, _ = self.make_tree("folder")
with patch("mdrsclient.client.FoldersApi") as folders_api:
client.mv("myremote:/mylab/a/sub", "myremote:/mylab/b/sub")
folders_api.return_value.move.assert_called_once_with(folders["/a"].sub_folders[0], "fb", "sub")
def test_a_destination_that_already_holds_the_name_is_refused(self):
sub = make_sub_folder("fsub", "sub")
folders = {"/a": make_folder("fa", "a", "/a/", [sub]), "/b": make_folder("fb", "b", "/b/")}
files = {"fa": [], "fb": [make_file("x2", "sub")]}
client = self.make_client(folders, files)
with self.assertRaises(IllegalArgumentException) as caught:
client.mv("myremote:/mylab/a/sub", "myremote:/mylab/b/sub")
self.assertIn("Cannot overwrite non-folder", str(caught.exception))
def test_a_transfer_across_laboratories_is_refused(self):
client, _, _ = self.make_tree("file")
with self.assertRaises(IllegalArgumentException) as caught:
client.cp("myremote:/mylab/a/data.txt", "myremote:/otherlab/b/data.txt")
self.assertIn("Laboratory mismatched", str(caught.exception))
def test_a_source_that_does_not_exist_is_refused(self):
client, _, _ = self.make_tree("file")
with self.assertRaises(IllegalArgumentException) as caught:
client.mv("myremote:/mylab/a/missing.txt", "myremote:/mylab/b/missing.txt")
self.assertIn("not found", str(caught.exception))
if __name__ == "__main__":
unittest.main()
+55
View File
@@ -0,0 +1,55 @@
import unittest
from unittest.mock import MagicMock, patch
from mdrsclient.models import Doi, Folder, Laboratories, Laboratory
from mdrsclient.services import MdrsService
TIMESTAMP = "2026-01-01T00:00:00+09:00"
DOI = "10.60178/cbs.20260429-001"
LABORATORY = Laboratory(id=1, name="mylab", pi_name="PI", full_name="My Laboratory")
def make_locked_folder() -> Folder:
return Folder(
id="f1",
pid=None,
name="root",
access_level=4,
lock=True,
size=0,
laboratory_id=1,
description="",
created_at=TIMESTAMP,
updated_at=TIMESTAMP,
restrict_opened_at=None,
metadata=[],
sub_folders=[],
path="/root/",
)
class TestFindFolderByDoi(unittest.TestCase):
"""The DOI response carries the folder id as a field, not as a nested object."""
def test_a_locked_doi_folder_is_unlocked_with_its_folder_id(self):
laboratories = Laboratories()
laboratories.append(LABORATORY)
with (
patch("mdrsclient.services.DoiApi") as doi_api,
patch("mdrsclient.services.FoldersApi") as folders_api,
patch("mdrsclient.services.LaboratoriesApi") as laboratories_api,
):
doi_api.return_value.retrieve.return_value = Doi(id="20260429-001", doi=DOI, folder_id="f1")
folders_api.return_value.retrieve.return_value = make_locked_folder()
laboratories_api.return_value.list.return_value = laboratories
folder, laboratory = MdrsService(MagicMock()).find_folder_by_doi(DOI, "secret")
folders_api.return_value.auth.assert_called_once_with("f1", "secret")
self.assertEqual(folder.id, "f1")
self.assertEqual(laboratory.name, "mylab")
if __name__ == "__main__":
unittest.main()