Add generics to the KeycloakContainer class

[nils-christian]

Description

Please add generics to the KeycloakContainer class and use them throughout the class.

public class KeycloakContainer<SELF extends KeycloakContainer<SELF>> extends GenericContainer<SELF> {
...
  public SELF withRealmImportFile(String importFile) {
    this.importFiles.add(importFile);
    return self();
  }
...

Motivation

In our tests we are using a custom implementation of the KeycloakContainer. It adds some additional behaviour (specific for our tests) as well as some additional methods. This works fine up until the point where we use the with-methods to configure the container, as one "looses" our custom class.

public final class MyCustomKeycloakContainer extends KeycloakContainer {

	public MyCustomKeycloakContainer( final String dockerImageName ) {
		super( dockerImageName );
	}
	public void myCustomMethod( ) {
		...
	}

}

@Container
private static final MyCustomKeycloakContainer keycloakContainer = ( MyCustomKeycloakContainer ) new MyCustomKeycloakContainer( "quay.io/keycloak/keycloak:18.0" )
		.withAdminUsername( "admin" )
		.withAdminPassword( "password" );

Details

Note that some other frameworks are using the generics as well. For example Postgresql:

public class PostgreSQLContainer<SELF extends PostgreSQLContainer<SELF>> extends JdbcDatabaseContainer<SELF>

[dasniko]

Hi @nils-christian thanks for this issue.
I totally agree with you, this should be changed.
Is it possible for you to create a PR with this change?

[nils-christian]

Hi @dasniko,

Sure. I added a PR (#99).

Please note: Everyone upgrading the library will be faced with raw type warnings. Also, in rare cases a compile error can now occur when one does not use the generics. This happened in the KeycloakContainerExtensionReuseTest with the withReuse-method (so basically when you do not use the generics, but call a method not overriden by KeycloakContainer).

[OLibutzki]

Hi @dasniko and @nils-christian,

sorry for kicking in, but I would like to share my thoughts on this issue.

Approx. 95% of the KeycloakContainers users use the class as-is. Introducing the generic encourages them to add at least <?> in order to remove the raw type warning.

Personally, I don't like that need to provide a generic for the PostgreSQLContainer, too.

What about this idea:

1. Rename KeycloakContainer to AbstractKeycloakContainer or ExtendableKeycloakContainer (an abstract class). The abstract class provides the generic proposed by @nils-christian.
2. Add a KeycloakContainer class which just extends the abstract class and provides the type argument <?>. This class could be final some time in order to clarify that it's not meant to be extended. But that would be a breaking change...
3. Clients are free to write custom classes which extend the abstract class using a type argument which reflects the specialized type itself.

The main benefit is that the KeycloakContainer class doesn't change from a client's point of view. You still don't have to deal with the generic. Nevertheless you provide a way to extend the abstract KeycloakContainer without losing the type information on the way through the fluent API.

[nils-christian]

Sounds good to me, @OLibutzki, especially the part that nothing would change from a client's perspective. I will make the changes tomorrow.

[nils-christian]

I included Olivers's suggestions and created a new PR: #100

[dasniko]

Thanks @nils-christian for the PR and @OLibutzki for the reasoning. I'll have a deeper look into it the next few days.

While it might be true that most of the users of this lib are just using the testcontainer as it is, I heard from quite a few people that they extend and overwrite the container, adding custom functionality, special to their environment, like @nils-christian did, which is not worth contributing to the lib. So, extensibility is a good and important thing.
The approach with an ExtendableKeycloakContainer sounds good to me.

[dasniko]

Thanks for this issue and corresponding PR! 🙏