Skip to content

Commit

Permalink
Merge branch 'master' into feat/donateObservabilityPlugin
Browse files Browse the repository at this point in the history
  • Loading branch information
christosarvanitis authored Feb 14, 2025
2 parents 8a34d3a + e81dffc commit a161c34
Show file tree
Hide file tree
Showing 13 changed files with 418 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
# Given a tag, determine what branch we are on, so we can bump dependencies in the correct branch
- name: Get Branch
run: |
BRANCHES=$(git branch -r --contains ${{ github.ref }})
BRANCHES=$(git branch -r --contains ${{ github.ref }} | grep -v 'HEAD')
echo "BRANCHES is '${BRANCHES}'"
# Check for no branches explicitly...Otherwise echo adds a newline so wc thinks there's
# one branch. And echo -n makes it appears that there's one less branch than there
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public Response<T> execute() {
}
} catch (JsonProcessingException jpe) {
throw new SpinnakerConversionException(
"Failed to process response body", jpe, delegate.request());
"Failed to process response body: " + jpe.getMessage(), jpe, delegate.request());
} catch (IOException e) {
throw new SpinnakerNetworkException(e, delegate.request());
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException;
import java.io.IOException;
import retrofit2.Call;
import retrofit2.Response;

public class Retrofit2SyncCall<T> {

Expand All @@ -30,8 +31,20 @@ public class Retrofit2SyncCall<T> {
* @param <T> Successful response body type.
*/
public static <T> T execute(Call<T> call) {
return executeCall(call).body();
}

/**
* Handle IOExceptions from {@link Call}.execute method centrally, instead of all places that make
* retrofit2 API calls.
*
* @throws SpinnakerNetworkException if IOException occurs.
* @param call call to be executed
* @return Response<T> response after execution
*/
public static <T> Response<T> executeCall(Call<T> call) {
try {
return call.execute().body();
return call.execute();
} catch (IOException e) {
throw new SpinnakerNetworkException(e, call.request());
}
Expand Down
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 @@ -213,7 +213,10 @@ void testSpinnakerConversionException() {
() -> retrofit2Service.getRetrofit2().execute(), SpinnakerConversionException.class);
assertThat(spinnakerConversionException.getRetryable()).isNotNull();
assertThat(spinnakerConversionException.getRetryable()).isFalse();
assertThat(spinnakerConversionException).hasMessage("Failed to process response body");
assertThat(spinnakerConversionException)
.hasMessage(
"Failed to process response body: Cannot deserialize value of type `java.lang.String` from Object value (token `JsonToken.START_OBJECT`)\n"
+ " at [Source: (okhttp3.ResponseBody$BomAwareReader); line: 1, column: 1]");
assertThat(spinnakerConversionException.getUrl())
.isEqualTo(mockWebServer.url("/retrofit2").toString());
}
Expand Down
2 changes: 2 additions & 0 deletions kork-retrofit2/kork-retrofit2.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ apply from: "$rootDir/gradle/kotlin-test.gradle"
dependencies {
api(platform(project(":spinnaker-dependencies")))
implementation project(":kork-web")
implementation project(":kork-retrofit")
implementation "com.squareup.retrofit2:retrofit"
implementation "com.squareup.retrofit2:converter-jackson"
implementation "com.squareup.okhttp3:logging-interceptor"
Expand All @@ -15,5 +16,6 @@ dependencies {

testImplementation "com.squareup.okhttp3:mockwebserver"
testImplementation "com.squareup.retrofit2:retrofit-mock"
testImplementation "com.github.tomakehurst:wiremock-jre8"

}
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,12 +42,22 @@ 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())))
.client(okHttpClient)
.addConverterFactory(JacksonConverterFactory.create(objectMapper))
.addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
.build()
.create(type);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
/*
* 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.kork.retrofit;

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
import static com.github.tomakehurst.wiremock.client.WireMock.verify;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.client.WireMock;
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
import com.netflix.spinnaker.config.DefaultServiceClientProvider;
import com.netflix.spinnaker.config.DefaultServiceEndpoint;
import com.netflix.spinnaker.config.ServiceEndpoint;
import com.netflix.spinnaker.config.okhttp3.DefaultOkHttpClientBuilderProvider;
import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider;
import com.netflix.spinnaker.kork.client.ServiceClientFactory;
import com.netflix.spinnaker.kork.client.ServiceClientProvider;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerConversionException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties;
import java.util.List;
import java.util.Map;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import retrofit2.Call;
import retrofit2.Response;
import retrofit2.http.GET;

@SpringBootTest(
webEnvironment = SpringBootTest.WebEnvironment.NONE,
classes = {
OkHttpClient.class,
OkHttpClientConfigurationProperties.class,
OkHttpClientProvider.class,
Retrofit2ServiceFactoryAutoConfiguration.class,
Retrofit2ServiceFactoryTest.Retrofit2TestConfig.class,
DefaultOkHttpClientBuilderProvider.class
})
public class Retrofit2ServiceFactoryTest {

@Autowired ServiceClientProvider serviceClientProvider;

static int port;
static WireMockServer wireMockServer;

@BeforeAll
static void setUp() {
wireMockServer = new WireMockServer(WireMockConfiguration.options().dynamicPort());
wireMockServer.start();
port = wireMockServer.port();
WireMock.configureFor("localhost", port);
}

@AfterAll
static void tearDown() {
wireMockServer.stop();
}

@Test
void testRetrofit2Client() {
stubFor(
get(urlEqualTo("/test"))
.willReturn(
aResponse()
.withHeader("Content-Type", "application/json")
.withBody("{\"message\": \"success\", \"code\": 200}")));

ServiceEndpoint serviceEndpoint =
new DefaultServiceEndpoint("retrofit2service", "http://localhost:" + port);
Retrofit2TestService retrofit2TestService =
serviceClientProvider.getService(Retrofit2TestService.class, serviceEndpoint);
Map<String, String> response = Retrofit2SyncCall.execute(retrofit2TestService.getSomething());

assertEquals("success", response.get("message"));
}

@Test
void testRetrofit2ClientWithResponse() {
stubFor(
get(urlEqualTo("/test"))
.willReturn(
aResponse()
.withHeader("Content-Type", "application/json")
.withBody("{\"message\": \"success\"}")));

ServiceEndpoint serviceEndpoint =
new DefaultServiceEndpoint("retrofit2service", "http://localhost:" + port);
Retrofit2TestService retrofit2TestService =
serviceClientProvider.getService(Retrofit2TestService.class, serviceEndpoint);
Response<Map<String, String>> response =
Retrofit2SyncCall.executeCall(retrofit2TestService.getSomething());
assertEquals(200, response.code());
assertEquals("application/json", response.headers().get("Content-Type"));
assertEquals("success", response.body().get("message"));
}

@Test
void testRetrofit2Client_withInterceptor() {

Interceptor interceptor =
chain -> {
Request originalRequest = chain.request();
Request modifiedRequest =
originalRequest.newBuilder().header("Authorization", "Bearer my-token").build();
return chain.proceed(modifiedRequest);
};

stubFor(
get(urlEqualTo("/test"))
.willReturn(
aResponse()
.withHeader("Content-Type", "application/json")
.withBody("{\"message\": \"success\", \"code\": 200}")));

ServiceEndpoint serviceEndpoint =
new DefaultServiceEndpoint("retrofit2service", "http://localhost:" + port);
Retrofit2TestService retrofit2TestService =
serviceClientProvider.getService(
Retrofit2TestService.class, serviceEndpoint, new ObjectMapper(), List.of(interceptor));
Retrofit2SyncCall.execute(retrofit2TestService.getSomething());

verify(
getRequestedFor(urlEqualTo("/test"))
.withHeader("Authorization", equalTo("Bearer my-token")));
}

@Test
void testRetrofit2Client_withHttpException() {
stubFor(
get(urlEqualTo("/test"))
.willReturn(
aResponse()
.withHeader("Content-Type", "application/json")
.withStatus(400)
.withBody("{\"message\": \"error\"}")));

ServiceEndpoint serviceEndpoint =
new DefaultServiceEndpoint("retrofit2service", "http://localhost:" + port);

Retrofit2TestService retrofit2TestService =
serviceClientProvider.getService(Retrofit2TestService.class, serviceEndpoint);

SpinnakerHttpException exception =
assertThrows(
SpinnakerHttpException.class,
() -> Retrofit2SyncCall.executeCall(retrofit2TestService.getSomething()));
assertEquals(400, exception.getResponseCode());
assertEquals(
"Status: 400, Method: GET, URL: http://localhost:" + port + "/test, Message: error",
exception.getMessage());
}

@Test
void testRetrofit2Client_withConversionException() {
stubFor(
get(urlEqualTo("/test"))
.willReturn(
aResponse()
.withHeader("Content-Type", "application/json")
.withBody("{\"message\": \"incorrect json}")));

ServiceEndpoint serviceEndpoint =
new DefaultServiceEndpoint("retrofit2service", "http://localhost:" + port);

Retrofit2TestService retrofit2TestService =
serviceClientProvider.getService(Retrofit2TestService.class, serviceEndpoint);

SpinnakerServerException exception =
assertThrows(
SpinnakerConversionException.class,
() -> Retrofit2SyncCall.executeCall(retrofit2TestService.getSomething()));
assertEquals(
"Failed to process response body: Unexpected end-of-input: was expecting closing quote for a string value\n"
+ " at [Source: (okhttp3.ResponseBody$BomAwareReader); line: 1, column: 29]",
exception.getMessage());
}

@Configuration
public static class Retrofit2TestConfig {

@Bean
public ServiceClientProvider serviceClientProvider(
List<ServiceClientFactory> serviceClientFactories) {
return new DefaultServiceClientProvider(serviceClientFactories, new ObjectMapper());
}
}

public interface Retrofit2TestService {

@GET("test")
Call<Map<String, String>> getSomething();
}
}
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
Loading

0 comments on commit a161c34

Please sign in to comment.