From b6dcb17753daa5846aec6feb332be46b2154a658 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Apr 2025 09:56:30 +0200 Subject: [PATCH 1/7] Derive basic `Builder` for `App` struct --- src/app.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/app.rs b/src/app.rs index 4d57f9ab83d..cd53c0f8b10 100644 --- a/src/app.rs +++ b/src/app.rs @@ -9,6 +9,7 @@ use crate::metrics::{InstanceMetrics, ServiceMetrics}; use crate::rate_limiter::RateLimiter; use crate::storage::Storage; use axum::extract::{FromRef, FromRequestParts, State}; +use bon::Builder; use crates_io_github::GitHubClient; use deadpool_diesel::Runtime; use derive_more::Deref; @@ -25,6 +26,7 @@ type DeadpoolResult = Result< /// The `App` struct holds the main components of the application like /// the database connection pool and configurations +#[derive(Builder)] pub struct App { /// Database connection pool connected to the primary database pub primary_database: DeadpoolPool, @@ -128,18 +130,18 @@ impl App { None }; - App { - primary_database, - replica_database, - github, - github_oauth, - emails, - storage: Arc::new(Storage::from_config(&config.storage)), - service_metrics: ServiceMetrics::new().expect("could not initialize service metrics"), - instance_metrics, - rate_limiter: RateLimiter::new(config.rate_limiter.clone()), - config: Arc::new(config), - } + App::builder() + .primary_database(primary_database) + .maybe_replica_database(replica_database) + .github(github) + .github_oauth(github_oauth) + .emails(emails) + .storage(Arc::new(Storage::from_config(&config.storage))) + .service_metrics(ServiceMetrics::new().expect("could not initialize service metrics")) + .instance_metrics(instance_metrics) + .rate_limiter(RateLimiter::new(config.rate_limiter.clone())) + .config(Arc::new(config)) + .build() } /// A unique key to generate signed cookies From d77501ef8927c0b5a0f9001afd6961461cfe324a Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Apr 2025 09:58:57 +0200 Subject: [PATCH 2/7] AppBuilder: Use default metrics --- src/app.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/app.rs b/src/app.rs index cd53c0f8b10..81f518213ef 100644 --- a/src/app.rs +++ b/src/app.rs @@ -51,9 +51,11 @@ pub struct App { pub storage: Arc, /// Metrics related to the service as a whole + #[builder(default = ServiceMetrics::new().expect("could not initialize service metrics"))] pub service_metrics: ServiceMetrics, /// Metrics related to this specific instance of the service + #[builder(default = InstanceMetrics::new().expect("could not initialize instance metrics"))] pub instance_metrics: InstanceMetrics, /// Rate limit select actions. @@ -71,9 +73,6 @@ impl App { pub fn new(config: config::Server, emails: Emails, github: Box) -> App { use oauth2::{AuthUrl, TokenUrl}; - let instance_metrics = - InstanceMetrics::new().expect("could not initialize instance metrics"); - let auth_url = "https://github.com/login/oauth/authorize"; let auth_url = AuthUrl::new(auth_url.into()).unwrap(); let token_url = "https://github.com/login/oauth/access_token"; @@ -137,8 +136,6 @@ impl App { .github_oauth(github_oauth) .emails(emails) .storage(Arc::new(Storage::from_config(&config.storage))) - .service_metrics(ServiceMetrics::new().expect("could not initialize service metrics")) - .instance_metrics(instance_metrics) .rate_limiter(RateLimiter::new(config.rate_limiter.clone())) .config(Arc::new(config)) .build() From c83d381900194651ec7ee3cb7d7ebbae901b25d9 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Apr 2025 10:05:08 +0200 Subject: [PATCH 3/7] AppBuilder: Extract `github_oauth_from_config()` fn --- src/app.rs | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/app.rs b/src/app.rs index 81f518213ef..32019b575b3 100644 --- a/src/app.rs +++ b/src/app.rs @@ -62,15 +62,14 @@ pub struct App { pub rate_limiter: RateLimiter, } -impl App { - /// Creates a new `App` with a given `Config` and an optional HTTP `Client` - /// - /// Configures and sets up: - /// - /// - GitHub OAuth - /// - Database connection pools - /// - A `git2::Repository` instance from the index repo checkout (that server.rs ensures exists) - pub fn new(config: config::Server, emails: Emails, github: Box) -> App { +impl AppBuilder { + pub fn github_oauth_from_config( + self, + config: &config::Server, + ) -> AppBuilder> + where + S::GithubOauth: app_builder::IsUnset, + { use oauth2::{AuthUrl, TokenUrl}; let auth_url = "https://github.com/login/oauth/authorize"; @@ -83,6 +82,19 @@ impl App { .set_auth_uri(auth_url) .set_token_uri(token_url); + self.github_oauth(github_oauth) + } +} + +impl App { + /// Creates a new `App` with a given `Config` and an optional HTTP `Client` + /// + /// Configures and sets up: + /// + /// - GitHub OAuth + /// - Database connection pools + /// - A `git2::Repository` instance from the index repo checkout (that server.rs ensures exists) + pub fn new(config: config::Server, emails: Emails, github: Box) -> App { let primary_database = { use secrecy::ExposeSecret; @@ -133,7 +145,7 @@ impl App { .primary_database(primary_database) .maybe_replica_database(replica_database) .github(github) - .github_oauth(github_oauth) + .github_oauth_from_config(&config) .emails(emails) .storage(Arc::new(Storage::from_config(&config.storage))) .rate_limiter(RateLimiter::new(config.rate_limiter.clone())) From 1a17ba09e8aa41342733de99fa7dcc53d99e2b26 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Apr 2025 10:13:22 +0200 Subject: [PATCH 4/7] AppBuilder: Extract `databases_from_config()` fn --- src/app.rs | 57 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/app.rs b/src/app.rs index 32019b575b3..3046256056f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -84,54 +84,52 @@ impl AppBuilder { self.github_oauth(github_oauth) } -} -impl App { - /// Creates a new `App` with a given `Config` and an optional HTTP `Client` - /// - /// Configures and sets up: - /// - /// - GitHub OAuth - /// - Database connection pools - /// - A `git2::Repository` instance from the index repo checkout (that server.rs ensures exists) - pub fn new(config: config::Server, emails: Emails, github: Box) -> App { + pub fn databases_from_config( + self, + config: &config::DatabasePools, + ) -> AppBuilder>> + where + S::PrimaryDatabase: app_builder::IsUnset, + S::ReplicaDatabase: app_builder::IsUnset, + { let primary_database = { use secrecy::ExposeSecret; let primary_db_connection_config = ConnectionConfig { - statement_timeout: config.db.statement_timeout, - read_only: config.db.primary.read_only_mode, + statement_timeout: config.statement_timeout, + read_only: config.primary.read_only_mode, }; - let url = connection_url(&config.db, config.db.primary.url.expose_secret()); - let manager_config = make_manager_config(config.db.enforce_tls); + let url = connection_url(config, config.primary.url.expose_secret()); + let manager_config = make_manager_config(config.enforce_tls); let manager = AsyncDieselConnectionManager::new_with_config(url, manager_config); DeadpoolPool::builder(manager) .runtime(Runtime::Tokio1) - .max_size(config.db.primary.pool_size) - .wait_timeout(Some(config.db.connection_timeout)) + .max_size(config.primary.pool_size) + .wait_timeout(Some(config.connection_timeout)) .post_create(primary_db_connection_config) .build() .unwrap() }; - let replica_database = if let Some(pool_config) = config.db.replica.as_ref() { + let replica_database = if let Some(pool_config) = config.replica.as_ref() { use secrecy::ExposeSecret; let replica_db_connection_config = ConnectionConfig { - statement_timeout: config.db.statement_timeout, + statement_timeout: config.statement_timeout, read_only: pool_config.read_only_mode, }; - let url = connection_url(&config.db, pool_config.url.expose_secret()); - let manager_config = make_manager_config(config.db.enforce_tls); + let url = connection_url(config, pool_config.url.expose_secret()); + let manager_config = make_manager_config(config.enforce_tls); let manager = AsyncDieselConnectionManager::new_with_config(url, manager_config); let pool = DeadpoolPool::builder(manager) .runtime(Runtime::Tokio1) .max_size(pool_config.pool_size) - .wait_timeout(Some(config.db.connection_timeout)) + .wait_timeout(Some(config.connection_timeout)) .post_create(replica_db_connection_config) .build() .unwrap(); @@ -141,9 +139,22 @@ impl App { None }; - App::builder() - .primary_database(primary_database) + self.primary_database(primary_database) .maybe_replica_database(replica_database) + } +} + +impl App { + /// Creates a new `App` with a given `Config` and an optional HTTP `Client` + /// + /// Configures and sets up: + /// + /// - GitHub OAuth + /// - Database connection pools + /// - A `git2::Repository` instance from the index repo checkout (that server.rs ensures exists) + pub fn new(config: config::Server, emails: Emails, github: Box) -> App { + App::builder() + .databases_from_config(&config.db) .github(github) .github_oauth_from_config(&config) .emails(emails) From 3d34dfc619597866eee26d578f496dcd151466d7 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Apr 2025 10:15:57 +0200 Subject: [PATCH 5/7] AppBuilder: Extract `storage_from_config()` fn --- src/app.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/app.rs b/src/app.rs index 3046256056f..99bccbc5208 100644 --- a/src/app.rs +++ b/src/app.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use crate::email::Emails; use crate::metrics::{InstanceMetrics, ServiceMetrics}; use crate::rate_limiter::RateLimiter; -use crate::storage::Storage; +use crate::storage::{Storage, StorageConfig}; use axum::extract::{FromRef, FromRequestParts, State}; use bon::Builder; use crates_io_github::GitHubClient; @@ -142,6 +142,16 @@ impl AppBuilder { self.primary_database(primary_database) .maybe_replica_database(replica_database) } + + pub fn storage_from_config( + self, + config: &StorageConfig, + ) -> AppBuilder> + where + S::Storage: app_builder::IsUnset, + { + self.storage(Arc::new(Storage::from_config(config))) + } } impl App { @@ -158,7 +168,7 @@ impl App { .github(github) .github_oauth_from_config(&config) .emails(emails) - .storage(Arc::new(Storage::from_config(&config.storage))) + .storage_from_config(&config.storage) .rate_limiter(RateLimiter::new(config.rate_limiter.clone())) .config(Arc::new(config)) .build() From d9d463c11b1b03480ee1505bf4bde74be2c24846 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Apr 2025 10:18:17 +0200 Subject: [PATCH 6/7] AppBuilder: Extract `rate_limiter_from_config()` fn --- src/app.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/app.rs b/src/app.rs index 99bccbc5208..cd7a679873a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2,11 +2,12 @@ use crate::config; use crate::db::{ConnectionConfig, connection_url, make_manager_config}; +use std::collections::HashMap; use std::sync::Arc; use crate::email::Emails; use crate::metrics::{InstanceMetrics, ServiceMetrics}; -use crate::rate_limiter::RateLimiter; +use crate::rate_limiter::{LimitedAction, RateLimiter, RateLimiterConfig}; use crate::storage::{Storage, StorageConfig}; use axum::extract::{FromRef, FromRequestParts, State}; use bon::Builder; @@ -152,6 +153,16 @@ impl AppBuilder { { self.storage(Arc::new(Storage::from_config(config))) } + + pub fn rate_limiter_from_config( + self, + config: HashMap, + ) -> AppBuilder> + where + S::RateLimiter: app_builder::IsUnset, + { + self.rate_limiter(RateLimiter::new(config)) + } } impl App { @@ -169,7 +180,7 @@ impl App { .github_oauth_from_config(&config) .emails(emails) .storage_from_config(&config.storage) - .rate_limiter(RateLimiter::new(config.rate_limiter.clone())) + .rate_limiter_from_config(config.rate_limiter.clone()) .config(Arc::new(config)) .build() } From a5105d69b7068b81ba845c7ff2cf0349fbdf61b2 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Apr 2025 10:23:34 +0200 Subject: [PATCH 7/7] Inline `App::new()` fn Using the `AppBuilder` directly gives us more flexibility for additional fields in the future. --- src/app.rs | 19 ------------------- src/bin/server.rs | 12 +++++++++++- src/tests/util/test_app.rs | 10 +++++++++- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/app.rs b/src/app.rs index cd7a679873a..079d433b8d3 100644 --- a/src/app.rs +++ b/src/app.rs @@ -166,25 +166,6 @@ impl AppBuilder { } impl App { - /// Creates a new `App` with a given `Config` and an optional HTTP `Client` - /// - /// Configures and sets up: - /// - /// - GitHub OAuth - /// - Database connection pools - /// - A `git2::Repository` instance from the index repo checkout (that server.rs ensures exists) - pub fn new(config: config::Server, emails: Emails, github: Box) -> App { - App::builder() - .databases_from_config(&config.db) - .github(github) - .github_oauth_from_config(&config) - .emails(emails) - .storage_from_config(&config.storage) - .rate_limiter_from_config(config.rate_limiter.clone()) - .config(Arc::new(config)) - .build() - } - /// A unique key to generate signed cookies pub fn session_key(&self) -> &cookie::Key { &self.config.session_key diff --git a/src/bin/server.rs b/src/bin/server.rs index a91d5e049f0..3303158c7e1 100644 --- a/src/bin/server.rs +++ b/src/bin/server.rs @@ -33,7 +33,17 @@ fn main() -> anyhow::Result<()> { let github = RealGitHubClient::new(client); let github = Box::new(github); - let app = Arc::new(App::new(config, emails, github)); + let app = App::builder() + .databases_from_config(&config.db) + .github(github) + .github_oauth_from_config(&config) + .emails(emails) + .storage_from_config(&config.storage) + .rate_limiter_from_config(config.rate_limiter.clone()) + .config(Arc::new(config)) + .build(); + + let app = Arc::new(app); // Start the background thread periodically logging instance metrics. log_instance_metrics_thread(app.clone()); diff --git a/src/tests/util/test_app.rs b/src/tests/util/test_app.rs index 2e155b3332b..4811bd82513 100644 --- a/src/tests/util/test_app.rs +++ b/src/tests/util/test_app.rs @@ -488,7 +488,15 @@ fn build_app(config: config::Server, github: Option) -> (Arc