diff options
| author | ojacobson <ojacobson@noreply.codeberg.org> | 2025-12-05 20:44:39 +0100 |
|---|---|---|
| committer | ojacobson <ojacobson@noreply.codeberg.org> | 2025-12-05 20:44:39 +0100 |
| commit | 6cd7e6dd5eb001779a83e9e07c7a3c8379b0548f (patch) | |
| tree | 39bb7e26fa7c2861394c5602f10d63deef65c70b | |
| parent | 9af71a82ca74fde48283d4e0a0adcd69f4fcb9dd (diff) | |
| parent | a2285c0e91063cf0f07637664c6055acdcacd9a8 (diff) | |
This covers two things:
* For _any_ static asset, send back an `ETag` header based on its content. If the request had an `If-None-Match` header, use that header to instead send back a `Not Modified` response, with no payload, if appropriate, saving the cost of transferring data the client already has.
* For _immutable_ static assets, send back a `Cache-Control` header to allow browsers to replay the response from cache, without making a request, for up to 90 days, saving whole HTTP round-trips.
In practice, startup time is now dominated by the time needed to check whether the user is logged in, and the time needed to satisfy the `/api/boot` request if they are.
"Static" here includes anything that's served in the HTML itself, so things like `/`, `/login`, `/c/:channelid`, and `/me` are all "static," and are all affected by this change even though the logical content of those endpoints also includes data that will vary from user to user and from time to time. All the dynamic data for those responses comes from separate API requests, which don't affect the cacheability of those pages. However, this change is conservative and _does not_ instruct the browser to cache those pages for long periods; it only supports telling the browser that the cached copy they have is fine, via a `Not Modified` response.
This change also includes a minimum Rust version bump, to the recently-released 1.91 - as it uses `Duration::from_hours` to compute the `Cache-Control` header. I could use `Duration::new` or `Duration::from_secs`, but really, working in increments of days from such tiny units is awkward. I'd prefer to use `Duration::from_days`, but it's not stable yet.
Merges asset-etags into main.
| -rw-r--r-- | Cargo.lock | 1 | ||||
| -rw-r--r-- | Cargo.toml | 3 | ||||
| -rw-r--r-- | src/ui/assets.rs | 154 | ||||
| -rw-r--r-- | src/ui/handlers/asset.rs | 11 | ||||
| -rw-r--r-- | src/ui/handlers/conversation.rs | 57 | ||||
| -rw-r--r-- | src/ui/handlers/index.rs | 18 | ||||
| -rw-r--r-- | src/ui/handlers/invite.rs | 44 | ||||
| -rw-r--r-- | src/ui/handlers/login.rs | 14 | ||||
| -rw-r--r-- | src/ui/handlers/me.rs | 18 | ||||
| -rw-r--r-- | src/ui/handlers/setup.rs | 18 | ||||
| -rw-r--r-- | src/ui/handlers/swatch.rs | 14 |
11 files changed, 237 insertions, 115 deletions
@@ -1968,6 +1968,7 @@ dependencies = [ "faker_rand", "futures", "headers", + "hex", "itertools", "mime", "nix", @@ -2,7 +2,7 @@ name = "pilcrow" version = "0.1.0" edition = "2024" -rust-version = "1.90" +rust-version = "1.91" authors = [ "Owen Jacobson <owen@grimoire.ca>", "Kit La Touche <kit@transneptune.net>", @@ -29,6 +29,7 @@ chrono = { version = "0.4.42", features = ["serde"] } clap = { version = "4.5.51", features = ["derive", "env"] } futures = "0.3.31" headers = "0.4.1" +hex = "0.4.3" itertools = "0.14.0" mime = "0.3.17" nix = { version = "0.30.1", features = ["fs"] } diff --git a/src/ui/assets.rs b/src/ui/assets.rs index ede1f7c..2fa703b 100644 --- a/src/ui/assets.rs +++ b/src/ui/assets.rs @@ -1,8 +1,12 @@ +use std::{str::FromStr, time::Duration}; + use ::mime::Mime; use axum::{ - http::{StatusCode, header}, - response::{IntoResponse, Response}, + http::StatusCode, + response::{self, IntoResponse, IntoResponseParts, ResponseParts}, }; +use axum_extra::TypedHeader; +use headers::{CacheControl, ContentType, ETag, IfNoneMatch}; use rust_embed::EmbeddedFile; use super::{error::NotFound, mime}; @@ -15,14 +19,16 @@ use crate::error::{ #[folder = "$OUT_DIR/ui"] pub struct Assets; +// Prefer the corresponding methods in `Response` unless you specifically do not want HTTP caching +// behaviours. impl Assets { - pub fn load(path: impl AsRef<str>) -> Result<Asset, Error> { - let path = path.as_ref(); - let mime = mime::from_path(path).fail("Failed to determine MIME type from asset path")?; + pub fn load(path: &str) -> Result<Asset, Error> { + let mime = mime::from_path(path) + .fail_with(|| format!("Failed to determine MIME type from asset path: {path}"))?; + let file = Self::get(path).ok_or(Error::NotFound(path.into()))?; + let asset = Asset::new(mime, file)?; - Self::get(path) - .map(|file| Asset(mime, file)) - .ok_or(Error::NotFound(path.into())) + Ok(asset) } pub fn index() -> Result<Asset, Internal> { @@ -33,20 +39,142 @@ impl Assets { } } -pub struct Asset(Mime, EmbeddedFile); +pub struct Asset { + mime: Mime, + file: EmbeddedFile, + etag: ETag, +} + +impl Asset { + fn new(mime: Mime, file: EmbeddedFile) -> Result<Self, Failed> { + let etag = Self::etag_from(&file).fail("Failed to compute ETag")?; + Ok(Self { mime, file, etag }) + } + + fn etag_from(file: &EmbeddedFile) -> Result<ETag, <ETag as FromStr>::Err> { + let digest = file.metadata.sha256_hash(); + let digest = hex::encode(digest); + + let tag = format!("\"{digest}\""); + let tag = ETag::from_str(&tag)?; + + Ok(tag) + } + + fn differs_from(&self, if_none_match: &IfNoneMatch) -> bool { + if_none_match.precondition_passes(&self.etag) + } +} impl IntoResponse for Asset { - fn into_response(self) -> Response { - let Self(mime, file) = self; + fn into_response(self) -> response::Response { + let Self { mime, file, etag } = self; ( StatusCode::OK, - [(header::CONTENT_TYPE, mime.as_ref())], + TypedHeader(ContentType::from(mime)), + TypedHeader(etag), file.data, ) .into_response() } } +// Clippy warns us here because the `Asset` variant is the size of, well, the Asset struct - itself +// quite large due to the embedded file metadata. There isn't a better alternative that I can see, +// and it's not causing any performance or stack exhaustion problems in practice. +#[expect(clippy::large_enum_variant)] +pub enum Response { + NotModified, + // I've opted to make Mutability part of the conditional-HTTP-response logic here and not part + // of the underlying Asset struct, even though it would be pretty reasonable to deem that it is + // the Asset that is immutable rather than its specific binding to an HTTP result. Assets don't + // know their own path, and there isn't a tidy way to tag that information onto Asset at + // construction without refactoring a _ton_ of things that are fine the way they are, though, + // and since mutability only affects an HTTP header and nothing else, the resulting structs + // work well enough. + Asset(Asset, Mutability), +} + +// This interface parallels the `Assets` interface, above. However, unlike `Assets`, this supports +// browser and intermediate server cache mechanisms, both by considering the provided +// `If-None-Match` header against each asset's `ETag`, and by tacking on cache control headers for +// static assets that we commit to never changing. +impl Response { + pub fn load(path: &str, if_none_match: &IfNoneMatch) -> Result<Self, Error> { + let mutability = Mutability::from_path(path); + let asset = Assets::load(path)?; + let response = Self::respond_with(asset, if_none_match, mutability); + Ok(response) + } + + pub fn index(if_none_match: &IfNoneMatch) -> Result<Self, Internal> { + let asset = Assets::index()?; + let response = Self::respond_with(asset, if_none_match, Mutability::Mutable); + Ok(response) + } + + fn respond_with(asset: Asset, if_none_match: &IfNoneMatch, mutability: Mutability) -> Self { + if asset.differs_from(if_none_match) { + Self::Asset(asset, mutability) + } else { + Self::NotModified + } + } +} + +impl IntoResponse for Response { + fn into_response(self) -> response::Response { + match self { + Self::NotModified => StatusCode::NOT_MODIFIED.into_response(), + Self::Asset(asset, mutability) => (mutability, asset).into_response(), + } + } +} + +pub enum Mutability { + Immutable, + Mutable, +} + +impl Mutability { + fn from_path(path: &str) -> Self { + // This is a complete hack and depends intimately on the output of SvelteKit's static site + // adapter in ways that are not documented anywhere I can find. On the other hand, I also + // can't find any sign of a better way to do this, and I think the odds of us (or SvelteKit) + // messing with these paths in a way that breaks this cacheability check are pretty low. + if path.starts_with("_app/immutable/") { + Self::Immutable + } else { + Self::Mutable + } + } +} + +impl IntoResponseParts for Mutability { + type Error = <TypedHeader<CacheControl> as IntoResponseParts>::Error; + + fn into_response_parts(self, res: ResponseParts) -> Result<ResponseParts, Self::Error> { + // 90 days is pretty arbitrary. Everything _works_ if immutable assets get + // re-requested more frequently than this (or even on every page view), but it eats + // up bandwidth and battery to transfer them, and they only ever change to and from + // "404 Not Found" (which we do not instruct browsers to cache). If it was free to + // cache them forever, we'd ask for that. + // + // If a user doesn't visit the app for 90 days, they're probably better off re-downloading + // the whole thing if they ever come back, and better off getting that cache space back for + // something else, until then. + match self { + Self::Immutable => TypedHeader( + CacheControl::new() + .with_immutable() + .with_max_age(Duration::from_hours(90 * 24)), + ) + .into_response_parts(res), + Self::Mutable => ().into_response_parts(res), + } + } +} + #[derive(Debug, thiserror::Error)] pub enum Error { #[error("not found: {0}")] @@ -56,7 +184,7 @@ pub enum Error { } impl IntoResponse for Error { - fn into_response(self) -> Response { + fn into_response(self) -> response::Response { match self { Self::NotFound(_) => NotFound(self.to_string()).into_response(), Self::Failed(_) => Internal::from(self).into_response(), diff --git a/src/ui/handlers/asset.rs b/src/ui/handlers/asset.rs index 1d5b8be..948d6d6 100644 --- a/src/ui/handlers/asset.rs +++ b/src/ui/handlers/asset.rs @@ -1,7 +1,12 @@ use axum::extract::Path; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; -use crate::ui::assets::{Asset, Assets, Error}; +use crate::ui::assets::{Error, Response}; -pub async fn handler(Path(path): Path<String>) -> Result<Asset, Error> { - Assets::load(path) +pub async fn handler( + Path(path): Path<String>, + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Error> { + Response::load(&path, &if_none_match) } diff --git a/src/ui/handlers/conversation.rs b/src/ui/handlers/conversation.rs index 102efc6..9a87d40 100644 --- a/src/ui/handlers/conversation.rs +++ b/src/ui/handlers/conversation.rs @@ -2,13 +2,15 @@ use axum::{ extract::{Path, State}, response::{self, IntoResponse, Redirect}, }; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; use crate::{ conversation::{self, app, app::Conversations}, error::Internal, token::extract::Identity, ui::{ - assets::{Asset, Assets}, + assets::{self, Asset, Assets}, error::NotFound, }, }; @@ -17,44 +19,39 @@ pub async fn handler( State(conversations): State<Conversations>, identity: Option<Identity>, Path(conversation): Path<conversation::Id>, -) -> Result<Asset, Error> { - let _ = identity.ok_or(Error::NotLoggedIn)?; - conversations - .get(&conversation) - .await - .map_err(Error::from)?; + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Internal> { + let response = if identity.is_none() { + Response::NotLoggedIn + } else { + match conversations.get(&conversation).await { + Ok(_) => { + let index = assets::Response::index(&if_none_match)?; + Response::Asset(index) + } + Err(app::GetError::NotFound(_) | app::GetError::Deleted(_)) => { + let index = Assets::index()?; + Response::NotFound(index) + } + Err(err @ app::GetError::Failed(_)) => return Err(err.into()), + } + }; - Assets::index().map_err(Error::Internal) + Ok(response) } -#[derive(Debug, thiserror::Error)] -pub enum Error { - #[error("conversation not found")] - NotFound, - #[error("not logged in")] +pub enum Response { NotLoggedIn, - #[error("{0}")] - Internal(Internal), -} - -impl From<app::GetError> for Error { - fn from(error: app::GetError) -> Self { - match error { - app::GetError::NotFound(_) | app::GetError::Deleted(_) => Self::NotFound, - app::GetError::Failed(_) => Self::Internal(error.into()), - } - } + NotFound(Asset), + Asset(assets::Response), } -impl IntoResponse for Error { +impl IntoResponse for Response { fn into_response(self) -> response::Response { match self { - Self::NotFound => match Assets::index() { - Ok(asset) => NotFound(asset).into_response(), - Err(internal) => internal.into_response(), - }, Self::NotLoggedIn => Redirect::temporary("/login").into_response(), - Self::Internal(error) => error.into_response(), + Self::NotFound(asset) => NotFound(asset).into_response(), + Self::Asset(asset) => asset.into_response(), } } } diff --git a/src/ui/handlers/index.rs b/src/ui/handlers/index.rs index 2fcb51c..de0b2b0 100644 --- a/src/ui/handlers/index.rs +++ b/src/ui/handlers/index.rs @@ -1,22 +1,20 @@ use axum::response::{self, IntoResponse, Redirect}; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; -use crate::{ - error::Internal, - token::extract::Identity, - ui::assets::{Asset, Assets}, -}; +use crate::{error::Internal, token::extract::Identity, ui::assets::Response}; -pub async fn handler(identity: Option<Identity>) -> Result<Asset, Error> { +pub async fn handler( + identity: Option<Identity>, + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Error> { let _ = identity.ok_or(Error::NotLoggedIn)?; - Assets::index().map_err(Error::Internal) + Response::index(&if_none_match).map_err(Error::Internal) } -#[derive(Debug, thiserror::Error)] pub enum Error { - #[error("not logged in")] NotLoggedIn, - #[error("{0}")] Internal(Internal), } diff --git a/src/ui/handlers/invite.rs b/src/ui/handlers/invite.rs index edd6dc1..e552318 100644 --- a/src/ui/handlers/invite.rs +++ b/src/ui/handlers/invite.rs @@ -2,12 +2,15 @@ use axum::{ extract::{Path, State}, response::{self, IntoResponse}, }; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; use crate::{ error::Internal, invite, invite::app::Invites, ui::{ + assets, assets::{Asset, Assets}, error::NotFound, }, @@ -16,38 +19,27 @@ use crate::{ pub async fn handler( State(invites): State<Invites>, Path(invite): Path<invite::Id>, -) -> Result<Asset, Error> { - invites - .get(&invite) - .await - .map_err(Error::internal)? - .ok_or(Error::NotFound)?; - - Assets::index().map_err(Error::Internal) -} - -#[derive(Debug, thiserror::Error)] -pub enum Error { - #[error("invite not found")] - NotFound, - #[error("{0}")] - Internal(Internal), + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Internal> { + if invites.get(&invite).await?.is_some() { + let index = assets::Response::index(&if_none_match)?; + Ok(Response::Found(index)) + } else { + let index = Assets::index()?; + Ok(Response::NotFound(index)) + } } -impl Error { - fn internal(err: impl Into<Internal>) -> Self { - Self::Internal(err.into()) - } +pub enum Response { + Found(assets::Response), + NotFound(Asset), } -impl IntoResponse for Error { +impl IntoResponse for Response { fn into_response(self) -> response::Response { match self { - Self::NotFound => match Assets::index() { - Ok(asset) => NotFound(asset).into_response(), - Err(internal) => internal.into_response(), - }, - Self::Internal(error) => error.into_response(), + Self::Found(asset) => asset.into_response(), + Self::NotFound(asset) => NotFound(asset).into_response(), } } } diff --git a/src/ui/handlers/login.rs b/src/ui/handlers/login.rs index 4562b04..c904d2a 100644 --- a/src/ui/handlers/login.rs +++ b/src/ui/handlers/login.rs @@ -1,8 +1,10 @@ -use crate::{ - error::Internal, - ui::assets::{Asset, Assets}, -}; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; -pub async fn handler() -> Result<Asset, Internal> { - Assets::index() +use crate::{error::Internal, ui::assets::Response}; + +pub async fn handler( + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Internal> { + Response::index(&if_none_match) } diff --git a/src/ui/handlers/me.rs b/src/ui/handlers/me.rs index 2fcb51c..de0b2b0 100644 --- a/src/ui/handlers/me.rs +++ b/src/ui/handlers/me.rs @@ -1,22 +1,20 @@ use axum::response::{self, IntoResponse, Redirect}; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; -use crate::{ - error::Internal, - token::extract::Identity, - ui::assets::{Asset, Assets}, -}; +use crate::{error::Internal, token::extract::Identity, ui::assets::Response}; -pub async fn handler(identity: Option<Identity>) -> Result<Asset, Error> { +pub async fn handler( + identity: Option<Identity>, + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Error> { let _ = identity.ok_or(Error::NotLoggedIn)?; - Assets::index().map_err(Error::Internal) + Response::index(&if_none_match).map_err(Error::Internal) } -#[derive(Debug, thiserror::Error)] pub enum Error { - #[error("not logged in")] NotLoggedIn, - #[error("{0}")] Internal(Internal), } diff --git a/src/ui/handlers/setup.rs b/src/ui/handlers/setup.rs index 5707765..ac91908 100644 --- a/src/ui/handlers/setup.rs +++ b/src/ui/handlers/setup.rs @@ -2,14 +2,15 @@ use axum::{ extract::State, response::{self, IntoResponse, Redirect}, }; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; -use crate::{ - error::Internal, - setup::app::Setup, - ui::assets::{Asset, Assets}, -}; +use crate::{error::Internal, setup::app::Setup, ui::assets::Response}; -pub async fn handler(State(setup): State<Setup>) -> Result<Asset, Error> { +pub async fn handler( + State(setup): State<Setup>, + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Error> { if setup .completed() .await @@ -18,15 +19,12 @@ pub async fn handler(State(setup): State<Setup>) -> Result<Asset, Error> { { Err(Error::SetupCompleted) } else { - Assets::index().map_err(Error::Internal) + Response::index(&if_none_match).map_err(Error::Internal) } } -#[derive(Debug, thiserror::Error)] pub enum Error { - #[error("setup already completed")] SetupCompleted, - #[error("{0}")] Internal(Internal), } diff --git a/src/ui/handlers/swatch.rs b/src/ui/handlers/swatch.rs index 4562b04..c904d2a 100644 --- a/src/ui/handlers/swatch.rs +++ b/src/ui/handlers/swatch.rs @@ -1,8 +1,10 @@ -use crate::{ - error::Internal, - ui::assets::{Asset, Assets}, -}; +use axum_extra::TypedHeader; +use headers::IfNoneMatch; -pub async fn handler() -> Result<Asset, Internal> { - Assets::index() +use crate::{error::Internal, ui::assets::Response}; + +pub async fn handler( + TypedHeader(if_none_match): TypedHeader<IfNoneMatch>, +) -> Result<Response, Internal> { + Response::index(&if_none_match) } |
