From 0a2cb037a80dee835c8fdc6c3fdfc21cae831fb2 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 4 Apr 2023 16:08:03 -0700 Subject: [PATCH 01/11] models: add initial user model bits for defining admins Based on #5376. Co-authored-by: Carol (Nichols || Goulding) --- .env.sample | 4 ++ Cargo.lock | 1 + Cargo.toml | 1 + src/models/helpers.rs | 1 + src/models/helpers/admin.rs | 81 +++++++++++++++++++++++++++++++++++++ src/models/user.rs | 49 +++++++++++++++++++++- 6 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/models/helpers/admin.rs diff --git a/.env.sample b/.env.sample index 58f047ed21e..4884de8b392 100644 --- a/.env.sample +++ b/.env.sample @@ -72,3 +72,7 @@ export GH_CLIENT_SECRET= # Credentials for connecting to the Sentry error reporting service. # export SENTRY_DSN_API= export SENTRY_ENV_API=local + +# GitHub users that are admins on this instance, separated by commas. Whitespace +# will be ignored. +export GH_ADMIN_USERS= diff --git a/Cargo.lock b/Cargo.lock index 3b0584fb21e..b33edfcf9e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -474,6 +474,7 @@ dependencies = [ "indicatif", "insta", "ipnetwork", + "lazy_static", "lettre", "minijinja", "moka", diff --git a/Cargo.toml b/Cargo.toml index aa0c3082766..fa0f4cf39eb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,6 +80,7 @@ tower-http = { version = "=0.4.0", features = ["fs", "catch-panic"] } tracing = "=0.1.37" tracing-subscriber = { version = "=0.3.16", features = ["env-filter"] } url = "=2.3.1" +lazy_static = "1.4.0" [dev-dependencies] cargo-registry-index = { path = "cargo-registry-index", features = ["testing"] } diff --git a/src/models/helpers.rs b/src/models/helpers.rs index 8d9b9a43893..062d5a15eae 100644 --- a/src/models/helpers.rs +++ b/src/models/helpers.rs @@ -1 +1,2 @@ +pub mod admin; pub mod with_count; diff --git a/src/models/helpers/admin.rs b/src/models/helpers/admin.rs new file mode 100644 index 00000000000..699c1a771b4 --- /dev/null +++ b/src/models/helpers/admin.rs @@ -0,0 +1,81 @@ +use std::collections::HashSet; + +use lazy_static::lazy_static; + +use crate::util::errors::{forbidden, AppResult}; + +lazy_static! { + static ref AUTHORIZED_ADMIN_USERS: HashSet = + parse_authorized_admin_users(dotenv::var("GH_ADMIN_USERS")); +} + +const DEFAULT_ADMIN_USERS: [&str; 3] = ["carols10cents", "jtgeibel", "Turbo87"]; + +fn parse_authorized_admin_users(maybe_users: dotenv::Result) -> HashSet { + match maybe_users { + Ok(users) => users + .split(|c: char| !(c.is_ascii_alphanumeric() || c == '-')) + .filter(|user| !user.is_empty()) + .map(String::from) + .collect(), + Err(_err) => DEFAULT_ADMIN_USERS.into_iter().map(String::from).collect(), + } +} + +/// Return `Ok(())` if the given GitHub user name matches a known admin (set +/// either via the `$GH_ADMIN_USERS` environment variable, or the builtin +/// fallback list if that variable is unset), or a forbidden error otherwise. +pub fn is_authorized_admin(username: &str) -> AppResult<()> { + // This hack is here to allow tests to have a consistent set of admin users + // (in this case, just the contents of the `DEFAULT_ADMIN_USERS` constant + // above). + + #[cfg(not(test))] + fn check_username(username: &str) -> bool { + AUTHORIZED_ADMIN_USERS.contains(username) + } + + #[cfg(test)] + fn check_username(username: &str) -> bool { + DEFAULT_ADMIN_USERS.contains(&username) + } + + if check_username(username) { + Ok(()) + } else { + Err(forbidden()) + } +} + +#[cfg(test)] +mod tests { + use std::io::{self, ErrorKind}; + + use super::{is_authorized_admin, parse_authorized_admin_users, DEFAULT_ADMIN_USERS}; + + #[test] + fn test_is_authorized_admin() { + assert_ok!(is_authorized_admin("Turbo87")); + assert_err!(is_authorized_admin("")); + assert_err!(is_authorized_admin("foo")); + } + + #[test] + fn test_parse_authorized_admin_users() { + fn assert_authorized(input: dotenv::Result<&str>, expected: &[&str]) { + assert_eq!( + parse_authorized_admin_users(input.map(String::from)), + expected.iter().map(|s| String::from(*s)).collect() + ); + } + + assert_authorized(Ok(""), &[]); + assert_authorized(Ok("foo"), &["foo"]); + assert_authorized(Ok("foo, bar"), &["foo", "bar"]); + assert_authorized(Ok(" foo bar "), &["foo", "bar"]); + assert_authorized(Ok("foo;bar"), &["foo", "bar"]); + + let not_found_error = dotenv::Error::Io(io::Error::new(ErrorKind::NotFound, "not found")); + assert_authorized(Err(not_found_error), DEFAULT_ADMIN_USERS.as_slice()); + } +} diff --git a/src/models/user.rs b/src/models/user.rs index 0073c9d8dec..b7929320fcc 100644 --- a/src/models/user.rs +++ b/src/models/user.rs @@ -6,7 +6,9 @@ use crate::app::App; use crate::email::Emails; use crate::util::errors::AppResult; -use crate::models::{ApiToken, Crate, CrateOwner, Email, NewEmail, Owner, OwnerKind, Rights}; +use crate::models::{ + helpers::admin, ApiToken, Crate, CrateOwner, Email, NewEmail, Owner, OwnerKind, Rights, +}; use crate::schema::{crate_owners, emails, users}; /// The model representing a row in the `users` database table. @@ -177,4 +179,49 @@ impl User { .first(conn) .optional() } + + /// Attempt to turn this user into an AdminUser + pub fn admin(&self) -> AppResult { + AdminUser::new(self) + } +} + +pub struct AdminUser(User); + +impl AdminUser { + pub fn new(user: &User) -> AppResult { + admin::is_authorized_admin(user.gh_login.as_str()).map(|_| Self(user.clone())) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hardcoded_admins() { + let user = User { + id: 3, + gh_access_token: "arbitrary".into(), + gh_login: "literally_anything".into(), + name: None, + gh_avatar: None, + gh_id: 7, + account_lock_reason: None, + account_lock_until: None, + }; + assert!(user.admin().is_err()); + + let sneaky_user = User { + gh_login: "carols10cents_plus_extra_stuff".into(), + ..user + }; + assert!(sneaky_user.admin().is_err()); + + let real_real_real = User { + gh_login: "carols10cents".into(), + ..sneaky_user + }; + assert!(real_real_real.admin().is_ok()); + } } From 7e45d9896a525d44cd621602ad96116183fdb41e Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 4 Apr 2023 17:23:41 -0700 Subject: [PATCH 02/11] WIP of initial wiring for admin TODO: refactor into multiple commits; credit Carol; make it actually route `admin`; nginx config updates --- Cargo.lock | 28 ++++++++++++++++++++++++++++ Cargo.toml | 2 ++ app/components/header.hbs | 3 +++ app/models/user.js | 1 + src/controllers.rs | 1 + src/controllers/admin.rs | 26 ++++++++++++++++++++++++++ src/models/user.rs | 1 + src/router.rs | 4 +++- src/views.rs | 3 +++ 9 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 src/controllers/admin.rs diff --git a/Cargo.lock b/Cargo.lock index b33edfcf9e6..4680a9abe9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -335,6 +335,18 @@ dependencies = [ "syn 2.0.15", ] +[[package]] +name = "axum-template" +version = "0.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f926c4ca412af0a88bd119a6d214b026a0c98d8c763eaead70032628d3d0a549" +dependencies = [ + "axum", + "handlebars", + "serde", + "thiserror", +] + [[package]] name = "backtrace" version = "0.3.67" @@ -447,6 +459,7 @@ dependencies = [ "aws-sigv4", "axum", "axum-extra", + "axum-template", "base64 0.13.1", "cargo-registry-index", "cargo-registry-markdown", @@ -465,6 +478,7 @@ dependencies = [ "flate2", "futures-channel", "futures-util", + "handlebars", "hex", "http", "http-body", @@ -1286,6 +1300,20 @@ dependencies = [ "tracing", ] +[[package]] +name = "handlebars" +version = "4.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "035ef95d03713f2c347a72547b7cd38cbc9af7cd51e6099fb62d586d4a6dee3a" +dependencies = [ + "log", + "pest", + "pest_derive", + "serde", + "serde_json", + "thiserror", +] + [[package]] name = "hashbrown" version = "0.12.3" diff --git a/Cargo.toml b/Cargo.toml index fa0f4cf39eb..cd150859378 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,6 +81,8 @@ tracing = "=0.1.37" tracing-subscriber = { version = "=0.3.16", features = ["env-filter"] } url = "=2.3.1" lazy_static = "1.4.0" +axum-template = { version = "0.15.1", features = ["handlebars"] } +handlebars = "4.3.6" [dev-dependencies] cargo-registry-index = { path = "cargo-registry-index", features = ["testing"] } diff --git a/app/components/header.hbs b/app/components/header.hbs index 1426d929173..2056704362d 100644 --- a/app/components/header.hbs +++ b/app/components/header.hbs @@ -29,6 +29,9 @@ Dashboard Account Settings Owner Invites + {{#if this.session.currentUser.admin }} + Site Admin + {{/if}} + + +
+

