Auto-configure the Postgres application_name when using Docker Compose

[nosan]

gh-40772

I added since 3.4.0 everywhere but maybe it could be merged earlier if everything is ok.

Thanks in advance

[nosan]

Should I also configure ApplicationName in PostgresR2dbcDockerComposeConnectionDetailsFactory?

[nosan]

Should I also configure ApplicationName in PostgresR2dbcDockerComposeConnectionDetailsFactory?

Auto-configure for r2dbc as well.

[snicoll]

Thanks for the PR @nosan. I feel that the testing bits of it are too involved and we should rather attempt to test the logic in simpler tests, if possible. Let us know if you have time to explore that route.

[snicoll]

Those tests shouldn't need to be full integration tests. The only thing that we assert here is that the ConnectionDetails auto-configured bean has the correct state. I can see that we don't have such tests so far but I wonder if it wouldn't be useful to put something in place so that we don't need all the docker machinery.

[wilkinsona]

+1. For this to warrant being a Docker-based test, I think it should somehow check that setting the application_name has had the desired effect on the Postgres side of things.

I'd then prefer that we only have a single Docker-based test that checks this and that the precedence of the different sources of the application name is unit tested without involving Docker. Right now we're adding a lot to the build time and I don't think the extra test coverage warrants that increase.

[snicoll]

Ditto. I can see this also changes the test infrastructure quite a bit and I wonder if that's warranted for this change.

[snicoll]

This is quite involved. I wonder if there wouldn't be a way to simplify this. And if not, perhaps getParameters may need to evolve so that such a handling of the raw string is unnecessary.

[nosan]

I will take a look at how it could be done better and easy to read. I have the same feeling that it looks quite messy.

[wilkinsona]

+1. For this to warrant being a Docker-based test, I think it should somehow check that setting the application_name has had the desired effect on the Postgres side of things.

I'd then prefer that we only have a single Docker-based test that checks this and that the precedence of the different sources of the application name is unit tested without involving Docker. Right now we're adding a lot to the build time and I don't think the extra test coverage warrants that increase.

[nosan]

Thank you for your feedback @snicoll and @wilkinsona

1. I can add unit tests for PostgresJdbcDockerComposeConnectionDetails and PostgresDbR2dbcDockerComposeConnectionDetails without involving docker, to verify that all options are set correctly.
2. I can add a docker integration test for PostgresJdbcDockerComposeConnectionDetailsFactory and PostgresR2dbcDockerComposeConnectionDetailsFactory to verify the application name is correctly set.

	@DockerComposeTest(composeFile = "postgres-compose.yaml", image = TestImage.POSTGRESQL,
			properties = "spring.application.name=my-app")
	void runCreatesConnectionDetailsWithAutoConfiguredApplicationName(JdbcConnectionDetails connectionDetails)
			throws ClassNotFoundException {
		assertThat(connectionDetails.getUsername()).isEqualTo("myuser");
		assertThat(connectionDetails.getPassword()).isEqualTo("secret");
		assertThat(connectionDetails.getJdbcUrl()).startsWith("jdbc:postgresql://").endsWith("?ApplicationName=my-app");
		assertThat(getApplicationName(connectionDetails)).isEqualTo("my-app");
	}

	private String getApplicationName(JdbcConnectionDetails connectionDetails) throws ClassNotFoundException {
		JdbcTemplate template = getJdbcTemplate(connectionDetails);
		return template.queryForObject("select current_setting('application_name')", String.class);
	}

and

@DockerComposeTest(composeFile = "postgres-compose.yaml", image = TestImage.POSTGRESQL,
			properties = "spring.application.name=my-app")
	void runCreatesConnectionDetailsWithAutoConfiguredApplicationName(R2dbcConnectionDetails connectionDetails) {
		assertConnectionDetails(connectionDetails);
		assertThat(getApplicationName(connectionDetails)).isEqualTo("my-app");
	}

	private String getApplicationName(R2dbcConnectionDetails connectionDetails) {
		ConnectionFactoryOptions connectionFactoryOptions = connectionDetails.getConnectionFactoryOptions();
		return DatabaseClient.create(ConnectionFactories.get(connectionFactoryOptions))
			.sql("select current_setting('application_name')")
			.map((row, metadata) -> row.get(0))
			.first()
			.map(Objects::toString)
			.block(Duration.ofSeconds(30));
	}

Does it make sense?

[nosan]

Also, should I revert my DockerComposeTestExtension changes? I can test the application name is set via docker-compose labels.

[nosan]

PR has been updated.

• I reverted my DockerComposeTestExtension and JdbcUrlBuilder changes.
• I added unit tests for both ConnectionDetails and removed docker-compose integration tests.
• I left only two docker-compose integration tests for checking that application_name had the desired effect on the Postgres side.

[wilkinsona]

We discussed this today and it feels a bit overly complicated to us. We think the same could be achieved with something similar to the code that disables encryption for SQL Server. One addition that's probably needed here is to encode the application name as its user-provided input and may contain characters that are restricted in a query string.

[nosan]

@wilkinsona
I initially used this variant

private static String getJdbcUrl(RunningService service, String database, Environment environment) {
			String jdbcUrl = jdbcUrlBuilder.build(service, database);
			return appendApplicationName(jdbcUrl, environment);
		}

private static String appendApplicationName(String jdbcUrl, Environment environment) {
	if (hasParameter(jdbcUrl, "ApplicationName")) {
		return jdbcUrl;
	}
	String applicationName = environment.getProperty("spring.application.name");
	if (!StringUtils.hasText(applicationName)) {
		return jdbcUrl;
	}
	return appendParameter(jdbcUrl, "ApplicationName", applicationName);
}

private static String appendParameter(String jdbcUrl, String name, String value) {
	StringBuilder jdbcUrlBuilder = new StringBuilder(jdbcUrl);
	if (!jdbcUrl.contains("?")) {
		jdbcUrlBuilder.append('?');
	}
	else if (!jdbcUrl.endsWith("&")) {
		jdbcUrlBuilder.append('&');
	}
	return jdbcUrlBuilder.append(name)
			.append('=')
			.append(URLEncoder.encode(value, StandardCharsets.UTF_8))
			.toString();
}

private static boolean hasParameter(String jdbcUrl, String parameterName) {
	return jdbcUrl.contains("?%s=".formatted(parameterName))
			|| jdbcUrl.contains("&%s=".formatted(parameterName));
}
		

[nosan]

Fixed