mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
test: remove stdlib mocks, strengthen integration coverage (#489)
Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,9 @@
|
||||
"""Cloud API client utilities."""
|
||||
|
||||
from collections.abc import AsyncIterator
|
||||
from typing import Optional
|
||||
from contextlib import asynccontextmanager
|
||||
from typing import AsyncContextManager, Callable
|
||||
|
||||
import httpx
|
||||
import typer
|
||||
@@ -11,6 +14,8 @@ from basic_memory.config import ConfigManager
|
||||
|
||||
console = Console()
|
||||
|
||||
HttpClientFactory = Callable[[], AsyncContextManager[httpx.AsyncClient]]
|
||||
|
||||
|
||||
class CloudAPIError(Exception):
|
||||
"""Exception raised for cloud API errors."""
|
||||
@@ -38,14 +43,14 @@ def get_cloud_config() -> tuple[str, str, str]:
|
||||
return config.cloud_client_id, config.cloud_domain, config.cloud_host
|
||||
|
||||
|
||||
async def get_authenticated_headers() -> dict[str, str]:
|
||||
async def get_authenticated_headers(auth: CLIAuth | None = None) -> dict[str, str]:
|
||||
"""
|
||||
Get authentication headers with JWT token.
|
||||
handles jwt refresh if needed.
|
||||
"""
|
||||
client_id, domain, _ = get_cloud_config()
|
||||
auth = CLIAuth(client_id=client_id, authkit_domain=domain)
|
||||
token = await auth.get_valid_token()
|
||||
auth_obj = auth or CLIAuth(client_id=client_id, authkit_domain=domain)
|
||||
token = await auth_obj.get_valid_token()
|
||||
if not token:
|
||||
console.print("[red]Not authenticated. Please run 'basic-memory cloud login' first.[/red]")
|
||||
raise typer.Exit(1)
|
||||
@@ -53,21 +58,31 @@ async def get_authenticated_headers() -> dict[str, str]:
|
||||
return {"Authorization": f"Bearer {token}"}
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def _default_http_client(timeout: float) -> AsyncIterator[httpx.AsyncClient]:
|
||||
async with httpx.AsyncClient(timeout=timeout) as client:
|
||||
yield client
|
||||
|
||||
|
||||
async def make_api_request(
|
||||
method: str,
|
||||
url: str,
|
||||
headers: Optional[dict] = None,
|
||||
json_data: Optional[dict] = None,
|
||||
timeout: float = 30.0,
|
||||
*,
|
||||
auth: CLIAuth | None = None,
|
||||
http_client_factory: HttpClientFactory | None = None,
|
||||
) -> httpx.Response:
|
||||
"""Make an API request to the cloud service."""
|
||||
headers = headers or {}
|
||||
auth_headers = await get_authenticated_headers()
|
||||
auth_headers = await get_authenticated_headers(auth=auth)
|
||||
headers.update(auth_headers)
|
||||
# Add debug headers to help with compression issues
|
||||
headers.setdefault("Accept-Encoding", "identity") # Disable compression for debugging
|
||||
|
||||
async with httpx.AsyncClient(timeout=timeout) as client:
|
||||
client_factory = http_client_factory or (lambda: _default_http_client(timeout))
|
||||
async with client_factory() as client:
|
||||
try:
|
||||
response = await client.request(method=method, url=url, headers=headers, json=json_data)
|
||||
response.raise_for_status()
|
||||
|
||||
@@ -16,7 +16,10 @@ class CloudUtilsError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
async def fetch_cloud_projects() -> CloudProjectList:
|
||||
async def fetch_cloud_projects(
|
||||
*,
|
||||
api_request=make_api_request,
|
||||
) -> CloudProjectList:
|
||||
"""Fetch list of projects from cloud API.
|
||||
|
||||
Returns:
|
||||
@@ -27,14 +30,18 @@ async def fetch_cloud_projects() -> CloudProjectList:
|
||||
config = config_manager.config
|
||||
host_url = config.cloud_host.rstrip("/")
|
||||
|
||||
response = await make_api_request(method="GET", url=f"{host_url}/proxy/projects/projects")
|
||||
response = await api_request(method="GET", url=f"{host_url}/proxy/projects/projects")
|
||||
|
||||
return CloudProjectList.model_validate(response.json())
|
||||
except Exception as e:
|
||||
raise CloudUtilsError(f"Failed to fetch cloud projects: {e}") from e
|
||||
|
||||
|
||||
async def create_cloud_project(project_name: str) -> CloudProjectCreateResponse:
|
||||
async def create_cloud_project(
|
||||
project_name: str,
|
||||
*,
|
||||
api_request=make_api_request,
|
||||
) -> CloudProjectCreateResponse:
|
||||
"""Create a new project on cloud.
|
||||
|
||||
Args:
|
||||
@@ -57,7 +64,7 @@ async def create_cloud_project(project_name: str) -> CloudProjectCreateResponse:
|
||||
set_default=False,
|
||||
)
|
||||
|
||||
response = await make_api_request(
|
||||
response = await api_request(
|
||||
method="POST",
|
||||
url=f"{host_url}/proxy/projects/projects",
|
||||
headers={"Content-Type": "application/json"},
|
||||
@@ -84,7 +91,7 @@ async def sync_project(project_name: str, force_full: bool = False) -> None:
|
||||
raise CloudUtilsError(f"Failed to sync project '{project_name}': {e}") from e
|
||||
|
||||
|
||||
async def project_exists(project_name: str) -> bool:
|
||||
async def project_exists(project_name: str, *, api_request=make_api_request) -> bool:
|
||||
"""Check if a project exists on cloud.
|
||||
|
||||
Args:
|
||||
@@ -94,7 +101,7 @@ async def project_exists(project_name: str) -> bool:
|
||||
True if project exists, False otherwise
|
||||
"""
|
||||
try:
|
||||
projects = await fetch_cloud_projects()
|
||||
projects = await fetch_cloud_projects(api_request=api_request)
|
||||
project_names = {p.name for p in projects.projects}
|
||||
return project_name in project_names
|
||||
except Exception:
|
||||
|
||||
@@ -14,7 +14,7 @@ import subprocess
|
||||
from dataclasses import dataclass
|
||||
from functools import lru_cache
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
from typing import Callable, Optional, Protocol
|
||||
|
||||
from loguru import logger
|
||||
from rich.console import Console
|
||||
@@ -27,6 +27,14 @@ console = Console()
|
||||
# Minimum rclone version for --create-empty-src-dirs support
|
||||
MIN_RCLONE_VERSION_EMPTY_DIRS = (1, 64, 0)
|
||||
|
||||
class RunResult(Protocol):
|
||||
returncode: int
|
||||
stdout: str
|
||||
|
||||
|
||||
RunFunc = Callable[..., RunResult]
|
||||
IsInstalledFunc = Callable[[], bool]
|
||||
|
||||
|
||||
class RcloneError(Exception):
|
||||
"""Exception raised for rclone command errors."""
|
||||
@@ -34,13 +42,13 @@ class RcloneError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
def check_rclone_installed() -> None:
|
||||
def check_rclone_installed(is_installed: IsInstalledFunc = is_rclone_installed) -> None:
|
||||
"""Check if rclone is installed and raise helpful error if not.
|
||||
|
||||
Raises:
|
||||
RcloneError: If rclone is not installed with installation instructions
|
||||
"""
|
||||
if not is_rclone_installed():
|
||||
if not is_installed():
|
||||
raise RcloneError(
|
||||
"rclone is not installed.\n\n"
|
||||
"Install rclone by running: bm cloud setup\n"
|
||||
@@ -50,7 +58,7 @@ def check_rclone_installed() -> None:
|
||||
|
||||
|
||||
@lru_cache(maxsize=1)
|
||||
def get_rclone_version() -> tuple[int, int, int] | None:
|
||||
def get_rclone_version(run: RunFunc = subprocess.run) -> tuple[int, int, int] | None:
|
||||
"""Get rclone version as (major, minor, patch) tuple.
|
||||
|
||||
Returns:
|
||||
@@ -60,7 +68,7 @@ def get_rclone_version() -> tuple[int, int, int] | None:
|
||||
Result is cached since rclone version won't change during runtime.
|
||||
"""
|
||||
try:
|
||||
result = subprocess.run(["rclone", "version"], capture_output=True, text=True, timeout=10)
|
||||
result = run(["rclone", "version"], capture_output=True, text=True, timeout=10)
|
||||
# Parse "rclone v1.64.2" or "rclone v1.60.1-DEV"
|
||||
match = re.search(r"v(\d+)\.(\d+)\.(\d+)", result.stdout)
|
||||
if match:
|
||||
@@ -72,13 +80,12 @@ def get_rclone_version() -> tuple[int, int, int] | None:
|
||||
return None
|
||||
|
||||
|
||||
def supports_create_empty_src_dirs() -> bool:
|
||||
def supports_create_empty_src_dirs(version: tuple[int, int, int] | None) -> bool:
|
||||
"""Check if installed rclone supports --create-empty-src-dirs flag.
|
||||
|
||||
Returns:
|
||||
True if rclone version >= 1.64.0, False otherwise.
|
||||
"""
|
||||
version = get_rclone_version()
|
||||
if version is None:
|
||||
# If we can't determine version, assume older and skip the flag
|
||||
return False
|
||||
@@ -167,6 +174,10 @@ def project_sync(
|
||||
bucket_name: str,
|
||||
dry_run: bool = False,
|
||||
verbose: bool = False,
|
||||
*,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
filter_path: Path | None = None,
|
||||
) -> bool:
|
||||
"""One-way sync: local → cloud.
|
||||
|
||||
@@ -184,14 +195,14 @@ def project_sync(
|
||||
Raises:
|
||||
RcloneError: If project has no local_sync_path configured or rclone not installed
|
||||
"""
|
||||
check_rclone_installed()
|
||||
check_rclone_installed(is_installed=is_installed)
|
||||
|
||||
if not project.local_sync_path:
|
||||
raise RcloneError(f"Project {project.name} has no local_sync_path configured")
|
||||
|
||||
local_path = Path(project.local_sync_path).expanduser()
|
||||
remote_path = get_project_remote(project, bucket_name)
|
||||
filter_path = get_bmignore_filter_path()
|
||||
filter_path = filter_path or get_bmignore_filter_path()
|
||||
|
||||
cmd = [
|
||||
"rclone",
|
||||
@@ -210,7 +221,7 @@ def project_sync(
|
||||
if dry_run:
|
||||
cmd.append("--dry-run")
|
||||
|
||||
result = subprocess.run(cmd, text=True)
|
||||
result = run(cmd, text=True)
|
||||
return result.returncode == 0
|
||||
|
||||
|
||||
@@ -220,6 +231,13 @@ def project_bisync(
|
||||
dry_run: bool = False,
|
||||
resync: bool = False,
|
||||
verbose: bool = False,
|
||||
*,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
version: tuple[int, int, int] | None = None,
|
||||
filter_path: Path | None = None,
|
||||
state_path: Path | None = None,
|
||||
is_initialized: Callable[[str], bool] = bisync_initialized,
|
||||
) -> bool:
|
||||
"""Two-way sync: local ↔ cloud.
|
||||
|
||||
@@ -242,15 +260,15 @@ def project_bisync(
|
||||
Raises:
|
||||
RcloneError: If project has no local_sync_path, needs --resync, or rclone not installed
|
||||
"""
|
||||
check_rclone_installed()
|
||||
check_rclone_installed(is_installed=is_installed)
|
||||
|
||||
if not project.local_sync_path:
|
||||
raise RcloneError(f"Project {project.name} has no local_sync_path configured")
|
||||
|
||||
local_path = Path(project.local_sync_path).expanduser()
|
||||
remote_path = get_project_remote(project, bucket_name)
|
||||
filter_path = get_bmignore_filter_path()
|
||||
state_path = get_project_bisync_state(project.name)
|
||||
filter_path = filter_path or get_bmignore_filter_path()
|
||||
state_path = state_path or get_project_bisync_state(project.name)
|
||||
|
||||
# Ensure state directory exists
|
||||
state_path.mkdir(parents=True, exist_ok=True)
|
||||
@@ -271,7 +289,8 @@ def project_bisync(
|
||||
]
|
||||
|
||||
# Add --create-empty-src-dirs if rclone version supports it (v1.64+)
|
||||
if supports_create_empty_src_dirs():
|
||||
version = version if version is not None else get_rclone_version(run=run)
|
||||
if supports_create_empty_src_dirs(version):
|
||||
cmd.append("--create-empty-src-dirs")
|
||||
|
||||
if verbose:
|
||||
@@ -286,13 +305,13 @@ def project_bisync(
|
||||
cmd.append("--resync")
|
||||
|
||||
# Check if first run requires resync
|
||||
if not resync and not bisync_initialized(project.name) and not dry_run:
|
||||
if not resync and not is_initialized(project.name) and not dry_run:
|
||||
raise RcloneError(
|
||||
f"First bisync for {project.name} requires --resync to establish baseline.\n"
|
||||
f"Run: bm project bisync --name {project.name} --resync"
|
||||
)
|
||||
|
||||
result = subprocess.run(cmd, text=True)
|
||||
result = run(cmd, text=True)
|
||||
return result.returncode == 0
|
||||
|
||||
|
||||
@@ -300,6 +319,10 @@ def project_check(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
one_way: bool = False,
|
||||
*,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
filter_path: Path | None = None,
|
||||
) -> bool:
|
||||
"""Check integrity between local and cloud.
|
||||
|
||||
@@ -316,14 +339,14 @@ def project_check(
|
||||
Raises:
|
||||
RcloneError: If project has no local_sync_path configured or rclone not installed
|
||||
"""
|
||||
check_rclone_installed()
|
||||
check_rclone_installed(is_installed=is_installed)
|
||||
|
||||
if not project.local_sync_path:
|
||||
raise RcloneError(f"Project {project.name} has no local_sync_path configured")
|
||||
|
||||
local_path = Path(project.local_sync_path).expanduser()
|
||||
remote_path = get_project_remote(project, bucket_name)
|
||||
filter_path = get_bmignore_filter_path()
|
||||
filter_path = filter_path or get_bmignore_filter_path()
|
||||
|
||||
cmd = [
|
||||
"rclone",
|
||||
@@ -337,7 +360,7 @@ def project_check(
|
||||
if one_way:
|
||||
cmd.append("--one-way")
|
||||
|
||||
result = subprocess.run(cmd, capture_output=True, text=True)
|
||||
result = run(cmd, capture_output=True, text=True)
|
||||
return result.returncode == 0
|
||||
|
||||
|
||||
@@ -345,6 +368,9 @@ def project_ls(
|
||||
project: SyncProject,
|
||||
bucket_name: str,
|
||||
path: Optional[str] = None,
|
||||
*,
|
||||
run: RunFunc = subprocess.run,
|
||||
is_installed: IsInstalledFunc = is_rclone_installed,
|
||||
) -> list[str]:
|
||||
"""List files in remote project.
|
||||
|
||||
@@ -360,12 +386,12 @@ def project_ls(
|
||||
subprocess.CalledProcessError: If rclone command fails
|
||||
RcloneError: If rclone is not installed
|
||||
"""
|
||||
check_rclone_installed()
|
||||
check_rclone_installed(is_installed=is_installed)
|
||||
|
||||
remote_path = get_project_remote(project, bucket_name)
|
||||
if path:
|
||||
remote_path = f"{remote_path}/{path}"
|
||||
|
||||
cmd = ["rclone", "ls", remote_path]
|
||||
result = subprocess.run(cmd, capture_output=True, text=True, check=True)
|
||||
result = run(cmd, capture_output=True, text=True, check=True)
|
||||
return result.stdout.splitlines()
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
from contextlib import AbstractAsyncContextManager
|
||||
from typing import Callable
|
||||
|
||||
import aiofiles
|
||||
import httpx
|
||||
@@ -20,6 +22,9 @@ async def upload_path(
|
||||
verbose: bool = False,
|
||||
use_gitignore: bool = True,
|
||||
dry_run: bool = False,
|
||||
*,
|
||||
client_cm_factory: Callable[[], AbstractAsyncContextManager[httpx.AsyncClient]] | None = None,
|
||||
put_func=call_put,
|
||||
) -> bool:
|
||||
"""
|
||||
Upload a file or directory to cloud project via WebDAV.
|
||||
@@ -85,8 +90,10 @@ async def upload_path(
|
||||
size_str = f"{size / (1024 * 1024):.1f} MB"
|
||||
print(f" {relative_path} ({size_str})")
|
||||
else:
|
||||
# Upload files using httpx
|
||||
async with get_client() as client:
|
||||
# Upload files using httpx.
|
||||
# Allow injection for tests (MockTransport) while keeping production default.
|
||||
cm_factory = client_cm_factory or get_client
|
||||
async with cm_factory() as client:
|
||||
for i, (file_path, relative_path) in enumerate(files_to_upload, 1):
|
||||
# Skip archive files (zip, tar, gz, etc.)
|
||||
if _is_archive_file(file_path):
|
||||
@@ -110,7 +117,7 @@ async def upload_path(
|
||||
|
||||
# Upload via HTTP PUT to WebDAV endpoint with mtime header
|
||||
# Using X-OC-Mtime (ownCloud/Nextcloud standard)
|
||||
response = await call_put(
|
||||
response = await put_func(
|
||||
client, remote_path, content=content, headers={"X-OC-Mtime": str(mtime)}
|
||||
)
|
||||
response.raise_for_status()
|
||||
|
||||
Reference in New Issue
Block a user