diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index d96235e5d..2bd0dde80 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -93,7 +93,7 @@ jobs: env: # Your code can read these, or just use the defaults above - TEST_PUBKY_CONNECTION_STRING: postgres://test_user:test_pass@localhost:5432/postgres?pubky-test=true + TEST_PUBKY_CONNECTION_STRING: postgres://test_user:test_pass@localhost:5432/postgres strategy: matrix: @@ -193,7 +193,7 @@ jobs: --health-retries=5 env: # Your code can read these, or just use the defaults above - TEST_PUBKY_CONNECTION_STRING: postgres://test_user:test_pass@localhost:5433/postgres?pubky-test=true + TEST_PUBKY_CONNECTION_STRING: postgres://test_user:test_pass@localhost:5433/postgres runs-on: ubuntu-latest steps: diff --git a/docs/TESTING.md b/docs/TESTING.md index e14664255..3ab7e7320 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -17,11 +17,11 @@ docker run --name pubky-postgres \ Then run tests with a test connection string: ```bash -TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true' \ +TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \ cargo test -p pubky-homeserver --all-features ``` -The `?pubky-test=true` parameter tells the test helpers to create an ephemeral `pubky_test_*` database inside the configured PostgreSQL instance. Databases are cleaned up after each test. +In test builds, each test automatically gets its own ephemeral `pubky_test_{uuid}` database on the configured server. Databases are cleaned up after each test. ## Automatic Database Cleanup @@ -93,7 +93,7 @@ async fn test_one() { The [`e2e`](../e2e) crate contains tests that cover cross-crate workflows using `pubky-testnet`. Run them with: ```bash -TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true' \ +TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \ cargo test -p e2e ``` @@ -102,7 +102,7 @@ TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgr Run the homeserver tests against external PostgreSQL: ```bash -TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true' \ +TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \ cargo test -p pubky-homeserver --all-features ``` @@ -115,6 +115,6 @@ cargo test -p pubky-testnet --features docker-postgres Run the full workspace test suite: ```bash -TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true' \ +TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \ cargo test --workspace --all-features ``` diff --git a/examples/rust/7-logging/README.md b/examples/rust/7-logging/README.md index 0b21a2fc8..e763c7c8e 100644 --- a/examples/rust/7-logging/README.md +++ b/examples/rust/7-logging/README.md @@ -22,7 +22,7 @@ cargo run --bin logging -- --level debug --external-postgres You can specify a custom connection via the `TEST_PUBKY_CONNECTION_STRING` environment variable: ```bash -TEST_PUBKY_CONNECTION_STRING=postgres://user:pass@localhost:5432/mydb?pubky-test=true cargo run --bin logging -- --level debug --external-postgres +TEST_PUBKY_CONNECTION_STRING=postgres://user:pass@localhost:5432/mydb cargo run --bin logging -- --level debug --external-postgres ``` -The `?pubky-test=true` parameter indicates that an ephemeral test database should be created. +Each testnet automatically gets its own ephemeral `pubky_test_{uuid}` database on the configured server. diff --git a/examples/rust/8-testnet/README.md b/examples/rust/8-testnet/README.md index 730e64e44..7d31aaa57 100644 --- a/examples/rust/8-testnet/README.md +++ b/examples/rust/8-testnet/README.md @@ -22,7 +22,7 @@ cargo run --bin testnet -- --external-postgres You can specify a custom connection via the `TEST_PUBKY_CONNECTION_STRING` environment variable: ```bash -TEST_PUBKY_CONNECTION_STRING=postgres://user:pass@localhost:5432/mydb?pubky-test=true cargo run --bin testnet -- --external-postgres +TEST_PUBKY_CONNECTION_STRING=postgres://user:pass@localhost:5432/mydb cargo run --bin testnet -- --external-postgres ``` -The `?pubky-test=true` parameter indicates that an ephemeral test database should be created. +Each testnet automatically gets its own ephemeral `pubky_test_{uuid}` database on the configured server. diff --git a/examples/rust/README.md b/examples/rust/README.md index 45d808524..ed6df5cf1 100644 --- a/examples/rust/README.md +++ b/examples/rust/README.md @@ -21,7 +21,7 @@ docker run --name pubky-postgres \ -d postgres:18 # Start the testnet (keep this terminal open) -TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true' \ +TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \ cargo run -p pubky-testnet ``` diff --git a/pubky-homeserver/src/app_context.rs b/pubky-homeserver/src/app_context.rs index 2a848369a..5008be8d1 100644 --- a/pubky-homeserver/src/app_context.rs +++ b/pubky-homeserver/src/app_context.rs @@ -36,6 +36,9 @@ pub enum AppContextConversionError { /// Failed to open SQL DB. #[error("Failed to open SQL DB: {0}")] SqlDb(sqlx::Error), + /// Failed to resolve the database mode (e.g. missing URL or invalid TEST_PUBKY_CONNECTION_STRING). + #[error("Failed to resolve database mode: {0}")] + DatabaseResolution(anyhow::Error), /// Failed to run migrations. #[error("Failed to run migrations: {0}")] Migrations(anyhow::Error), @@ -151,7 +154,12 @@ impl AppContext { .read_or_create_keypair() .map_err(AppContextConversionError::Keypair)?; - let sql_db = Self::connect_to_sql_db(&conf).await?; + let db_mode = dir + .resolve_database_mode(&conf) + .map_err(AppContextConversionError::DatabaseResolution)?; + let sql_db = SqlDb::connect(db_mode) + .await + .map_err(AppContextConversionError::SqlDb)?; Migrator::new(&sql_db) .run() .await @@ -203,7 +211,9 @@ impl AppContext { fn build_pkarr_builder_from_config(config_toml: &ConfigToml) -> pkarr::ClientBuilder { let mut builder = pkarr::ClientBuilder::default(); #[cfg(any(test, feature = "testing"))] - if config_toml.general.database_url.is_test_db() { + // In test builds, no explicit database_url means we're in a test environment + // where we must avoid contacting the public DHT. + if config_toml.general.database_url.is_none() { builder .no_default_network() // Keep the client buildable without contacting the public DHT. @@ -235,50 +245,33 @@ impl AppContext { } builder } - - /// Connect to the SQL database. - /// If we are in a test environment and it's a test db connection string, - /// we use an empheral test db. - /// Otherwise, we use the normal db connection. - async fn connect_to_sql_db( - config_toml: &ConfigToml, - ) -> Result { - #[cfg(any(test, feature = "testing"))] - { - // If we are in a test environment and it's a test db connection string, - // we use an empheral test db. - return if config_toml.general.database_url.is_test_db() { - Ok(SqlDb::test().await) - } else { - SqlDb::connect(&config_toml.general.database_url) - .await - .map_err(AppContextConversionError::SqlDb) - }; - } - - #[cfg(not(any(test, feature = "testing")))] - { - // If we are not in a test environment, we use the normal db connection. - return SqlDb::connect(&config_toml.general.database_url) - .await - .map_err(AppContextConversionError::SqlDb); - } - } } #[cfg(test)] mod tests { use super::*; + /// Verifies that the test pkarr builder doesn't contact the public DHT + /// when database_url is None (the default test config). #[test] - fn test_pkarr_builder_does_not_use_default_network() { - let builder = - AppContext::build_pkarr_builder_from_config(&ConfigToml::default_test_config()); + fn pkarr_builder_does_not_use_default_network() { + let config = ConfigToml::default_test_config(); + assert!( + config.general.database_url.is_none(), + "default_test_config should have database_url = None" + ); + let builder = AppContext::build_pkarr_builder_from_config(&config); let builder_debug = format!("{builder:?}"); - assert!(builder_debug.contains("127.0.0.1:9")); + assert!( + builder_debug.contains("127.0.0.1:9"), + "expected sentinel bootstrap node in builder: {builder_debug}" + ); for relay in pkarr::DEFAULT_RELAYS { - assert!(!builder_debug.contains(relay)); + assert!( + !builder_debug.contains(relay), + "default relay {relay} should not appear in test builder: {builder_debug}" + ); } builder.build().expect("isolated pkarr client should build"); } diff --git a/pubky-homeserver/src/client_server/app.rs b/pubky-homeserver/src/client_server/app.rs index 120be8d9a..23e49b938 100644 --- a/pubky-homeserver/src/client_server/app.rs +++ b/pubky-homeserver/src/client_server/app.rs @@ -259,8 +259,8 @@ mod tests { use crate::{ app_context::AppContext, client_server::ClientServer, + data_directory::{ConfigToml, MockDataDir}, shared::quota::{GlobPattern, HttpMethod, LimitKeyType, PathLimit}, - ConfigToml, MockDataDir, }; #[tokio::test] @@ -335,9 +335,9 @@ mod tests { #[pubky_test_utils::test] async fn storage_metrics_only_count_resolved_requests_with_low_cardinality_labels() { let data_dir = MockDataDir::new(ConfigToml::minimal_test_config(), None).unwrap(); - let context = AppContext::read_from(data_dir).await.unwrap(); + let context = Arc::new(AppContext::read_from(data_dir).await.unwrap()); let metrics = context.metrics.clone(); - let router = ClientServer::create_router(Arc::new(context)).unwrap(); + let router = ClientServer::create_router(Arc::clone(&context)).unwrap(); let server = TestServer::new(router).unwrap(); let user = Keypair::random(); let cookie = signup_cookie(&server, &user).await; @@ -380,25 +380,25 @@ mod tests { .collect::>(); assert_eq!(samples.len(), 4, "unexpected metric samples:\n{output}"); - assert!(samples.iter().any(|sample| { + assert!(samples.iter().any(|sample: &&str| { sample.contains("addressing_mode=\"path\"") && sample.contains("auth_method=\"cookie\"") && sample.contains("pubky_host_header=\"matching\"") && sample.contains("pubky_host_query=\"true\"") })); - assert!(samples.iter().any(|sample| { + assert!(samples.iter().any(|sample: &&str| { sample.contains("addressing_mode=\"legacy\"") && sample.contains("auth_method=\"none\"") && sample.contains("pubky_host_header=\"matching\"") && sample.contains("pubky_host_query=\"false\"") })); - assert!(samples.iter().any(|sample| { + assert!(samples.iter().any(|sample: &&str| { sample.contains("addressing_mode=\"path\"") && sample.contains("auth_method=\"none\"") && sample.contains("pubky_host_header=\"other\"") && sample.contains("pubky_host_query=\"false\"") })); - assert!(samples.iter().any(|sample| { + assert!(samples.iter().any(|sample: &&str| { sample.contains("addressing_mode=\"legacy\"") && sample.contains("auth_method=\"none\"") && sample.contains("pubky_host_header=\"absent\"") diff --git a/pubky-homeserver/src/data_directory/config_toml.rs b/pubky-homeserver/src/data_directory/config_toml.rs index ae8f19a49..7bf7b413f 100644 --- a/pubky-homeserver/src/data_directory/config_toml.rs +++ b/pubky-homeserver/src/data_directory/config_toml.rs @@ -91,7 +91,8 @@ pub struct GeneralToml { )] #[serde(default)] pub user_storage_quota_mb: u64, - pub database_url: ConnectionString, + #[serde(default)] + pub database_url: Option, } /// A config for Homeserver tracing subscriber configuration @@ -213,7 +214,7 @@ impl ConfigToml { #[cfg(any(test, feature = "testing"))] pub fn default_test_config() -> Self { let mut config = Self::default(); - config.general.database_url = ConnectionString::default_test_db(); // Mark this db as test. This indicates that the db is not real. + config.general.database_url = None; // Resolved downstream via env var or default fallback. config.general.signup_mode = SignupMode::Open; // Use ephemeral ports (0) so parallel tests don't collide. config.drive.icann_listen_socket = SocketAddr::from(([127, 0, 0, 1], 0)); diff --git a/pubky-homeserver/src/data_directory/data_dir.rs b/pubky-homeserver/src/data_directory/data_dir.rs index 75864ecef..c3f28f02b 100644 --- a/pubky-homeserver/src/data_directory/data_dir.rs +++ b/pubky-homeserver/src/data_directory/data_dir.rs @@ -1,4 +1,5 @@ use super::ConfigToml; +use crate::persistence::sql::DatabaseMode; use dyn_clone::DynClone; use std::path::Path; @@ -20,6 +21,11 @@ pub trait DataDir: std::fmt::Debug + DynClone + Send + Sync { /// Reads the secret file from the data directory. /// Creates a new secret file if it doesn't exist. fn read_or_create_keypair(&self) -> anyhow::Result; + + /// Resolve how the homeserver should connect to its database. + /// + /// Each implementation selects the appropriate database lifecycle. + fn resolve_database_mode(&self, conf: &ConfigToml) -> anyhow::Result; } dyn_clone::clone_trait_object!(DataDir); diff --git a/pubky-homeserver/src/data_directory/mock_data_dir.rs b/pubky-homeserver/src/data_directory/mock_data_dir.rs index 826040b6e..c3dab90f1 100644 --- a/pubky-homeserver/src/data_directory/mock_data_dir.rs +++ b/pubky-homeserver/src/data_directory/mock_data_dir.rs @@ -56,6 +56,14 @@ impl DataDir for MockDataDir { self.temp_dir.path() } + /// Creates a temporary database with [`DatabaseMode::EphemeralTest`](crate::persistence::sql::DatabaseMode::EphemeralTest). + fn resolve_database_mode( + &self, + conf: &super::ConfigToml, + ) -> anyhow::Result { + crate::persistence::sql::DatabaseMode::resolve_test(conf.general.database_url.clone()) + } + fn ensure_data_dir_exists_and_is_writable(&self) -> anyhow::Result<()> { Ok(()) // Always ok because this is validated by the tempfile crate. } @@ -68,3 +76,33 @@ impl DataDir for MockDataDir { Ok(self.keypair.clone()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::persistence::sql::{ConnectionString, DatabaseMode}; + + #[test] + fn resolve_database_mode_returns_ephemeral() { + let mock = MockDataDir::test(); + let conf = mock.config_toml.clone(); + let mode = mock.resolve_database_mode(&conf).unwrap(); + assert!( + matches!(mode, DatabaseMode::EphemeralTest(_)), + "MockDataDir should always resolve to EphemeralTest" + ); + } + + #[test] + fn resolve_database_mode_with_explicit_url_returns_ephemeral() { + let mut config = super::super::ConfigToml::default_test_config(); + config.general.database_url = + Some(ConnectionString::new("postgres://custom:5432/mydb").unwrap()); + let mock = MockDataDir::new(config.clone(), None).unwrap(); + let mode = mock.resolve_database_mode(&config).unwrap(); + assert!( + matches!(mode, DatabaseMode::EphemeralTest(_)), + "MockDataDir should return EphemeralTest even with an explicit URL" + ); + } +} diff --git a/pubky-homeserver/src/data_directory/persistent_data_dir.rs b/pubky-homeserver/src/data_directory/persistent_data_dir.rs index 4814a4061..e1807ada5 100644 --- a/pubky-homeserver/src/data_directory/persistent_data_dir.rs +++ b/pubky-homeserver/src/data_directory/persistent_data_dir.rs @@ -100,6 +100,22 @@ impl DataDir for PersistentDataDir { &self.expanded_path } + /// Connects to the configured URL with [`DatabaseMode::Direct`](crate::persistence::sql::DatabaseMode::Direct). + fn resolve_database_mode( + &self, + conf: &ConfigToml, + ) -> anyhow::Result { + conf.general + .database_url + .clone() + .map(crate::persistence::sql::DatabaseMode::Direct) + .ok_or_else(|| { + anyhow::anyhow!( + "No database_url configured. Set [general].database_url in config.toml." + ) + }) + } + /// Makes sure the data directory exists. /// Create the directory if it doesn't exist. fn ensure_data_dir_exists_and_is_writable(&self) -> anyhow::Result<()> { @@ -247,6 +263,37 @@ mod tests { assert_eq!(content, "test"); } + #[test] + fn resolve_database_mode_returns_direct_when_url_set() { + use crate::persistence::sql::{ConnectionString, DatabaseMode}; + + let mut conf = ConfigToml::default(); + conf.general.database_url = + Some(ConnectionString::new("postgres://localhost:5432/mydb").unwrap()); + + let temp_dir = TempDir::new().unwrap(); + let data_dir = PersistentDataDir::new(temp_dir.path().to_path_buf()); + let mode = data_dir.resolve_database_mode(&conf).unwrap(); + assert!( + matches!(mode, DatabaseMode::Direct(_)), + "PersistentDataDir should resolve to Direct" + ); + } + + #[test] + fn resolve_database_mode_errors_when_no_url() { + let mut conf = ConfigToml::default(); + conf.general.database_url = None; + + let temp_dir = TempDir::new().unwrap(); + let data_dir = PersistentDataDir::new(temp_dir.path().to_path_buf()); + let result = data_dir.resolve_database_mode(&conf); + assert!( + result.is_err(), + "PersistentDataDir should error when database_url is None" + ); + } + #[test] pub fn test_trim_secret_file_content() { let temp_dir = TempDir::new().unwrap(); diff --git a/pubky-homeserver/src/lib.rs b/pubky-homeserver/src/lib.rs index 84a19e3d4..f078d17f2 100644 --- a/pubky-homeserver/src/lib.rs +++ b/pubky-homeserver/src/lib.rs @@ -36,7 +36,7 @@ pub use data_directory::{ }; pub use homeserver_app::{HomeserverApp, HomeserverAppBuildError}; pub use metrics_server::{MetricsServer, MetricsServerBuildError}; -pub use persistence::sql::ConnectionString; +pub use persistence::sql::{ConnectionString, DatabaseMode}; pub use shared::quota::{ BandwidthQuota, DefaultQuotasToml, GlobPattern, HttpMethod, LimitKey, LimitKeyType, PathLimit, RequestCountQuota, TimeUnit, diff --git a/pubky-homeserver/src/persistence/sql/connection_string.rs b/pubky-homeserver/src/persistence/sql/connection_string.rs index a3ede42fc..835bb3b92 100644 --- a/pubky-homeserver/src/persistence/sql/connection_string.rs +++ b/pubky-homeserver/src/persistence/sql/connection_string.rs @@ -11,11 +11,16 @@ impl ConnectionString { /// Create a new connection string from a string. /// This function validates that the connection string is a postgres connection string. pub fn new(con_string: &str) -> anyhow::Result { - let con = Self(url::Url::parse(con_string)?); - if !con.is_postgres() { + Self::validated(url::Url::parse(con_string)?) + } + + /// Shared validation: ensures the URL uses a postgres scheme. + fn validated(url: url::Url) -> anyhow::Result { + let cs = Self(url); + if !cs.is_postgres() { anyhow::bail!("Only postgres database urls are supported"); } - Ok(con) + Ok(cs) } /// Get the connection string as a str. @@ -29,45 +34,50 @@ impl ConnectionString { /// Get the database name /// For postgres, this is the database name directly - /// For sqlite, this is the path to the database file pub fn database_name(&self) -> &str { self.0.path().trim_start_matches("/") } - /// Set the database name + /// Set the database name, clearing any `dbname` query parameter that would + /// otherwise override the path. See + /// pub fn set_database_name(&mut self, db_name: &str) { self.0.set_path(db_name); - } -} - -#[cfg(any(test, feature = "testing"))] -impl ConnectionString { - /// Returns a connection string for a test database. - /// This is a postgres database that is not real. - /// It is used as an indicator for a empheral test database. - pub fn default_test_db() -> Self { - Self::new("postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true").unwrap() + self.remove_query_param("dbname"); } - /// Returns true if the connection string is for a test database. - pub fn is_test_db(&self) -> bool { - self.0 + /// Remove all occurrences of a query parameter by key. + fn remove_query_param(&mut self, key: &str) { + if self.0.query().is_none() { + return; + } + let pairs: Vec<_> = self + .0 .query_pairs() - .any(|(key, value)| key == "pubky-test" && value == "true") + .filter(|(k, _)| k != key) + .map(|(k, v)| (k.into_owned(), v.into_owned())) + .collect(); + if pairs.is_empty() { + self.0.set_query(None); + } else { + self.0.query_pairs_mut().clear().extend_pairs(&pairs); + } } } -impl From for ConnectionString { - fn from(url: url::Url) -> Self { - Self(url) +impl TryFrom for ConnectionString { + type Error = anyhow::Error; + + fn try_from(url: url::Url) -> Result { + Self::validated(url) } } impl FromStr for ConnectionString { - type Err = url::ParseError; + type Err = anyhow::Error; fn from_str(s: &str) -> Result { - Ok(Self(url::Url::parse(s)?)) + Self::new(s) } } @@ -96,25 +106,61 @@ impl<'de> Deserialize<'de> for ConnectionString { } } -impl Default for ConnectionString { - fn default() -> Self { - Self::new("postgres://localhost:5432/pubky_homeserver").unwrap() - } -} - #[cfg(test)] mod tests { use super::*; - #[tokio::test] - #[pubky_test_utils::test] - async fn test_create_db() { - let con_strings = vec![ - "postgres://localhost:5432/pubky_homeserver", - "sqlite:///path/to/sqlite.db", - ]; - for con_string in con_strings { - let _: ConnectionString = con_string.parse().unwrap(); - } + #[test] + fn test_valid_postgres_url() { + let _: ConnectionString = "postgres://localhost:5432/pubky_homeserver" + .parse() + .unwrap(); + } + + #[test] + fn test_non_postgres_url_rejected() { + let result: Result = "sqlite:///path/to/sqlite.db".parse(); + assert!(result.is_err(), "sqlite URLs should be rejected"); + } + + #[test] + fn set_database_name_changes_path() { + let mut cs = ConnectionString::new("postgres://user:pass@localhost:5432/original").unwrap(); + cs.set_database_name("new_db"); + assert_eq!(cs.database_name(), "new_db"); + } + + #[test] + fn set_database_name_strips_dbname_query_param() { + let mut cs = + ConnectionString::new("postgres://user:pass@localhost:5432/postgres?dbname=postgres") + .unwrap(); + cs.set_database_name("pubky_test_abc123"); + assert_eq!(cs.database_name(), "pubky_test_abc123"); + assert!( + !cs.as_str().contains("dbname="), + "dbname query param should be removed, got: {}", + cs.as_str() + ); + } + + #[test] + fn set_database_name_preserves_other_query_params() { + let mut cs = ConnectionString::new( + "postgres://user:pass@localhost:5432/postgres?dbname=postgres&sslmode=require", + ) + .unwrap(); + cs.set_database_name("pubky_test_abc123"); + assert_eq!(cs.database_name(), "pubky_test_abc123"); + assert!( + !cs.as_str().contains("dbname="), + "dbname should be removed, got: {}", + cs.as_str() + ); + assert!( + cs.as_str().contains("sslmode=require"), + "other params should be preserved, got: {}", + cs.as_str() + ); } } diff --git a/pubky-homeserver/src/persistence/sql/database_mode.rs b/pubky-homeserver/src/persistence/sql/database_mode.rs new file mode 100644 index 000000000..8507079a5 --- /dev/null +++ b/pubky-homeserver/src/persistence/sql/database_mode.rs @@ -0,0 +1,120 @@ +use super::connection_string::ConnectionString; + +/// How the homeserver should connect to its database. +/// +/// This enum makes the distinction between "connect to an existing database" +/// and "create a fresh ephemeral database for this test" explicit. +#[derive(Debug, Clone)] +pub enum DatabaseMode { + /// Connect directly to the database identified by the URL. + /// Used in production and persistent testnets. + Direct(ConnectionString), + + /// Create an ephemeral `pubky_test_{uuid}` database on the server + /// identified by the URL, then connect to it. + /// + /// Dropping the [`SqlDb`](super::SqlDb) **registers** the database for + /// cleanup but does not delete it immediately. Actual deletion requires + /// the `#[pubky_testnet::test]` macro or an explicit call to + /// [`drop_test_databases()`](pubky_test_utils::drop_test_databases). + /// Without either, the database will be leaked. + /// + /// Only available in test / testing builds. + #[cfg(any(test, feature = "testing"))] + EphemeralTest(ConnectionString), +} + +impl DatabaseMode { + /// Returns the underlying connection string, regardless of mode. + #[cfg(test)] + pub fn connection_string(&self) -> &ConnectionString { + match self { + Self::Direct(url) => url, + #[cfg(any(test, feature = "testing"))] + Self::EphemeralTest(url) => url, + } + } +} + +#[cfg(any(test, feature = "testing"))] +const DEFAULT_TEST_SERVER: &str = "postgres://localhost:5432/postgres"; + +#[cfg(any(test, feature = "testing"))] +impl DatabaseMode { + /// Resolve a `database_url` from config into a `DatabaseMode`. + /// + /// Priority: + /// 1. Explicitly provided URL (e.g. from Docker Postgres or config) → `EphemeralTest` + /// 2. `TEST_PUBKY_CONNECTION_STRING` environment variable → `EphemeralTest` + /// 3. [`DEFAULT_TEST_SERVER`] fallback → `EphemeralTest` + /// + /// Tests always get ephemeral databases — the decision is encoded here, + /// not in a URL query parameter. + pub fn resolve_test(explicit: Option) -> anyhow::Result { + let env_val = std::env::var("TEST_PUBKY_CONNECTION_STRING").ok(); + Self::resolve_test_inner(explicit, env_val) + } + + /// Pure resolution logic, separated from env access for testability. + fn resolve_test_inner( + explicit: Option, + env_val: Option, + ) -> anyhow::Result { + let url = match (explicit, env_val) { + (Some(url), _) => url, + (None, Some(raw)) => ConnectionString::new(&raw).map_err(|e| { + anyhow::anyhow!("Invalid TEST_PUBKY_CONNECTION_STRING: {raw}. Error: {e}") + })?, + (None, None) => ConnectionString::new(DEFAULT_TEST_SERVER) + .expect("Default test connection string is valid"), + }; + Ok(Self::EphemeralTest(url)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolve_explicit_wins_over_env() { + let explicit = ConnectionString::new("postgres://custom:5432/mydb").unwrap(); + let env_val = Some("postgres://envhost:5432/envdb".to_string()); + let result = DatabaseMode::resolve_test_inner(Some(explicit.clone()), env_val).unwrap(); + assert_eq!(result.connection_string(), &explicit); + assert!(matches!(result, DatabaseMode::EphemeralTest(_))); + } + + #[test] + fn resolve_env_var_used_when_no_explicit() { + let env_val = Some("postgres://envhost:5432/envdb".to_string()); + let result = DatabaseMode::resolve_test_inner(None, env_val).unwrap(); + assert_eq!( + result.connection_string().as_str(), + "postgres://envhost:5432/envdb" + ); + assert!(matches!(result, DatabaseMode::EphemeralTest(_))); + } + + #[test] + fn resolve_falls_back_to_default() { + let result = DatabaseMode::resolve_test_inner(None, None).unwrap(); + assert_eq!(result.connection_string().as_str(), DEFAULT_TEST_SERVER); + assert!(matches!(result, DatabaseMode::EphemeralTest(_))); + } + + #[test] + fn resolve_invalid_env_var_errors() { + let env_val = Some("not-a-valid-url".to_string()); + let result = DatabaseMode::resolve_test_inner(None, env_val); + assert!(result.is_err()); + } + + #[test] + fn resolve_old_style_url_with_pubky_test_param_still_works() { + let env_val = + Some("postgres://user:pass@localhost:5432/postgres?pubky-test=true".to_string()); + let result = DatabaseMode::resolve_test_inner(None, env_val).unwrap(); + assert!(matches!(result, DatabaseMode::EphemeralTest(_))); + } +} diff --git a/pubky-homeserver/src/persistence/sql/mod.rs b/pubky-homeserver/src/persistence/sql/mod.rs index 24f87a1ab..2127c5ce7 100644 --- a/pubky-homeserver/src/persistence/sql/mod.rs +++ b/pubky-homeserver/src/persistence/sql/mod.rs @@ -6,6 +6,7 @@ //! both pooled connections and explicit transactions. mod connection_string; +mod database_mode; pub(crate) mod entities; mod migration; pub(crate) mod migrations; @@ -15,6 +16,7 @@ mod sql_db; mod unified_executor; pub use connection_string::ConnectionString; +pub use database_mode::DatabaseMode; pub use entities::entry; pub use entities::signup_code; pub(crate) use entities::user; diff --git a/pubky-homeserver/src/persistence/sql/sql_db.rs b/pubky-homeserver/src/persistence/sql/sql_db.rs index ee58f6779..4e0dee485 100644 --- a/pubky-homeserver/src/persistence/sql/sql_db.rs +++ b/pubky-homeserver/src/persistence/sql/sql_db.rs @@ -3,6 +3,7 @@ use sqlx::postgres::PgPool; use sqlx::postgres::PgPoolOptions; use crate::persistence::sql::connection_string::ConnectionString; +use crate::persistence::sql::database_mode::DatabaseMode; /// The SqlDb is a wrapper around the postgres connection pool. /// It is used to connect to the database and run queries. @@ -28,17 +29,23 @@ impl std::fmt::Debug for SqlDb { } impl SqlDb { - /// Connect to the database. Respects the pubky_test flag - pub async fn connect(con_string: &ConnectionString) -> Result { - #[cfg(any(test, feature = "testing"))] - if con_string.is_test_db() { - return Self::test_postgres_db(Some(con_string.clone())).await; + /// Connect to the database using the given [`DatabaseMode`]. + /// + /// - [`DatabaseMode::Direct`]: connects to the database identified by the URL. + /// - [`DatabaseMode::EphemeralTest`]: creates a fresh `pubky_test_{uuid}` database + /// on the server and connects to it. Dropping this `SqlDb` registers the + /// database for cleanup; see [`DatabaseMode::EphemeralTest`] for details. + pub async fn connect(mode: DatabaseMode) -> Result { + match mode { + DatabaseMode::Direct(url) => Self::connect_inner(&url).await, + #[cfg(any(test, feature = "testing"))] + DatabaseMode::EphemeralTest(admin_url) => { + Self::create_ephemeral_test_db(admin_url).await + } } - - Self::connect_inner(con_string).await } - /// Connect to the database. directly without any test db logic. + /// Connect to the database directly without any test db logic. async fn connect_inner(con_string: &ConnectionString) -> Result { let pool: PgPool = PgPool::connect(con_string.as_str()).await?; Ok(Self { @@ -54,7 +61,11 @@ impl SqlDb { } } -/// Helper struct to drop the postgres test database after the db connection is dropped. +/// Registers an ephemeral test database for cleanup when dropped. +/// +/// Dropping this struct does **not** delete the database immediately — it +/// queues it for later removal by [`drop_test_databases()`](pubky_test_utils::drop_test_databases), +/// which is called automatically by the `#[pubky_testnet::test]` macro. #[cfg(any(test, feature = "testing"))] struct TestDbDropper { db_name: String, @@ -83,87 +94,57 @@ impl Drop for TestDbDropper { } } -#[cfg(any(test, feature = "testing"))] -const DEFAULT_TEST_CONNECTION_STRING: &str = "postgres://localhost:5432/postgres"; - #[cfg(any(test, feature = "testing"))] impl SqlDb { - /// Creates a new test database with the name `pubky_test_{uuid}`. - /// The provided `admin_con_string` is used to create the test database. The database name defined by the admin connection string - /// is only used to create the actual test database. - /// If no connection string is passed, the connection string is read from the TEST_PUBKY_CONNECTION_STRING environment variable. - /// If the environment variable is not set, the default test connection string is used. - async fn create_test_database( + /// Creates an ephemeral `pubky_test_{uuid}` database and connects to it. + /// + /// `admin_con_string` is used for the initial admin connection that creates + /// the database. The returned `SqlDb` registers the database for cleanup + /// on drop (see [`TestDbDropper`]). + async fn create_ephemeral_test_db( admin_con_string: ConnectionString, - ) -> Result { - use uuid::Uuid; - let admin_con = Self::connect_inner(&admin_con_string).await?; - let test_db_name = format!("pubky_test_{}", Uuid::new_v4().as_simple()); - let query = format!("CREATE DATABASE {}", test_db_name); - sqlx::query(&query).execute(admin_con.pool()).await?; - let mut test_db_con_string = admin_con_string.clone(); - test_db_con_string.set_database_name(&test_db_name); - Ok(test_db_con_string) - } - /// Creates a new test database with the name `pubky_test_{uuid}`. - /// The provided `admin_con_string` is used to create the test database. The database name defined by the admin connection string - /// is only used to create the actual test database. - /// If no connection string is passed, the connection string is read from the TEST_PUBKY_CONNECTION_STRING environment variable. - /// If the environment variable is not set, the default test connection string is used. - pub async fn test_postgres_db( - admin_con_string: Option, ) -> Result { - let admin_con_string = Self::derive_connection_string(admin_con_string); - - let test_db_con_string = Self::create_test_database(admin_con_string.clone()).await?; + let (test_db_url, test_db_name) = + Self::create_ephemeral_db_on_server(&admin_con_string).await?; - // Connect to the test database. - let mut con = Self::connect_inner(&test_db_con_string).await?; + let mut con = Self::connect_inner(&test_db_url).await?; con.db_dropper = Some(std::sync::Arc::new(TestDbDropper::new( - test_db_con_string.database_name().to_string(), + test_db_name, admin_con_string.to_string(), ))); Ok(con) } - /// Derives the admin connection string to use for the test database creation. - /// If the user passed a connection string, use it. - /// If the user passed a connection string as a env variable, use it. - /// If no connection string is passed, use the default test connection string. - pub fn derive_connection_string( - admin_con_string: Option, - ) -> ConnectionString { - if let Some(con_string) = admin_con_string { - // If the user passed a connection string, use it. - return con_string.clone(); - } - if let Ok(raw_con_string) = std::env::var("TEST_PUBKY_CONNECTION_STRING") { - // If the user passed a connection string as a env variable, use it. - match ConnectionString::new(&raw_con_string) { - Ok(con_string) => return con_string, - Err(e) => { - tracing::warn!("Invalid database connection string in TEST_PUBKY_CONNECTION_STRING environment variable: {}. Fallback to default test connection string. Error: {e}", raw_con_string); - } - } - } + /// Create an ephemeral `pubky_test_{uuid}` database on the given server. + /// + /// Returns the connection string to the new database and its name. + async fn create_ephemeral_db_on_server( + admin_con_string: &ConnectionString, + ) -> Result<(ConnectionString, String), sqlx::Error> { + use uuid::Uuid; - // If no connection string is passed, use the default test connection string. - ConnectionString::new(DEFAULT_TEST_CONNECTION_STRING) - .expect("Default test connection string is valid") + let admin_con = Self::connect_inner(admin_con_string).await?; + let test_db_name = format!("pubky_test_{}", Uuid::new_v4().as_simple()); + let query = format!("CREATE DATABASE \"{}\"", test_db_name); + sqlx::query(&query).execute(admin_con.pool()).await?; + + let mut test_db_url = admin_con_string.clone(); + test_db_url.set_database_name(&test_db_name); + Ok((test_db_url, test_db_name)) } - /// Create a test database without running migrations - /// If the DB_CONNECTION_STRING environment variable is not set, a temporary directory is used for the sqlite database - /// If the DB_CONNECTION_STRING environment variable is set, the test database is created on the existing database + /// Create a test database without running migrations. + #[cfg(test)] pub async fn test_without_migrations() -> Self { - Self::test_postgres_db(None) + let mode = DatabaseMode::resolve_test(None).expect("Failed to resolve test database mode"); + Self::connect(mode) .await .expect("Failed to create test database") } - /// Create a test database and run migrations - /// If the DB_CONNECTION_STRING environment variable is not set, a temporary directory is used for the sqlite database - /// If the DB_CONNECTION_STRING environment variable is set, the migrations are run on the existing database + /// Create a test database and run migrations. + /// Convenience wrapper around [`Self::test_without_migrations`] + [`Migrator::run`]. + #[cfg(test)] pub async fn test() -> Self { use crate::persistence::sql::migrator::Migrator; let db = Self::test_without_migrations().await; @@ -179,21 +160,25 @@ impl SqlDb { max_connections: u32, acquire_timeout: std::time::Duration, ) -> Self { - let admin_con_string = Self::derive_connection_string(None); - let test_db_con_string = Self::create_test_database(admin_con_string.clone()) + let mode = DatabaseMode::resolve_test(None).expect("Failed to resolve test database mode"); + let admin_url = match mode { + DatabaseMode::EphemeralTest(url) | DatabaseMode::Direct(url) => url, + }; + let (test_db_url, test_db_name) = Self::create_ephemeral_db_on_server(&admin_url) .await .expect("Failed to create test database"); + let pool = PgPoolOptions::new() .max_connections(max_connections) .acquire_timeout(acquire_timeout) - .connect(test_db_con_string.as_str()) + .connect(test_db_url.as_str()) .await .expect("Failed to connect to test database"); let db = Self { pool, db_dropper: Some(std::sync::Arc::new(TestDbDropper::new( - test_db_con_string.database_name().to_string(), - admin_con_string.to_string(), + test_db_name, + admin_url.to_string(), ))), }; let migrator = crate::persistence::sql::migrator::Migrator::new(&db); @@ -208,7 +193,8 @@ mod tests { #[tokio::test] #[pubky_test_utils::test] - async fn test_pg_db_available() { - let _db = SqlDb::test_postgres_db(None).await.unwrap(); + async fn pg_db_available() { + let mode = DatabaseMode::resolve_test(None).unwrap(); + let _db = SqlDb::connect(mode).await.unwrap(); } } diff --git a/pubky-testnet/README.md b/pubky-testnet/README.md index 3dea04fea..f3ee1647d 100644 --- a/pubky-testnet/README.md +++ b/pubky-testnet/README.md @@ -34,10 +34,10 @@ TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgr cargo run -p pubky-testnet -- --homeserver-config my-config.toml persist ./my-testnet-data ``` -If you don't need persistent state, omit the `persist` subcommand and add `?pubky-test=true` to the connection string. The database is auto-created on startup and cleaned up on shutdown: +If you don't need persistent state, simply omit the `persist` subcommand. An ephemeral database is auto-created on startup and cleaned up on shutdown: ```bash -TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true' \ +TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \ cargo run -p pubky-testnet ``` @@ -80,13 +80,15 @@ async fn my_test() { ### Postgres for tests -You need a running PostgreSQL instance (see [Quick start](#quick-start) for a Docker one-liner). By default, `EphemeralTestnet` reads the `TEST_PUBKY_CONNECTION_STRING` environment variable. The `?pubky-test=true` parameter tells the homeserver to create an ephemeral `pubky_test_*` database. The `#[pubky_testnet::test]` macro ensures the database is cleaned up after the test completes or panics. +You need a running PostgreSQL instance (see [Quick start](#quick-start) for a Docker one-liner). By default, `EphemeralTestnet` reads the `TEST_PUBKY_CONNECTION_STRING` environment variable: ```bash -TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true' \ +TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \ cargo test -p my-crate ``` +Each test automatically gets its own ephemeral `pubky_test_{uuid}` database on the configured server. The `#[pubky_testnet::test]` macro ensures the database is cleaned up after the test completes or panics. + You can also pass the connection string programmatically: ```rust,no_run @@ -96,7 +98,7 @@ use pubky_testnet::{EphemeralTestnet, pubky_homeserver::ConnectionString}; #[pubky_testnet::test] async fn my_test() { let connection_string = ConnectionString::new( - "postgres://postgres:postgres@localhost:5432/postgres?pubky-test=true" + "postgres://postgres:postgres@localhost:5432/postgres" ).unwrap(); let testnet = EphemeralTestnet::builder() @@ -133,7 +135,13 @@ async fn main() { } ``` -Each call to `.with_docker_postgres()` starts a **separate** container. To share **one** container across all tests, use `DockerPostgres::shared()`: +> **Important**: If you have multiple tests, see [Sharing Docker Postgres Across Tests](#sharing-docker-postgres-across-tests) below. + +### Sharing Docker Postgres Across Tests + +Each `.with_docker_postgres()` starts a **separate** container. To avoid that overhead, +use `DockerPostgres::shared()` to start one container and reuse it. Tests remain isolated +— each testnet gets its own ephemeral database. ```rust # #[cfg(feature = "docker-postgres")] @@ -165,9 +173,7 @@ async fn test_two() { # } ``` -Each testnet still gets its own ephemeral database within the shared PostgreSQL instance, so tests remain isolated. - -### Custom configuration +### Custom Configuration ```rust,no_run use pubky_testnet::{EphemeralTestnet, pubky_homeserver::ConfigToml, pubky::Keypair}; diff --git a/pubky-testnet/src/docker_postgres.rs b/pubky-testnet/src/docker_postgres.rs index dfdbe097c..1c60ab79a 100644 --- a/pubky-testnet/src/docker_postgres.rs +++ b/pubky-testnet/src/docker_postgres.rs @@ -36,29 +36,11 @@ extern "C" fn cleanup_shared_container() { /// A containerized PostgreSQL instance for testing. /// -/// This wraps a testcontainers `Postgres` container and manages its lifecycle. -/// The container is automatically stopped and removed when this struct is dropped. +/// The container is automatically cleaned up on drop, on Ctrl+C/SIGTERM, +/// and (for the shared instance) on normal process exit via an `atexit` hook. /// -/// # Sharing Across Tests (Recommended) -/// -/// Each `DockerPostgres::start()` starts a **separate** PostgreSQL container. -/// Use [`DockerPostgres::shared()`] to start **one** instance and share it across tests: -/// -/// ```ignore -/// use pubky_testnet::docker_postgres::DockerPostgres; -/// use pubky_testnet::EphemeralTestnet; -/// -/// #[tokio::test] -/// async fn my_test() { -/// let pg = DockerPostgres::shared().await; -/// let testnet = EphemeralTestnet::builder() -/// .postgres(pg.connection_string().unwrap()) -/// .build() -/// .await -/// .unwrap(); -/// // Each testnet gets its own ephemeral database — tests remain isolated. -/// } -/// ``` +/// Multiple testnets can safely share one container — each gets its own +/// isolated database. See [`Self::connection_string()`] for details. pub struct DockerPostgres { _container: ContainerAsync, host: String, @@ -70,14 +52,10 @@ pub struct DockerPostgres { pub type EmbeddedPostgres = DockerPostgres; impl DockerPostgres { - /// Return a shared Docker PostgreSQL instance, starting it on first call. - /// - /// This is the recommended way to share a single PostgreSQL container across - /// multiple tests. Docker handles all cleanup automatically. + /// Return a shared Docker PostgreSQL container, starting it on first call. /// - /// An `atexit` hook is registered to ensure the container is removed even on - /// normal process exit (Rust never drops statics, so the testcontainers - /// `Drop` impl alone is not sufficient). + /// Avoids the overhead of starting a separate container per test. + /// Each testnet still gets its own isolated database. /// /// # Panics /// @@ -127,6 +105,10 @@ impl DockerPostgres { } /// Get the connection string for this Docker PostgreSQL instance. + /// + /// The [`DatabaseMode`](pubky_homeserver::DatabaseMode) enum — not the URL + /// itself — controls whether the homeserver creates an ephemeral + /// `pubky_test_{uuid}` database. pub fn connection_string(&self) -> anyhow::Result { let url = format!( "postgres://postgres:postgres@{}:{}/postgres", @@ -146,6 +128,7 @@ mod tests { use super::DockerPostgres; use crate::EphemeralTestnet; use pubky::Keypair; + use pubky_common::auth::jws::ClientId; const CONTAINER_ID_PREFIX: &str = "CONTAINER_ID="; @@ -180,6 +163,7 @@ mod tests { /// Basic integration test: start a testnet with docker postgres + http relay, /// signup a user, store and retrieve data. #[tokio::test] + #[crate::test] async fn test_docker_postgres_with_testnet() { let testnet = EphemeralTestnet::builder() .with_docker_postgres() @@ -199,10 +183,14 @@ mod tests { let keypair = Keypair::random(); let signer = pubky.signer(keypair); - let session = signer - .signup_cookie(&testnet.homeserver_app().public_key(), None) + signer + .signup(&testnet.homeserver_app().public_key(), None) .await .expect("Failed to signup user"); + let session = signer + .signin(ClientId::new("test").unwrap()) + .await + .expect("Failed to signin user"); // Store and retrieve data let path = "/pub/test.txt"; @@ -298,6 +286,56 @@ mod tests { ); } + /// Verify that two testnets sharing a `DockerPostgres` get isolated databases. + /// + /// Signs up a user on testnet A, then verifies that same keypair can sign up + /// on testnet B (proving it has a separate, empty database). + /// + /// This also validates that the configured connection string (random Docker + /// port) flows through `DatabaseMode::resolve_test` → `EphemeralTest` → + /// `create_ephemeral_test_db()`. Without that, it would fall back to + /// `localhost:5432` and fail to connect. + #[tokio::test] + #[crate::test] + async fn test_shared_docker_postgres_provides_db_isolation() { + let pg = DockerPostgres::start() + .await + .expect("Failed to start docker postgres"); + + let keypair = Keypair::random(); + + // Build two independent testnets sharing the same Postgres container. + let testnet_a = EphemeralTestnet::builder() + .postgres(pg.connection_string().unwrap()) + .build() + .await + .expect("Failed to start testnet A"); + + let testnet_b = EphemeralTestnet::builder() + .postgres(pg.connection_string().unwrap()) + .keypair(Keypair::random()) // different homeserver identity + .build() + .await + .expect("Failed to start testnet B"); + + // Sign up the user on testnet A. + let sdk_a = testnet_a.sdk().expect("Failed to create SDK A"); + let signer_a = sdk_a.signer(keypair.clone()); + signer_a + .signup(&testnet_a.homeserver_app().public_key(), None) + .await + .expect("Signup on testnet A should succeed"); + + // The same keypair should be able to sign up on testnet B, + // proving it has its own isolated database. + let sdk_b = testnet_b.sdk().expect("Failed to create SDK B"); + let signer_b = sdk_b.signer(keypair); + signer_b + .signup(&testnet_b.homeserver_app().public_key(), None) + .await + .expect("Signup on testnet B should succeed (proves DB isolation)"); + } + /// Test that specifying both docker postgres and a custom connection string fails. #[tokio::test] async fn test_docker_postgres_and_custom_connection_string_fails() { diff --git a/pubky-testnet/src/ephemeral_testnet.rs b/pubky-testnet/src/ephemeral_testnet.rs index 988020dd8..9fe4bb035 100644 --- a/pubky-testnet/src/ephemeral_testnet.rs +++ b/pubky-testnet/src/ephemeral_testnet.rs @@ -195,9 +195,10 @@ impl EphemeralTestnetBuilder { .homeserver_config .unwrap_or_else(ConfigToml::minimal_test_config); - if let Some(connection_string) = testnet.postgres_connection_string.as_ref() { - config.general.database_url = connection_string.clone(); - } + config.general.database_url = testnet + .postgres_connection_string + .clone() + .or(config.general.database_url); let keypair = self .homeserver_keypair @@ -328,9 +329,11 @@ impl EphemeralTestnet { ) -> anyhow::Result<&HomeserverApp> { let mut config = config.unwrap_or_else(ConfigToml::minimal_test_config); - if let Some(connection_string) = self.testnet.postgres_connection_string.as_ref() { - config.general.database_url = connection_string.clone(); - } + config.general.database_url = self + .testnet + .postgres_connection_string + .clone() + .or(config.general.database_url); let mock_dir = MockDataDir::new(config, Some(Keypair::random()))?; self.testnet.create_homeserver_app_with_mock(mock_dir).await @@ -383,6 +386,7 @@ mod test { /// This is to prevent the case where the testnet is not cleaned up properly. /// For example, if the port is not released after the testnet is stopped. #[tokio::test] + #[crate::test] async fn test_two_testnet_in_a_row() { { let _ = EphemeralTestnet::builder().build().await.unwrap(); @@ -394,6 +398,7 @@ mod test { } #[tokio::test] + #[crate::test] async fn test_homeserver_with_random_keypair() { // Start with just DHT + http relay, no homeserver let mut testnet = Testnet::new().await.unwrap(); @@ -417,6 +422,7 @@ mod test { } #[tokio::test] + #[crate::test] async fn test_builder_default() { // Verify builder creates homeserver with minimal config (admin disabled) let network = EphemeralTestnet::builder().build().await.unwrap(); @@ -434,6 +440,7 @@ mod test { } #[tokio::test] + #[crate::test] async fn test_builder_with_custom_config() { // Verify custom config is used (e.g., metrics enabled) let mut config = ConfigToml::minimal_test_config(); @@ -457,6 +464,7 @@ mod test { } #[tokio::test] + #[crate::test] async fn test_builder_with_custom_keypair() { // Verify custom keypair is used let keypair = Keypair::random(); diff --git a/pubky-testnet/src/static_testnet.rs b/pubky-testnet/src/static_testnet.rs index ebd011692..8d7139fd2 100644 --- a/pubky-testnet/src/static_testnet.rs +++ b/pubky-testnet/src/static_testnet.rs @@ -424,6 +424,13 @@ impl DataDir for TestnetDataDir { self.inner.path() } + fn resolve_database_mode( + &self, + conf: &ConfigToml, + ) -> anyhow::Result { + self.inner.resolve_database_mode(conf) + } + fn ensure_data_dir_exists_and_is_writable(&self) -> anyhow::Result<()> { self.inner.ensure_data_dir_exists_and_is_writable() } @@ -431,13 +438,14 @@ impl DataDir for TestnetDataDir { fn read_or_create_config_file(&self) -> anyhow::Result { let mut config = self.inner.read_or_create_config_file()?; apply_static_testnet_overrides(&mut config, self.dht_bootstrap_nodes.clone()); - if let Some(connection_string) = &self.postgres_connection_string { - config.general.database_url = connection_string.clone(); - } - if config.general.database_url.is_test_db() { + config.general.database_url = self + .postgres_connection_string + .clone() + .or(config.general.database_url); + if config.general.database_url.is_none() { anyhow::bail!( - "Persistent testnet requires a real database. \ - Remove `?pubky-test=true` from the connection string to use a persistent database." + "Persistent testnet requires an explicit database URL. \ + Set `database_url` in config.toml under [general]." ); } Ok(config) diff --git a/pubky-testnet/src/testnet.rs b/pubky-testnet/src/testnet.rs index 160f95847..92e99e74a 100644 --- a/pubky-testnet/src/testnet.rs +++ b/pubky-testnet/src/testnet.rs @@ -39,8 +39,7 @@ impl Testnet { http_relays: vec![], homeservers: vec![], temp_dirs: vec![], - postgres_connection_string: Self::extract_postgres_connection_string_from_env_variable( - ), + postgres_connection_string: None, }; Ok(testnet) @@ -75,29 +74,13 @@ impl Testnet { Ok(testnet) } - /// Extract the postgres connection string from the TEST_PUBKY_CONNECTION_STRING environment variable. - /// If the environment variable is not set, None is returned. - /// If the environment variable is set, but the connection string is invalid, a warning is logged and None is returned. - fn extract_postgres_connection_string_from_env_variable() -> Option { - if let Ok(raw_con_string) = std::env::var("TEST_PUBKY_CONNECTION_STRING") { - if let Ok(con_string) = ConnectionString::new(&raw_con_string) { - return Some(con_string); - } else { - tracing::warn!("Invalid database connection string in TEST_PUBKY_CONNECTION_STRING environment variable. Ignoring it."); - } - } - None - } - /// Run the full homeserver app with core and admin server. /// /// Uses [`ConfigToml::default_test_config()`] which enables the admin server. /// Automatically listens on ephemeral ports and uses this Testnet's bootstrap nodes and relays. pub async fn create_homeserver(&mut self) -> Result<&HomeserverApp> { let mut config = ConfigToml::default_test_config(); - if let Some(connection_string) = self.postgres_connection_string.as_ref() { - config.general.database_url = connection_string.clone(); - } + config.general.database_url = self.postgres_connection_string.clone(); let mock_dir = MockDataDir::new(config, Some(crate::common::testnet_keypair()))?; self.create_homeserver_app_with_mock(mock_dir).await } @@ -108,9 +91,7 @@ impl Testnet { /// Automatically listens on ephemeral ports and uses this Testnet's bootstrap nodes and relays. pub async fn create_random_homeserver(&mut self) -> Result<&HomeserverApp> { let mut config = ConfigToml::default_test_config(); - if let Some(connection_string) = self.postgres_connection_string.as_ref() { - config.general.database_url = connection_string.clone(); - } + config.general.database_url = self.postgres_connection_string.clone(); let mock_dir = MockDataDir::new(config, Some(Keypair::random()))?; self.create_homeserver_app_with_mock(mock_dir).await } @@ -257,6 +238,7 @@ impl Testnet { mod test { use crate::Testnet; use pubky::Keypair; + use pubky_common::auth::jws::ClientId; /// Make sure the components are kept alive even when dropped. #[tokio::test] @@ -291,7 +273,8 @@ mod test { let signer = sdk.signer(Keypair::random()); - let session = signer.signup_cookie(&hs.public_key(), None).await.unwrap(); + signer.signup(&hs.public_key(), None).await.unwrap(); + let session = signer.signin(ClientId::new("test").unwrap()).await.unwrap(); assert_eq!(session.info().public_key(), &signer.public_key()); } @@ -305,6 +288,7 @@ mod test { /// If everything is linked correctly, the hs_pubky should be resolvable from the pkarr client. #[tokio::test] + #[crate::test] async fn test_homeserver_resolvable() { let mut testnet = Testnet::new().await.unwrap(); let hs_pubky = testnet.create_homeserver().await.unwrap().public_key(); diff --git a/test_utils/drop_db_helper/src/lib.rs b/test_utils/drop_db_helper/src/lib.rs index 591d1dd18..a51db4379 100644 --- a/test_utils/drop_db_helper/src/lib.rs +++ b/test_utils/drop_db_helper/src/lib.rs @@ -20,7 +20,7 @@ impl DbToDrop { /// Drop the database. pub async fn drop(&self) -> Result<(), sqlx::Error> { let pool = PgPool::connect(&self.connection_string).await?; - let query = format!("DROP DATABASE {} WITH (FORCE)", self.db_name); + let query = format!("DROP DATABASE \"{}\" WITH (FORCE)", self.db_name); sqlx::query(&query).execute(&pool).await?; let _ = pool.close().await; // Close connection properly. Ok(())