fix(api): Fix S3 backup path-style configuration and improve UX.

- Fix critical S3 client configuration bug for path-style addressing
- Add toast notifications for better user feedback on S3 config operations
- Set up Toaster component in root layout for app-wide notifications
- Clean up unused imports in test files

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Elisiário Couto
2025-09-28 22:55:15 +01:00
committed by Elisiário Couto
parent 0122913052
commit 22ec0e36b1
7 changed files with 150 additions and 126 deletions

View File

@@ -1,6 +1,7 @@
import { useState, useEffect } from "react";
import { useMutation, useQueryClient } from "@tanstack/react-query";
import { Cloud, TestTube } from "lucide-react";
import { toast } from "sonner";
import { apiClient } from "../lib/api";
import { Button } from "./ui/button";
import { Input } from "./ui/input";
@@ -55,9 +56,12 @@ export default function S3BackupConfigDrawer({
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ["backupSettings"] });
setOpen(false);
toast.success("S3 backup configuration saved successfully");
},
onError: (error) => {
onError: (error: any) => {
console.error("Failed to update S3 backup configuration:", error);
const message = error?.response?.data?.detail || "Failed to save S3 configuration. Please check your settings and try again.";
toast.error(message);
},
});
@@ -69,9 +73,12 @@ export default function S3BackupConfigDrawer({
}),
onSuccess: () => {
console.log("S3 connection test successful");
toast.success("S3 connection test successful! Your configuration is working correctly.");
},
onError: (error) => {
onError: (error: any) => {
console.error("Failed to test S3 connection:", error);
const message = error?.response?.data?.detail || "S3 connection test failed. Please verify your credentials and settings.";
toast.error(message);
},
});
@@ -245,4 +252,4 @@ export default function S3BackupConfigDrawer({
</DrawerContent>
</Drawer>
);
}
}

View File

@@ -5,6 +5,7 @@ import { PWAInstallPrompt, PWAUpdatePrompt } from "../components/PWAPrompts";
import { usePWA } from "../hooks/usePWA";
import { useVersionCheck } from "../hooks/useVersionCheck";
import { SidebarInset, SidebarProvider } from "../components/ui/sidebar";
import { Toaster } from "../components/ui/sonner";
function RootLayout() {
const { updateAvailable, updateSW, forceReload } = usePWA();
@@ -48,6 +49,9 @@ function RootLayout() {
updateAvailable={updateAvailable}
onUpdate={handlePWAUpdate}
/>
{/* Toast Notifications */}
<Toaster />
</SidebarProvider>
);
}

View File

@@ -7,7 +7,7 @@ from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware
from loguru import logger
from leggen.api.routes import accounts, banks, backup, notifications, sync, transactions
from leggen.api.routes import accounts, backup, banks, notifications, sync, transactions
from leggen.background.scheduler import scheduler
from leggen.utils.config import config
from leggen.utils.paths import path_manager

View File