{{> title}}

+ {{> body}} +
+ + \ No newline at end of file diff --git a/src/app.rs b/src/app.rs index ae069722f5c..ea73e5e95ba 100644 --- a/src/app.rs +++ b/src/app.rs @@ -11,10 +11,13 @@ use crate::email::Emails; use crate::github::{GitHubClient, RealGitHubClient}; use crate::metrics::{InstanceMetrics, ServiceMetrics}; use axum::extract::{FromRef, FromRequestParts, State}; +use axum_template::engine::Engine; use diesel::r2d2; +use handlebars::Handlebars; use moka::future::{Cache, CacheBuilder}; use oauth2::basic::BasicClient; use reqwest::blocking::Client; +use rust_embed::RustEmbed; use scheduled_thread_pool::ScheduledThreadPool; /// The `App` struct holds the main components of the application like @@ -65,6 +68,9 @@ pub struct App { /// In-flight request counters for the `balance_capacity` middleware. pub balance_capacity: BalanceCapacityState, + + /// Admin templating engine. + pub admin_engine: Engine>, } impl App { @@ -175,6 +181,10 @@ impl App { _ => None, }; + let admin_engine = Engine::from( + templating::registry().expect("could not initialise admin template engine"), + ); + App { primary_database, read_only_replica_database: replica_database, @@ -189,6 +199,7 @@ impl App { fastboot_client, balance_capacity: Default::default(), config, + admin_engine, } } @@ -300,3 +311,31 @@ impl FromRef for cookie::Key { app.session_key().clone() } } + +mod templating { + use handlebars::{Handlebars, TemplateError}; + + #[cfg(not(debug_assertions))] + #[derive(rust_embed::RustEmbed)] + #[folder = "admin/templates/"] + struct Assets; + + pub(super) fn registry() -> Result, TemplateError> { + let mut hbs = Handlebars::new(); + hbs.set_strict_mode(true); + + #[cfg(debug_assertions)] + { + hbs.set_dev_mode(true); + hbs.register_templates_directory(".hbs", "admin/templates")?; + } + + #[cfg(not(debug_assertions))] + { + // TODO: fix to match names by actually walking the embed. + hbs.register_embed_templates::()?; + } + + Ok(hbs) + } +} diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index 7fdcd2d9c35..cfcea67a38d 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -1,26 +1,49 @@ -use axum::response::Html; +use axum_template::RenderHtml; -use crate::{auth::AuthCheck, models::User, schema::users}; +use crate::{extractors::admin::AdminUser, views::admin::krates::CrateVersion}; use super::prelude::*; /// Handles the `GET /admin/` route. -pub async fn index(app: AppState, req: Parts) -> AppResult> { - tracing::warn!("in admin index"); - +pub async fn index(app: AppState, _user: AdminUser) -> AppResult { conduit_compat(move || { - let conn = &mut *app.db_read_prefer_primary()?; - let user_id = AuthCheck::only_cookie().check(&req, conn)?.user_id(); + use crate::schema::{crates, versions}; + + let conn = &mut *app.db_read()?; - let user = users::table - .find(user_id) - .select(users::all_columns) - .first::(conn)? - .admin()?; + // TODO: move to a new controller and redirect when hitting /admin/. + // TODO: refactor into something that's not a spaghetti query. + // TODO: pagination. + // TODO: search. - dbg!(user); + // XXX: can we send an iterator to RenderHtml? + let recent_versions: Vec = versions::table + .inner_join(crates::table) + .select(( + versions::id, + versions::num, + versions::created_at, + crates::name, + )) + .order(versions::created_at.desc()) + .limit(50) + .load(conn)? + .into_iter() + .map(|(id, num, created_at, name)| -> CrateVersion { + CrateVersion { + id, + num, + created_at, + name, + } + }) + .collect(); - Ok(Html("a wolf at the door".into())) + Ok(RenderHtml( + "crates", + app.admin_engine.clone(), + recent_versions, + )) }) .await } diff --git a/src/extractors.rs b/src/extractors.rs new file mode 100644 index 00000000000..92918b0982b --- /dev/null +++ b/src/extractors.rs @@ -0,0 +1 @@ +pub mod admin; diff --git a/src/extractors/admin.rs b/src/extractors/admin.rs new file mode 100644 index 00000000000..4ebb51dc68d --- /dev/null +++ b/src/extractors/admin.rs @@ -0,0 +1,43 @@ +use axum::{ + async_trait, + extract::{FromRequestParts, State}, +}; +use http::request::Parts; + +use crate::{app::AppState, auth::AuthCheck, models, util::errors::BoxedAppError}; + +/// An authorisation extractor that requires that the current user be a valid +/// admin. +/// +/// If there is no logged in user, or if the user isn't an admin, then a 403 +/// Forbidden will be returned without the controller being invoked. +/// +/// Note that the ordering of extractors is often important: most notably, this +/// extractor _must_ be used before any extractor that accesses the `Request` or +/// `RequestParts`. +#[derive(Debug)] +pub struct AdminUser(pub models::user::AdminUser); + +#[async_trait] +impl FromRequestParts for AdminUser { + type Rejection = BoxedAppError; + + async fn from_request_parts( + parts: &mut Parts, + state: &AppState, + ) -> Result { + let app = State::::from_request_parts(parts, state) + .await + .expect("accessing AppState"); + let conn = &mut *app.db_read_prefer_primary()?; + + // TODO: allow other authentication methods, such as tokens. + // TODO: allow token scopes to be required. + Ok(Self( + AuthCheck::only_cookie() + .check(parts, conn)? + .user() + .admin()?, + )) + } +} diff --git a/src/lib.rs b/src/lib.rs index b5ecbe055cb..4c4473c73ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -58,6 +58,7 @@ pub mod worker; pub mod auth; pub mod controllers; +pub mod extractors; pub mod models; mod router; pub mod sentry; diff --git a/src/views.rs b/src/views.rs index b0c6cd461f2..dab4022314e 100644 --- a/src/views.rs +++ b/src/views.rs @@ -9,6 +9,8 @@ use crate::models::{ }; use crate::util::rfc3339; +pub mod admin; + pub mod krate_publish; pub use self::krate_publish::{EncodableCrateDependency, EncodableCrateUpload}; diff --git a/src/views/admin.rs b/src/views/admin.rs new file mode 100644 index 00000000000..8116a00689a --- /dev/null +++ b/src/views/admin.rs @@ -0,0 +1 @@ +pub mod krates; diff --git a/src/views/admin/krates.rs b/src/views/admin/krates.rs new file mode 100644 index 00000000000..620000872c3 --- /dev/null +++ b/src/views/admin/krates.rs @@ -0,0 +1,18 @@ +use chrono::NaiveDateTime; +use serde::Serializer; + +#[derive(Serialize)] +pub struct CrateVersion { + pub id: i32, + pub name: String, + pub num: String, + #[serde(serialize_with = "format_date_time")] + pub created_at: NaiveDateTime, +} + +fn format_date_time(time: &NaiveDateTime, ser: S) -> Result +where + S: Serializer, +{ + ser.serialize_str(&time.format("%a %-d %b %Y %H:%M:%S").to_string()) +} From 1c09e9c2f119215b40475ba26f45cd074d16c734 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Tue, 11 Apr 2023 21:19:13 -0700 Subject: [PATCH 05/11] Minimal caddy setup. --- .ember-cli | 3 ++- Caddyfile | 8 ++++++++ config/nginx.conf.erb | 4 ++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 Caddyfile diff --git a/.ember-cli b/.ember-cli index ee64cfed2a8..fdc746c3cc4 100644 --- a/.ember-cli +++ b/.ember-cli @@ -5,5 +5,6 @@ Setting `disableAnalytics` to true will prevent any data from being sent. */ - "disableAnalytics": false + "disableAnalytics": false, + "port": 4201 } diff --git a/Caddyfile b/Caddyfile new file mode 100644 index 00000000000..63fe8535083 --- /dev/null +++ b/Caddyfile @@ -0,0 +1,8 @@ +http://localhost:4200, http://127.0.0.1:4200 { + @backend { + path /api/* /admin/* /git/* + } + + reverse_proxy @backend 127.0.0.1:8888 + reverse_proxy 127.0.0.1:4201 +} diff --git a/config/nginx.conf.erb b/config/nginx.conf.erb index 5afcefc59a0..356dc8687ac 100644 --- a/config/nginx.conf.erb +++ b/config/nginx.conf.erb @@ -237,6 +237,10 @@ http { <% if ENV['USE_FASTBOOT'] == "staging-experimental" %> # Experimentally send all non-backend requests to FastBoot + location /admin/ { + proxy_pass http://app_server; + } + location /api/ { proxy_pass http://app_server; } From d8c7360e83bed8501ff970d5465e329065f6ce00 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Wed, 12 Apr 2023 11:31:03 -0700 Subject: [PATCH 06/11] Various admin template improvements. --- admin/templates/crates.hbs | 11 +++- src/app.rs | 35 +--------- src/controllers/admin.rs | 25 ++++--- src/views/admin.rs | 1 + src/views/admin/krates.rs | 2 + src/views/admin/templating.rs | 121 ++++++++++++++++++++++++++++++++++ 6 files changed, 153 insertions(+), 42 deletions(-) create mode 100644 src/views/admin/templating.rs diff --git a/admin/templates/crates.hbs b/admin/templates/crates.hbs index 7048c1b23f9..e8913212e3f 100644 --- a/admin/templates/crates.hbs +++ b/admin/templates/crates.hbs @@ -5,6 +5,7 @@ Crate Version Published at + Published by @@ -13,10 +14,18 @@ - + {{this.num}} {{this.created_at}} + + + {{#if this.published_by_avatar}} + + {{~/if~}} + {{~this.published_by_username}} + + {{/each}} diff --git a/src/app.rs b/src/app.rs index ea73e5e95ba..1e7a4ad42fc 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,6 +1,7 @@ //! Application-wide components in a struct accessible from each request use crate::db::{ConnectionConfig, DieselPool, DieselPooledConn, PoolError}; +use crate::views::admin::templating; use crate::{config, Env}; use std::ops::Deref; use std::sync::atomic::AtomicUsize; @@ -17,7 +18,6 @@ use handlebars::Handlebars; use moka::future::{Cache, CacheBuilder}; use oauth2::basic::BasicClient; use reqwest::blocking::Client; -use rust_embed::RustEmbed; use scheduled_thread_pool::ScheduledThreadPool; /// The `App` struct holds the main components of the application like @@ -181,9 +181,8 @@ impl App { _ => None, }; - let admin_engine = Engine::from( - templating::registry().expect("could not initialise admin template engine"), - ); + let admin_engine = + templating::engine().expect("could not initialise admin templating engine"); App { primary_database, @@ -311,31 +310,3 @@ impl FromRef for cookie::Key { app.session_key().clone() } } - -mod templating { - use handlebars::{Handlebars, TemplateError}; - - #[cfg(not(debug_assertions))] - #[derive(rust_embed::RustEmbed)] - #[folder = "admin/templates/"] - struct Assets; - - pub(super) fn registry() -> Result, TemplateError> { - let mut hbs = Handlebars::new(); - hbs.set_strict_mode(true); - - #[cfg(debug_assertions)] - { - hbs.set_dev_mode(true); - hbs.register_templates_directory(".hbs", "admin/templates")?; - } - - #[cfg(not(debug_assertions))] - { - // TODO: fix to match names by actually walking the embed. - hbs.register_embed_templates::()?; - } - - Ok(hbs) - } -} diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index cfcea67a38d..dbdba205cda 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -7,7 +7,7 @@ use super::prelude::*; /// Handles the `GET /admin/` route. pub async fn index(app: AppState, _user: AdminUser) -> AppResult { conduit_compat(move || { - use crate::schema::{crates, versions}; + use crate::schema::{crates, users, versions}; let conn = &mut *app.db_read()?; @@ -19,24 +19,31 @@ pub async fn index(app: AppState, _user: AdminUser) -> AppResult = versions::table .inner_join(crates::table) + .inner_join(users::table) .select(( versions::id, versions::num, versions::created_at, crates::name, + users::gh_login, + users::gh_avatar, )) .order(versions::created_at.desc()) .limit(50) .load(conn)? .into_iter() - .map(|(id, num, created_at, name)| -> CrateVersion { - CrateVersion { - id, - num, - created_at, - name, - } - }) + .map( + |(id, num, created_at, name, published_by_username, published_by_avatar)| { + CrateVersion { + id, + num, + created_at, + name, + published_by_username, + published_by_avatar, + } + }, + ) .collect(); Ok(RenderHtml( diff --git a/src/views/admin.rs b/src/views/admin.rs index 8116a00689a..e8a986665ab 100644 --- a/src/views/admin.rs +++ b/src/views/admin.rs @@ -1 +1,2 @@ pub mod krates; +pub mod templating; diff --git a/src/views/admin/krates.rs b/src/views/admin/krates.rs index 620000872c3..fec855a792c 100644 --- a/src/views/admin/krates.rs +++ b/src/views/admin/krates.rs @@ -8,6 +8,8 @@ pub struct CrateVersion { pub num: String, #[serde(serialize_with = "format_date_time")] pub created_at: NaiveDateTime, + pub published_by_username: String, + pub published_by_avatar: Option, } fn format_date_time(time: &NaiveDateTime, ser: S) -> Result diff --git a/src/views/admin/templating.rs b/src/views/admin/templating.rs new file mode 100644 index 00000000000..7730b2d639f --- /dev/null +++ b/src/views/admin/templating.rs @@ -0,0 +1,121 @@ +use std::string::FromUtf8Error; + +use axum_template::engine::Engine; +use handlebars::{handlebars_helper, Handlebars, TemplateError}; +use thiserror::Error; + +#[derive(rust_embed::RustEmbed)] +#[folder = "admin/templates/"] +struct Assets; + +pub fn engine() -> Result>, Error> { + let mut hbs = Handlebars::new(); + hbs.set_strict_mode(true); + + #[cfg(debug_assertions)] + configure_handlebars_debug(&mut hbs)?; + + #[cfg(not(debug_assertions))] + configure_handlebars_release(&mut hbs)?; + + handlebars_helper!(crate_index_path: |name: str| helpers::crate_index_path(name)); + hbs.register_helper("crate-index-path", Box::new(crate_index_path)); + + Ok(Engine::from(hbs)) +} + +#[allow(dead_code)] +fn configure_handlebars_debug(hbs: &mut Handlebars<'_>) -> Result<(), Error> { + hbs.set_dev_mode(true); + hbs.register_templates_directory(".hbs", "admin/templates")?; + Ok(()) +} + +#[allow(dead_code)] +fn configure_handlebars_release(hbs: &mut Handlebars<'_>) -> Result<(), Error> { + // RustEmbed doesn't strip the `.hbs` extension from each template when + // using `Handlebars::register_embed_templates`, whereas + // `Handlebars::register_templates_directory` does. We'll walk the assets + // and register them individually to strip the extensions. + // + // In theory, we could do it the other way around in debug mode, but then + // the dev mode auto-reload wouldn't discover new templates without + // restarting the service. + for file in Assets::iter() { + let content = String::from_utf8( + Assets::get(&file) + .ok_or_else(|| Error::MissingFile(file.to_string()))? + .data + .into_owned(), + ) + .map_err(|e| Error::MalformedContent(file.to_string(), e))?; + + let name = file + .strip_suffix(".hbs") + .ok_or_else(|| Error::UnexpectedExtension(file.to_string()))?; + + hbs.register_template_string(name, content)?; + } + + Ok(()) +} + +#[derive(Debug, Error)] +pub enum Error { + #[error("template {0} content is not valid UTF-8")] + MalformedContent(String, #[source] FromUtf8Error), + + #[error("missing template file {0}")] + MissingFile(String), + + #[error(transparent)] + Template(#[from] Box), + + #[error("template file {0} does not have the expected .hbs extension")] + UnexpectedExtension(String), +} + +impl From for Error { + fn from(value: TemplateError) -> Self { + // TemplateError is big, so we box it to avoid clippy warnings. + Self::Template(Box::new(value)) + } +} + +mod helpers { + use cargo_registry_index::Repository; + + pub(super) fn crate_index_path(name: &str) -> String { + String::from( + Repository::relative_index_file(name) + .to_str() + .expect("invalid UTF-8 in crate name"), + ) + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + use anyhow::Error; + + #[test] + fn template_names_match_in_debug_and_release_modes() -> Result<(), Error> { + let mut debug = Handlebars::new(); + debug.set_strict_mode(true); + configure_handlebars_debug(&mut debug)?; + + let mut release = Handlebars::new(); + release.set_strict_mode(true); + configure_handlebars_release(&mut release)?; + + let debug_templates: HashSet<&String> = debug.get_templates().keys().collect(); + let release_templates: HashSet<&String> = release.get_templates().keys().collect(); + + assert_eq!(debug_templates, release_templates); + + Ok(()) + } +} From 9db3203819bc09192ebda2ce663131f1600f33ad Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Wed, 12 Apr 2023 16:08:45 -0700 Subject: [PATCH 07/11] Improve admin display and code layout. --- Cargo.lock | 10 ++++++ Cargo.toml | 1 + admin/templates/components/datetime.hbs | 1 + admin/templates/components/user.hbs | 13 ++++++++ admin/templates/crates.hbs | 13 +++----- src/controllers/admin.rs | 11 ++++--- src/views/admin/krates.rs | 16 ++-------- src/views/admin/templating.rs | 15 ++------- src/views/admin/templating/components.rs | 6 ++++ .../admin/templating/components/datetime.rs | 31 +++++++++++++++++++ src/views/admin/templating/components/user.rs | 11 +++++++ src/views/admin/templating/helpers.rs | 9 ++++++ 12 files changed, 99 insertions(+), 38 deletions(-) create mode 100644 admin/templates/components/datetime.hbs create mode 100644 admin/templates/components/user.hbs create mode 100644 src/views/admin/templating/components.rs create mode 100644 src/views/admin/templating/components/datetime.rs create mode 100644 src/views/admin/templating/components/user.rs create mode 100644 src/views/admin/templating/helpers.rs diff --git a/Cargo.lock b/Cargo.lock index 3c634b5a6cf..eb42b0c42f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -465,6 +465,7 @@ dependencies = [ "cargo-registry-markdown", "cargo-registry-s3", "chrono", + "chrono-human-duration", "claims", "clap", "cookie", @@ -603,6 +604,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "chrono-human-duration" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "19283df3a144dfdeee6568e42ad7f0939d3c261bcdfe954b1a1098f6f7c1b908" +dependencies = [ + "chrono", +] + [[package]] name = "cipher" version = "0.4.4" diff --git a/Cargo.toml b/Cargo.toml index 41025283943..33d7371ef01 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ cargo-registry-index = { path = "cargo-registry-index" } cargo-registry-markdown = { path = "cargo-registry-markdown" } cargo-registry-s3 = { path = "cargo-registry-s3" } chrono = { version = "=0.4.24", features = ["serde"] } +chrono-human-duration = "0.1.1" clap = { version = "=4.2.3", features = ["derive", "env", "unicode", "wrap_help"] } cookie = { version = "=0.17.0", features = ["secure"] } dashmap = { version = "=5.4.0", features = ["raw-api"] } diff --git a/admin/templates/components/datetime.hbs b/admin/templates/components/datetime.hbs new file mode 100644 index 00000000000..eeaee423825 --- /dev/null +++ b/admin/templates/components/datetime.hbs @@ -0,0 +1 @@ +{{this.human}} \ No newline at end of file diff --git a/admin/templates/components/user.hbs b/admin/templates/components/user.hbs new file mode 100644 index 00000000000..4d5c3d6e169 --- /dev/null +++ b/admin/templates/components/user.hbs @@ -0,0 +1,13 @@ + + {{#if this.avatar}} + + {{~/if~}} + {{~this.username}} + \ No newline at end of file diff --git a/admin/templates/crates.hbs b/admin/templates/crates.hbs index e8913212e3f..e50baa81325 100644 --- a/admin/templates/crates.hbs +++ b/admin/templates/crates.hbs @@ -4,8 +4,7 @@ Crate Version - Published at - Published by + Published @@ -17,14 +16,10 @@ {{this.num}} - {{this.created_at}} - - {{#if this.published_by_avatar}} - - {{~/if~}} - {{~this.published_by_username}} - + {{> components/datetime this.created_at}} + by + {{> components/user this.publisher }} diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index dbdba205cda..224e9886ddf 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -1,6 +1,7 @@ use axum_template::RenderHtml; +use chrono::NaiveDateTime; -use crate::{extractors::admin::AdminUser, views::admin::krates::CrateVersion}; +use crate::{extractors::admin::AdminUser, views::admin::{krates::CrateVersion, templating::components}}; use super::prelude::*; @@ -37,10 +38,12 @@ pub async fn index(app: AppState, _user: AdminUser) -> AppResult, -} - -fn format_date_time(time: &NaiveDateTime, ser: S) -> Result -where - S: Serializer, -{ - ser.serialize_str(&time.format("%a %-d %b %Y %H:%M:%S").to_string()) + pub created_at: DateTime, + pub publisher: User, } diff --git a/src/views/admin/templating.rs b/src/views/admin/templating.rs index 7730b2d639f..6eacabe4ae0 100644 --- a/src/views/admin/templating.rs +++ b/src/views/admin/templating.rs @@ -4,6 +4,9 @@ use axum_template::engine::Engine; use handlebars::{handlebars_helper, Handlebars, TemplateError}; use thiserror::Error; +pub mod components; +mod helpers; + #[derive(rust_embed::RustEmbed)] #[folder = "admin/templates/"] struct Assets; @@ -82,18 +85,6 @@ impl From for Error { } } -mod helpers { - use cargo_registry_index::Repository; - - pub(super) fn crate_index_path(name: &str) -> String { - String::from( - Repository::relative_index_file(name) - .to_str() - .expect("invalid UTF-8 in crate name"), - ) - } -} - #[cfg(test)] mod tests { use std::collections::HashSet; diff --git a/src/views/admin/templating/components.rs b/src/views/admin/templating/components.rs new file mode 100644 index 00000000000..8f84b51116d --- /dev/null +++ b/src/views/admin/templating/components.rs @@ -0,0 +1,6 @@ +mod datetime; +pub use datetime::DateTime; + +mod user; +pub use user::User; + diff --git a/src/views/admin/templating/components/datetime.rs b/src/views/admin/templating/components/datetime.rs new file mode 100644 index 00000000000..e8a6e1fcdcf --- /dev/null +++ b/src/views/admin/templating/components/datetime.rs @@ -0,0 +1,31 @@ +use chrono::{NaiveDateTime, Utc}; +use chrono_human_duration::ChronoHumanDuration; +use serde::{ser::SerializeMap, Serialize}; + +#[derive(Debug, Clone, Copy)] +pub struct DateTime(NaiveDateTime); + +impl From for DateTime { + fn from(value: NaiveDateTime) -> Self { + Self(value) + } +} + +impl Serialize for DateTime { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let mut map = serializer.serialize_map(Some(2))?; + map.serialize_entry("absolute", &format_time(self.0))?; + map.serialize_entry( + "human", + &(Utc::now().naive_utc() - self.0).format_human().to_string(), + )?; + map.end() + } +} + +fn format_time(time: NaiveDateTime) -> String { + time.format("%a %-d %b %Y %H:%M:%S").to_string() +} diff --git a/src/views/admin/templating/components/user.rs b/src/views/admin/templating/components/user.rs new file mode 100644 index 00000000000..d041e8f4f7b --- /dev/null +++ b/src/views/admin/templating/components/user.rs @@ -0,0 +1,11 @@ +#[derive(Debug, Clone, Serialize)] +pub struct User { + avatar: Option, + username: String, +} + +impl User { + pub fn new(username: String, avatar: Option) -> Self { + Self { avatar, username } + } +} diff --git a/src/views/admin/templating/helpers.rs b/src/views/admin/templating/helpers.rs new file mode 100644 index 00000000000..362301c9ab5 --- /dev/null +++ b/src/views/admin/templating/helpers.rs @@ -0,0 +1,9 @@ +use cargo_registry_index::Repository; + +pub(super) fn crate_index_path(name: &str) -> String { + String::from( + Repository::relative_index_file(name) + .to_str() + .expect("invalid UTF-8 in crate name"), + ) +} From 7edcc1abe3f2681b17eacebe9cc8820553702859 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Wed, 12 Apr 2023 18:51:42 -0700 Subject: [PATCH 08/11] More admin improvements. --- Cargo.lock | 10 --- admin/templates/crates.hbs | 66 +++++++++++++- admin/templates/page.hbs | 22 ++--- src/controllers/admin.rs | 85 +++++++++---------- src/controllers/version/yank.rs | 2 +- src/models/krate.rs | 2 +- src/models/user.rs | 2 +- src/models/version.rs | 2 +- src/router.rs | 3 +- src/views/admin.rs | 2 +- src/views/admin/crates.rs | 41 +++++++++ src/views/admin/krates.rs | 10 --- .../admin/templating/components/datetime.rs | 75 ++++++++++++++-- src/views/admin/templating/components/user.rs | 13 ++- 14 files changed, 243 insertions(+), 92 deletions(-) create mode 100644 src/views/admin/crates.rs delete mode 100644 src/views/admin/krates.rs diff --git a/Cargo.lock b/Cargo.lock index eb42b0c42f2..3c634b5a6cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -465,7 +465,6 @@ dependencies = [ "cargo-registry-markdown", "cargo-registry-s3", "chrono", - "chrono-human-duration", "claims", "clap", "cookie", @@ -604,15 +603,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "chrono-human-duration" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19283df3a144dfdeee6568e42ad7f0939d3c261bcdfe954b1a1098f6f7c1b908" -dependencies = [ - "chrono", -] - [[package]] name = "cipher" version = "0.4.4" diff --git a/admin/templates/crates.hbs b/admin/templates/crates.hbs index e50baa81325..f3fc6564834 100644 --- a/admin/templates/crates.hbs +++ b/admin/templates/crates.hbs @@ -1,4 +1,11 @@ {{#*inline "body"}} +
+

