Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ApiClient in retrofit2 be able to use own OkHttpClient #6699

Merged
merged 9 commits into from
Jul 2, 2020
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public class ApiClient {
public <S> S createService(Class<S> serviceClass) {
if (okHttpClient != null) {
return adapterBuilder.client(okHttpClient).build().create(serviceClass);
else {
}else {
return adapterBuilder.client(okBuilder.build()).build().create(serviceClass);
}
}
Expand Down Expand Up @@ -365,9 +365,11 @@ public class ApiClient {
throw new RuntimeException("auth name \"" + authName + "\" already in api authorizations");
}
apiAuthorizations.put(authName, authorization);
if(okBuilder != null){
okBuilder.addInterceptor(authorization);
if(okBuilder == null){
throw new Exception("okBuilder is null")
}
okBuilder.addInterceptor(authorization);

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public void createDefaultAdapter() {
public <S> S createService(Class<S> serviceClass) {
if (okHttpClient != null) {
return adapterBuilder.client(okHttpClient).build().create(serviceClass);
else {
}else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space missing

return adapterBuilder.client(okBuilder.build()).build().create(serviceClass);
}
}
Expand Down Expand Up @@ -311,9 +311,11 @@ public ApiClient addAuthorization(String authName, Interceptor authorization) {
throw new RuntimeException("auth name \"" + authName + "\" already in api authorizations");
}
apiAuthorizations.put(authName, authorization);
if(okBuilder != null){
okBuilder.addInterceptor(authorization);
if(okBuilder == null){
throw new Exception("okBuilder is null")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use RuntimeException ? (or even better : create an ApiClientException)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message could be more explicit. Something like "the ApiClient was created with a built okClient so it's not possible to add an authorization interceptor to it".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the advantage of creating an own ApiClientException comparing to the RuntimeException here in this case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://rules.sonarsource.com/java/RSPEC-112 . But we already have a bunch of RuntimeException so...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok I see but as you said we already have many RuntimeExceptions so I will use a RuntimeException here if that's ok and then maybe in future you/we can change them to own Exceptions.

}
okBuilder.addInterceptor(authorization);

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void createDefaultAdapter() {
public <S> S createService(Class<S> serviceClass) {
if (okHttpClient != null) {
return adapterBuilder.client(okHttpClient).build().create(serviceClass);
else {
}else {
return adapterBuilder.client(okBuilder.build()).build().create(serviceClass);
}
}
Expand Down Expand Up @@ -313,9 +313,11 @@ public ApiClient addAuthorization(String authName, Interceptor authorization) {
throw new RuntimeException("auth name \"" + authName + "\" already in api authorizations");
}
apiAuthorizations.put(authName, authorization);
if(okBuilder != null){
okBuilder.addInterceptor(authorization);
if(okBuilder == null){
throw new Exception("okBuilder is null")
}
okBuilder.addInterceptor(authorization);

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void createDefaultAdapter() {
public <S> S createService(Class<S> serviceClass) {
if (okHttpClient != null) {
return adapterBuilder.client(okHttpClient).build().create(serviceClass);
else {
}else {
return adapterBuilder.client(okBuilder.build()).build().create(serviceClass);
}
}
Expand Down Expand Up @@ -313,9 +313,11 @@ public ApiClient addAuthorization(String authName, Interceptor authorization) {
throw new RuntimeException("auth name \"" + authName + "\" already in api authorizations");
}
apiAuthorizations.put(authName, authorization);
if(okBuilder != null){
okBuilder.addInterceptor(authorization);
if(okBuilder == null){
throw new Exception("okBuilder is null")
}
okBuilder.addInterceptor(authorization);

return this;
}

Expand Down