Optimize upload_visit_images by offloading I/O while preserving thread safety and security

backend/routers/field_officer.py

@@ -4,6 +4,7 @@
 Issue #288: Field Officer Check-In System With Location Verification
 """
 
+from starlette.concurrency import run_in_threadpool
 from fastapi import APIRouter, Depends, HTTPException, UploadFile, File, Form, Response
 from sqlalchemy.orm import Session
 from sqlalchemy import func, case
@@ -34,6 +35,8 @@
 )
 from backend.cache import visit_last_hash_cache, visit_stats_cache
 from backend.schemas import BlockchainVerificationResponse
+from backend.utils import process_uploaded_image
+
 
 logger = logging.getLogger(__name__)
 
@@ -302,6 +305,17 @@ async def upload_visit_images(
 
         image_paths = []
 
+        def save_images(images_data):
+            saved_paths = []
+            for safe_filename, img_bytes in images_data:
+                file_path = os.path.join(VISIT_IMAGES_DIR, safe_filename)
+                with open(file_path, 'wb') as f:
+                    f.write(img_bytes)
+                relative_path = os.path.join("data", "visit_images", safe_filename)
+                saved_paths.append(relative_path)
+            return saved_paths
+
+        images_to_save = []
         for idx, image in enumerate(images):
             # Validate content_type is present
             if not image.content_type:
@@ -310,45 +324,61 @@ async def upload_visit_images(
             # Validate file type
             if not image.content_type.startswith('image/'):
                 raise HTTPException(status_code=400, detail=f"File must be an image, got {image.content_type}")
-            
+
             # Validate filename is present
             if not image.filename:
                 raise HTTPException(status_code=400, detail="File must have a filename")
 
-            # Validate extension
-            extension = image.filename.split('.')[-1].lower() if '.' in image.filename else ''
+            # Validate extension using existing allowlist to prevent unrestricted file upload
+            if '.' not in image.filename:
+                raise HTTPException(status_code=400, detail="File must have an extension")
+
+            extension = image.filename.split('.')[-1].lower()
             if extension not in ALLOWED_IMAGE_EXTENSIONS:
                 raise HTTPException(
                     status_code=400,
                     detail=f"File extension '{extension}' not allowed. Allowed: {', '.join(ALLOWED_IMAGE_EXTENSIONS)}"
                 )
 
-            # Read and validate file size
+            # Read and validate file size directly here since utils max size is 20MB, but route requires 10MB
             content = await image.read()
             if len(content) > MAX_UPLOAD_SIZE:
                 raise HTTPException(
                     status_code=400,
                     detail=f"File {image.filename} exceeds maximum size of {MAX_UPLOAD_SIZE / 1024 / 1024:.1f} MB"
                 )
 
+            # Reset file pointer for the utility processing
+            await image.seek(0)
+
+            # Offload processing to threadpool using central utility
+            try:
+                pil_img, image_bytes = await process_uploaded_image(image)
+                # Ensure the saved extension matches the actual image format if possible, otherwise fallback to safe validated extension
+                actual_ext = pil_img.format.lower() if pil_img and pil_img.format else extension
+                # Map some formats to standard extensions
+                if actual_ext == 'jpeg': actual_ext = 'jpg'
+                if actual_ext not in ALLOWED_IMAGE_EXTENSIONS:
+                    actual_ext = extension
+            except HTTPException:
+                raise
+            except Exception as e:
+                logger.error(f"Image processing failed: {e}")
+                raise HTTPException(status_code=400, detail="Invalid image file.")
+
             # Generate secure filename
             timestamp = datetime.now(timezone.utc).strftime('%Y%m%d_%H%M%S')
-            safe_filename = f"visit_{visit_id}_{timestamp}_{idx}.{extension}"
-            file_path = os.path.join(VISIT_IMAGES_DIR, safe_filename)
-
-            # Save file
-            with open(file_path, 'wb') as f:
-                f.write(content)
+            safe_filename = f"visit_{visit_id}_{timestamp}_{idx}.{actual_ext}"
 
-            # Store relative path
-            relative_path = os.path.join("data", "visit_images", safe_filename)
-            image_paths.append(relative_path)
+            images_to_save.append((safe_filename, image_bytes))
+
+        # Offload only file writing to threadpool
+        image_paths = await run_in_threadpool(save_images, images_to_save)
 
-        # Update visit with image paths
+        # Keep SQLAlchemy operations in main async context
         existing_images.extend(image_paths)
         visit.visit_images = existing_images
         visit.updated_at = datetime.now(timezone.utc)
-
         db.commit()
 
         logger.info(f"Uploaded {len(images)} images for visit {visit_id}")