From b4097e5946f078eee2c243ffc4a5449b4ea53c8d Mon Sep 17 00:00:00 2001 From: Kiran Godishala <53332225+kirangodishala@users.noreply.github.com> Date: Thu, 9 Jan 2025 23:42:12 +0530 Subject: [PATCH] feat(retrofit2): Improvise OkHttpClientProvider to accept interceptors (#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 --- .../kork/retrofit/RetrofitServiceFactory.java | 14 +++ .../retrofit/Retrofit2ServiceFactory.java | 13 ++- .../config/DefaultServiceClientProvider.java | 11 +++ .../config/okhttp3/OkHttpClientProvider.java | 10 ++- .../kork/client/ServiceClientFactory.java | 22 ++++- .../kork/client/ServiceClientProvider.java | 22 ++++- .../okhttp3/OkHttpClientProviderTest.java | 86 +++++++++++++++++++ 7 files changed, 171 insertions(+), 7 deletions(-) create mode 100644 kork-web/src/test/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProviderTest.java diff --git a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/RetrofitServiceFactory.java b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/RetrofitServiceFactory.java index 53b32c5ba..76ce6bf74 100644 --- a/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/RetrofitServiceFactory.java +++ b/kork-retrofit/src/main/java/com/netflix/spinnaker/kork/retrofit/RetrofitServiceFactory.java @@ -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; @@ -62,4 +64,16 @@ public T create(Class type, ServiceEndpoint serviceEndpoint, ObjectMapper .build() .create(type); } + + @Override + public T create( + Class type, + ServiceEndpoint serviceEndpoint, + ObjectMapper objectMapper, + List interceptors) { + throw new IllegalArgumentException( + String.format( + "Retrofit1 client doesn't support okhttp3 Interceptors. Failed to build %s ", + type.getName())); + } } diff --git a/kork-retrofit2/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2ServiceFactory.java b/kork-retrofit2/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2ServiceFactory.java index 33bbb12b5..bc6a93f81 100644 --- a/kork-retrofit2/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2ServiceFactory.java +++ b/kork-retrofit2/src/main/java/com/netflix/spinnaker/kork/retrofit/Retrofit2ServiceFactory.java @@ -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; @@ -40,7 +42,16 @@ public Retrofit2ServiceFactory(OkHttpClientProvider clientProvider) { @Override public T create(Class type, ServiceEndpoint serviceEndpoint, ObjectMapper objectMapper) { - OkHttpClient okHttpClient = clientProvider.getClient(serviceEndpoint); + return create(type, serviceEndpoint, objectMapper, List.of()); + } + + @Override + public T create( + Class type, + ServiceEndpoint serviceEndpoint, + ObjectMapper objectMapper, + List interceptors) { + OkHttpClient okHttpClient = clientProvider.getClient(serviceEndpoint, interceptors); return new Retrofit.Builder() .baseUrl(Objects.requireNonNull(HttpUrl.parse(serviceEndpoint.getBaseUrl()))) diff --git a/kork-web/src/main/java/com/netflix/spinnaker/config/DefaultServiceClientProvider.java b/kork-web/src/main/java/com/netflix/spinnaker/config/DefaultServiceClientProvider.java index 00d2e9ff3..984272d38 100644 --- a/kork-web/src/main/java/com/netflix/spinnaker/config/DefaultServiceClientProvider.java +++ b/kork-web/src/main/java/com/netflix/spinnaker/config/DefaultServiceClientProvider.java @@ -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. */ @@ -54,6 +55,16 @@ public T getService( return serviceClientFactory.create(type, serviceEndpoint, objectMapper); } + @Override + public T getService( + Class type, + ServiceEndpoint serviceEndpoint, + ObjectMapper objectMapper, + List 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)) diff --git a/kork-web/src/main/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProvider.java b/kork-web/src/main/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProvider.java index ef0ae707e..c3a2cb25d 100644 --- a/kork-web/src/main/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProvider.java +++ b/kork-web/src/main/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProvider.java @@ -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; @@ -41,8 +42,13 @@ public OkHttpClientProvider(List 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 interceptors) { + OkHttpClient.Builder builder = findProvider(service).get(service); + interceptors.forEach(builder::addInterceptor); + return builder.build(); } private OkHttpClientBuilderProvider findProvider(ServiceEndpoint service) { diff --git a/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientFactory.java b/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientFactory.java index b8c071419..f969b4966 100644 --- a/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientFactory.java +++ b/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientFactory.java @@ -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 { @@ -29,11 +31,27 @@ public interface ServiceClientFactory { * @param type client type * @param serviceEndpoint endpoint configuration * @param objectMapper mapper - * @param type of client , usually a interface with all the remote method definitions. - * @return a implementation of the type of client given. + * @param type of client , usually an interface with all the remote method definitions. + * @return an implementation of the type of client given. */ public T create(Class 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 type of client , usually an interface with all the remote method definitions. + * @return an implementation of the type of client given. + */ + public T create( + Class type, + ServiceEndpoint serviceEndpoint, + ObjectMapper objectMapper, + List interceptors); + /** * Decide if this factory can support the endpoint provided. * diff --git a/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientProvider.java b/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientProvider.java index 030a9e351..36f929fef 100644 --- a/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientProvider.java +++ b/kork-web/src/main/java/com/netflix/spinnaker/kork/client/ServiceClientProvider.java @@ -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 { @@ -29,7 +31,7 @@ public interface ServiceClientProvider { * * @param type retrofit interface type * @param serviceEndpoint endpoint definition - * @param type of client , usually a interface with all the remote method definitions. + * @param type of client , usually an interface with all the remote method definitions. * @return the retrofit interface implementation */ public T getService(Class type, ServiceEndpoint serviceEndpoint); @@ -40,9 +42,25 @@ public interface ServiceClientProvider { * @param type retrofit interface type * @param serviceEndpoint endpoint definition * @param objectMapper object mapper for conversion - * @param type of client , usually a interface with all the remote method definitions. + * @param type of client , usually an interface with all the remote method definitions. * @return the retrofit interface implementation */ public T getService( Class 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 type of client , usually an interface with all the remote method definitions. + * @return the retrofit interface implementation + */ + public T getService( + Class type, + ServiceEndpoint serviceEndpoint, + ObjectMapper objectMapper, + List interceptors); } diff --git a/kork-web/src/test/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProviderTest.java b/kork-web/src/test/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProviderTest.java new file mode 100644 index 000000000..20a307a3f --- /dev/null +++ b/kork-web/src/test/java/com/netflix/spinnaker/config/okhttp3/OkHttpClientProviderTest.java @@ -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."); + } +}