pathlib, Background Tasks, Logging Updates (#38)

* updated CacheManager to use pathlib

* updated some ops to be background tasks

* fixed ruff lint errors

* finally fixed logging. logs writing to app.log again

* added test for delete file

logging.conf

@@ -2,14 +2,14 @@
 keys=root, hypercorn.error, hypercorn.access
 
 [handlers]
-keys=console, error_file, access_file
+keys=console, error_file, access_file, logfile
 
 [formatters]
-keys=generic, access
+keys=generic, access, logfile
 
 [logger_root]
 level=DEBUG
-handlers=console
+handlers=console, logfile
 
 [logger_hypercorn.error]
 level=INFO
@@ -38,11 +38,20 @@ class=logging.FileHandler
 formatter=access
 args=('hypercorn.access.log',)
 
+[handler_logfile]
+class=handlers.RotatingFileHandler
+level=INFO
+args=('app.log', 'a', 100000, 10)
+formatter=logfile
+
 [formatter_generic]
-format=%(asctime)s [%(process)d] [%(levelname)s] %(message)s
+format=%(asctime)s [%(levelname)s] %(name)s: %(message)s
 datefmt=%Y-%m-%d %H:%M:%S
 class=logging.Formatter
 
 [formatter_access]
 format=%(message)s
 class=logging.Formatter
+
+[formatter_logfile]
+format=%(asctime)s [%(levelname)s] %(name)s: %(message)s

pyproject.toml

@@ -34,7 +34,7 @@ indent-width = 4
 target-version = "py311"
 
 [tool.ruff.lint]
-select = ["E", "F", "W", "C90", "I", "N", "UP", "ASYNC", "S", "B", "ERA", "PLE", "PLW", "PERF", "RUF", "SIM", "PT", "T20"]
+select = ["E", "F", "W", "C90", "I", "N", "UP", "ASYNC", "S", "B", "ERA", "PLE", "PLW", "PERF", "RUF", "SIM", "PT", "T20", "PTH"]
 ignore = ["E501", "S101"]
 
 # Allow fix for all enabled rules (when `--fix`) is provided.

src/smolvault/cache/cache_manager.py

@@ -1,15 +1,20 @@
-import os
+import pathlib
 
 
 class CacheManager:
     def __init__(self, cache_dir: str) -> None:
-        self.cache_dir = cache_dir
+        self.cache_dir = pathlib.Path(cache_dir)
 
     def file_exists(self, filename: str) -> bool:
-        return os.path.exists(os.path.join(self.cache_dir, filename))
+        file_path = self.cache_dir / filename
+        return file_path.exists()
 
     def save_file(self, filename: str, data: bytes) -> str:
-        file_path = os.path.join(self.cache_dir, filename)
-        with open(file_path, "wb") as f:
+        file_path = self.cache_dir / filename
+        with file_path.open("wb") as f:
             f.write(data)
-        return file_path
+        return file_path.as_posix()
+
+    def delete_file(self, local_path: str) -> None:
+        file_path = pathlib.Path(local_path)
+        file_path.unlink(missing_ok=True)

src/smolvault/clients/aws.py

@@ -11,23 +11,22 @@
 
 class S3Client:
     def __init__(self, bucket_name: str) -> None:
-        logger.debug(f"Creating S3 client for bucket {bucket_name}")
+        logger.info(f"Creating S3 client for bucket {bucket_name}")
         self.settings = get_settings()
         self.bucket_name = bucket_name
         self.session = boto3.Session()
         self.client = self.session.client("s3")
         self.bucket = self.session.resource("s3").Bucket(bucket_name)
 
     def upload(self, data: FileUploadDTO) -> str:
-        logger.debug(f"Uploading file {data.name} to S3")
         key = data.name
         self.bucket.put_object(Key=key, Body=data.content)
+        logger.info("File %s uploaded successfully", key)
         return key
 
     def download(self, key: str) -> Any:
-        logger.debug(f"Downloading file {key} from S3")
         response = self.client.get_object(Bucket=self.bucket_name, Key=key)
-        logger.info("File downloaded successfully")
+        logger.info("File %s downloaded successfully", key)
         return response["Body"].read()
 
     def delete(self, key: str) -> None:

src/smolvault/main.py

@@ -1,12 +1,12 @@
 import json
 import logging
-import os
+import pathlib
 import sys
 import urllib.parse
 from logging.handlers import RotatingFileHandler
 from typing import Annotated
 
-from fastapi import Depends, FastAPI, File, Form, UploadFile
+from fastapi import BackgroundTasks, Depends, FastAPI, File, Form, UploadFile
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.responses import FileResponse, Response
 
@@ -50,8 +50,10 @@ async def upload_file(
 ) -> Response:
     contents = await file.read()
     if file.filename is None:
+        logger.error("Filename not received in request")
         raise ValueError("Filename is required")
     file_upload = FileUploadDTO(name=file.filename, size=len(contents), content=contents, tags=tags)
+    logger.info("Uploading file to S3 with name %s", file_upload.name)
     object_key = s3_client.upload(data=file_upload)
     db_client.add_metadata(file_upload, object_key)
     return Response(
@@ -62,16 +64,21 @@ async def upload_file(
 
 
 @app.get("/file/{name}")
-async def get_file(db_client: Annotated[DatabaseClient, Depends(DatabaseClient)], name: str) -> Response:
+async def get_file(
+    db_client: Annotated[DatabaseClient, Depends(DatabaseClient)], name: str, background_tasks: BackgroundTasks
+) -> Response:
     record = db_client.get_metadata(urllib.parse.unquote(name))
     if record is None:
+        logger.info("File %s not found in database", name)
         return Response(content=json.dumps({"error": "File not found"}), status_code=404, media_type="application/json")
-    if record.local_path is None or cache.file_exists(record.local_path) is False:
+    if record.local_path is None or cache.file_exists(record.file_name) is False:
+        logger.info("File %s not found in cache, downloading from S3", record.file_name)
         content = s3_client.download(record.object_key)
         record.local_path = cache.save_file(record.file_name, content)
-        record.cache_timestamp = int(os.path.getmtime(record.local_path))
-        db_client.update_metadata(record)
-
+        record.cache_timestamp = int(pathlib.Path(record.local_path).stat().st_mtime)
+        logger.info("Saved file %s at time %d", record.local_path, record.cache_timestamp)
+        background_tasks.add_task(db_client.update_metadata, record)
+    logger.info("Serving file %s from cache", record.file_name)
     return FileResponse(path=record.local_path, filename=record.file_name)
 
 
@@ -88,13 +95,15 @@ async def get_file_metadata(
 @app.get("/files")
 async def get_files(db_client: Annotated[DatabaseClient, Depends(DatabaseClient)]) -> list[FileMetadata]:
     raw_metadata = db_client.get_all_metadata()
+    logger.info("Retrieved %d records from database", len(raw_metadata))
     results = [FileMetadata.model_validate(metadata.model_dump()) for metadata in raw_metadata]
     return results
 
 
 @app.get("/files/search")
 async def search_files(db_client: Annotated[DatabaseClient, Depends(DatabaseClient)], tag: str) -> list[FileMetadata]:
     raw_metadata = db_client.select_metadata_by_tag(tag)
+    logger.info("Retrieved %d records from database with tag %s", len(raw_metadata), tag)
     results = [FileMetadata.model_validate(metadata.model_dump()) for metadata in raw_metadata]
     return results
 
@@ -118,12 +127,16 @@ async def update_file_tags(
 
 
 @app.delete("/file/{name}")
-async def delete_file(db_client: Annotated[DatabaseClient, Depends(DatabaseClient)], name: str) -> Response:
+async def delete_file(
+    db_client: Annotated[DatabaseClient, Depends(DatabaseClient)], name: str, background_tasks: BackgroundTasks
+) -> Response:
     record: FileMetadataRecord | None = db_client.get_metadata(name)
     if record is None:
         return Response(content=json.dumps({"error": "File not found"}), status_code=404, media_type="application/json")
     s3_client.delete(record.object_key)
     db_client.delete_metadata(record)
+    if record.local_path:
+        background_tasks.add_task(cache.delete_file, record.local_path)
     return Response(
         content=json.dumps({"message": "File deleted successfully", "record": record.model_dump()}),
         status_code=200,

tests/conftest.py

@@ -1,4 +1,5 @@
 import os
+import pathlib
 from collections.abc import Generator
 from datetime import datetime
 from typing import Any
@@ -55,13 +56,13 @@ def _test_bucket(aws: S3Client) -> None:
 @pytest.fixture()
 def _bucket_w_camera_img(_test_bucket: None) -> None:
     client = boto3.client("s3")
-    with open("tests/mock_data/camera.png", "rb") as f:
+    with pathlib.Path("tests/mock_data/camera.png").open("rb") as f:
         client.put_object(Bucket="test-bucket", Key="camera.png", Body=f.read())
 
 
 @pytest.fixture(scope="session")
 def camera_img() -> bytes:
-    with open("tests/mock_data/camera.png", "rb") as f:
+    with pathlib.Path("tests/mock_data/camera.png").open("rb") as f:
         return f.read()
 
 

tests/test_delete_file.py

@@ -0,0 +1,27 @@
+from typing import Any
+from uuid import uuid4
+
+import pytest
+from httpx import AsyncClient
+from smolvault.models import FileUploadDTO
+
+
+@pytest.mark.asyncio()
+@pytest.mark.usefixtures("_test_bucket")
+async def test_delete_file(client: AsyncClient, camera_img: bytes) -> None:
+    # first upload the file
+    filename = f"{uuid4().hex[:6]}-camera.png"
+    expected_obj = FileUploadDTO(name=filename, size=len(camera_img), content=camera_img, tags="camera,photo")
+    expected = expected_obj.model_dump(exclude={"content", "upload_timestamp", "tags"})
+    response = await client.post(
+        "/file/upload", files={"file": (filename, camera_img, "image/png")}, data={"tags": "camera,photo"}
+    )
+    actual: dict[str, Any] = response.json()
+    actual.pop("upload_timestamp")
+    assert response.status_code == 201
+    assert actual == expected
+
+    # now delete the file
+    response = await client.delete(f"/file/{filename}")
+    assert response.status_code == 200
+    assert response.json() == {"message": "File deleted successfully", "record": expected}  # TODO: Fix this assertion