diff --git a/Dockerfile b/Dockerfile index 432ea425..89e77d7f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -28,7 +28,7 @@ RUN mix release FROM base AS runner RUN apt-get update -y && \ - apt-get install -y libstdc++6 openssl libncurses6 libtinfo6 locales ca-certificates imagemagick && \ + apt-get install -y libstdc++6 openssl libncurses6 libtinfo6 locales ca-certificates imagemagick nginx && \ apt-get clean && rm -rf /var/lib/apt/lists/* # Set the locale @@ -41,10 +41,13 @@ ENV LC_ALL=en_US.UTF-8 WORKDIR /app COPY --from=builder /app/_build/prod/rel/vmemo /app +COPY config/nginx/prod.conf /etc/nginx/nginx.conf COPY rel/entrypoint.sh /usr/local/bin/entrypoint.sh RUN chmod +x /usr/local/bin/entrypoint.sh ENV HOME=/app +ENV VMEMO_ENABLE_NGINX=true +ENV PHX_PORT=4001 ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] CMD ["start"] diff --git a/assets/vendor/heroicons.js b/assets/vendor/heroicons.js index 296f80e4..fae6cb7a 100644 --- a/assets/vendor/heroicons.js +++ b/assets/vendor/heroicons.js @@ -2,8 +2,19 @@ const plugin = require("tailwindcss/plugin") const fs = require("fs") const path = require("path") +function resolveDepsRoot() { + let envDepsRoot = process.env.MIX_DEPS_PATH && path.resolve(process.env.MIX_DEPS_PATH) + let localDepsRoot = path.join(__dirname, "../../deps") + + if (envDepsRoot && fs.existsSync(path.join(envDepsRoot, "heroicons"))) { + return envDepsRoot + } + + return localDepsRoot +} + module.exports = plugin(function({matchComponents, theme}) { - let iconsDir = path.join(__dirname, "../../deps/heroicons/optimized") + let iconsDir = path.join(resolveDepsRoot(), "heroicons/optimized") let values = {} let icons = [ ["", "/24/outline"], diff --git a/config/config.exs b/config/config.exs index 4003bc9b..4ffc70b6 100644 --- a/config/config.exs +++ b/config/config.exs @@ -90,6 +90,13 @@ config :vmemo, VmemoWeb.Endpoint, # at the `config/runtime.exs`. config :vmemo, Vmemo.Mailer, adapter: Swoosh.Adapters.Local +deps_path = + case System.get_env("MIX_DEPS_PATH") do + nil -> Path.expand("../deps", __DIR__) + "" -> Path.expand("../deps", __DIR__) + path -> Path.expand(path) + end + # Configure esbuild (the version is required) config :esbuild, version: "0.17.11", @@ -97,7 +104,7 @@ config :esbuild, args: ~w(js/app.js --bundle --target=es2017 --outdir=../priv/static/assets --external:/fonts/* --external:/images/*), cd: Path.expand("../assets", __DIR__), - env: %{"NODE_PATH" => Path.expand("../deps", __DIR__)} + env: %{"NODE_PATH" => deps_path} ] # Configure tailwind (the version is required) diff --git a/config/nginx/dev.conf b/config/nginx/dev.conf new file mode 100644 index 00000000..f52f7d52 --- /dev/null +++ b/config/nginx/dev.conf @@ -0,0 +1,35 @@ +events {} + +http { + include /etc/nginx/mime.types; + default_type application/octet-stream; + + map $http_upgrade $connection_upgrade { + default upgrade; + "" close; + } + + server { + listen 80; + server_name localhost; + client_max_body_size 50m; + + location /storage/v1/_internal/ { + internal; + alias /app/storage/v1/; + } + + location / { + proxy_pass http://host.docker.internal:4000; + proxy_http_version 1.1; + proxy_set_header Host $host; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + } + } +} diff --git a/config/nginx/prod.conf b/config/nginx/prod.conf new file mode 100644 index 00000000..1178e718 --- /dev/null +++ b/config/nginx/prod.conf @@ -0,0 +1,50 @@ +events {} + +http { + include /etc/nginx/mime.types; + default_type application/octet-stream; + + map $http_upgrade $connection_upgrade { + default upgrade; + "" close; + } + + map $http_x_forwarded_proto $proxy_x_forwarded_proto { + default $http_x_forwarded_proto; + "" $scheme; + } + + map $http_x_forwarded_host $proxy_x_forwarded_host { + default $http_x_forwarded_host; + "" $host; + } + + map $http_x_forwarded_port $proxy_x_forwarded_port { + default $http_x_forwarded_port; + "" $server_port; + } + + server { + listen 4000; + server_name _; + client_max_body_size 50m; + + location /storage/v1/_internal/ { + internal; + alias /app/storage/v1/; + } + + location / { + proxy_pass http://127.0.0.1:4001; + proxy_http_version 1.1; + proxy_set_header Host $host; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $proxy_x_forwarded_host; + proxy_set_header X-Forwarded-Port $proxy_x_forwarded_port; + proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection $connection_upgrade; + } + } +} diff --git a/config/runtime.exs b/config/runtime.exs index 25c308ed..af2f4699 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -165,7 +165,7 @@ if config_env() == :prod do root_source_code_paths: [File.cwd!()] host = System.get_env("PHX_HOST") || "vmemo.app" - port = String.to_integer(System.get_env("PORT") || "4000") + port = String.to_integer(System.get_env("PHX_PORT") || System.get_env("PORT") || "4000") config :vmemo, VmemoWeb.Endpoint, url: [host: host, port: 443, scheme: "https"], diff --git a/docker-compose.yml b/docker-compose.yml index 8776ea02..e882c0a7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,21 @@ -# Default `docker compose up` starts postgres + typesense only. +# Default `docker compose up` starts local dependency services. +# Use `docker compose --profile proxy up -d` to add the local Nginx proxy. # For test DB/search: `docker compose --profile test up` (or `COMPOSE_PROFILES=test`). services: + nginx: + profiles: + - proxy + image: nginx:1.27.5-alpine + hostname: nginx + restart: on-failure + ports: + - "4080:80" + volumes: + - ./config/nginx/dev.conf:/etc/nginx/nginx.conf:ro + - ./storage/v1:/app/storage/v1:ro + extra_hosts: + - "host.docker.internal:host-gateway" + postgres: image: postgres:18 hostname: postgres diff --git a/docs/guides/deployment/docker.md b/docs/guides/deployment/docker.md index 1ca4b487..7bf8f076 100644 --- a/docs/guides/deployment/docker.md +++ b/docs/guides/deployment/docker.md @@ -48,7 +48,8 @@ docker run --rm -p 4000:4000 \ Release startup behavior: 1. `bin/vmemo eval "Vmemo.Release.migrate()"` -2. `bin/vmemo start` +2. Start Nginx when `VMEMO_ENABLE_NGINX=true` +3. `bin/vmemo start` Remote IEx: @@ -93,6 +94,7 @@ docker manifest inspect thaddeusjiang/vmemo:latest >/dev/null && echo ok - `rel/entrypoint.sh` exists and is executable. - Entrypoint runs `bin/vmemo eval "Vmemo.Release.migrate()"`. - Dockerfile runner starts via `ENTRYPOINT + CMD ["start"]`. +- Dockerfile runner starts Nginx before Phoenix when `VMEMO_ENABLE_NGINX=true`. - Dockerfile runner includes ImageMagick (`magick`) for AI vision request preprocessing. ### Required Environment Variables @@ -109,6 +111,28 @@ docker manifest inspect thaddeusjiang/vmemo:latest >/dev/null && echo ok Optional: `MOONDREAM_URL`, `OPENROUTER_VISION_MODEL`, `SENTRY_ENV` +Storage acceleration: + +- Browser-facing storage URLs stay under `/storage/v1`. +- Phoenix performs the lightweight owner check, then returns `X-Accel-Redirect` + under `/storage/v1/_internal`. +- Nginx sends the file bytes from the app's `storage/v1` directory. The production + Docker image enables this by default: Nginx listens on `4000`, while Phoenix + listens internally on `PHX_PORT=4001`. + +Example Nginx location: + +```nginx +location /storage/v1/_internal/ { + internal; + alias /app/storage/v1/; +} +``` + +Keep `/storage/v1/_internal/` internal-only. Browser-facing requests should continue to use +`/storage/v1/:user_id/images/:filename` and `/storage/v1/:user_id/avatars/:filename` +so they pass through Phoenix authorization before Nginx serves the file bytes. + ### Troubleshooting Migration failure: diff --git a/docs/guides/development/setup.md b/docs/guides/development/setup.md index 9eabb85a..a84ada2a 100644 --- a/docs/guides/development/setup.md +++ b/docs/guides/development/setup.md @@ -19,11 +19,21 @@ From repository root: ```bash mise trust && mise install -docker compose up -d +docker compose --profile proxy up -d mix setup iex -S mix phx.server ``` +Open the app through the local reverse proxy: + +```text +http://localhost:4080 +``` + +Phoenix still runs on `http://localhost:4000`, but storage image responses use +`X-Accel-Redirect`. Use the Nginx proxy on port `4080` when validating image +loading performance or `/storage/v1` image URLs locally. + Test dependencies: ```bash diff --git a/docs/guides/self-hosting/zeabur/README.md b/docs/guides/self-hosting/zeabur/README.md index 3a8ad000..28562ad6 100644 --- a/docs/guides/self-hosting/zeabur/README.md +++ b/docs/guides/self-hosting/zeabur/README.md @@ -15,3 +15,8 @@ SENTRY_DSN= # optional RESEND_API_KEY= # optional OPENROUTER_API_KEY= # optional ``` + +The template exposes the Vmemo service on port `4000`. The production image starts +Nginx on that public port and runs Phoenix internally on `4001`, so `/storage/v1` +image requests keep the normal browser URL while Nginx handles the internal +`X-Accel-Redirect` file response. diff --git a/docs/guides/self-hosting/zeabur/vmemo.yml b/docs/guides/self-hosting/zeabur/vmemo.yml index 9e2d28b8..561e7181 100644 --- a/docs/guides/self-hosting/zeabur/vmemo.yml +++ b/docs/guides/self-hosting/zeabur/vmemo.yml @@ -50,6 +50,12 @@ spec: PHX_SERVER: default: "true" expose: false + PHX_PORT: + default: "4001" + expose: false + VMEMO_ENABLE_NGINX: + default: "true" + expose: false SECRET_KEY_BASE: default: ${TYPESENSE_SERVICE_API_KEY}_${TYPESENSE_SERVICE_API_KEY} expose: false diff --git a/lib/vmemo_web/controllers/file_controller.ex b/lib/vmemo_web/controllers/file_controller.ex index 536330b0..2cb464be 100644 --- a/lib/vmemo_web/controllers/file_controller.ex +++ b/lib/vmemo_web/controllers/file_controller.ex @@ -2,6 +2,7 @@ defmodule VmemoWeb.FileController do use VmemoWeb, :controller @storage_root Path.expand("storage/v1") + @storage_accel_redirect_prefix "/storage/v1/_internal" @allowed_mime_types %{ ".png" => "image/png", ".jpg" => "image/jpeg", @@ -14,6 +15,7 @@ defmodule VmemoWeb.FileController do def show(conn, %{"user_id" => user_id, "filename" => filename}) do with {:ok, safe_user_id} <- normalize_user_id(user_id), + :ok <- authorize_storage_user(conn, safe_user_id), {:ok, safe_filename} <- normalize_filename(filename), {:ok, file_path} <- image_path(safe_user_id, safe_filename), {:ok, resolved_path} <- resolve_image_path(file_path) do @@ -25,6 +27,7 @@ defmodule VmemoWeb.FileController do def show_avatar(conn, %{"user_id" => user_id, "filename" => filename}) do with {:ok, safe_user_id} <- normalize_user_id(user_id), + :ok <- authorize_storage_user(conn, safe_user_id), {:ok, safe_filename} <- normalize_filename(filename), {:ok, file_path} <- avatar_path(safe_user_id, safe_filename) do if File.exists?(file_path) do @@ -39,19 +42,12 @@ defmodule VmemoWeb.FileController do defp send_storage_file(conn, file_path) do with {:ok, stat} <- File.stat(file_path), - {:ok, file_bin} <- read_file_binary(file_path, stat.size), etag <- build_etag(file_path, stat), last_modified <- build_last_modified(stat), false <- fresh?(conn, etag, stat) do - conn = - conn - |> put_resp_header("content-type", detect_safe_mime(file_path)) - |> put_resp_header("content-disposition", "inline") - |> put_resp_header("cache-control", "public, max-age=31536000, immutable") - |> put_resp_header("etag", etag) - |> put_resp_header("last-modified", last_modified) - - send_resp(conn, 200, file_bin) + conn + |> put_storage_headers(file_path, etag, last_modified) + |> send_storage_body(file_path) else true -> conn @@ -67,6 +63,36 @@ defmodule VmemoWeb.FileController do end end + defp authorize_storage_user(conn, user_id) do + case conn.assigns[:current_user] do + %{id: ^user_id} -> :ok + %{id: current_user_id} when is_binary(current_user_id) -> :error + _ -> :error + end + end + + defp put_storage_headers(conn, file_path, etag, last_modified) do + conn + |> put_resp_header("content-type", detect_safe_mime(file_path)) + |> put_resp_header("content-disposition", "inline") + |> put_resp_header("cache-control", "public, max-age=31536000, immutable") + |> put_resp_header("etag", etag) + |> put_resp_header("last-modified", last_modified) + end + + defp send_storage_body(conn, file_path) do + conn + |> put_resp_header("x-accel-redirect", storage_accel_redirect_path(file_path)) + |> send_resp(200, "") + end + + defp storage_accel_redirect_path(file_path) do + file_path + |> Path.expand() + |> Path.relative_to(@storage_root) + |> then(&Path.join(@storage_accel_redirect_prefix, &1)) + end + defp fresh?(conn, etag, stat) do if_none_match_present? = get_req_header(conn, "if-none-match") != [] @@ -212,23 +238,4 @@ defmodule VmemoWeb.FileController do extension = Path.extname(file_path) |> String.downcase() Map.get(@allowed_mime_types, extension, "application/octet-stream") end - - defp read_file_binary(file_path, size) - when is_binary(file_path) and is_integer(size) and size >= 0 do - case :file.open(String.to_charlist(file_path), [:read, :binary]) do - {:ok, io} -> - try do - case :file.read(io, size) do - {:ok, data} -> {:ok, data} - :eof -> {:ok, <<>>} - {:error, _} = error -> error - end - after - :file.close(io) - end - - {:error, _} = error -> - error - end - end end diff --git a/lib/vmemo_web/router.ex b/lib/vmemo_web/router.ex index 722f8ac5..961497b7 100644 --- a/lib/vmemo_web/router.ex +++ b/lib/vmemo_web/router.ex @@ -28,6 +28,11 @@ defmodule VmemoWeb.Router do plug VmemoWeb.ApiAuth end + pipeline :storage do + plug :fetch_session + plug :fetch_current_user + end + # MCP pipeline - optional authentication for MCP server # Allows unauthenticated access for public tools, but sets actor if API token is provided # Only supports StreamableHttp (POST requests), not SSE (GET requests) @@ -160,7 +165,7 @@ defmodule VmemoWeb.Router do end scope "/storage/v1/", VmemoWeb do - pipe_through :browser + pipe_through :storage get "/:user_id/images/:filename", FileController, :show get "/:user_id/avatars/:filename", FileController, :show_avatar diff --git a/others/e2e-test/README.md b/others/e2e-test/README.md index dbf998a9..53b62a22 100644 --- a/others/e2e-test/README.md +++ b/others/e2e-test/README.md @@ -29,6 +29,9 @@ bun run e2e -- tests/home-page.spec.ts # UI mode (scoped) bun run e2e:ui -- tests/home-page.spec.ts + +# Image display performance probe +E2E_BASE_URL=http://localhost:4000 bun run perf:images ``` Do not run the full suite unless explicitly requested. @@ -46,6 +49,26 @@ For local prerequisites and execution flow, follow: - `/.agents/skills/vmemo-e2e-testing/SKILL.md` +## Image Performance Probe + +`bun run perf:images` verifies the `/images` page against a running Vmemo target and prints a JSON report with: + +- first image ready time +- `/storage/v1` response status, duration, content type, server, and bytes +- browser image resource timing and decoded dimensions + +The default budgets target the local Docker e2e environment: + +```bash +PHOTOS_INDEX_READY_BUDGET_MS=3000 \ +STORAGE_IMAGE_RESPONSE_BUDGET_MS=1500 \ +PHOTOS_INDEX_MIN_IMAGES=1 \ +E2E_BASE_URL=http://localhost:4000 \ +bun run perf:images +``` + +The report is also attached to the Playwright output as `photos-index-image-performance.json`. + ## CI Notes - CI e2e workflow trigger label: `run-e2e-test` diff --git a/others/e2e-test/global-setup.ts b/others/e2e-test/global-setup.ts index 9dc6a716..1a8b8318 100644 --- a/others/e2e-test/global-setup.ts +++ b/others/e2e-test/global-setup.ts @@ -20,11 +20,16 @@ export default async function globalSetup(config: FullConfig) { try { await page.goto("/login", { waitUntil: "domcontentloaded" }); - const loginButton = page.getByRole("button", { name: /Login/i }); - await expect(loginButton).toBeVisible({ timeout: 10_000 }); - await page.getByLabel("Email").fill(email); - await page.getByLabel("Password").fill(password); - await loginButton.click(); + const csrfToken = await page.locator('input[name="_csrf_token"]').inputValue(); + await page.request.post("/login", { + form: { + _csrf_token: csrfToken, + "user[email]": email, + "user[password]": password, + "user[remember_me]": "false", + }, + }); + await page.goto("/home", { waitUntil: "domcontentloaded" }); await expect(page).toHaveURL(/\/home/, { timeout: 10_000 }); lastError = undefined; break; diff --git a/others/e2e-test/package.json b/others/e2e-test/package.json index e2cc46f2..aedc8aea 100644 --- a/others/e2e-test/package.json +++ b/others/e2e-test/package.json @@ -5,7 +5,8 @@ "scripts": { "e2e": "playwright test", "e2e:ui": "playwright test --ui", - "e2e:update-snapshots": "playwright test --update-snapshots" + "e2e:update-snapshots": "playwright test --update-snapshots", + "perf:images": "playwright test tests/photos-index-performance.spec.ts --project=macbook-13" }, "devDependencies": { "@playwright/test": "^1.58.2", diff --git a/others/e2e-test/tests/photos-index-page.spec.ts b/others/e2e-test/tests/photos-index-page.spec.ts index ed6bb33a..05fba532 100644 --- a/others/e2e-test/tests/photos-index-page.spec.ts +++ b/others/e2e-test/tests/photos-index-page.spec.ts @@ -1,7 +1,19 @@ -import { test } from "@playwright/test"; +import { expect, test } from "@playwright/test"; import { expectVisual, gotoAndAssertAttached } from "./visual-helpers.js"; test("photos index page visual snapshot", async ({ page }) => { await gotoAndAssertAttached(page, "/images", page.locator("#infinite-scroll")); - await expectVisual(page, "photos-index-page", [page.locator("#waterfall-images")]); + const firstImage = page.locator("#waterfall-images img").first(); + + await expect(firstImage).toBeVisible({ timeout: 20_000 }); + await expect + .poll(() => + firstImage.evaluate((image) => (image as HTMLImageElement).naturalWidth), + ) + .toBeGreaterThan(0); + + await expectVisual(page, "photos-index-page", [ + page.locator(".page-shell"), + page.getByLabel("Notifications"), + ]); }); diff --git a/others/e2e-test/tests/photos-index-page.spec.ts-snapshots/photos-index-page-macbook-13-darwin.png b/others/e2e-test/tests/photos-index-page.spec.ts-snapshots/photos-index-page-macbook-13-darwin.png index 54075f20..f65368da 100644 Binary files a/others/e2e-test/tests/photos-index-page.spec.ts-snapshots/photos-index-page-macbook-13-darwin.png and b/others/e2e-test/tests/photos-index-page.spec.ts-snapshots/photos-index-page-macbook-13-darwin.png differ diff --git a/others/e2e-test/tests/photos-index-performance.spec.ts b/others/e2e-test/tests/photos-index-performance.spec.ts new file mode 100644 index 00000000..36751d93 --- /dev/null +++ b/others/e2e-test/tests/photos-index-performance.spec.ts @@ -0,0 +1,173 @@ +import { expect, test, type Request } from "@playwright/test"; + +type BrowserImageMetric = { + url: string; + naturalWidth: number; + naturalHeight: number; + complete: boolean; + durationMs: number; + transferSize: number; + encodedBodySize: number; + decodedBodySize: number; +}; + +type ResponseMetric = { + url: string; + status: number; + durationMs: number; + contentLength: number | null; + contentType: string | null; + server: string | null; +}; + +const pageReadyBudgetMs = readNumberEnv("PHOTOS_INDEX_READY_BUDGET_MS", 3_000); +const imageResponseBudgetMs = readNumberEnv("STORAGE_IMAGE_RESPONSE_BUDGET_MS", 1_500); +const minExpectedImages = readNumberEnv("PHOTOS_INDEX_MIN_IMAGES", 1); + +test("photos index image display performance", async ({ page }, testInfo) => { + const storageResponses: ResponseMetric[] = []; + const storageRequestStartMs = new Map(); + + page.on("request", (request) => { + if (isStorageImageUrl(request.url())) { + storageRequestStartMs.set(request, Date.now()); + } + }); + + page.on("response", (response) => { + const url = response.url(); + + if (!isStorageImageUrl(url)) { + return; + } + + const request = response.request(); + const headers = response.headers(); + const startedAt = storageRequestStartMs.get(request); + storageRequestStartMs.delete(request); + + storageResponses.push({ + url: new URL(url).pathname, + status: response.status(), + durationMs: startedAt ? Date.now() - startedAt : 0, + contentLength: parseNullableInteger(headers["content-length"]), + contentType: headers["content-type"] ?? null, + server: headers.server ?? null, + }); + }); + + const startMs = Date.now(); + + await page.goto("/images", { waitUntil: "domcontentloaded" }); + await expect(page.locator("#infinite-scroll")).toHaveCount(1, { timeout: 20_000 }); + + const firstImage = page.locator("#waterfall-images img").first(); + await expect(firstImage).toBeVisible({ timeout: 20_000 }); + await expect + .poll(() => + firstImage.evaluate( + (image) => (image as HTMLImageElement).complete && (image as HTMLImageElement).naturalWidth > 0, + ), + ) + .toBe(true); + + const firstImageReadyMs = Date.now() - startMs; + const browserImages = await collectBrowserImageMetrics(page); + const loadedImages = browserImages.filter((image) => image.complete && image.naturalWidth > 0); + const report = buildReport(firstImageReadyMs, browserImages, storageResponses); + + console.log(JSON.stringify(report, null, 2)); + + await testInfo.attach("photos-index-image-performance.json", { + body: JSON.stringify(report, null, 2), + contentType: "application/json", + }); + + expect(loadedImages.length).toBeGreaterThanOrEqual(minExpectedImages); + expect(storageResponses.length).toBeGreaterThanOrEqual(minExpectedImages); + expect(storageResponses.map((response) => response.status)).toEqual( + expect.arrayContaining([200]), + ); + expect(storageResponses.every((response) => response.status < 400)).toBe(true); + expect(firstImageReadyMs).toBeLessThanOrEqual(pageReadyBudgetMs); + expect(maxDuration(storageResponses)).toBeLessThanOrEqual(imageResponseBudgetMs); +}); + +async function collectBrowserImageMetrics(page: import("@playwright/test").Page) { + return page.locator("#waterfall-images img").evaluateAll((images) => + images.map((image) => { + const htmlImage = image as HTMLImageElement; + const entry = performance + .getEntriesByName(htmlImage.currentSrc) + .at(-1) as PerformanceResourceTiming | undefined; + + return { + url: new URL(htmlImage.currentSrc).pathname, + naturalWidth: htmlImage.naturalWidth, + naturalHeight: htmlImage.naturalHeight, + complete: htmlImage.complete, + durationMs: Math.round(entry?.duration ?? 0), + transferSize: entry?.transferSize ?? 0, + encodedBodySize: entry?.encodedBodySize ?? 0, + decodedBodySize: entry?.decodedBodySize ?? 0, + } satisfies BrowserImageMetric; + }), + ); +} + +function buildReport( + firstImageReadyMs: number, + browserImages: BrowserImageMetric[], + storageResponses: ResponseMetric[], +) { + const totalEncodedBytes = browserImages.reduce( + (sum, image) => sum + image.encodedBodySize, + 0, + ); + + return { + budgets: { + pageReadyBudgetMs, + imageResponseBudgetMs, + minExpectedImages, + }, + summary: { + firstImageReadyMs, + imageCount: browserImages.length, + storageRequestCount: storageResponses.length, + maxBrowserImageDurationMs: maxDuration(browserImages), + maxStorageResponseDurationMs: maxDuration(storageResponses), + totalEncodedBytes, + }, + browserImages, + storageResponses, + }; +} + +function maxDuration(metrics: Array<{ durationMs: number }>) { + return metrics.reduce((max, metric) => Math.max(max, metric.durationMs), 0); +} + +function readNumberEnv(name: string, fallback: number) { + const value = process.env[name]; + + if (!value) { + return fallback; + } + + const parsed = Number(value); + return Number.isFinite(parsed) ? parsed : fallback; +} + +function parseNullableInteger(value: string | undefined) { + if (!value) { + return null; + } + + const parsed = Number.parseInt(value, 10); + return Number.isFinite(parsed) ? parsed : null; +} + +function isStorageImageUrl(url: string) { + return url.includes("/storage/v1/") && !url.includes("/storage/v1/_internal/"); +} diff --git a/rel/entrypoint.sh b/rel/entrypoint.sh index 60872bf9..ae2001e3 100644 --- a/rel/entrypoint.sh +++ b/rel/entrypoint.sh @@ -3,4 +3,9 @@ set -eu /app/bin/vmemo eval "Vmemo.Release.migrate()" +if [ "${1:-}" = "start" ] && [ "${VMEMO_ENABLE_NGINX:-}" = "true" ]; then + nginx -t + nginx +fi + exec /app/bin/vmemo "$@" diff --git a/test/vmemo_web/controllers/file_controller_test.exs b/test/vmemo_web/controllers/file_controller_test.exs index 56feaf46..c36802c9 100644 --- a/test/vmemo_web/controllers/file_controller_test.exs +++ b/test/vmemo_web/controllers/file_controller_test.exs @@ -1,31 +1,42 @@ defmodule VmemoWeb.FileControllerTest do - use VmemoWeb.ConnCase, async: true + use VmemoWeb.ConnCase, async: false + + import Vmemo.AccountFixtures @base_dir Path.join(["storage", "v1"]) - setup do - user_id = "test-user-#{System.unique_integer([:positive])}" - image_dir = Path.join([@base_dir, user_id, "images"]) + setup %{conn: conn} do + user = user_fixture() + other_user = user_fixture() + conn = log_in_user(conn, user) + + image_dir = Path.join([@base_dir, user.id, "images"]) File.mkdir_p!(image_dir) on_exit(fn -> - File.rm_rf!(Path.join([@base_dir, user_id])) + File.rm_rf!(Path.join([@base_dir, user.id])) + File.rm_rf!(Path.join([@base_dir, other_user.id])) end) - {:ok, user_id: user_id, image_dir: image_dir} + {:ok, conn: conn, user: user, other_user: other_user, image_dir: image_dir} end - test "returns original image when thumbnail is missing", %{ + test "accelerates fallback original image when thumbnail is missing", %{ conn: conn, - user_id: user_id, + user: user, image_dir: image_dir } do original = Path.join(image_dir, "sample.png") File.write!(original, "png-data") - conn = get(conn, ~p"/storage/v1/#{user_id}/images/sample--m.png") + conn = get(conn, ~p"/storage/v1/#{user.id}/images/sample--m.png") + + assert response(conn, 200) == "" + + assert get_resp_header(conn, "x-accel-redirect") == [ + "/storage/v1/_internal/#{user.id}/images/sample.png" + ] - assert response(conn, 200) == "png-data" assert get_resp_header(conn, "content-type") == ["image/png"] assert get_resp_header(conn, "content-disposition") == ["inline"] assert get_resp_header(conn, "cache-control") == ["public, max-age=31536000, immutable"] @@ -35,37 +46,47 @@ defmodule VmemoWeb.FileControllerTest do test "falls back to another extension when exact original does not exist", %{ conn: conn, - user_id: user_id, + user: user, image_dir: image_dir } do png = Path.join(image_dir, "sample.png") File.write!(png, "png-fallback") - conn = get(conn, ~p"/storage/v1/#{user_id}/images/sample--m.webp") + conn = get(conn, ~p"/storage/v1/#{user.id}/images/sample--m.webp") + + assert response(conn, 200) == "" + + assert get_resp_header(conn, "x-accel-redirect") == [ + "/storage/v1/_internal/#{user.id}/images/sample.png" + ] - assert response(conn, 200) == "png-fallback" assert get_resp_header(conn, "content-type") == ["image/png"] end - test "serves tiff images with image/tiff content type", %{ + test "accelerates tiff images with image/tiff content type", %{ conn: conn, - user_id: user_id, + user: user, image_dir: image_dir } do tiff = Path.join(image_dir, "sample.tiff") File.write!(tiff, "tiff-data") - conn = get(conn, ~p"/storage/v1/#{user_id}/images/sample.tiff") + conn = get(conn, ~p"/storage/v1/#{user.id}/images/sample.tiff") + + assert response(conn, 200) == "" + + assert get_resp_header(conn, "x-accel-redirect") == [ + "/storage/v1/_internal/#{user.id}/images/sample.tiff" + ] - assert response(conn, 200) == "tiff-data" assert get_resp_header(conn, "content-type") == ["image/tiff"] end test "returns 404 with no-store when both thumbnail and original are missing", %{ conn: conn, - user_id: user_id + user: user } do - conn = get(conn, ~p"/storage/v1/#{user_id}/images/not-found--s.png") + conn = get(conn, ~p"/storage/v1/#{user.id}/images/not-found--s.png") assert response(conn, 404) == "File not found" assert get_resp_header(conn, "cache-control") == ["no-store"] @@ -74,40 +95,98 @@ defmodule VmemoWeb.FileControllerTest do test "returns 304 when if-none-match matches etag", %{ conn: conn, - user_id: user_id, + user: user, image_dir: image_dir } do original = Path.join(image_dir, "etag.png") File.write!(original, "etag-data") - first = get(conn, ~p"/storage/v1/#{user_id}/images/etag.png") - assert response(first, 200) == "etag-data" + first = get(conn, ~p"/storage/v1/#{user.id}/images/etag.png") + assert response(first, 200) == "" [etag] = get_resp_header(first, "etag") second = conn |> put_req_header("if-none-match", etag) - |> get(~p"/storage/v1/#{user_id}/images/etag.png") + |> get(~p"/storage/v1/#{user.id}/images/etag.png") assert response(second, 304) == "" assert get_resp_header(second, "cache-control") == ["public, max-age=0, must-revalidate"] end - test "returns 404 for invalid filename pattern", %{conn: conn, user_id: user_id} do - conn = get(conn, "/storage/v1/#{user_id}/images/evil file.png") + test "returns 404 for invalid filename pattern", %{conn: conn, user: user} do + conn = get(conn, "/storage/v1/#{user.id}/images/evil file.png") assert response(conn, 404) == "File not found" end - test "serves avatar when file exists", %{conn: conn, user_id: user_id} do - avatar_dir = Path.join([@base_dir, user_id, "avatars"]) + test "accelerates avatar when file exists", %{conn: conn, user: user} do + avatar_dir = Path.join([@base_dir, user.id, "avatars"]) File.mkdir_p!(avatar_dir) avatar = Path.join(avatar_dir, "me.jpg") File.write!(avatar, "jpg-data") - conn = get(conn, ~p"/storage/v1/#{user_id}/avatars/me.jpg") + conn = get(conn, ~p"/storage/v1/#{user.id}/avatars/me.jpg") + + assert response(conn, 200) == "" + + assert get_resp_header(conn, "x-accel-redirect") == [ + "/storage/v1/_internal/#{user.id}/avatars/me.jpg" + ] - assert response(conn, 200) == "jpg-data" assert get_resp_header(conn, "content-type") == ["image/jpeg"] assert get_resp_header(conn, "cache-control") == ["public, max-age=31536000, immutable"] end + + test "does not serve image files to anonymous users", %{ + user: user, + image_dir: image_dir + } do + original = Path.join(image_dir, "private.png") + File.write!(original, "private-data") + + conn = + Phoenix.ConnTest.build_conn() + |> get(~p"/storage/v1/#{user.id}/images/private.png") + + assert response(conn, 404) == "File not found" + assert get_resp_header(conn, "cache-control") == ["no-store"] + end + + test "does not serve another user's image files", %{ + other_user: other_user, + user: user, + image_dir: image_dir + } do + original = Path.join(image_dir, "private.png") + File.write!(original, "private-data") + + conn = + Phoenix.ConnTest.build_conn() + |> log_in_user(other_user) + |> get(~p"/storage/v1/#{user.id}/images/private.png") + + assert response(conn, 404) == "File not found" + assert get_resp_header(conn, "cache-control") == ["no-store"] + end + + test "uses x-accel-redirect by default", %{ + conn: conn, + user: user, + image_dir: image_dir + } do + original = Path.join(image_dir, "accelerated.png") + File.write!(original, "accelerated-data") + + conn = get(conn, ~p"/storage/v1/#{user.id}/images/accelerated.png") + + assert response(conn, 200) == "" + + assert get_resp_header(conn, "x-accel-redirect") == [ + "/storage/v1/_internal/#{user.id}/images/accelerated.png" + ] + + assert get_resp_header(conn, "content-type") == ["image/png"] + assert get_resp_header(conn, "content-disposition") == ["inline"] + assert get_resp_header(conn, "cache-control") == ["public, max-age=31536000, immutable"] + end end