{{> title}}

+
+ +
+
+ @@ -12,7 +19,16 @@ {{#each this}} @@ -21,11 +37,57 @@ by {{> components/user this.publisher }} - + {{/each}}
- + {{this.num}} + {{#if this.yanked}} + + {{else}} + + {{/if}} +
+ + {{/inline}} {{#*inline "title"}}Crates{{/inline}} diff --git a/admin/templates/page.hbs b/admin/templates/page.hbs index a846e3c67b2..61ff0f0242f 100644 --- a/admin/templates/page.hbs +++ b/admin/templates/page.hbs @@ -5,26 +5,26 @@ {{> title}} :: crates.io +
-

{{> title}}

{{> body}}
diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index 224e9886ddf..16e3a628fa3 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -1,59 +1,56 @@ -use axum_template::RenderHtml; -use chrono::NaiveDateTime; - -use crate::{extractors::admin::AdminUser, views::admin::{krates::CrateVersion, templating::components}}; - use super::prelude::*; +use axum::extract::Query; + +use crate::{ + extractors::admin::AdminUser, + models::{Crate, User, Version}, + views::admin::crates::{render_versions, CrateVersion}, +}; /// Handles the `GET /admin/` route. -pub async fn index(app: AppState, _user: AdminUser) -> AppResult { +pub async fn index(_user: AdminUser) -> impl IntoResponse { + redirect("/admin/crates/".to_string()) +} + +#[derive(Deserialize)] +pub struct CrateQuery { + q: Option, + page: Option, +} + +/// Handles the `GET /admin/crates/` route. +pub async fn crates( + app: AppState, + q: Query, + _user: AdminUser, +) -> AppResult { + const PER_PAGE: i64 = 50; + conduit_compat(move || { use crate::schema::{crates, users, versions}; let conn = &mut *app.db_read()?; - // TODO: move to a new controller and redirect when hitting /admin/. - // TODO: refactor into something that's not a spaghetti query. - // TODO: pagination. - // TODO: search. - - // XXX: can we send an iterator to RenderHtml? - let recent_versions: Vec = versions::table + let mut query = versions::table .inner_join(crates::table) .inner_join(users::table) - .select(( - versions::id, - versions::num, - versions::created_at, - crates::name, - users::gh_login, - users::gh_avatar, - )) + .select((Version::as_select(), Crate::as_select(), User::as_select())) .order(versions::created_at.desc()) - .limit(50) - .load(conn)? + .limit(PER_PAGE) + .offset(PER_PAGE * q.page.unwrap_or(0) as i64) + .into_boxed(); + + if let Some(q) = &q.q { + // TODO: this is overly simplistic. + query = query.filter(crates::name.ilike(format!("%{}%", q))); + } + + let recent_versions = query + .load::<(Version, Crate, User)>(conn)? .into_iter() - .map( - |(id, num, created_at, name, published_by_username, published_by_avatar)| { - CrateVersion { - id, - num, - created_at: NaiveDateTime::into(created_at), - name, - publisher: components::User::new( - published_by_username, - published_by_avatar, - ), - } - }, - ) - .collect(); - - Ok(RenderHtml( - "crates", - app.admin_engine.clone(), - recent_versions, - )) + .map(|(version, krate, user)| CrateVersion::new(version, krate, user)); + + Ok(render_versions(&app.admin_engine, recent_versions)) }) .await } diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 45313ee93a8..51571f5cfdf 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -63,7 +63,7 @@ fn modify_yank( let user = auth.user(); let owners = krate.owners(conn)?; - if user.rights(state, &owners)? < Rights::Publish { + if user.rights(state, &owners)? < Rights::Publish && user.admin().is_err() { return Err(cargo_err("must already be an owner to yank or unyank")); } diff --git a/src/models/krate.rs b/src/models/krate.rs index e4b1d540b60..9dca18912ee 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -28,7 +28,7 @@ pub struct RecentCrateDownloads { pub downloads: i32, } -#[derive(Debug, Clone, Queryable, Identifiable, AsChangeset, QueryableByName)] +#[derive(Debug, Clone, Queryable, Identifiable, AsChangeset, QueryableByName, Selectable)] #[diesel(table_name = crates)] pub struct Crate { pub id: i32, diff --git a/src/models/user.rs b/src/models/user.rs index a655381d6e5..26da42eaaba 100644 --- a/src/models/user.rs +++ b/src/models/user.rs @@ -12,7 +12,7 @@ use crate::models::{ use crate::schema::{crate_owners, emails, users}; /// The model representing a row in the `users` database table. -#[derive(Clone, Debug, PartialEq, Eq, Queryable, Identifiable, AsChangeset)] +#[derive(Clone, Debug, PartialEq, Eq, Queryable, Identifiable, AsChangeset, Selectable)] pub struct User { pub id: i32, pub gh_access_token: String, diff --git a/src/models/version.rs b/src/models/version.rs index d493f304289..7c29e558fc5 100644 --- a/src/models/version.rs +++ b/src/models/version.rs @@ -9,7 +9,7 @@ use crate::models::{Crate, Dependency, User}; use crate::schema::*; // Queryable has a custom implementation below -#[derive(Clone, Identifiable, Associations, Debug, Queryable, Deserialize, Serialize)] +#[derive(Clone, Identifiable, Associations, Debug, Queryable, Selectable, Deserialize, Serialize)] #[diesel(belongs_to(Crate))] pub struct Version { pub id: i32, diff --git a/src/router.rs b/src/router.rs index 50025c6460a..6fe68037cf5 100644 --- a/src/router.rs +++ b/src/router.rs @@ -158,7 +158,8 @@ pub fn build_axum_router(state: AppState) -> Router { post(github::secret_scanning::verify), ) // Admin console - .route("/admin/", get(admin::index)); + .route("/admin/", get(admin::index)) + .route("/admin/crates/", get(admin::crates)); // Only serve the local checkout of the git index in development mode. // In production, for crates.io, cargo gets the index from diff --git a/src/views/admin.rs b/src/views/admin.rs index e8a986665ab..18b5115b530 100644 --- a/src/views/admin.rs +++ b/src/views/admin.rs @@ -1,2 +1,2 @@ -pub mod krates; +pub mod crates; pub mod templating; diff --git a/src/views/admin/crates.rs b/src/views/admin/crates.rs new file mode 100644 index 00000000000..70174c4dc57 --- /dev/null +++ b/src/views/admin/crates.rs @@ -0,0 +1,41 @@ +use axum::response::IntoResponse; +use axum_template::{engine::Engine, RenderHtml}; +use handlebars::Handlebars; + +use crate::models::{Crate, User, Version}; + +use super::templating::components; + +#[derive(Serialize)] +pub struct CrateVersion { + pub id: i32, + pub name: String, + pub num: String, + pub created_at: components::DateTime, + pub publisher: components::User, + pub yanked: bool, +} + +impl CrateVersion { + pub fn new(version: Version, krate: Crate, user: User) -> Self { + Self { + id: version.id, + name: krate.name, + num: version.num, + created_at: version.created_at.into(), + publisher: user.into(), + yanked: version.yanked, + } + } +} + +pub fn render_versions( + engine: &Engine>, + iter: impl Iterator, +) -> impl IntoResponse { + RenderHtml( + "crates", + engine.clone(), + iter.collect::>(), + ) +} diff --git a/src/views/admin/krates.rs b/src/views/admin/krates.rs deleted file mode 100644 index 71fb3a01a84..00000000000 --- a/src/views/admin/krates.rs +++ /dev/null @@ -1,10 +0,0 @@ -use super::templating::components::{DateTime, User}; - -#[derive(Serialize)] -pub struct CrateVersion { - pub id: i32, - pub name: String, - pub num: String, - pub created_at: DateTime, - pub publisher: User, -} diff --git a/src/views/admin/templating/components/datetime.rs b/src/views/admin/templating/components/datetime.rs index e8a6e1fcdcf..77e01b51d79 100644 --- a/src/views/admin/templating/components/datetime.rs +++ b/src/views/admin/templating/components/datetime.rs @@ -1,5 +1,4 @@ -use chrono::{NaiveDateTime, Utc}; -use chrono_human_duration::ChronoHumanDuration; +use chrono::{Duration, NaiveDateTime, Utc}; use serde::{ser::SerializeMap, Serialize}; #[derive(Debug, Clone, Copy)] @@ -18,10 +17,7 @@ impl Serialize for DateTime { { let mut map = serializer.serialize_map(Some(2))?; map.serialize_entry("absolute", &format_time(self.0))?; - map.serialize_entry( - "human", - &(Utc::now().naive_utc() - self.0).format_human().to_string(), - )?; + map.serialize_entry("human", &format_duration(Utc::now().naive_utc() - self.0))?; map.end() } } @@ -29,3 +25,70 @@ impl Serialize for DateTime { fn format_time(time: NaiveDateTime) -> String { time.format("%a %-d %b %Y %H:%M:%S").to_string() } + +fn format_duration(duration: Duration) -> String { + // This originally delegated out to chrono-human-duration or + // pretty-duration, but the former had bugs around handling plurals and the + // latter could only handle std::time::Duration and didn't have any options + // to not include every unit. What we really need is so simple that it's + // easier to just implement it here. + let abs = if duration < Duration::zero() { + -duration + } else { + duration + }; + let adverb = adverb(duration); + + match ( + abs.num_weeks(), + abs.num_days(), + abs.num_hours(), + abs.num_minutes(), + abs.num_seconds(), + ) { + (_, days, _, _, _) if days >= 365 => { + // Technically, leap years exist. Practically, it doesn't matter for + // a fuzzy duration anyway. + format!("{} year{} {}", days / 365, plural(days / 365), adverb) + } + (_, days, _, _, _) if days >= 30 => { + // Same for months, honestly. + format!("{} month{} {}", days / 30, plural(days / 30), adverb) + } + (weeks, _, _, _, _) if weeks > 0 => { + format!("{} week{} {}", weeks, plural(weeks), adverb) + } + (_, days, _, _, _) if days > 0 => { + format!("{} day{} {}", days, plural(days), adverb) + } + (_, _, hours, _, _) if hours > 0 => { + format!("{} hour{} {}", hours, plural(hours), adverb) + } + (_, _, _, minutes, _) if minutes > 0 => { + format!("{} minute{} {}", minutes, plural(minutes), adverb) + } + (_, _, _, seconds, _) if seconds > 0 => { + format!("{} second{} {}", seconds, plural(seconds), adverb) + } + _ => String::from("just now"), + } +} + +fn plural(value: i64) -> &'static str { + if value == 1 { + "" + } else { + "s" + } +} + +fn adverb(duration: Duration) -> &'static str { + // These durations are technically reversed; see the code in + // `DateTime::serialize` for the full details of how the durations are + // calculated. + if duration > Duration::zero() { + "ago" + } else { + "from now" + } +} diff --git a/src/views/admin/templating/components/user.rs b/src/views/admin/templating/components/user.rs index d041e8f4f7b..481b516a0a3 100644 --- a/src/views/admin/templating/components/user.rs +++ b/src/views/admin/templating/components/user.rs @@ -1,11 +1,18 @@ +use crate::models; + #[derive(Debug, Clone, Serialize)] pub struct User { + id: i32, avatar: Option, username: String, } -impl User { - pub fn new(username: String, avatar: Option) -> Self { - Self { avatar, username } +impl From for User { + fn from(value: models::User) -> Self { + Self { + id: value.id, + avatar: value.gh_avatar, + username: value.gh_login, + } } } From 6a98a3dae7632255c3d45b652d1b22b944d1194d Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Mon, 17 Apr 2023 17:01:35 -0700 Subject: [PATCH 09/11] Rough WIP pre-pagination. --- admin/templates/crates.hbs | 6 +++--- src/controllers/admin.rs | 42 ++++++++++++++++++++++++++++---------- src/views/admin/crates.rs | 35 +++++++++++++++++++++---------- 3 files changed, 58 insertions(+), 25 deletions(-) diff --git a/admin/templates/crates.hbs b/admin/templates/crates.hbs index f3fc6564834..b22d6e50df0 100644 --- a/admin/templates/crates.hbs +++ b/admin/templates/crates.hbs @@ -2,7 +2,7 @@

{{> title}}

- +
@@ -16,7 +16,7 @@ - {{#each this}} + {{#each this.versions}}
@@ -37,7 +37,7 @@ by {{> components/user this.publisher }} - + {{#if this.yanked}} {{else}} diff --git a/src/controllers/admin.rs b/src/controllers/admin.rs index 16e3a628fa3..1eeafcd9e67 100644 --- a/src/controllers/admin.rs +++ b/src/controllers/admin.rs @@ -1,10 +1,12 @@ use super::prelude::*; use axum::extract::Query; +use diesel::sql_types::Text; +use diesel_full_text_search::{TsQuery, TsQueryExtensions}; use crate::{ extractors::admin::AdminUser, models::{Crate, User, Version}, - views::admin::crates::{render_versions, CrateVersion}, + views::admin::crates::render, }; /// Handles the `GET /admin/` route. @@ -18,6 +20,15 @@ pub struct CrateQuery { page: Option, } +impl CrateQuery { + fn query_string(&self) -> Option<&str> { + match &self.q { + Some(q) if !q.is_empty() => Some(q.as_str()), + _ => None, + } + } +} + /// Handles the `GET /admin/crates/` route. pub async fn crates( app: AppState, @@ -28,29 +39,38 @@ pub async fn crates( conduit_compat(move || { use crate::schema::{crates, users, versions}; + use diesel::dsl::*; let conn = &mut *app.db_read()?; let mut query = versions::table .inner_join(crates::table) .inner_join(users::table) - .select((Version::as_select(), Crate::as_select(), User::as_select())) .order(versions::created_at.desc()) .limit(PER_PAGE) .offset(PER_PAGE * q.page.unwrap_or(0) as i64) + .select((Version::as_select(), Crate::as_select(), User::as_select())) .into_boxed(); - if let Some(q) = &q.q { - // TODO: this is overly simplistic. - query = query.filter(crates::name.ilike(format!("%{}%", q))); + if let Some(q_string) = q.query_string() { + // FIXME: this is stolen from the public search controller, and + // should be refactored into a common helper. + let q = sql::("plainto_tsquery('english', ") + .bind::(q_string) + .sql(")"); + query = query + .filter( + q.clone() + .matches(crates::textsearchable_index_col) + .or(Crate::loosly_matches_name(q_string)), + ) + .select((Version::as_select(), Crate::as_select(), User::as_select())) + .order(Crate::with_name(q_string).desc()) + .then_order_by(versions::created_at.desc()); } - let recent_versions = query - .load::<(Version, Crate, User)>(conn)? - .into_iter() - .map(|(version, krate, user)| CrateVersion::new(version, krate, user)); - - Ok(render_versions(&app.admin_engine, recent_versions)) + let recent_versions = query.load::<(Version, Crate, User)>(conn)?.into_iter(); + Ok(render(&app.admin_engine, q.query_string(), recent_versions)) }) .await } diff --git a/src/views/admin/crates.rs b/src/views/admin/crates.rs index 70174c4dc57..3f43235ab0c 100644 --- a/src/views/admin/crates.rs +++ b/src/views/admin/crates.rs @@ -7,17 +7,24 @@ use crate::models::{Crate, User, Version}; use super::templating::components; #[derive(Serialize)] -pub struct CrateVersion { - pub id: i32, - pub name: String, - pub num: String, - pub created_at: components::DateTime, - pub publisher: components::User, - pub yanked: bool, +struct View { + // TODO: pagination. + q: Option, + versions: Vec, +} + +#[derive(Serialize)] +struct CrateVersion { + id: i32, + name: String, + num: String, + created_at: components::DateTime, + publisher: components::User, + yanked: bool, } impl CrateVersion { - pub fn new(version: Version, krate: Crate, user: User) -> Self { + fn new(version: Version, krate: Crate, user: User) -> Self { Self { id: version.id, name: krate.name, @@ -29,13 +36,19 @@ impl CrateVersion { } } -pub fn render_versions( +pub fn render( engine: &Engine>, - iter: impl Iterator, + q: Option<&str>, + version_iter: impl Iterator, ) -> impl IntoResponse { RenderHtml( "crates", engine.clone(), - iter.collect::>(), + View { + q: q.map(|s| s.to_string()), + versions: version_iter + .map(|(version, krate, user)| CrateVersion::new(version, krate, user)) + .collect(), + }, ) } From 7b806776c354756e590151c79e8e4967f729616a Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Wed, 19 Apr 2023 12:28:22 -0700 Subject: [PATCH 10/11] Clean up Cargo.toml after rebase. --- Cargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 33d7371ef01..734c32445c2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,6 @@ cargo-registry-index = { path = "cargo-registry-index" } cargo-registry-markdown = { path = "cargo-registry-markdown" } cargo-registry-s3 = { path = "cargo-registry-s3" } chrono = { version = "=0.4.24", features = ["serde"] } -chrono-human-duration = "0.1.1" clap = { version = "=4.2.3", features = ["derive", "env", "unicode", "wrap_help"] } cookie = { version = "=0.17.0", features = ["secure"] } dashmap = { version = "=5.4.0", features = ["raw-api"] } @@ -41,7 +40,6 @@ dialoguer = "=0.10.4" diesel = { version = "=2.0.4", features = ["postgres", "serde_json", "chrono", "r2d2"] } diesel_full_text_search = "=2.0.0" diesel_migrations = { version = "=2.0.0", features = ["postgres"] } -diesel = { version = "=2.0.3", features = ["postgres", "serde_json", "chrono", "r2d2"] } dotenv = "=0.15.0" flate2 = "=1.0.25" futures-channel = { version = "=0.3.28", default-features = false } From 988d0fc6569d4d7e9ccfa45cc874e6bdf7672913 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Wed, 19 Apr 2023 16:40:12 -0700 Subject: [PATCH 11/11] Implement pagination. --- admin/templates/components/page.hbs | 27 +++++++ admin/templates/crates.hbs | 2 + src/controllers/admin.rs | 27 +++++-- src/controllers/helpers/pagination.rs | 75 +++++++++++++++++ src/views/admin/crates.rs | 13 ++- src/views/admin/templating/components.rs | 4 +- src/views/admin/templating/components/page.rs | 80 +++++++++++++++++++ 7 files changed, 216 insertions(+), 12 deletions(-) create mode 100644 admin/templates/components/page.hbs create mode 100644 src/views/admin/templating/components/page.rs diff --git a/admin/templates/components/page.hbs b/admin/templates/components/page.hbs new file mode 100644 index 00000000000..00f1abac0b1 --- /dev/null +++ b/admin/templates/components/page.hbs @@ -0,0 +1,27 @@ +{{#if this.paginated}} + +{{/if}} \ No newline at end of file diff --git a/admin/templates/crates.hbs b/admin/templates/crates.hbs index b22d6e50df0..2d654dc9178 100644 --- a/admin/templates/crates.hbs +++ b/admin/templates/crates.hbs @@ -49,6 +49,8 @@ + {{> components/page this.page}} +