fix(testnet/homeserver)!: DockerPostgres::shared() to share Postgres instance but isolate databases and remove pubky-test URL query param

[86667]

The idea behind DockerPostgres::shared() is that the work of downloading, building and starting up Postgres shouldn't need to be done for each testnet which needs to use the Postgres instance. For example, in a test suite in which each test requires its own testnet.

The existing behaviour was that the Postgres instance and database was being shared. The fix here is to share the Postgres instance but give each test a fresh, ephemeral database.

These changes motivated a re-write of how databse urls are set, as the logic was spread across different places and behaviour not well-defined.

Previously we configured ephemeral database by a ?pubky-test=true query parameter in the connection URL, a fragile convention that is easy to forget and adds confusion. We replaced it with a DatabaseMode enum that is resolved at startup based on compile-time cfg flags. In test builds you always get EphemeralTest, in production you always get Direct. The URL is now just a server address, it no longer carries behavioral intent.

We also change database_url from ConnectionString to Option<ConnectionString>. Test configs set it to None, deferring resolution to the env var or built-in default. Explicit urls (e.g. Docker Postgres) are Some and used as-is. Production builds throw and error if None.

The priority for database URL resolution in test builds is:

• Explicit URL set in config (e.g. from Docker Postgres or config.toml)
• TEST_PUBKY_CONNECTION_STRING env var
• Built-in test default

[86667]

@MCarlomagno @ok300

Thanks both for review and finding that issue. My bad - I must have made the removal of ?pubky-testnet=true change after testing the persistent StaticTestnet, then not re-tested after.

I have removed the "ephemeral db" decision from the testing cfg and instead drive it off of DataDir. Ie, If a data dir is persisted then so is the db, if datat dir is not persisted then neither is the db.

Im going to look into refactoring the whole DataDir trait with MockDataDir and PersistentDataDir implementors. I dont find it a clean abstraction. I didnt want to make this PR any larger though so used what i had.

[ok300]

we must avoid contacting the public DHT

This only happens if the database_url is unset. So if a MockDataDir is created with an explicit database_url:

let mut config = ConfigToml::minimal_test_config();
config.general.database_url = Some(postgres_url);

let dir = MockDataDir::new(config, None)?;
HomeserverApp::start_with_mock_data_dir(dir).await?;

then the test can publish test records to the public DHT, and / or fail the test if the environment is sandboxed and outbound connections are denied.

I guess in our test suites we wouldn't do that, but the README documents how library users might write their own tests, so others could inadvertently trigger this edge-case:

pubky-homeserver/pubky-homeserver/README.md

Lines 77 to 79 in b4100e7

	let config = ConfigToml::default_test_config();
	let mock_dir = MockDataDir::new(config, None).unwrap();
	let app = HomeserverApp::start_with_mock_data_dir(mock_dir).await.unwrap();

One solution could be to pass db_mode: &DatabaseMode as an extra arg to build_pkarr_builder_from_config then internally check it directly:

- if config_toml.general.database_url.is_none()
+ if matches!(db_mode, DatabaseMode::EphemeralTest(_))

[86667]

This is equal in fragility as before, when a provide database_url without ?pubky-test=true would not isolate pkarr. I'll fix this properly when i refactor the whole data dir/db/file system config situation, if that okay with you.

[ok300]

This change means the persistent command from pubky-testnet/README.md

TEST_PUBKY_CONNECTION_STRING='postgres://postgres:postgres@localhost:5432/postgres' \
  cargo run -p pubky-testnet -- persist ./my-testnet-data

now ignores TEST_PUBKY_CONNECTION_STRING and instead uses the generated config default

 database_url = "postgres://localhost:5432/pubky_homeserver"

from config.sample.toml, which doesn't have the user/pass or the right DB name for the Docker DB setup.

[86667]

this also is pre-existing and also will be addressed in the follow-up refactor

[ok300]

The database is dropped when the SqlDb is dropped.

Dropping SqlDb only queues cleanup; deletion requires #[pubky_testnet::test] or an explicit drop_test_databases() call. Callers without either leak databases on external PostgreSQL instances.

Simplest fix is probably to change these doc lines to say that dropping registers the DB for cleanup and that callers must use the #[pubky_testnet::test] macro or call drop_test_databases(). Ideally the README examples¹ should include or mention that too.

Footnotes

1. https://github.com/pubky/pubky-homeserver/blob/b4100e7849d2db18cc2c8800ad57f6aa0b9f724a/pubky-testnet/README.md?plain=1#L146-L174 ↩

[86667]

Updated doc here and also added missing [crate::test]!

[86667]

I'll also look into making it clearer where/when this is needed, since it was missed on some tests already.

[MCarlomagno]

LGTM