From d7717bd7ce78e51fdbfc81e768f703c091be61d0 Mon Sep 17 00:00:00 2001 From: Liam Keegan Date: Wed, 20 Nov 2024 10:56:46 +0100 Subject: [PATCH] Optimise images when they are uploaded and save in webp format (#177) * Optimise images when they are uploaded and save in webp format - backend - open uploaded images using pillow - resize if width > 1024 - export to webp format using pywebp - add some png & jpeg files to the tests - frontend-admin - allow png uploads as well as jpg - frontend - update image urls - resolves #166 * set `media_type` to `image/webp` for get_child_image FileResponse - automatically inferred type caused `Failed to execute 'createObjectURL' on 'URL': Overload resolution failed.` runtime error in frontend --- frontend/src/lib/admin.svelte.ts | 2 +- .../Admin/EditMilestoneGroupModal.svelte | 2 +- .../Admin/EditMilestoneModal.svelte | 4 +- .../components/Admin/MilestoneGroups.svelte | 2 +- frontend/src/lib/components/Milestone.svelte | 2 +- mondey_backend/pyproject.toml | 2 + .../routers/admin_routers/milestones.py | 6 +-- .../src/mondey_backend/routers/users.py | 6 +-- .../src/mondey_backend/routers/utils.py | 27 +++++++--- mondey_backend/tests/conftest.py | 33 +++++++++--- .../admin_routers/test_admin_milestones.py | 50 +++++++++++++------ mondey_backend/tests/routers/test_users.py | 21 ++++---- 12 files changed, 104 insertions(+), 53 deletions(-) diff --git a/frontend/src/lib/admin.svelte.ts b/frontend/src/lib/admin.svelte.ts index 6ce98ca2..40e46bf3 100644 --- a/frontend/src/lib/admin.svelte.ts +++ b/frontend/src/lib/admin.svelte.ts @@ -21,7 +21,7 @@ export async function refreshMilestoneGroups() { } export function milestoneGroupImageUrl(id: number) { - return `${import.meta.env.VITE_MONDEY_API_URL}/static/mg/${id}.jpg`; + return `${import.meta.env.VITE_MONDEY_API_URL}/static/mg/${id}.webp`; } export async function refreshUserQuestions() { diff --git a/frontend/src/lib/components/Admin/EditMilestoneGroupModal.svelte b/frontend/src/lib/components/Admin/EditMilestoneGroupModal.svelte index 8fb09867..80c6b393 100644 --- a/frontend/src/lib/components/Admin/EditMilestoneGroupModal.svelte +++ b/frontend/src/lib/components/Admin/EditMilestoneGroupModal.svelte @@ -98,7 +98,7 @@ export async function saveChanges() { diff --git a/frontend/src/lib/components/Admin/EditMilestoneModal.svelte b/frontend/src/lib/components/Admin/EditMilestoneModal.svelte index 79bbfbde..fb4358df 100644 --- a/frontend/src/lib/components/Admin/EditMilestoneModal.svelte +++ b/frontend/src/lib/components/Admin/EditMilestoneModal.svelte @@ -110,7 +110,7 @@ async function deleteMilestoneImageAndUpdate() {
{#each milestone.images as milestoneImage (milestoneImage.id)} { currentMilestoneImageId = milestoneImage.id; showDeleteMilestoneImageModal = true; @@ -125,7 +125,7 @@ async function deleteMilestoneImageAndUpdate() { bind:files on:change={updateImagesToUpload} multiple - accept=".jpg, .jpeg" + accept=".jpg, .jpeg, .png" id="img_upload" class="mb-2 flex-grow-0" /> diff --git a/frontend/src/lib/components/Admin/MilestoneGroups.svelte b/frontend/src/lib/components/Admin/MilestoneGroups.svelte index 15682ff9..88f387bf 100644 --- a/frontend/src/lib/components/Admin/MilestoneGroups.svelte +++ b/frontend/src/lib/components/Admin/MilestoneGroups.svelte @@ -193,7 +193,7 @@ async function doDeleteMilestone() { {#if milestone?.images?.length} {milestoneTitle} {/each} diff --git a/mondey_backend/pyproject.toml b/mondey_backend/pyproject.toml index f1dd92a3..8173de4e 100644 --- a/mondey_backend/pyproject.toml +++ b/mondey_backend/pyproject.toml @@ -19,6 +19,8 @@ dependencies = [ "python-multipart", "pydantic-settings", "click", + "pillow", + "webp", ] dynamic = ["version"] diff --git a/mondey_backend/src/mondey_backend/routers/admin_routers/milestones.py b/mondey_backend/src/mondey_backend/routers/admin_routers/milestones.py index 226897fe..0385d94a 100644 --- a/mondey_backend/src/mondey_backend/routers/admin_routers/milestones.py +++ b/mondey_backend/src/mondey_backend/routers/admin_routers/milestones.py @@ -24,7 +24,7 @@ from ..utils import update_item_orders from ..utils import update_milestone_group_text from ..utils import update_milestone_text -from ..utils import write_file +from ..utils import write_image_file def create_router() -> APIRouter: @@ -80,7 +80,7 @@ async def upload_milestone_group_image( session: SessionDep, milestone_group_id: int, file: UploadFile ): get(session, MilestoneGroup, milestone_group_id) - write_file(file, milestone_group_image_path(milestone_group_id)) + write_image_file(file, milestone_group_image_path(milestone_group_id)) return {"ok": True} @router.post("/milestones/{milestone_group_id}", response_model=MilestoneAdmin) @@ -126,7 +126,7 @@ async def upload_milestone_image( milestone = get(session, Milestone, milestone_id) milestone_image = MilestoneImage(milestone_id=milestone.id) add(session, milestone_image) - write_file(file, milestone_image_path(milestone_image.id)) + write_image_file(file, milestone_image_path(milestone_image.id)) return milestone_image @router.delete("/milestone-images/{milestone_image_id}") diff --git a/mondey_backend/src/mondey_backend/routers/users.py b/mondey_backend/src/mondey_backend/routers/users.py index c3c685aa..b5ff7e40 100644 --- a/mondey_backend/src/mondey_backend/routers/users.py +++ b/mondey_backend/src/mondey_backend/routers/users.py @@ -28,7 +28,7 @@ from .utils import get from .utils import get_db_child from .utils import get_or_create_current_milestone_answer_session -from .utils import write_file +from .utils import write_image_file def create_router() -> APIRouter: @@ -92,7 +92,7 @@ async def get_child_image( child = get_db_child(session, current_active_user, child_id) image_path = child_image_path(child_id) if child.has_image and image_path.exists(): - return image_path + return FileResponse(image_path, media_type="image/webp") raise HTTPException(404) @router.put("/children-images/{child_id}") @@ -105,7 +105,7 @@ async def upload_child_image( child = get_db_child(session, current_active_user, child_id) child.has_image = True session.commit() - write_file(file, child_image_path(child_id)) + write_image_file(file, child_image_path(child_id)) return {"ok": True} @router.delete("/children-images/{child_id}") diff --git a/mondey_backend/src/mondey_backend/routers/utils.py b/mondey_backend/src/mondey_backend/routers/utils.py index 40c67765..68323a15 100644 --- a/mondey_backend/src/mondey_backend/routers/utils.py +++ b/mondey_backend/src/mondey_backend/routers/utils.py @@ -8,8 +8,10 @@ from typing import TypeVar import numpy as np +import webp from fastapi import HTTPException from fastapi import UploadFile +from PIL import Image from sqlmodel import SQLModel from sqlmodel import col from sqlmodel import select @@ -40,13 +42,20 @@ OrderedItem = Milestone | MilestoneGroup | UserQuestion | ChildQuestion -def write_file(file: UploadFile, filename: pathlib.Path | str): - logging.warning(f"Saving file {file.filename} to {filename}") +def write_image_file(file: UploadFile, filename: pathlib.Path | str): + max_image_width = 1024 + image_quality = 90 try: pathlib.Path(filename).parent.mkdir(exist_ok=True) - contents = file.file.read() - with open(filename, "wb") as f: - f.write(contents) + img = Image.open(file.file) + if img.width > max_image_width: + img = img.resize( + ( + max_image_width, + int(img.height * max_image_width / img.width), + ) + ) + webp.save_image(img, filename, quality=image_quality) except Exception as e: logging.exception(e) raise HTTPException(status_code=404, detail="Error saving uploaded file") from e @@ -224,15 +233,17 @@ def calculate_milestone_age_scores( def child_image_path(child_id: int | None) -> pathlib.Path: - return pathlib.Path(f"{app_settings.PRIVATE_FILES_PATH}/children/{child_id}.jpg") + return pathlib.Path(f"{app_settings.PRIVATE_FILES_PATH}/children/{child_id}.webp") def milestone_image_path(milestone_image_id: int | None) -> pathlib.Path: - return pathlib.Path(f"{app_settings.STATIC_FILES_PATH}/m/{milestone_image_id}.jpg") + return pathlib.Path(f"{app_settings.STATIC_FILES_PATH}/m/{milestone_image_id}.webp") def milestone_group_image_path(milestone_group_id: int) -> pathlib.Path: - return pathlib.Path(f"{app_settings.STATIC_FILES_PATH}/mg/{milestone_group_id}.jpg") + return pathlib.Path( + f"{app_settings.STATIC_FILES_PATH}/mg/{milestone_group_id}.webp" + ) def i18n_language_path(language_id: str) -> pathlib.Path: diff --git a/mondey_backend/tests/conftest.py b/mondey_backend/tests/conftest.py index 4e3c2ded..a8292f42 100644 --- a/mondey_backend/tests/conftest.py +++ b/mondey_backend/tests/conftest.py @@ -7,6 +7,7 @@ import pytest_asyncio from fastapi import FastAPI from fastapi.testclient import TestClient +from PIL import Image from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.ext.asyncio import async_sessionmaker from sqlalchemy.ext.asyncio import create_async_engine @@ -49,8 +50,8 @@ def static_dir(tmp_path_factory: pytest.TempPathFactory): milestone_images_dir.mkdir() # add some milestone image files for milestone_image_id in [1, 2, 3]: - with (milestone_images_dir / "f{milestone_image_id}.jpg").open("w") as f: - f.write(f"{milestone_image_id}") + img = Image.new("RGB", (201, 414)) + img.save(milestone_images_dir / f"{milestone_image_id}.webp") return static_dir @@ -60,9 +61,9 @@ def private_dir(tmp_path_factory: pytest.TempPathFactory): children_dir = private_dir / "children" children_dir.mkdir() # add some child image files - for filename in ["2.jpg", "3.jpg"]: - with (children_dir / filename).open("w") as f: - f.write(filename) + for child_id in [2, 3]: + img = Image.new("RGBA", (67, 38)) + img.save(children_dir / f"{child_id}.webp") return private_dir @@ -716,13 +717,29 @@ def admin_client( @pytest.fixture -def jpg_file(tmp_path: pathlib.Path): +def image_file_jpg_1600_1200(tmp_path: pathlib.Path): jpg_path = tmp_path / "test.jpg" - with jpg_path.open("w") as f: - f.write("test") + img = Image.new("RGB", (1600, 1200)) + img.save(jpg_path) return jpg_path +@pytest.fixture +def image_file_jpg_64_64(tmp_path: pathlib.Path): + jpg_path = tmp_path / "test.jpg" + img = Image.new("RGB", (64, 64)) + img.save(jpg_path) + return jpg_path + + +@pytest.fixture +def image_file_png_1100_1100(tmp_path: pathlib.Path): + png_path = tmp_path / "test.png" + img = Image.new("RGBA", (1100, 1100)) + img.save(png_path) + return png_path + + @pytest.fixture def milestone_group1(): return { diff --git a/mondey_backend/tests/routers/admin_routers/test_admin_milestones.py b/mondey_backend/tests/routers/admin_routers/test_admin_milestones.py index 4ac9e26f..02826bca 100644 --- a/mondey_backend/tests/routers/admin_routers/test_admin_milestones.py +++ b/mondey_backend/tests/routers/admin_routers/test_admin_milestones.py @@ -2,6 +2,7 @@ import pytest from fastapi.testclient import TestClient +from PIL import Image def test_get_milestone_groups( @@ -82,18 +83,33 @@ def test_delete_endpoints_invalid_admin_user( assert response.status_code == 401 +@pytest.mark.parametrize( + ["image_path_fixture", "image_tag"], + [ + ("image_file_jpg_1600_1200", "image/jpeg"), + ("image_file_jpg_64_64", "image/jpeg"), + ("image_file_png_1100_1100", "image/png"), + ], +) def test_put_milestone_group_image( - admin_client: TestClient, static_dir: pathlib.Path, jpg_file: pathlib.Path + admin_client: TestClient, + static_dir: pathlib.Path, + image_path_fixture: str, + image_tag: str, + request: pytest.FixtureRequest, ): - static_dir_jpg = static_dir / "mg" / "1.jpg" - assert not static_dir_jpg.is_file() - with open(jpg_file, "rb") as f: + image_path = request.getfixturevalue(image_path_fixture) + static_dir_image_file = static_dir / "mg" / "1.webp" + assert not static_dir_image_file.is_file() + original_width = Image.open(image_path).size[0] + with open(image_path, "rb") as f: response = admin_client.put( "/admin/milestone-group-images/1", - files={"file": ("filename", f, "image/jpeg")}, + files={"file": ("filename", f, image_tag)}, ) assert response.status_code == 200 - assert static_dir_jpg.is_file() + assert static_dir_image_file.is_file() + assert Image.open(static_dir_image_file).size[0] == min(original_width, 1024) def test_post_milestone(admin_client: TestClient): @@ -159,7 +175,9 @@ def test_delete_milestone(admin_client: TestClient): def test_post_milestone_image( - admin_client: TestClient, static_dir: pathlib.Path, jpg_file: pathlib.Path + admin_client: TestClient, + static_dir: pathlib.Path, + image_file_jpg_1600_1200: pathlib.Path, ): # 3 milestone images already exist assert len(admin_client.get("/milestones/1").json()["images"]) == 2 @@ -172,16 +190,16 @@ def test_post_milestone_image( # add an image to each milestone for milestone_id in [1, 2, 3, 4, 5]: milestone_image_id += 1 - filename = f"{milestone_image_id}.jpg" - static_dir_jpg = static_dir / "m" / filename - assert not static_dir_jpg.is_file() - with open(jpg_file, "rb") as f: + filename = f"{milestone_image_id}.webp" + static_dir_image_file = static_dir / "m" / filename + assert not static_dir_image_file.is_file() + with open(image_file_jpg_1600_1200, "rb") as f: response = admin_client.post( f"/admin/milestone-images/{milestone_id}", files={"file": ("filename", f, "image/jpeg")}, ) assert response.status_code == 200 - assert static_dir_jpg.is_file() + assert static_dir_image_file.is_file() assert len(admin_client.get("/milestones/1").json()["images"]) == 3 assert len(admin_client.get("/milestones/2").json()["images"]) == 2 assert len(admin_client.get("/milestones/3").json()["images"]) == 1 @@ -189,16 +207,16 @@ def test_post_milestone_image( assert len(admin_client.get("/milestones/5").json()["images"]) == 1 # remove added images for milestone_image_id in range(4, 9): - filename = f"{milestone_image_id}.jpg" - static_dir_jpg = static_dir / "m" / filename - assert static_dir_jpg.is_file() + filename = f"{milestone_image_id}.webp" + static_dir_image_file = static_dir / "m" / filename + assert static_dir_image_file.is_file() assert ( admin_client.delete( f"/admin/milestone-images/{milestone_image_id}" ).status_code == 200 ) - assert not static_dir_jpg.is_file() + assert not static_dir_image_file.is_file() assert len(admin_client.get("/milestones/1").json()["images"]) == 2 assert len(admin_client.get("/milestones/2").json()["images"]) == 1 assert len(admin_client.get("/milestones/3").json()["images"]) == 0 diff --git a/mondey_backend/tests/routers/test_users.py b/mondey_backend/tests/routers/test_users.py index 0289c0eb..e4c705bd 100644 --- a/mondey_backend/tests/routers/test_users.py +++ b/mondey_backend/tests/routers/test_users.py @@ -56,7 +56,6 @@ def test_get_children_invalid_user(public_client: TestClient): def test_get_child_image(user_client: TestClient): response = user_client.get("/users/children-images/2") assert response.status_code == 200 - assert response.content == b"2.jpg" def test_get_child_image_no_image(user_client: TestClient): @@ -108,35 +107,39 @@ def test_create_update_and_delete_child(user_client: TestClient): def test_upload_child_image( - user_client: TestClient, private_dir: pathlib.Path, jpg_file: pathlib.Path + user_client: TestClient, + private_dir: pathlib.Path, + image_file_jpg_1600_1200: pathlib.Path, ): - private_dir_jpg_file = private_dir / "children" / "1.jpg" + private_dir_image_file = private_dir / "children" / "1.webp" # child 1 does not have an image: - assert not private_dir_jpg_file.is_file() + assert not private_dir_image_file.is_file() assert user_client.get("/users/children/").json()[0]["has_image"] is False assert user_client.get("/users/children-images/1").status_code == 404 # add an image for the first child - with open(jpg_file, "rb") as f: + with open(image_file_jpg_1600_1200, "rb") as f: response = user_client.put( "/users/children-images/1", files={"file": ("filename", f, "image/jpeg")}, ) assert response.status_code == 200 - assert private_dir_jpg_file.is_file() + assert private_dir_image_file.is_file() assert user_client.get("/users/children/").json()[0]["has_image"] is True assert user_client.get("/users/children-images/1").status_code == 200 def test_delete_child_image( - user_client: TestClient, private_dir: pathlib.Path, jpg_file: pathlib.Path + user_client: TestClient, + private_dir: pathlib.Path, + image_file_png_1100_1100: pathlib.Path, ): children_dir = private_dir / "children" # add an image for the first child - with open(jpg_file, "rb") as f: + with open(image_file_png_1100_1100, "rb") as f: response = user_client.put( "/users/children-images/1", - files={"file": ("filename", f, "image/jpeg")}, + files={"file": ("filename", f, "image/png")}, ) # delete the image