Import type names instead of writing their paths (#355)

The style guide imports types and uses them unqualified. 222 places
wrote a path such as std::collections::HashMap or std::time::Duration
inline; each file now imports the type once, one use a module, and
names it bare. Done with a script, file by file, only where the name is
unambiguous there:
- one path for the name throughout the file, and not a name the file
  already binds otherwise (its own types, other imports, a function's
  local use);
- not a name generic enough to mislead bare (Error, Result, Value,
  Mutex, Client, Request, Response, Command, Event, Context, Map, Ok);
- only code a normal build compiles, so no import is unused outside
  tests or names a crate only tests depend on.

193 paths stay, those cases above. No behaviour change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-24 01:19:10 +00:00
parent d33d83043d
commit f22c1993b1
25 changed files with 307 additions and 246 deletions

View File

@@ -13,6 +13,7 @@ use serde::Deserialize;
use tracing::Instrument;
use crate::{Shared, SnapshotEntry};
use std::collections::{BTreeSet, HashSet};
/// What the charts are filed under, as a server's snapshot is under its token's name.
pub const INSTANCE: &str = "apple-podcasts";
@@ -83,7 +84,7 @@ async fn sync(dir: &Shared, client: &reqwest::Client, country: &str) -> Result<(
}
tokio::time::sleep(PAUSE).await;
}
let mut seen = std::collections::HashSet::new();
let mut seen = HashSet::new();
ids.retain(|id| seen.insert(id.clone()));
anyhow::ensure!(!ids.is_empty(), "no chart had a show in it");
@@ -150,9 +151,8 @@ fn differ(theirs: &BTreeMap<String, Vec<String>>, ours: &[(&str, &[&str])]) -> (
let flat = |c: &str, subs: &mut dyn Iterator<Item = String>| {
std::iter::once(c.to_string()).chain(subs.map(|s| format!("{c} / {s}"))).collect::<Vec<_>>()
};
let theirs: std::collections::BTreeSet<String> =
theirs.iter().flat_map(|(c, subs)| flat(c, &mut subs.iter().cloned())).collect();
let ours: std::collections::BTreeSet<String> =
let theirs: BTreeSet<String> = theirs.iter().flat_map(|(c, subs)| flat(c, &mut subs.iter().cloned())).collect();
let ours: BTreeSet<String> =
ours.iter().flat_map(|(c, subs)| flat(c, &mut subs.iter().map(|s| s.to_string()))).collect();
(theirs.difference(&ours).cloned().collect(), ours.difference(&theirs).cloned().collect())
}

View File

