Skip to content

Commit

Permalink
feat(retrofit2): Improvise OkHttpClientProvider to accept interceptors (
Browse files Browse the repository at this point in the history
#1220)

* feat(web): add method to get OkHttpClient with given list of interceptors added

* feat(web): add a method each to ServiceClientFactory and ServiceClientProvider to support list of interceptors.

* test(web): add OkHttpClientProviderTest class
  • Loading branch information
kirangodishala authored Jan 9, 2025
1 parent 2fef420 commit b4097e5
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import com.netflix.spinnaker.kork.client.ServiceClientFactory;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler;
import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
import java.util.List;
import okhttp3.Interceptor;
import retrofit.Endpoint;
import retrofit.RequestInterceptor;
import retrofit.RestAdapter;
Expand Down Expand Up @@ -62,4 +64,16 @@ public <T> T create(Class<T> type, ServiceEndpoint serviceEndpoint, ObjectMapper
.build()
.create(type);
}

@Override
public <T> T create(
Class<T> type,
ServiceEndpoint serviceEndpoint,
ObjectMapper objectMapper,
List<Interceptor> interceptors) {
throw new IllegalArgumentException(
String.format(
"Retrofit1 client doesn't support okhttp3 Interceptors. Failed to build %s ",
type.getName()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider;
import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
import com.netflix.spinnaker.kork.client.ServiceClientFactory;
import java.util.List;
import java.util.Objects;
import okhttp3.HttpUrl;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import retrofit2.Call;
import retrofit2.Retrofit;
Expand All @@ -40,7 +42,16 @@ public Retrofit2ServiceFactory(OkHttpClientProvider clientProvider) {

@Override
public <T> T create(Class<T> type, ServiceEndpoint serviceEndpoint, ObjectMapper objectMapper) {
OkHttpClient okHttpClient = clientProvider.getClient(serviceEndpoint);
return create(type, serviceEndpoint, objectMapper, List.of());
}

@Override
public <T> T create(
Class<T> type,
ServiceEndpoint serviceEndpoint,
ObjectMapper objectMapper,
List<Interceptor> interceptors) {
OkHttpClient okHttpClient = clientProvider.getClient(serviceEndpoint, interceptors);

return new Retrofit.Builder()
.baseUrl(Objects.requireNonNull(HttpUrl.parse(serviceEndpoint.getBaseUrl())))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.netflix.spinnaker.kork.client.ServiceClientProvider;
import com.netflix.spinnaker.kork.exceptions.SystemException;
import java.util.List;
import okhttp3.Interceptor;
import org.springframework.stereotype.Component;

/** Provider that returns a suitable service client capable of making http calls. */
Expand Down Expand Up @@ -54,6 +55,16 @@ public <T> T getService(
return serviceClientFactory.create(type, serviceEndpoint, objectMapper);
}

@Override
public <T> T getService(
Class<T> type,
ServiceEndpoint serviceEndpoint,
ObjectMapper objectMapper,
List<Interceptor> interceptors) {
ServiceClientFactory serviceClientFactory = findProvider(type, serviceEndpoint);
return serviceClientFactory.create(type, serviceEndpoint, objectMapper, interceptors);
}

private ServiceClientFactory findProvider(Class<?> type, ServiceEndpoint service) {
return serviceClientFactories.stream()
.filter(provider -> provider.supports(type, service))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.netflix.spinnaker.config.ServiceEndpoint;
import com.netflix.spinnaker.kork.exceptions.SystemException;
import java.util.List;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import org.springframework.stereotype.Component;

Expand All @@ -41,8 +42,13 @@ public OkHttpClientProvider(List<OkHttpClientBuilderProvider> providers) {
* @return okHttpClient
*/
public OkHttpClient getClient(ServiceEndpoint service) {
OkHttpClientBuilderProvider provider = findProvider(service);
return provider.get(service).build();
return getClient(service, List.of());
}

public OkHttpClient getClient(ServiceEndpoint service, List<Interceptor> interceptors) {
OkHttpClient.Builder builder = findProvider(service).get(service);
interceptors.forEach(builder::addInterceptor);
return builder.build();
}

private OkHttpClientBuilderProvider findProvider(ServiceEndpoint service) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.config.ServiceEndpoint;
import java.util.List;
import okhttp3.Interceptor;

/** Factory to build a client for a service. */
public interface ServiceClientFactory {
Expand All @@ -29,11 +31,27 @@ public interface ServiceClientFactory {
* @param type client type
* @param serviceEndpoint endpoint configuration
* @param objectMapper mapper
* @param <T> type of client , usually a interface with all the remote method definitions.
* @return a implementation of the type of client given.
* @param <T> type of client , usually an interface with all the remote method definitions.
* @return an implementation of the type of client given.
*/
public <T> T create(Class<T> type, ServiceEndpoint serviceEndpoint, ObjectMapper objectMapper);

/**
* Builds a concrete client capable of making HTTP calls.
*
* @param type client type
* @param serviceEndpoint endpoint configuration
* @param objectMapper mapper
* @param interceptors list of interceptors
* @param <T> type of client , usually an interface with all the remote method definitions.
* @return an implementation of the type of client given.
*/
public <T> T create(
Class<T> type,
ServiceEndpoint serviceEndpoint,
ObjectMapper objectMapper,
List<Interceptor> interceptors);

/**
* Decide if this factory can support the endpoint provided.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.config.ServiceEndpoint;
import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
import java.util.List;
import okhttp3.Interceptor;

@NonnullByDefault
public interface ServiceClientProvider {
Expand All @@ -29,7 +31,7 @@ public interface ServiceClientProvider {
*
* @param type retrofit interface type
* @param serviceEndpoint endpoint definition
* @param <T> type of client , usually a interface with all the remote method definitions.
* @param <T> type of client , usually an interface with all the remote method definitions.
* @return the retrofit interface implementation
*/
public <T> T getService(Class<T> type, ServiceEndpoint serviceEndpoint);
Expand All @@ -40,9 +42,25 @@ public interface ServiceClientProvider {
* @param type retrofit interface type
* @param serviceEndpoint endpoint definition
* @param objectMapper object mapper for conversion
* @param <T> type of client , usually a interface with all the remote method definitions.
* @param <T> type of client , usually an interface with all the remote method definitions.
* @return the retrofit interface implementation
*/
public <T> T getService(
Class<T> type, ServiceEndpoint serviceEndpoint, ObjectMapper objectMapper);

/**
* Returns the concrete retrofit service client
*
* @param type retrofit interface type
* @param serviceEndpoint endpoint definition
* @param objectMapper object mapper for conversion
* @param interceptors list of interceptors
* @param <T> type of client , usually an interface with all the remote method definitions.
* @return the retrofit interface implementation
*/
public <T> T getService(
Class<T> type,
ServiceEndpoint serviceEndpoint,
ObjectMapper objectMapper,
List<Interceptor> interceptors);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright 2025 OpsMx, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.netflix.spinnaker.config.okhttp3;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.when;

import com.netflix.spinnaker.config.ServiceEndpoint;
import com.netflix.spinnaker.kork.exceptions.SystemException;
import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties;
import java.util.List;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.springframework.boot.test.context.SpringBootTest;

@SpringBootTest(
webEnvironment = SpringBootTest.WebEnvironment.NONE,
classes = {OkHttpClient.class, OkHttpClientConfigurationProperties.class})
public class OkHttpClientProviderTest {

@Mock private DefaultOkHttpClientBuilderProvider defaultProvider;

@Mock private ServiceEndpoint service;

private OkHttpClient.Builder builder;

@Mock private Interceptor interceptor;

@Mock private Interceptor interceptor2;

private OkHttpClientProvider clientProvider;

@BeforeEach
void setUp() {
MockitoAnnotations.openMocks(this);
builder = new OkHttpClient.Builder();
clientProvider = new OkHttpClientProvider(List.of(defaultProvider));
}

@Test
void getClient_withInterceptors_shouldAddInterceptors() {
when(defaultProvider.supports(service)).thenReturn(true);
when(defaultProvider.get(service)).thenReturn(builder);

OkHttpClient result = clientProvider.getClient(service, List.of(interceptor, interceptor2));

assertEquals(result.interceptors().size(), 2);
assertEquals(result.interceptors().get(0), interceptor);
assertEquals(result.interceptors().get(1), interceptor2);
}

@Test
void getClient_noProviderFound_shouldThrowException() {
when(defaultProvider.supports(service)).thenReturn(false);
when(service.getBaseUrl()).thenReturn("http://example.com");

SystemException exception =
assertThrows(
SystemException.class,
() -> clientProvider.getClient(service),
"Expected an exception if no provider supports the service");
assertEquals(
"No client provider found for url (http://example.com)",
exception.getMessage(),
"The exception message should indicate no provider was found.");
}
}

0 comments on commit b4097e5

Please sign in to comment.