Use beanName as default name of @ServiceConnection

[quaff]

Before this commit, ConnectionDetailsNotFoundException will be raised if unnamed @ServiceConnection is annotated on @Bean method and containerType is not accepted by ContainerConnectionSource.

For example:

	@Bean
	@ServiceConnection(name = "redis")
	GenericContainer<?> redis() {
		return new GenericContainer<>("redis").withExposedPorts(6379);
	}

After this commit, name of @ServiceConnection could be omitted.

[wilkinsona]

Thanks for the proposal, @quaff. @snicoll suggested this a few weeks ago but I wasn't in favor as it feels a bit too magical. It also won't work for containers with / in their name such as openzipkin/zipkin. Let's see what the rest of the team thinks.

[quaff]

Thanks for the proposal, @quaff. @snicoll suggested this a few weeks ago but I wasn't in favor as it feels a bit too magical. It also won't work for containers with / in their name such as openzipkin/zipkin. Let's see what the rest of the team thinks.

What about bean name as openzipkin_zipkin or openzipkinZipkin?

[eddumelendez]

IIRC, this was introduced in order to make sure the ConnectionFactory to be used. This is due there is no specific Testcontainers implementation. Redis and Zipkin fall into this because both use GenericContainer but the issue comes with custom images ("my-custom-kv-store") which will always requires @ServiceName(name = "redis"). The existing approach is good, instead of making it optional and rely on the bean's name and increase cognitive complexity

[quaff]

If people migrate

@ServiceConnection
GenericContainer<?> redis = return new GenericContainer<>("redis").withExposedPorts(6379);

to

@Bean
@ServiceConnection
GenericContainer<?> redis() {
	return new GenericContainer<>("redis").withExposedPorts(6379);
}

They will encounter unexpected ConnectionDetailsNotFoundException .

[wilkinsona]

That's not because the field is called redis but because the GenericContainer instance is available, allowing us to call getDockerImageName() on it:

By default Container.getDockerImageName() is used to obtain the name used to find connection details. If you are using a custom docker image, you can use the name attribute of @ServiceConnection to override it.

When you're using @Bean, the container instance isn't available without calling the method which we can't do as it would trigger eager initialization. As a result, all we have available is the return type of the @Bean method. We should improve the documentation for this as it's not explained at the moment and I think it should be. I've opened #36071.

[quaff]

That's not because the field is called redis but because the GenericContainer instance is available, allowing us to call getDockerImageName() on it:

By default Container.getDockerImageName() is used to obtain the name used to find connection details. If you are using a custom docker image, you can use the name attribute of @ServiceConnection to override it.

When you're using @Bean, the container instance isn't available without calling the method which we can't do as it would trigger eager initialization. As a result, all we have available is the return type of the @Bean method. We should improve the documentation for this as it's not explained at the moment and I think it should be. I've opened #36071.

I know that because I read sources before preparing this PR, but most people doesn't read sources even documents, they want intuitional programming experience.

[quaff]

Using beanName as default name of @ServiceConnection is better choice, I've updated the commit.

[wilkinsona]

We've discussed this today and decided that we don't want to make any changes here. We think there's too much magic in using the bean name. Thanks anyway for the suggestion, @quaff. We do think the docs should be improved though. #36071 is tracking that.