@@ -4,6 +4,8 @@
use anyhow::{Context, Result, bail};
use serde::Deserialize;
use std::collections::HashSet;
use std::fs::File;
use std::path::Path;
/// `directory.toml`. See docs/directory.md.
@@ -87,7 +89,7 @@ impl Config {
/// instances' snapshots together.
pub fn parse(text: &str) -> Result<Self> {
let cfg: Config = toml::from_str(text)?;
let mut seen = std::collections::HashSet::new();
let mut seen = HashSet::new();
for t in &cfg.tokens {
if t.name.trim().is_empty() {
bail!("a token has an empty name");
@@ -111,7 +113,7 @@ impl Config {
/// The tokens as the server checks them. A hash that is not 64 hex digits, or the same
/// hash under two names, is an error: which name a request is would be ambiguous.
pub fn tokens(&self) -> Result<Vec<Token>> {
let mut seen = std::collections::HashSet::new();
let mut seen = HashSet::new();
self.tokens
.iter()
.map(|t| {
@@ -148,9 +150,7 @@ pub fn new_token() -> String {
use std::io::Read;
let mut bytes = [0u8; 32];
// Falling back to the clock would make a guessable token. Better to fail.
std::fs::File::open("/dev/urandom")
.and_then(|mut f| f.read_exact(&mut bytes))
.expect("no source of randomness for a token");
File::open("/dev/urandom").and_then(|mut f| f.read_exact(&mut bytes)).expect("no source of randomness for a token");
format!("ipxd_{}", bytes.iter().map(|b| format!("{b:02x}")).collect::<String>())
}

View File

@@ -26,7 +26,13 @@ use axum::{Extension, Json, Router};
use serde::{Deserialize, Serialize};
use config::{Access, Token};
use reqwest::redirect::Policy;
use std::path::PathBuf;
use store::{Refreshed, Store};
use tokio::sync::Notify;
use tower_http::compression::CompressionLayer;
use tower_http::decompression::RequestDecompressionLayer;
use tracing::Instrument;
/// A feed as the directory lists it.
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -134,7 +140,7 @@ pub struct Directory {
tokens: RwLock<Vec<Token>>,
client: reqwest::Client,
/// Wakes the refresher, so a feed a snapshot brings in is fetched now, not in a minute.
wake: tokio::sync::Notify,
wake: Notify,
/// The public pages' copy of the list, and when it was read.
pages_list: tokio::sync::Mutex<Option<(Instant, Arc<Vec<Listing>>)>>,
}
@@ -155,9 +161,9 @@ impl Directory {
) -> Result<Shared, reqwest::Error> {
// No redirects: ipx_feed::fetch follows them itself, so it sees each hop.
let client = reqwest::Client::builder()
.connect_timeout(std::time::Duration::from_secs(10))
.connect_timeout(Duration::from_secs(10))
.read_timeout(ipx_feed::FEED_SILENCE)
.redirect(reqwest::redirect::Policy::none())
.redirect(Policy::none())
.user_agent(concat!("ipx-directory/", env!("CARGO_PKG_VERSION")))
// The feeds are named by whoever holds a write token, and redirect where their
// publishers like: nothing on this network is fetched unless listed (#329).
@@ -167,7 +173,7 @@ impl Directory {
store,
tokens: RwLock::new(tokens),
client,
wake: tokio::sync::Notify::new(),
wake: Notify::new(),
pages_list: Default::default(),
}))
}
@@ -239,14 +245,14 @@ pub async fn refresher(dir: Shared) {
if !due.is_empty() {
// A round is a trace, each feed's check a span in it (#213).
let round = tracing::info_span!("refresh", feeds = due.len());
tracing::Instrument::instrument(
Instrument::instrument(
async {
// A few at a time: in turn, two publishers timing out held a round
// for a minute and more (#227).
futures_util::StreamExt::for_each_concurrent(futures_util::stream::iter(&due), 5, |d| {
let check = tracing::info_span!("feed", feed = %d.id);
async {
if let Err(e) = tracing::Instrument::instrument(dir.refresh(d), check).await {
if let Err(e) = Instrument::instrument(dir.refresh(d), check).await {
tracing::warn!(feed = %d.id, error = %format!("{e:#}"), "could not store a feed check");
}
}
@@ -272,7 +278,7 @@ pub async fn refresher(dir: Shared) {
/// Reads the config file again whenever it changes, checked each minute. A file that fails to
/// load leaves the tokens as they were, so a typo locks nobody out.
pub async fn reloader(dir: Shared, path: std::path::PathBuf) {
pub async fn reloader(dir: Shared, path: PathBuf) {
let mtime = |p: &std::path::Path| std::fs::metadata(p).and_then(|m| m.modified()).ok();
let mut last = mtime(&path);
loop {
@@ -321,9 +327,9 @@ pub fn router(dir: Shared) -> Router {
// Brotli, zstd or gzip as the caller takes them: the feed list went to ipx as 603 KB of
// JSON, most of a Directory page's time (#253). Inside the access log, which records what
// was sent.
.layer(tower_http::compression::CompressionLayer::new().br(true).zstd(true).gzip(true))
.layer(CompressionLayer::new().br(true).zstd(true).gzip(true))
// And takes a request's body gzipped: ipx sends its snapshot so, about 100 KB as JSON.
.layer(tower_http::decompression::RequestDecompressionLayer::new().gzip(true))
.layer(RequestDecompressionLayer::new().gzip(true))
.layer(axum::middleware::from_fn(access_log))
.with_state(dir)
}
@@ -368,7 +374,7 @@ async fn access_log(req: Request, next: Next) -> Response {
use tracing_opentelemetry::OpenTelemetrySpanExt;
let _ = span.set_parent(parent);
}
let resp = tracing::Instrument::instrument(next.run(req), span.clone()).await;
let resp = Instrument::instrument(next.run(req), span.clone()).await;
span.record("http.response.status_code", resp.status().as_u16());
let _in = span.enter();
let who = resp.extensions().get::<Who>().map_or("", |w| w.0.as_str()).to_owned();

View File

@@ -4,6 +4,11 @@ use anyhow::{Context, Result};
use clap::{Parser, Subcommand};
use ipx_directory::store::Store;
use ipx_directory::{Directory, config};
use std::path::{Path, PathBuf};
use std::time::Duration;
use tokio::net::TcpListener;
use tokio::signal::unix::SignalKind;
use tracing_subscriber::EnvFilter;
use tracing_subscriber::prelude::*;
#[derive(Parser)]
@@ -12,7 +17,7 @@ struct Cli {
/// The config file: where to listen, the database, and the tokens it accepts. Otherwise
/// DIRECTORY_CONFIG, or else /config/directory.toml.
#[arg(long)]
config: Option<std::path::PathBuf>,
config: Option<PathBuf>,
#[command(subcommand)]
cmd: Option<Cmd>,
}
@@ -62,7 +67,7 @@ async fn main() -> Result<()> {
let port = cfg.bind.rsplit(':').next().unwrap_or("8100");
let resp = reqwest::Client::new()
.get(format!("http://127.0.0.1:{port}/healthz"))
.timeout(std::time::Duration::from_secs(4))
.timeout(Duration::from_secs(4))
.send()
.await
.context("the directory does not answer")?;
@@ -72,11 +77,10 @@ async fn main() -> Result<()> {
}
}
async fn serve(path: &std::path::Path) -> Result<()> {
async fn serve(path: &Path) -> Result<()> {
// One JSON object a line for Loki when DIRECTORY_LOG_FORMAT=json, as ipx does; text otherwise.
let json = std::env::var("DIRECTORY_LOG_FORMAT").is_ok_and(|f| f.eq_ignore_ascii_case("json"));
let filter =
|| tracing_subscriber::EnvFilter::try_from_env("DIRECTORY_LOG").unwrap_or_else(|_| "ipx_directory=info".into());
let filter = || EnvFilter::try_from_env("DIRECTORY_LOG").unwrap_or_else(|_| "ipx_directory=info".into());
// Traces to Tempo when OTEL_EXPORTER_OTLP_ENDPOINT is set, joining ipx's (#213).
let otel = ipx_directory::otel::provider("ipx-directory")?;
tracing_subscriber::registry()
@@ -92,7 +96,7 @@ async fn serve(path: &std::path::Path) -> Result<()> {
use opentelemetry::trace::TracerProvider;
tracing_opentelemetry::layer()
.with_tracer(p.tracer("ipx-directory"))
.with_filter(tracing_subscriber::EnvFilter::new("ipx_directory=info,ipx_feed=info"))
.with_filter(EnvFilter::new("ipx_directory=info,ipx_feed=info"))
}))
.init();
@@ -106,11 +110,10 @@ async fn serve(path: &std::path::Path) -> Result<()> {
let dir = Directory::new(store, vec![], ipx_feed::guard::Allowed::parse(&cfg.fetch_internal)?)?;
let dropped = dir.set_tokens(tokens).await?;
let listener =
tokio::net::TcpListener::bind(&cfg.bind).await.with_context(|| format!("listening on {}", cfg.bind))?;
let listener = TcpListener::bind(&cfg.bind).await.with_context(|| format!("listening on {}", cfg.bind))?;
tracing::info!(version = env!("CARGO_PKG_VERSION"), bind = %cfg.bind, tokens = cfg.tokens.len(), dropped, "directory started");
if !cfg.metrics_bind.is_empty() {
let metrics = tokio::net::TcpListener::bind(&cfg.metrics_bind)
let metrics = TcpListener::bind(&cfg.metrics_bind)
.await
.with_context(|| format!("listening for metrics on {}", cfg.metrics_bind))?;
tracing::info!(bind = %cfg.metrics_bind, "serving metrics for Prometheus at /metrics");
@@ -125,8 +128,7 @@ async fn serve(path: &std::path::Path) -> Result<()> {
tokio::spawn(ipx_directory::reloader(dir.clone(), path.to_owned()));
axum::serve(listener, ipx_directory::router(dir))
.with_graceful_shutdown(async {
let mut term =
tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()).expect("SIGTERM handler");
let mut term = tokio::signal::unix::signal(SignalKind::terminate()).expect("SIGTERM handler");
tokio::select! {
_ = term.recv() => {}
_ = tokio::signal::ctrl_c() => {}

View File

@@ -6,7 +6,11 @@
//! mimalloc gives freed pages back, so resident memory follows what is in use: a spike that
//! does not come down again is a leak. Not jemalloc, whose upstream was archived in 2025.
use axum::Router;
use std::sync::atomic::Ordering::Relaxed;
use std::time::Instant;
use tokio::net::TcpListener;
use tower_http::compression::CompressionLayer;
#[global_allocator]
static ALLOC: mimalloc::MiMalloc = mimalloc::MiMalloc;
@@ -25,7 +29,7 @@ pub fn process() -> (Option<f64>, Option<f64>) {
/// The directory's /metrics, in Prometheus's text format: memory, open files, and the feeds it
/// has fetched. Nothing from the database, so a scrape costs nothing and answers while it is down.
fn render(started: std::time::Instant) -> String {
fn render(started: Instant) -> String {
let mut out = String::new();
let mut gauge = |name: &str, kind: &str, help: &str, labels: &str, v: f64| {
out.push_str(&format!("# HELP {name} {help}\n# TYPE {name} {kind}\n{name}{labels} {v}\n"));
@@ -70,9 +74,9 @@ fn render(started: std::time::Instant) -> String {
/// Serves /metrics on a listener of its own: the public port is behind the tunnel with no
/// Access in front, and these are for the LAN.
pub async fn serve(listener: tokio::net::TcpListener) {
let started = std::time::Instant::now();
let app = axum::Router::new()
pub async fn serve(listener: TcpListener) {
let started = Instant::now();
let app = Router::new()
.route(
"/metrics",
axum::routing::get(move || async move {
@@ -80,7 +84,7 @@ pub async fn serve(listener: tokio::net::TcpListener) {
}),
)
// Prometheus asks for gzip (#253).
.layer(tower_http::compression::CompressionLayer::new().gzip(true));
.layer(CompressionLayer::new().gzip(true));
if let Err(e) = axum::serve(listener, app).await {
tracing::error!(error = %e, "metrics stopped");
}

View File

@@ -2,6 +2,8 @@
//! caller and out to a callee, so one trace follows a request from the browser through ipx-web
//! and ipx into the directory.
use axum::http::HeaderMap;
/// Traces over OTLP, to Tempo for one, when OTEL_EXPORTER_OTLP_ENDPOINT names a collector
/// (`http://host:4318`: the exporter speaks OTLP over HTTP and adds `/v1/traces`). The exporter
/// reads that and the other `OTEL_` variables itself.
@@ -24,7 +26,7 @@ pub fn provider(service: &'static str) -> anyhow::Result<Option<opentelemetry_sd
/// header can be set: a position saved by sendBeacon, and the playlist Safari's player fetches
/// itself (#192).
#[must_use]
pub fn remote_parent(headers: &axum::http::HeaderMap, query: Option<&str>) -> Option<opentelemetry::Context> {
pub fn remote_parent(headers: &HeaderMap, query: Option<&str>) -> Option<opentelemetry::Context> {
use opentelemetry::trace::{SpanContext, SpanId, TraceContextExt, TraceFlags, TraceId, TraceState};
let v = match headers.get("traceparent") {
Some(h) => h.to_str().ok()?,

View File

@@ -16,6 +16,7 @@ use axum::response::{IntoResponse, Response};
use axum::routing::get;
use crate::{Listing, Shared};
use url::form_urlencoded::Serializer;
pub fn router() -> Router<Shared> {
Router::new()
@@ -140,7 +141,7 @@ async fn listed(dir: &Shared, title: &str, here: &str, f: &Filter, pick: impl Fn
// The query a link keeps, with one of its keys changed.
let link = |kind: Option<&str>, sub: Option<&str>| {
let mut q = url::form_urlencoded::Serializer::new(String::new());
let mut q = Serializer::new(String::new());
if let Some(v) = f.q.as_deref() {
q.append_pair("q", v);
}

View File

@@ -6,6 +6,11 @@ use anyhow::Result;
use sea_orm::{ConnectionTrait, DatabaseConnection, Statement, TransactionTrait, Value};
use crate::{Blocked, FeedPage, Instance, Item, Listing, SnapshotEntry, SnapshotResult};
use sea_orm::{ConnectOptions, Database, QueryResult};
use std::cmp::Reverse;
use std::collections::{BTreeMap, HashSet};
use std::time::SystemTime;
use url::Url;
/// A feed's items as its page shows them, and as many as ipx's own Directory page showed.
pub const ITEMS: usize = 20;
@@ -42,14 +47,14 @@ pub enum Refreshed {
/// Unix seconds now.
#[must_use]
pub fn now() -> i64 {
std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |d| d.as_secs() as i64)
SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |d| d.as_secs() as i64)
}
/// The address as every instance would write it: scheme and host lower-cased, a default port
/// and a fragment dropped. None for anything that is not http(s).
#[must_use]
pub fn normalise(url: &str) -> Option<String> {
let mut u = url::Url::parse(url.trim()).ok()?;
let mut u = Url::parse(url.trim()).ok()?;
if !matches!(u.scheme(), "http" | "https") || u.host_str().is_none() {
return None;
}
@@ -146,7 +151,7 @@ impl Store {
let sep = if url.contains('?') { '&' } else { '?' };
url = format!("{url}{sep}options=-c%20client_min_messages%3Dwarning");
}
let mut opts = sea_orm::ConnectOptions::new(url.clone());
let mut opts = ConnectOptions::new(url.clone());
opts.sqlx_logging(false);
// An in-memory SQLite database is one per connection: a pool of them would be several.
if url.contains(":memory:") {
@@ -157,7 +162,7 @@ impl Store {
if keep_open && url.starts_with("postgres") {
opts.max_connections(10).min_connections(10);
}
let db = sea_orm::Database::connect(opts).await?;
let db = Database::connect(opts).await?;
for sql in SCHEMA {
db.execute_unprepared(sql).await?;
}
@@ -177,7 +182,7 @@ impl Store {
Ok(self.db.execute_raw(self.stmt(sql, values)).await?.rows_affected())
}
async fn rows(&self, sql: &str, values: Vec<Value>) -> Result<Vec<sea_orm::QueryResult>> {
async fn rows(&self, sql: &str, values: Vec<Value>) -> Result<Vec<QueryResult>> {
Ok(self.db.query_all_raw(self.stmt(sql, values)).await?)
}
@@ -209,14 +214,14 @@ impl Store {
/// blocked feed is left out and counted; the rest are added to the feeds, new ones to be
/// fetched by the refresher. Feeds no instance sends any more go.
pub async fn snapshot(&self, instance: &str, entries: &[SnapshotEntry]) -> Result<SnapshotResult> {
let blocked: std::collections::HashSet<String> = self
let blocked: HashSet<String> = self
.rows("SELECT id FROM blocked", vec![])
.await?
.iter()
.map(|r| r.try_get("", "id"))
.collect::<Result<_, _>>()?;
let mut result = SnapshotResult::default();
let mut keep = std::collections::BTreeMap::new();
let mut keep = BTreeMap::new();
for e in entries {
let Some(url) = normalise(&e.url) else {
result.refused_invalid += 1;
@@ -307,7 +312,7 @@ impl Store {
f.podcast, CAST(sum(s.subscribers) AS BIGINT) AS subscribers
FROM feeds f JOIN snapshots s ON s.feed_id = f.id WHERE f.items IS NOT NULL";
fn listing(r: &sea_orm::QueryResult) -> Result<Listing> {
fn listing(r: &QueryResult) -> Result<Listing> {
Ok(Listing {
id: r.try_get("", "id")?,
url: r.try_get("", "url")?,
@@ -483,7 +488,7 @@ impl Store {
.flat_map(|e| &e.enclosures)
.any(|e| e.mime.as_deref().is_some_and(|m| m.starts_with("audio/") || m.starts_with("video/")));
let mut entries = parsed.entries;
entries.sort_by_key(|e| std::cmp::Reverse(e.published));
entries.sort_by_key(|e| Reverse(e.published));
// What an item is, never its files: their addresses are where a key would be.
let items: Vec<Item> = entries
.into_iter()