From 13a912d28a2bee0669730248107cf90bade78cab Mon Sep 17 00:00:00 2001 From: Chethan Regala Date: Mon, 23 Feb 2026 19:27:04 +0530 Subject: [PATCH 1/3] setup --- backend/app/database/images.py | 12 ++++++++++++ frontend/package-lock.json | 12 ++++++++---- frontend/package.json | 1 + frontend/src-tauri/src/main.rs | 1 + 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/backend/app/database/images.py b/backend/app/database/images.py index dadbc2020..2cd6e3917 100644 --- a/backend/app/database/images.py +++ b/backend/app/database/images.py @@ -78,6 +78,18 @@ def db_create_images_table() -> None: """ ) + # Ensure Memories feature columns exist on older databases + cursor.execute("PRAGMA table_info(images)") + existing_columns = {row[1] for row in cursor.fetchall()} + + for column_name, column_type in [ + ("latitude", "REAL"), + ("longitude", "REAL"), + ("captured_at", "DATETIME"), + ]: + if column_name not in existing_columns: + cursor.execute(f"ALTER TABLE images ADD COLUMN {column_name} {column_type}") + # Create indexes for Memories feature queries cursor.execute("CREATE INDEX IF NOT EXISTS ix_images_latitude ON images(latitude)") cursor.execute( diff --git a/frontend/package-lock.json b/frontend/package-lock.json index ab218ecaf..1948df7b5 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -73,6 +73,7 @@ "@vitejs/plugin-react": "^4.2.1", "autoprefixer": "^10.4.20", "babel-jest": "^29.7.0", + "baseline-browser-mapping": "^2.10.0", "eslint": "^8.57.1", "eslint-config-prettier": "^9.1.0", "eslint-config-react-app": "^7.0.1", @@ -6588,13 +6589,16 @@ "license": "MIT" }, "node_modules/baseline-browser-mapping": { - "version": "2.8.20", - "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.8.20.tgz", - "integrity": "sha512-JMWsdF+O8Orq3EMukbUN1QfbLK9mX2CkUmQBcW2T0s8OmdAUL5LLM/6wFwSrqXzlXB13yhyK9gTKS1rIizOduQ==", + "version": "2.10.0", + "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.10.0.tgz", + "integrity": "sha512-lIyg0szRfYbiy67j9KN8IyeD7q7hcmqnJ1ddWmNt19ItGpNN64mnllmxUNFIOdOm6by97jlL6wfpTTJrmnjWAA==", "dev": true, "license": "Apache-2.0", "bin": { - "baseline-browser-mapping": "dist/cli.js" + "baseline-browser-mapping": "dist/cli.cjs" + }, + "engines": { + "node": ">=6.0.0" } }, "node_modules/brace-expansion": { diff --git a/frontend/package.json b/frontend/package.json index 0a53f1b8d..daaf1f5bb 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -88,6 +88,7 @@ "@vitejs/plugin-react": "^4.2.1", "autoprefixer": "^10.4.20", "babel-jest": "^29.7.0", + "baseline-browser-mapping": "^2.10.0", "eslint": "^8.57.1", "eslint-config-prettier": "^9.1.0", "eslint-config-react-app": "^7.0.1", diff --git a/frontend/src-tauri/src/main.rs b/frontend/src-tauri/src/main.rs index 5b7aa6acc..2f643d2b6 100644 --- a/frontend/src-tauri/src/main.rs +++ b/frontend/src-tauri/src/main.rs @@ -6,6 +6,7 @@ mod services; use sysinfo::System; use tauri::path::BaseDirectory; use tauri::{Manager, Window, WindowEvent}; +#[cfg(feature = "ci")] use tauri_plugin_shell::ShellExt; const ENDPOINTS: [(&str, &str, &str); 2] = [ From 82644befbfce7ac1bd16261b6a7c4a43f8652f06 Mon Sep 17 00:00:00 2001 From: Chethan Regala Date: Mon, 23 Feb 2026 19:38:10 +0530 Subject: [PATCH 2/3] feat: allow renaming images from media info panel Co-authored-by: Cursor --- backend/app/database/images.py | 24 +++ backend/app/routes/images.py | 101 ++++++++++++- backend/app/schemas/images.py | 14 ++ backend/tests/test_images.py | 139 +++++++++++++++++ docs/backend/backend_python/openapi.json | 110 ++++++++++++++ frontend/src/api/api-functions/images.ts | 11 ++ frontend/src/api/apiEndpoints.ts | 1 + .../src/components/Media/MediaInfoPanel.tsx | 140 ++++++++++++++++-- frontend/src/hooks/useKeyboardNavigation.ts | 10 ++ 9 files changed, 537 insertions(+), 13 deletions(-) create mode 100644 backend/tests/test_images.py diff --git a/backend/app/database/images.py b/backend/app/database/images.py index 2cd6e3917..345002b0a 100644 --- a/backend/app/database/images.py +++ b/backend/app/database/images.py @@ -261,6 +261,30 @@ def db_get_all_images(tagged: Union[bool, None] = None) -> List[dict]: conn.close() +def db_get_image_path_by_id(image_id: ImageId) -> Optional[ImagePath]: + """ + Get the filesystem path for a single image by its ID. + + Args: + image_id: ID of the image to look up + + Returns: + The image path as a string, or None if the image is not found or an error occurs. + """ + conn = _connect() + cursor = conn.cursor() + + try: + cursor.execute("SELECT path FROM images WHERE id = ?", (image_id,)) + row = cursor.fetchone() + return row[0] if row else None + except Exception as e: + logger.error(f"Error getting image path for id {image_id}: {e}") + return None + finally: + conn.close() + + def db_get_untagged_images() -> List[UntaggedImageRecord]: """ Find all images that need AI tagging. diff --git a/backend/app/routes/images.py b/backend/app/routes/images.py index 2e40cd825..6f7b26e98 100644 --- a/backend/app/routes/images.py +++ b/backend/app/routes/images.py @@ -1,7 +1,8 @@ +import os from fastapi import APIRouter, HTTPException, Query, status from typing import List, Optional -from app.database.images import db_get_all_images -from app.schemas.images import ErrorResponse +from app.database.images import db_get_all_images, db_get_image_path_by_id +from app.schemas.images import ErrorResponse, RenameImageRequest, RenameImageResponse from app.utils.images import image_util_parse_metadata from pydantic import BaseModel from app.database.images import db_toggle_image_favourite_status @@ -128,3 +129,99 @@ class ImageInfoResponse(BaseModel): isTagged: bool isFavourite: bool tags: Optional[List[str]] = None + + +@router.put( + "/rename-image", + response_model=RenameImageResponse, + responses={code: {"model": ErrorResponse} for code in [400, 404, 500]}, +) +def rename_image(request: RenameImageRequest): + """ + Rename an image file on disk based on its image ID. + + The database record is updated separately by the sync microservice. + """ + try: + image_id = request.image_id.strip() + new_name = request.rename.strip() + + if not image_id: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="Validation Error", + message="Image ID cannot be empty", + ).model_dump(), + ) + + if not new_name: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="Validation Error", + message="New image name cannot be empty", + ).model_dump(), + ) + + # Disallow filesystem separators and common invalid characters (especially on Windows), + # plus a few extra characters that are prone to shell/filesystem issues. + invalid_chars = set('<>:"/\\\\|?*!^') + if any(ch in invalid_chars for ch in new_name): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="Validation Error", + message="New image name contains invalid characters.", + ).model_dump(), + ) + + image_path = db_get_image_path_by_id(image_id) + if not image_path: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=ErrorResponse( + success=False, + error="Image Not Found", + message=f"Image with ID '{image_id}' does not exist.", + ).model_dump(), + ) + + folder_path = os.path.dirname(image_path) + extension = os.path.splitext(image_path)[1] + new_file_path = os.path.join(folder_path, new_name + extension) + + # Avoid overwriting an existing file with the same name. + if os.path.exists(new_file_path): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="File Exists", + message="A file with the new name already exists.", + ).model_dump(), + ) + + os.rename(image_path, new_file_path) + + return RenameImageResponse( + success=True, + message=f"Successfully renamed image to '{new_name}{extension}'", + ) + + except HTTPException as e: + # Re-raise HTTPExceptions to preserve their status codes and details. + raise e + except Exception as e: + logger.error(f"Error renaming image: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=ErrorResponse( + success=False, + error="Internal server error", + message=f"Unable to rename image: {str(e)}", + ).model_dump(), + ) diff --git a/backend/app/schemas/images.py b/backend/app/schemas/images.py index 47f939c4f..2fb0dacdd 100644 --- a/backend/app/schemas/images.py +++ b/backend/app/schemas/images.py @@ -132,3 +132,17 @@ class DeleteThumbnailsResponse(BaseModel): class GetThumbnailPathResponse(BaseModel): success: bool thumbnailPath: str + + +class RenameImageRequest(BaseModel): + """Request model for renaming an image.""" + + image_id: str + rename: str + + +class RenameImageResponse(BaseModel): + """Response model for image rename operations.""" + + success: bool + message: str diff --git a/backend/tests/test_images.py b/backend/tests/test_images.py new file mode 100644 index 000000000..a7bb4f1ef --- /dev/null +++ b/backend/tests/test_images.py @@ -0,0 +1,139 @@ +import os +from unittest.mock import patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from app.routes.images import router as images_router + +app = FastAPI() +app.include_router(images_router, prefix="/images") +client = TestClient(app) + + +@pytest.fixture +def sample_image_id(): + return "image_123" + + +@pytest.fixture +def sample_image_path(tmp_path): + image_file = tmp_path / "old_name.jpg" + image_file.write_bytes(b"dummy image data") + return str(image_file) + + +class TestRenameImageAPI: + """Tests for the /images/rename-image endpoint.""" + + @patch("app.routes.images.db_get_image_path_by_id") + @patch("app.routes.images.os.rename") + def test_rename_image_success( + self, mock_rename, mock_get_path, sample_image_id, sample_image_path + ): + mock_get_path.return_value = sample_image_path + + response = client.put( + "/images/rename-image", + json={"image_id": sample_image_id, "rename": "new_name"}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert "Successfully renamed image" in data["message"] + + folder = os.path.dirname(sample_image_path) + ext = os.path.splitext(sample_image_path)[1] + expected_new_path = os.path.join(folder, "new_name" + ext) + + mock_get_path.assert_called_once_with(sample_image_id) + mock_rename.assert_called_once_with(sample_image_path, expected_new_path) + + def test_rename_image_empty_id(self): + response = client.put( + "/images/rename-image", + json={"image_id": " ", "rename": "new_name"}, + ) + + assert response.status_code == 400 + data = response.json() + assert data["detail"]["success"] is False + assert data["detail"]["error"] == "Validation Error" + assert "Image ID cannot be empty" in data["detail"]["message"] + + def test_rename_image_empty_name(self, sample_image_id): + response = client.put( + "/images/rename-image", + json={"image_id": sample_image_id, "rename": " "}, + ) + + assert response.status_code == 400 + data = response.json() + assert data["detail"]["success"] is False + assert data["detail"]["error"] == "Validation Error" + assert "New image name cannot be empty" in data["detail"]["message"] + + @pytest.mark.parametrize("invalid_char", ['*', '^', '!', '/', '\\\\', '?', '|']) + def test_rename_image_invalid_characters(self, sample_image_id, invalid_char): + response = client.put( + "/images/rename-image", + json={"image_id": sample_image_id, "rename": f"bad{invalid_char}name"}, + ) + + assert response.status_code == 400 + data = response.json() + assert data["detail"]["success"] is False + assert data["detail"]["error"] == "Validation Error" + + @patch("app.routes.images.db_get_image_path_by_id") + def test_rename_image_not_found(self, mock_get_path, sample_image_id): + mock_get_path.return_value = None + + response = client.put( + "/images/rename-image", + json={"image_id": sample_image_id, "rename": "new_name"}, + ) + + assert response.status_code == 404 + data = response.json() + assert data["detail"]["success"] is False + assert data["detail"]["error"] == "Image Not Found" + + @patch("app.routes.images.db_get_image_path_by_id") + @patch("app.routes.images.os.path.exists") + def test_rename_image_target_exists( + self, mock_exists, mock_get_path, sample_image_id, sample_image_path + ): + mock_get_path.return_value = sample_image_path + mock_exists.return_value = True + + response = client.put( + "/images/rename-image", + json={"image_id": sample_image_id, "rename": "new_name"}, + ) + + assert response.status_code == 400 + data = response.json() + assert data["detail"]["success"] is False + assert data["detail"]["error"] == "File Exists" + + @patch("app.routes.images.db_get_image_path_by_id") + @patch("app.routes.images.os.rename") + def test_rename_image_unexpected_error( + self, mock_rename, mock_get_path, sample_image_id, sample_image_path + ): + mock_get_path.return_value = sample_image_path + mock_rename.side_effect = OSError("disk error") + + response = client.put( + "/images/rename-image", + json={"image_id": sample_image_id, "rename": "new_name"}, + ) + + assert response.status_code == 500 + data = response.json() + assert data["detail"]["success"] is False + assert data["detail"]["error"] == "Internal server error" + diff --git a/docs/backend/backend_python/openapi.json b/docs/backend/backend_python/openapi.json index 389ab3020..3a3d732d3 100644 --- a/docs/backend/backend_python/openapi.json +++ b/docs/backend/backend_python/openapi.json @@ -926,6 +926,78 @@ } } }, + "/images/rename-image": { + "put": { + "tags": [ + "Images" + ], + "summary": "Rename Image", + "description": "Rename an image file on disk based on its image ID.\n\nThe database record is updated separately by the sync microservice.", + "operationId": "rename_image_images_rename_image_put", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RenameImageRequest" + } + } + }, + "required": true + }, + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RenameImageResponse" + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/app__schemas__images__ErrorResponse" + } + } + } + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/app__schemas__images__ErrorResponse" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/app__schemas__images__ErrorResponse" + } + } + } + }, + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + } + } + } + } + }, "/face-clusters/{cluster_id}": { "put": { "tags": [ @@ -2676,6 +2748,44 @@ ], "title": "RenameClusterResponse" }, + "RenameImageRequest": { + "properties": { + "image_id": { + "type": "string", + "title": "Image Id" + }, + "rename": { + "type": "string", + "title": "Rename" + } + }, + "type": "object", + "required": [ + "image_id", + "rename" + ], + "title": "RenameImageRequest", + "description": "Request model for renaming an image." + }, + "RenameImageResponse": { + "properties": { + "success": { + "type": "boolean", + "title": "Success" + }, + "message": { + "type": "string", + "title": "Message" + } + }, + "type": "object", + "required": [ + "success", + "message" + ], + "title": "RenameImageResponse", + "description": "Response model for image rename operations." + }, "ShutdownResponse": { "properties": { "status": { diff --git a/frontend/src/api/api-functions/images.ts b/frontend/src/api/api-functions/images.ts index dda3b21ce..4a22b4afd 100644 --- a/frontend/src/api/api-functions/images.ts +++ b/frontend/src/api/api-functions/images.ts @@ -12,3 +12,14 @@ export const fetchAllImages = async ( ); return response.data; }; + +export const renameImage = async ( + imageId: string, + newName: string, +): Promise<{ success: boolean; message: string }> => { + const response = await apiClient.put(imagesEndpoints.renameImage, { + image_id: imageId, + rename: newName, + }); + return response.data; +}; diff --git a/frontend/src/api/apiEndpoints.ts b/frontend/src/api/apiEndpoints.ts index f0e749197..6b98c33d8 100644 --- a/frontend/src/api/apiEndpoints.ts +++ b/frontend/src/api/apiEndpoints.ts @@ -1,6 +1,7 @@ export const imagesEndpoints = { getAllImages: '/images/', setFavourite: '/images/toggle-favourite', + renameImage: '/images/rename-image', }; export const faceClustersEndpoints = { diff --git a/frontend/src/components/Media/MediaInfoPanel.tsx b/frontend/src/components/Media/MediaInfoPanel.tsx index 1b0c09a0e..b4440fdba 100644 --- a/frontend/src/components/Media/MediaInfoPanel.tsx +++ b/frontend/src/components/Media/MediaInfoPanel.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; import { motion, AnimatePresence } from 'framer-motion'; import { open } from '@tauri-apps/plugin-shell'; import { @@ -11,6 +11,9 @@ import { SquareArrowOutUpRight, } from 'lucide-react'; import { Image } from '@/types/Media'; +import { Input } from '@/components/ui/input'; +import { Button } from '@/components/ui/button'; +import { renameImage as renameImageApi } from '@/api/api-functions'; interface MediaInfoPanelProps { show: boolean; @@ -27,6 +30,11 @@ export const MediaInfoPanel: React.FC = ({ currentIndex, totalImages, }) => { + const [isRenaming, setIsRenaming] = useState(false); + const [newName, setNewName] = useState(''); + const [isSaving, setIsSaving] = useState(false); + const [error, setError] = useState(null); + const getFormattedDate = () => { if (currentImage?.metadata?.date_created) { return new Date(currentImage.metadata.date_created).toLocaleDateString( @@ -41,11 +49,26 @@ export const MediaInfoPanel: React.FC = ({ return 'Date not available'; }; - const getImageName = () => { + const getFileName = useCallback(() => { if (!currentImage) return 'Image'; - // Handle both Unix (/) and Windows (\) path separators return currentImage.path?.split(/[/\\]/).pop() || 'Image'; - }; + }, [currentImage]); + + const getBaseName = useCallback(() => { + const fileName = getFileName(); + const lastDotIndex = fileName.lastIndexOf('.'); + if (lastDotIndex <= 0) return fileName; + return fileName.substring(0, lastDotIndex); + }, [getFileName]); + + const getExtension = useCallback(() => { + const fileName = getFileName(); + const lastDotIndex = fileName.lastIndexOf('.'); + if (lastDotIndex === -1 || lastDotIndex === fileName.length - 1) return ''; + return fileName.substring(lastDotIndex); + }, [getFileName]); + + const displayName = useMemo(() => getFileName(), [getFileName]); const handleLocationClick = async () => { if (currentImage?.metadata?.latitude && currentImage?.metadata?.longitude) { @@ -58,6 +81,39 @@ export const MediaInfoPanel: React.FC = ({ } }; + const startRenaming = () => { + setError(null); + setNewName(getBaseName()); + setIsRenaming(true); + }; + + const cancelRenaming = () => { + setIsRenaming(false); + setError(null); + }; + + const handleRenameSubmit = async () => { + if (!currentImage || !newName.trim()) { + setError('Name cannot be empty.'); + return; + } + + try { + setIsSaving(true); + setError(null); + await renameImageApi(currentImage.id, newName.trim()); + setIsRenaming(false); + } catch (e: any) { + const message = + e?.response?.data?.detail?.message || + e?.response?.data?.message || + 'Failed to rename image.'; + setError(message); + } finally { + setIsSaving(false); + } + }; + return ( {show && ( @@ -85,13 +141,75 @@ export const MediaInfoPanel: React.FC = ({
-