@@ -38,35 +38,40 @@ class BackupService:
s3_kwargs["endpoint_url"] = current_config.endpoint_url
if current_config.path_style:
s3_kwargs["config"] = boto3.client("s3").meta.config
s3_kwargs["config"].s3 = {"addressing_style": "path"}
from botocore.config import Config
s3_kwargs["config"] = Config(s3={"addressing_style": "path"})
return session.client("s3", **s3_kwargs)
async def test_connection(self, config: S3BackupConfig) -> bool:
"""Test S3 connection with provided configuration.
Args:
config: S3 configuration to test
Returns:
True if connection successful, False otherwise
"""
try:
s3_client = self._get_s3_client(config)
# Try to list objects in the bucket (limited to 1 to minimize cost)
s3_client.list_objects_v2(Bucket=config.bucket_name, MaxKeys=1)
logger.info(f"S3 connection test successful for bucket: {config.bucket_name}")
logger.info(
f"S3 connection test successful for bucket: {config.bucket_name}"
)
return True
except NoCredentialsError:
logger.error("S3 credentials not found or invalid")
return False
except ClientError as e:
error_code = e.response["Error"]["Code"]
logger.error(f"S3 connection test failed: {error_code} - {e.response['Error']['Message']}")
logger.error(
f"S3 connection test failed: {error_code} - {e.response['Error']['Message']}"
)
return False
except Exception as e:
logger.error(f"Unexpected error during S3 connection test: {str(e)}")
@@ -74,10 +79,10 @@ class BackupService:
async def backup_database(self, database_path: Path) -> bool:
"""Backup database file to S3.
Args:
database_path: Path to the SQLite database file
Returns:
True if backup successful, False otherwise
"""
@@ -91,29 +96,27 @@ class BackupService:
try:
s3_client = self._get_s3_client()
# Generate backup filename with timestamp
timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
backup_key = f"leggen_backups/database_backup_{timestamp}.db"
# Upload database file
logger.info(f"Starting database backup to S3: {backup_key}")
s3_client.upload_file(
str(database_path),
self.s3_config.bucket_name,
backup_key
str(database_path), self.s3_config.bucket_name, backup_key
)
logger.info(f"Database backup completed successfully: {backup_key}")
return True
except Exception as e:
logger.error(f"Database backup failed: {str(e)}")
return False
async def list_backups(self) -> list[dict]:
"""List available backups in S3.
Returns:
List of backup metadata dictionaries
"""
@@ -123,37 +126,38 @@ class BackupService:
try:
s3_client = self._get_s3_client()
# List objects with backup prefix
response = s3_client.list_objects_v2(
Bucket=self.s3_config.bucket_name,
Prefix="leggen_backups/"
Bucket=self.s3_config.bucket_name, Prefix="leggen_backups/"
)
backups = []
for obj in response.get("Contents", []):
backups.append({
"key": obj["Key"],
"last_modified": obj["LastModified"].isoformat(),
"size": obj["Size"],
})
backups.append(
{
"key": obj["Key"],
"last_modified": obj["LastModified"].isoformat(),
"size": obj["Size"],
}
)
# Sort by last modified (newest first)
backups.sort(key=lambda x: x["last_modified"], reverse=True)
return backups
except Exception as e:
logger.error(f"Failed to list backups: {str(e)}")
return []
async def restore_database(self, backup_key: str, restore_path: Path) -> bool:
"""Restore database from S3 backup.
Args:
backup_key: S3 key of the backup to restore
restore_path: Path where to restore the database
Returns:
True if restore successful, False otherwise
"""
@@ -163,28 +167,26 @@ class BackupService:
try:
s3_client = self._get_s3_client()
# Download backup file
logger.info(f"Starting database restore from S3: {backup_key}")
# Create parent directory if it doesn't exist
restore_path.parent.mkdir(parents=True, exist_ok=True)
# Download to temporary file first, then move to final location
with tempfile.NamedTemporaryFile(delete=False) as temp_file:
s3_client.download_file(
self.s3_config.bucket_name,
backup_key,
temp_file.name
self.s3_config.bucket_name, backup_key, temp_file.name
)
# Move temp file to final location
temp_path = Path(temp_file.name)
temp_path.replace(restore_path)
logger.info(f"Database restore completed successfully: {restore_path}")
return True
except Exception as e:
logger.error(f"Database restore failed: {str(e)}")
return False
return False

View File

@@ -89,5 +89,5 @@ markers = [
]
[[tool.mypy.overrides]]
module = ["apscheduler.*", "discord_webhook.*"]
module = ["apscheduler.*", "discord_webhook.*", "botocore.*", "boto3.*"]
ignore_missing_imports = true

View File

