From 398304c76049432169cafc2d84a9a7b77c940c8a Mon Sep 17 00:00:00 2001 From: Michael Milton Date: Fri, 17 Jan 2025 09:38:30 +1100 Subject: [PATCH 1/3] Remove keywords --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 7c15c92..760dedb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,6 @@ description = "FileSender Python CLI and API client" version = "2.0.0" readme = "README.md" requires-python = ">=3.8" -keywords = ["one", "two"] license = {text = "BSD-3-Clause"} classifiers = [ "Programming Language :: Python :: 3", From 67e1371368b3228d6cfc6342a7566292209bf446 Mon Sep 17 00:00:00 2001 From: Michael Milton Date: Fri, 17 Jan 2025 11:33:11 +1100 Subject: [PATCH 2/3] Refactor URL handling into separate class, fix 28 --- filesender/api.py | 58 +++++++++++++++++++++++++++++++++------------ test/test_client.py | 13 ++++++++++ 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/filesender/api.py b/filesender/api.py index 3dd160f..cca809e 100644 --- a/filesender/api.py +++ b/filesender/api.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass from typing import Any, Iterable, List, Optional, Tuple, AsyncIterator, Union from filesender.download import files_from_page, DownloadFile import filesender.response_types as response @@ -99,13 +100,41 @@ def iter_files(paths: Iterable[Path], root: Optional[Path] = None) -> Iterable[T # If this is a nested file, use the relative path from the root directory as the name yield str(path.relative_to(root)), path +@dataclass +class EndpointHandler: + base: str + + def api(self) -> str: + return f"{self.base}/rest.php" + + def download(self) -> str: + return f"{self.base}/download.php" + + def create_transfer(self) -> str: + return f"{self.api()}/transfer" + + def single_transfer(self, transfer_id: int) -> str: + return f"{self.api()}/transfer/{transfer_id}" + + def chunk(self, file_id: int, offset: int) -> str: + return f"{self.api()}/file/{file_id}/chunk/{offset}" + + def file(self, file_id: int) -> str: + return f"{self.api()}/file/{file_id}" + + def guest(self) -> str: + return f"{self.api()}/guest" + + def server_info(self) -> str: + return f"{self.api()}/info" + class FileSenderClient: """ A client that can be used to programmatically interact with FileSender. """ #: The base url of the file sender's API. For example https://filesender.aarnet.edu.au/rest.php - base_url: str + urls: EndpointHandler #: Size of upload chunks chunk_size: Optional[int] #: Authentication provider that will be used for all privileged requests @@ -141,7 +170,8 @@ def __init__( speed up transfers, or reduce this number to reduce memory usage and network errors. This can be set to `None` to enable unlimited concurrency, but use at your own risk. """ - self.base_url = base_url + # self.base_url = base_url + self.urls = EndpointHandler(base_url) self.auth = auth # FileSender seems to sometimes use redirects self.http_client = AsyncClient(timeout=None, follow_redirects=True) @@ -149,6 +179,7 @@ def __init__( self.concurrent_chunks = concurrent_chunks self.concurrent_files = concurrent_files + async def prepare(self) -> None: """ Checks that the chunk size is appropriate and/or sets the chunk size based on the server info. @@ -177,7 +208,7 @@ def on_retry(state: RetryCallState) -> None: if e is not None: message = exception_to_message(e) - logger.warn(f"Attempt {state.attempt_number}. {message}") + logger.warning(f"Attempt {state.attempt_number}. {message}") @retry( retry=retry_if_exception(should_retry), @@ -209,7 +240,7 @@ async def create_transfer( return await self._sign_send( self.http_client.build_request( "POST", - f"{self.base_url}/transfer", + self.urls.create_transfer(), json=body, ) ) @@ -228,12 +259,12 @@ async def update_transfer( body: See [`TransferUpdate`][filesender.request_types.TransferUpdate] Returns: - : See [`Transfer`][filesender.response_types.Transfer] + See [`Transfer`][filesender.response_types.Transfer] """ return await self._sign_send( self.http_client.build_request( "PUT", - f"{self.base_url}/transfer/{transfer_id}", + self.urls.single_transfer(transfer_id), json=body, ) ) @@ -254,7 +285,7 @@ async def update_file( await self._sign_send( self.http_client.build_request( "PUT", - f"{self.base_url}/file/{file_info['id']}", + self.urls.file(file_info['id']), params={"key": file_info["uid"]}, json=body, ) @@ -300,7 +331,7 @@ async def _upload_chunk( return await self._sign_send( self.http_client.build_request( "PUT", - f"{self.base_url}/file/{file_info['id']}/chunk/{offset}", + self.urls.chunk(file_info["id"], offset), params={"key": file_info["uid"]}, content=chunk, headers={ @@ -323,7 +354,7 @@ async def create_guest(self, body: request.Guest) -> response.Guest: : See [`Guest`][filesender.response_types.Guest] """ return await self._sign_send( - self.http_client.build_request("POST", f"{self.base_url}/guest", json=body) + self.http_client.build_request("POST", self.urls.guest(), json=body) ) async def _files_from_token(self, token: str) -> Iterable[DownloadFile]: @@ -331,7 +362,7 @@ async def _files_from_token(self, token: str) -> Iterable[DownloadFile]: Internal function that returns a list of file IDs for a given guest token """ download_page = await self.http_client.get( - "https://filesender.aarnet.edu.au", params={"s": "download", "token": token} + self.urls.base, params={"s": "download", "token": token} ) return files_from_page(download_page.content) @@ -377,11 +408,8 @@ async def download_file( file_size: The file size in bytes, optionally. file_name: The file name of the file being downloaded. This will impact the name by which it's saved. """ - download_endpoint = urlunparse( - urlparse(self.base_url)._replace(path="/download.php") - ) async with self.http_client.stream( - "GET", download_endpoint, params={"files_ids": file_id, "token": token} + "GET", self.urls.download(), params={"files_ids": file_id, "token": token} ) as res: # Determine filename from response, if not provided if file_name is None: @@ -411,7 +439,7 @@ async def get_server_info(self) -> response.ServerInfo: Returns: : See [`ServerInfo`][filesender.response_types.ServerInfo]. """ - return (await self.http_client.get(f"{self.base_url}/info")).json() + return (await self.http_client.get(self.urls.server_info())).json() async def upload_workflow( self, files: List[Path], transfer_args: request.PartialTransfer = {} diff --git a/test/test_client.py b/test/test_client.py index 4d36d0f..15b35fc 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -5,6 +5,7 @@ import pytest from filesender.request_types import GuestOptions from filesender.benchmark import make_tempfile, make_tempfiles, benchmark +from unittest.mock import MagicMock, patch def count_files_recursively(path: Path) -> int: """ @@ -152,3 +153,15 @@ async def test_upload_semaphore( limited, unlimited = benchmark(paths, [1, float("inf")], [1, float("inf")], base_url, username, apikey, recipient) assert unlimited.time < limited.time assert unlimited.memory > limited.memory + +@pytest.mark.asyncio +async def test_client_download_url(): + mock_http_client = MagicMock() + token = "NOT A REAL TOKEN" + client = FileSenderClient(base_url="http://localhost:8080") + client.http_client = mock_http_client + try: + await client.download_files(token, out_dir=Path("NOT A REAL DIR")) + except Exception: + pass + mock_http_client.get.assert_called_once_with("http://localhost:8080", params=dict(s="download", token=token)) From 7dc14484f87fc0951394d7248eb96ce0d9674587 Mon Sep 17 00:00:00 2001 From: Michael Milton Date: Fri, 17 Jan 2025 11:34:21 +1100 Subject: [PATCH 3/3] Docstring for new test --- test/test_client.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_client.py b/test/test_client.py index 15b35fc..8ce1db0 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -156,6 +156,9 @@ async def test_upload_semaphore( @pytest.mark.asyncio async def test_client_download_url(): + """ + Tests that the client constructs the correct download URL when downloading a file + """ mock_http_client = MagicMock() token = "NOT A REAL TOKEN" client = FileSenderClient(base_url="http://localhost:8080")