Fix for @Wither in couchbase container (issues: #784) #810
Conversation
| @@ -50,49 +49,38 @@ | |||
| * optimized by Tayeb Chlyah | |||
| */ | |||
| @AllArgsConstructor | |||
| public class CouchbaseContainer extends GenericContainer<CouchbaseContainer> { | |||
| public class CouchbaseContainer <SELF extends CouchbaseContainer<SELF>> extends GenericContainer<SELF> { | |||
There was a problem hiding this comment.
CouchbaseContainer is a final container and doesn't need SELF typing. Any good reason to add it back?
There was a problem hiding this comment.
I don't see why testcontainers should forbid subclassing the couchbase container. This gives a user the possiblity to add something they need.
Aside from a slightly more complex definition on the class line, this adds a lot of flexibility for adding new with methods without disallowing chaining at the configuration level.
Example:
MyCouchbaseContainer c = new MyCouchbaseContainer()
.withClusterUsername("admin") // returns CouchbaseContainer
.withRoles("role1", "role2"...) // implemented in MyCouchbaseContainer and cannot be chained from CouchbaseContainer
.withClusterPassword("mops");There was a problem hiding this comment.
Another reason is consistency. A lot of containers ins the modules project provide exactly the same style.
There was a problem hiding this comment.
the problem is that some languages (ahem Kotlin ahem) really suck at handling such types :D
This is why we decided to use self-typing only on base classes and not the final ones (new modules have that)
Also, Testcontainers 2.0 will have a new, generic-less DSL
There was a problem hiding this comment.
So you suggest to revert that change?
And using return self(); is still okay?
There was a problem hiding this comment.
Yes, please.
return self() is okay :)
These methods do not clone the instance and therefore do not default to the standard alpine:3.5 image - Added test to verify that the image name is not overridden by withXXX - Made CouchbaseContainer conforming to other modules' containers Replaced the extends mechanism by the project standard
|
@Kaidowei merged, thanks! 👍 |
Replaced Wither annotation with own with methods
These methods do not clone the instance and therefore do not default to
the standard alpine:3.5 image
Added test to verify that the image name is not overridden by withXXX
Made CouchbaseContainer conforming to other modules' containers
Replaced the extends mechanism by the project standard
Thanks to @RobNE for the contribution