Name

-

- {getImageName()} -

+
+

Name

+ {!isRenaming && ( + + )} +
+ {isRenaming ? ( +
+
+
+ setNewName(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault(); + handleRenameSubmit(); + } else if (e.key === 'Escape') { + e.preventDefault(); + cancelRenaming(); + } + }} + className="h-8 text-sm" + placeholder="Enter new name" + /> + + {getExtension()} + +
+ {error && ( +

{error}

+ )} +
+
+ + +
+
+ ) : ( +

+ {displayName} +

+ )}
diff --git a/frontend/src/hooks/useKeyboardNavigation.ts b/frontend/src/hooks/useKeyboardNavigation.ts index c373a4875..a4fa4bc6a 100644 --- a/frontend/src/hooks/useKeyboardNavigation.ts +++ b/frontend/src/hooks/useKeyboardNavigation.ts @@ -13,6 +13,16 @@ interface KeyboardHandlers { export const useKeyboardNavigation = (handlers: KeyboardHandlers) => { const handleKeyDown = useCallback( (e: KeyboardEvent) => { + const target = e.target as HTMLElement | null; + if ( + target && + (target.tagName === 'INPUT' || + target.tagName === 'TEXTAREA' || + target.isContentEditable) + ) { + return; + } + switch (e.key) { case 'Escape': handlers.onClose(); From 778eda9e1a58e09b24cac5d87a34bc461e0488b6 Mon Sep 17 00:00:00 2001 From: Chethan Regala Date: Mon, 23 Feb 2026 20:30:21 +0530 Subject: [PATCH 3/3] fix: harden image rename validation and media info UI --- backend/app/routes/images.py | 98 ++++++++++++++++--- backend/tests/test_images.py | 32 ++++-- .../src/components/Media/MediaInfoPanel.tsx | 37 +++++-- 3 files changed, 139 insertions(+), 28 deletions(-) diff --git a/backend/app/routes/images.py b/backend/app/routes/images.py index 6f7b26e98..3ff484103 100644 --- a/backend/app/routes/images.py +++ b/backend/app/routes/images.py @@ -144,7 +144,8 @@ def rename_image(request: RenameImageRequest): """ try: image_id = request.image_id.strip() - new_name = request.rename.strip() + raw_name = request.rename + new_name = raw_name.strip() if not image_id: raise HTTPException( @@ -179,6 +180,52 @@ def rename_image(request: RenameImageRequest): ).model_dump(), ) + # Additional Windows-safe validations + # Reject names that are '.' or '..' or that consist only of dots + if all(ch == "." for ch in new_name): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="Validation Error", + message="New image name cannot be '.' , '..' or consist only of dots.", + ).model_dump(), + ) + + # Reject names that end with a dot or a space (Windows trims these) + if raw_name and (raw_name.endswith(".") or raw_name.endswith(" ")): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="Validation Error", + message="New image name cannot end with a dot or space.", + ).model_dump(), + ) + + # Reject names whose base stem matches Windows reserved device names + reserved_device_names = { + "CON", + "PRN", + "NUL", + "AUX", + *[f"COM{i}" for i in range(1, 10)], + *[f"LPT{i}" for i in range(1, 10)], + } + base_stem = new_name.split(".", 1)[0] + if base_stem and base_stem.upper() in reserved_device_names: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="Validation Error", + message=( + "New image name cannot use a reserved Windows device name " + "(CON, PRN, NUL, AUX, COM1–COM9, LPT1–LPT9)." + ), + ).model_dump(), + ) + image_path = db_get_image_path_by_id(image_id) if not image_path: raise HTTPException( @@ -194,18 +241,43 @@ def rename_image(request: RenameImageRequest): extension = os.path.splitext(image_path)[1] new_file_path = os.path.join(folder_path, new_name + extension) - # Avoid overwriting an existing file with the same name. - if os.path.exists(new_file_path): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ErrorResponse( - success=False, - error="File Exists", - message="A file with the new name already exists.", - ).model_dump(), - ) - - os.rename(image_path, new_file_path) + # Atomically reserve the target path to avoid TOCTOU between existence + # checks and rename operations. + placeholder_created = False + try: + try: + fd = os.open( + new_file_path, + os.O_CREAT | os.O_EXCL | os.O_WRONLY, + ) + os.close(fd) + placeholder_created = True + except FileExistsError: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=ErrorResponse( + success=False, + error="File Exists", + message="A file with the new name already exists.", + ).model_dump(), + ) + + # Perform the actual rename now that the name is reserved. + os.rename(image_path, new_file_path) + except HTTPException: + # Bubble up HTTP errors (e.g., file already exists). + raise + except Exception as e: + # Clean up the placeholder if the rename failed unexpectedly. + if placeholder_created: + try: + os.unlink(new_file_path) + except OSError as cleanup_err: + logger.error( + f"Failed to clean up placeholder file '{new_file_path}': {cleanup_err}" + ) + # Re-raise for the outer exception handler to translate. + raise e return RenameImageResponse( success=True, diff --git a/backend/tests/test_images.py b/backend/tests/test_images.py index a7bb4f1ef..11aef7440 100644 --- a/backend/tests/test_images.py +++ b/backend/tests/test_images.py @@ -29,10 +29,17 @@ class TestRenameImageAPI: @patch("app.routes.images.db_get_image_path_by_id") @patch("app.routes.images.os.rename") + @patch("app.routes.images.os.open") def test_rename_image_success( - self, mock_rename, mock_get_path, sample_image_id, sample_image_path + self, + mock_open, + mock_rename, + mock_get_path, + sample_image_id, + sample_image_path, ): mock_get_path.return_value = sample_image_path + mock_open.return_value = 3 # dummy file descriptor response = client.put( "/images/rename-image", @@ -49,6 +56,7 @@ def test_rename_image_success( expected_new_path = os.path.join(folder, "new_name" + ext) mock_get_path.assert_called_once_with(sample_image_id) + mock_open.assert_called_once() mock_rename.assert_called_once_with(sample_image_path, expected_new_path) def test_rename_image_empty_id(self): @@ -75,7 +83,9 @@ def test_rename_image_empty_name(self, sample_image_id): assert data["detail"]["error"] == "Validation Error" assert "New image name cannot be empty" in data["detail"]["message"] - @pytest.mark.parametrize("invalid_char", ['*', '^', '!', '/', '\\\\', '?', '|']) + @pytest.mark.parametrize( + "invalid_char", ['*', '^', '!', '/', '\\', '?', '|', '<', '>', ':', '"'] + ) def test_rename_image_invalid_characters(self, sample_image_id, invalid_char): response = client.put( "/images/rename-image", @@ -102,12 +112,12 @@ def test_rename_image_not_found(self, mock_get_path, sample_image_id): assert data["detail"]["error"] == "Image Not Found" @patch("app.routes.images.db_get_image_path_by_id") - @patch("app.routes.images.os.path.exists") + @patch("app.routes.images.os.open") def test_rename_image_target_exists( - self, mock_exists, mock_get_path, sample_image_id, sample_image_path + self, mock_open, mock_get_path, sample_image_id, sample_image_path ): mock_get_path.return_value = sample_image_path - mock_exists.return_value = True + mock_open.side_effect = FileExistsError() response = client.put( "/images/rename-image", @@ -120,11 +130,20 @@ def test_rename_image_target_exists( assert data["detail"]["error"] == "File Exists" @patch("app.routes.images.db_get_image_path_by_id") + @patch("app.routes.images.os.unlink") @patch("app.routes.images.os.rename") + @patch("app.routes.images.os.open") def test_rename_image_unexpected_error( - self, mock_rename, mock_get_path, sample_image_id, sample_image_path + self, + mock_open, + mock_rename, + mock_unlink, + mock_get_path, + sample_image_id, + sample_image_path, ): mock_get_path.return_value = sample_image_path + mock_open.return_value = 3 mock_rename.side_effect = OSError("disk error") response = client.put( @@ -136,4 +155,5 @@ def test_rename_image_unexpected_error( data = response.json() assert data["detail"]["success"] is False assert data["detail"]["error"] == "Internal server error" + mock_unlink.assert_called_once() diff --git a/frontend/src/components/Media/MediaInfoPanel.tsx b/frontend/src/components/Media/MediaInfoPanel.tsx index b4440fdba..f9c3823e3 100644 --- a/frontend/src/components/Media/MediaInfoPanel.tsx +++ b/frontend/src/components/Media/MediaInfoPanel.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { motion, AnimatePresence } from 'framer-motion'; import { open } from '@tauri-apps/plugin-shell'; import { @@ -34,6 +34,16 @@ export const MediaInfoPanel: React.FC = ({ const [newName, setNewName] = useState(''); const [isSaving, setIsSaving] = useState(false); const [error, setError] = useState(null); + const [renamedName, setRenamedName] = useState(null); + + useEffect(() => { + // Reset rename state when the current image changes + setIsRenaming(false); + setNewName(''); + setError(null); + setIsSaving(false); + setRenamedName(null); + }, [currentImage?.id]); const getFormattedDate = () => { if (currentImage?.metadata?.date_created) { @@ -55,20 +65,23 @@ export const MediaInfoPanel: React.FC = ({ }, [currentImage]); const getBaseName = useCallback(() => { - const fileName = getFileName(); + const fileName = renamedName || getFileName(); const lastDotIndex = fileName.lastIndexOf('.'); if (lastDotIndex <= 0) return fileName; return fileName.substring(0, lastDotIndex); }, [getFileName]); const getExtension = useCallback(() => { - const fileName = getFileName(); + const fileName = renamedName || getFileName(); const lastDotIndex = fileName.lastIndexOf('.'); if (lastDotIndex === -1 || lastDotIndex === fileName.length - 1) return ''; return fileName.substring(lastDotIndex); }, [getFileName]); - const displayName = useMemo(() => getFileName(), [getFileName]); + const displayName = useMemo( + () => renamedName || getFileName(), + [renamedName, getFileName], + ); const handleLocationClick = async () => { if (currentImage?.metadata?.latitude && currentImage?.metadata?.longitude) { @@ -101,13 +114,19 @@ export const MediaInfoPanel: React.FC = ({ try { setIsSaving(true); setError(null); - await renameImageApi(currentImage.id, newName.trim()); + const trimmed = newName.trim(); + await renameImageApi(currentImage.id, trimmed); + const fullName = `${trimmed}${getExtension()}`; + setRenamedName(fullName); setIsRenaming(false); - } catch (e: any) { + } catch (e: unknown) { + const maybeAxiosError = e as { + response?: { data?: { detail?: { message?: string }; message?: string } }; + }; const message = - e?.response?.data?.detail?.message || - e?.response?.data?.message || - 'Failed to rename image.'; + maybeAxiosError.response?.data?.detail?.message || + maybeAxiosError.response?.data?.message || + (e instanceof Error ? e.message : 'Failed to rename image.'); setError(message); } finally { setIsSaving(false);