@@ -1,12 +1,9 @@
"""Tests for backup API endpoints."""
from unittest.mock import AsyncMock, patch
from unittest.mock import patch
import pytest
from leggen.api.models.backup import BackupSettings, S3Config
from leggen.models.config import S3BackupConfig
@pytest.mark.api
class TestBackupAPI:
@@ -16,10 +13,10 @@ class TestBackupAPI:
"""Test getting backup settings with no configuration."""
# Mock empty backup config by updating the config dict
mock_config._config["backup"] = {}
with patch("leggen.utils.config.config", mock_config):
response = api_client.get("/api/v1/backup/settings")
assert response.status_code == 200
data = response.json()
assert data["success"] is True
@@ -39,15 +36,15 @@ class TestBackupAPI:
"enabled": True,
}
}
with patch("leggen.utils.config.config", mock_config):
response = api_client.get("/api/v1/backup/settings")
assert response.status_code == 200
data = response.json()
assert data["success"] is True
assert data["data"]["s3"] is not None
s3_config = data["data"]["s3"]
assert s3_config["access_key_id"] == "***" # Masked
assert s3_config["secret_access_key"] == "***" # Masked
@@ -56,11 +53,13 @@ class TestBackupAPI:
assert s3_config["enabled"] is True
@patch("leggen.services.backup_service.BackupService.test_connection")
def test_update_backup_settings_success(self, mock_test_connection, api_client, mock_config):
def test_update_backup_settings_success(
self, mock_test_connection, api_client, mock_config
):
"""Test successful backup settings update."""
mock_test_connection.return_value = True
mock_config._config["backup"] = {}
request_data = {
"s3": {
"access_key_id": "AKIATEST123",
@@ -72,24 +71,26 @@ class TestBackupAPI:
"enabled": True,
}
}
with patch("leggen.utils.config.config", mock_config):
response = api_client.put("/api/v1/backup/settings", json=request_data)
assert response.status_code == 200
data = response.json()
assert data["success"] is True
assert data["data"]["updated"] is True
# Verify connection test was called
mock_test_connection.assert_called_once()
@patch("leggen.services.backup_service.BackupService.test_connection")
def test_update_backup_settings_connection_failure(self, mock_test_connection, api_client, mock_config):
def test_update_backup_settings_connection_failure(
self, mock_test_connection, api_client, mock_config
):
"""Test backup settings update with connection test failure."""
mock_test_connection.return_value = False
mock_config._config["backup"] = {}
request_data = {
"s3": {
"access_key_id": "AKIATEST123",
@@ -101,10 +102,10 @@ class TestBackupAPI:
"enabled": True,
}
}
with patch("leggen.utils.config.config", mock_config):
response = api_client.put("/api/v1/backup/settings", json=request_data)
assert response.status_code == 400
data = response.json()
assert "S3 connection test failed" in data["detail"]
@@ -113,7 +114,7 @@ class TestBackupAPI:
def test_test_backup_connection_success(self, mock_test_connection, api_client):
"""Test successful backup connection test."""
mock_test_connection.return_value = True
request_data = {
"service": "s3",
"config": {
@@ -124,16 +125,16 @@ class TestBackupAPI:
"endpoint_url": None,
"path_style": False,
"enabled": True,
}
},
}
response = api_client.post("/api/v1/backup/test", json=request_data)
assert response.status_code == 200
data = response.json()
assert data["success"] is True
assert data["data"]["connected"] is True
# Verify connection test was called
mock_test_connection.assert_called_once()
@@ -141,7 +142,7 @@ class TestBackupAPI:
def test_test_backup_connection_failure(self, mock_test_connection, api_client):
"""Test failed backup connection test."""
mock_test_connection.return_value = False
request_data = {
"service": "s3",
"config": {
@@ -152,11 +153,11 @@ class TestBackupAPI:
"endpoint_url": None,
"path_style": False,
"enabled": True,
}
},
}
response = api_client.post("/api/v1/backup/test", json=request_data)
assert response.status_code == 200
data = response.json()
assert data["success"] is False
@@ -173,11 +174,11 @@ class TestBackupAPI:
"endpoint_url": None,
"path_style": False,
"enabled": True,
}
},
}
response = api_client.post("/api/v1/backup/test", json=request_data)
assert response.status_code == 400
data = response.json()
assert "Only 's3' service is supported" in data["detail"]
@@ -197,7 +198,7 @@ class TestBackupAPI:
"size": 512,
},
]
mock_config._config["backup"] = {
"s3": {
"access_key_id": "AKIATEST123",
@@ -207,23 +208,26 @@ class TestBackupAPI:
"enabled": True,
}
}
with patch("leggen.utils.config.config", mock_config):
response = api_client.get("/api/v1/backup/list")
assert response.status_code == 200
data = response.json()
assert data["success"] is True
assert len(data["data"]) == 2
assert data["data"][0]["key"] == "leggen_backups/database_backup_20250101_120000.db"
assert (
data["data"][0]["key"]
== "leggen_backups/database_backup_20250101_120000.db"
)
def test_list_backups_no_config(self, api_client, mock_config):
"""Test backup listing with no configuration."""
mock_config._config["backup"] = {}
with patch("leggen.utils.config.config", mock_config):
response = api_client.get("/api/v1/backup/list")
assert response.status_code == 200
data = response.json()
assert data["success"] is True
@@ -231,13 +235,15 @@ class TestBackupAPI:
@patch("leggen.services.backup_service.BackupService.backup_database")
@patch("leggen.utils.paths.path_manager.get_database_path")
def test_backup_operation_success(self, mock_get_db_path, mock_backup_db, api_client, mock_config):
def test_backup_operation_success(
self, mock_get_db_path, mock_backup_db, api_client, mock_config
):
"""Test successful backup operation."""
from pathlib import Path
mock_get_db_path.return_value = Path("/test/database.db")
mock_backup_db.return_value = True
mock_config._config["backup"] = {
"s3": {
"access_key_id": "AKIATEST123",
@@ -247,30 +253,30 @@ class TestBackupAPI:
"enabled": True,
}
}
request_data = {"operation": "backup"}
with patch("leggen.utils.config.config", mock_config):
response = api_client.post("/api/v1/backup/operation", json=request_data)
assert response.status_code == 200
data = response.json()
assert data["success"] is True
assert data["data"]["operation"] == "backup"
assert data["data"]["completed"] is True
# Verify backup was called
mock_backup_db.assert_called_once()
def test_backup_operation_no_config(self, api_client, mock_config):
"""Test backup operation with no configuration."""
mock_config._config["backup"] = {}
request_data = {"operation": "backup"}
with patch("leggen.utils.config.config", mock_config):
response = api_client.post("/api/v1/backup/operation", json=request_data)
assert response.status_code == 400
data = response.json()
assert "S3 backup is not configured" in data["detail"]
@@ -286,12 +292,12 @@ class TestBackupAPI:
"enabled": True,
}
}
request_data = {"operation": "invalid"}
with patch("leggen.utils.config.config", mock_config):
response = api_client.post("/api/v1/backup/operation", json=request_data)
assert response.status_code == 400
data = response.json()
assert "Invalid operation" in data["detail"]
assert "Invalid operation" in data["detail"]

