@edudar @vitalijr2
Guys, I have a serious question for ya. I am working on a new feign-oauth2 module now and having an issue with current implementation of BaseBuilder. If I clone the builder using method Object#clone and then add an request or response interceptors, those interceptors are simply forgotten. Here an example:
B enrichedBuilder = (B) builder.clone();
return enrichedBuilder
.requestInterceptor(new AuthenticationInterceptor()); // <-- this interceptor is lostWhy does it happens:
BaseBuilder has the private field B thisB pointing to itself.- When builder is cloned, the value of that field is copied as is, and continue to point on the object it was cloned from.
- The references
requestInterceptors and responseInterceptors point to the new ArrayList instances, because their values are initialized outside constructor.
- Method
BaseBuilder#requestInterceptor adds an interceptor into the list from the new builder object, but returns an old builder then:
/** Adds a single request interceptor to the builder. */
public B requestInterceptor(RequestInterceptor requestInterceptor) {
this.requestInterceptors.add(requestInterceptor); // <-- interceptor added to new list
return thisB; // <-- returns an old builder object without added interceptor
}So here my questions:
-
@edudar why do we even need to catch value of this into instance field thisB? I removed it, by returning this from every builder method and it seems to works. At least, all tests pass.
-
@vitalijr2 why to use Object#clone that is going against all best practices of programming with modern Java? Using a copy constructor would be a more clean and relyable solution.
I can fix both of those problems, but I need a confirmation from you that I didn't miss something.
@edudar @vitalijr2
Guys, I have a serious question for ya. I am working on a new
feign-oauth2module now and having an issue with current implementation ofBaseBuilder. If I clone the builder using methodObject#cloneand then add an request or response interceptors, those interceptors are simply forgotten. Here an example:Why does it happens:
BaseBuilderhas the private fieldB thisBpointing to itself.requestInterceptorsandresponseInterceptorspoint to the newArrayListinstances, because their values are initialized outside constructor.BaseBuilder#requestInterceptoradds an interceptor into the list from the new builder object, but returns an old builder then:So here my questions:
@edudar why do we even need to catch value of
thisinto instance fieldthisB? I removed it, by returningthisfrom every builder method and it seems to works. At least, all tests pass.@vitalijr2 why to use
Object#clonethat is going against all best practices of programming with modern Java? Using a copy constructor would be a more clean and relyable solution.I can fix both of those problems, but I need a confirmation from you that I didn't miss something.