Skip to content

Commit

Permalink
Optimise images when they are uploaded and save in webp format (#177)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
lkeegan authored Nov 20, 2024
1 parent dca2681 commit d7717bd
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 53 deletions.
2 changes: 1 addition & 1 deletion frontend/src/lib/admin.svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export async function saveChanges() {
<Fileupload
bind:files
onchange={updateImageToUpload}
accept=".jpg, .jpeg"
accept=".jpg, .jpeg, .png"
id="img_upload"
class="mb-2 flex-grow-0"
/>
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/components/Admin/EditMilestoneModal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ async function deleteMilestoneImageAndUpdate() {
<div class="flex flex-row">
{#each milestone.images as milestoneImage (milestoneImage.id)}
<EditImage
filename={`m/${milestoneImage.id}.jpg`}
filename={`m/${milestoneImage.id}.webp`}
ondelete={() => {
currentMilestoneImageId = milestoneImage.id;
showDeleteMilestoneImageModal = true;
Expand All @@ -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"
/>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/components/Admin/MilestoneGroups.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ async function doDeleteMilestone() {
<TableBodyCell>
{#if milestone?.images?.length}
<img
src={`${import.meta.env.VITE_MONDEY_API_URL}/static/m/${milestone.images[0].id}.jpg`}
src={`${import.meta.env.VITE_MONDEY_API_URL}/static/m/${milestone.images[0].id}.webp`}
width="64"
height="64"
alt={milestoneTitle}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/components/Milestone.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ const breadcrumbdata = $derived([
{#each currentMilestone.images as image, imageIndex}
<img
class={`absolute top-0 left-0 w-full h-full object-cover transition duration-1000 ease-in-out ${imageIndex === currentImageIndex ? 'opacity-100' : 'opacity-0'}`}
src={`${import.meta.env.VITE_MONDEY_API_URL}/static/m/${image.id}.jpg`}
src={`${import.meta.env.VITE_MONDEY_API_URL}/static/m/${image.id}.webp`}
alt=""
/>
{/each}
Expand Down
2 changes: 2 additions & 0 deletions mondey_backend/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ dependencies = [
"python-multipart",
"pydantic-settings",
"click",
"pillow",
"webp",
]
dynamic = ["version"]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}")
Expand Down
6 changes: 3 additions & 3 deletions mondey_backend/src/mondey_backend/routers/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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}")
Expand All @@ -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}")
Expand Down
27 changes: 19 additions & 8 deletions mondey_backend/src/mondey_backend/routers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
33 changes: 25 additions & 8 deletions mondey_backend/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand All @@ -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


Expand Down Expand Up @@ -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 {
Expand Down
50 changes: 34 additions & 16 deletions mondey_backend/tests/routers/admin_routers/test_admin_milestones.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import pytest
from fastapi.testclient import TestClient
from PIL import Image


def test_get_milestone_groups(
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -172,33 +190,33 @@ 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
assert len(admin_client.get("/milestones/4").json()["images"]) == 1
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
Expand Down
21 changes: 12 additions & 9 deletions mondey_backend/tests/routers/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d7717bd

Please sign in to comment.