View File

@@ -2,7 +2,7 @@
import tempfile
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock, patch
from unittest.mock import MagicMock, patch
import pytest
from botocore.exceptions import ClientError, NoCredentialsError
@@ -41,20 +41,20 @@ class TestBackupService:
bucket_name="test-bucket",
region="us-east-1",
)
service = BackupService()
# Mock S3 client
with patch("boto3.Session") as mock_session:
mock_client = MagicMock()
mock_session.return_value.client.return_value = mock_client
# Mock successful list_objects_v2 call
mock_client.list_objects_v2.return_value = {"Contents": []}
result = await service.test_connection(s3_config)
assert result is True
# Verify the client was called correctly
mock_client.list_objects_v2.assert_called_once_with(
Bucket="test-bucket", MaxKeys=1
@@ -69,15 +69,15 @@ class TestBackupService:
bucket_name="test-bucket",
region="us-east-1",
)
service = BackupService()
# Mock S3 client to raise NoCredentialsError
with patch("boto3.Session") as mock_session:
mock_client = MagicMock()
mock_session.return_value.client.return_value = mock_client
mock_client.list_objects_v2.side_effect = NoCredentialsError()
result = await service.test_connection(s3_config)
assert result is False
@@ -90,16 +90,20 @@ class TestBackupService:
bucket_name="test-bucket",
region="us-east-1",
)
service = BackupService()
# Mock S3 client to raise ClientError
with patch("boto3.Session") as mock_session:
mock_client = MagicMock()
mock_session.return_value.client.return_value = mock_client
error_response = {"Error": {"Code": "NoSuchBucket", "Message": "Bucket not found"}}
mock_client.list_objects_v2.side_effect = ClientError(error_response, "ListObjectsV2")
error_response = {
"Error": {"Code": "NoSuchBucket", "Message": "Bucket not found"}
}
mock_client.list_objects_v2.side_effect = ClientError(
error_response, "ListObjectsV2"
)
result = await service.test_connection(s3_config)
assert result is False
@@ -107,11 +111,11 @@ class TestBackupService:
async def test_backup_database_no_config(self):
"""Test backup database with no configuration."""
service = BackupService()
with tempfile.TemporaryDirectory() as tmpdir:
db_path = Path(tmpdir) / "test.db"
db_path.write_text("test database content")
result = await service.backup_database(db_path)
assert result is False
@@ -126,11 +130,11 @@ class TestBackupService:
enabled=False,
)
service = BackupService(s3_config)
with tempfile.TemporaryDirectory() as tmpdir:
db_path = Path(tmpdir) / "test.db"
db_path.write_text("test database content")
result = await service.backup_database(db_path)
assert result is False
@@ -144,7 +148,7 @@ class TestBackupService:
region="us-east-1",
)
service = BackupService(s3_config)
non_existent_path = Path("/non/existent/path.db")
result = await service.backup_database(non_existent_path)
assert result is False
@@ -159,19 +163,19 @@ class TestBackupService:
region="us-east-1",
)
service = BackupService(s3_config)
with tempfile.TemporaryDirectory() as tmpdir:
db_path = Path(tmpdir) / "test.db"
db_path.write_text("test database content")
# Mock S3 client
with patch("boto3.Session") as mock_session:
mock_client = MagicMock()
mock_session.return_value.client.return_value = mock_client
result = await service.backup_database(db_path)
assert result is True
# Verify upload_file was called
mock_client.upload_file.assert_called_once()
args = mock_client.upload_file.call_args[0]
@@ -189,13 +193,14 @@ class TestBackupService:
region="us-east-1",
)
service = BackupService(s3_config)
# Mock S3 client response
with patch("boto3.Session") as mock_session:
mock_client = MagicMock()
mock_session.return_value.client.return_value = mock_client
from datetime import datetime
mock_response = {
"Contents": [
{
@@ -211,11 +216,11 @@ class TestBackupService:
]
}
mock_client.list_objects_v2.return_value = mock_response
backups = await service.list_backups()
assert len(backups) == 2
# Check that backups are sorted by last modified (newest first)
assert backups[0]["last_modified"] > backups[1]["last_modified"]
assert backups[0]["size"] == 2048
assert backups[1]["size"] == 1024
assert backups[1]["size"] == 1024