Skip to content

Commit

Permalink
fix: HttpClient.Factory instances can be prioritized
Browse files Browse the repository at this point in the history
Introduce a scoring system so that HttpClient.Factory instances
provided by the SPI can be prioritized.
The higher the priority, the higher the chances
that the factory instance will be selected automatically
to create HttpClient instances.

Signed-off-by: GitHub <bot@example.com>
  • Loading branch information
GitHub authored and manusa committed Jan 31, 2023
1 parent 55357e1 commit 5ffca33
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ default boolean isDefault() {
return false;
}

/**
* The priority of the implementation. The higher the priority the more likely it will be used.
*
* @return the priority.
*/
default int priority() {
return 0;
}

}

interface DerivedClientBuilder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ private static HttpClient.Factory getFactory(ServiceLoader<HttpClient.Factory> l
+ "choosing the first non-default implementation. "
+ "You should exclude dependencies that aren't needed or use an explicit association of the HttpClient.Factory.");
}
if (factory == null || (factory.isDefault() && !possible.isDefault())) {
if (factory == null || (factory.isDefault() && !possible.isDefault())
|| (!factory.isDefault()) && factory.priority() < possible.priority()) {
factory = possible;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.ConfigBuilder;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.http.HttpClient;
import io.fabric8.kubernetes.client.http.Interceptor;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
Expand All @@ -27,17 +30,28 @@
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.MockedStatic;

import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.ServiceLoader;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.when;

class HttpClientUtilsTest {

Expand Down Expand Up @@ -182,4 +196,100 @@ void withNoHttpProxyProvidedReturnsNull() throws MalformedURLException {
assertThat(url).isNull();
}
}

@Nested
@DisplayName("getHttpClientFactory")
class GetHttpClientFactory {

private List<HttpClient.Factory> factories;
@SuppressWarnings("rawtypes")
private MockedStatic<ServiceLoader> mockedStaticServiceLoader;

@BeforeEach
void setUp() {
// n.b. Since ServiceLoader is mocked for this test, it's not possible to use AssertJ assertions.
factories = new ArrayList<>();
mockedStaticServiceLoader = mockStatic(ServiceLoader.class);
@SuppressWarnings("unchecked")
final ServiceLoader<HttpClient.Factory> mockServiceLoader = mock(ServiceLoader.class);
when(mockServiceLoader.iterator()).thenAnswer(i -> new ArrayList<>(factories).iterator());
when(ServiceLoader.load(eq(HttpClient.Factory.class), any())).thenReturn(mockServiceLoader);
mockedStaticServiceLoader.when(() -> ServiceLoader.load(eq(HttpClient.Factory.class)))
.thenReturn(mockServiceLoader);
}

@AfterEach
void tearDown() {
factories.clear();
mockedStaticServiceLoader.close();
}

@Test
@DisplayName("With no implementations, should throw exception")
void withNoImplementations() {
final KubernetesClientException result = assertThrows(KubernetesClientException.class,
HttpClientUtils::getHttpClientFactory);
assertEquals(
"No httpclient implementations found on the context classloader, please ensure your classpath includes an implementation jar",
result.getMessage());
}

@Test
@DisplayName("With default implementation, should return default")
void withDefault() {
factories.add(new DefaultHttpClientFactory());
assertEquals(DefaultHttpClientFactory.class, HttpClientUtils.getHttpClientFactory().getClass());
}

@Test
@DisplayName("With default and other implementation, should return other")
void withDefaultAndOther() {
factories.add(new OtherHttpClientFactory());
factories.add(new DefaultHttpClientFactory());
assertEquals(OtherHttpClientFactory.class, HttpClientUtils.getHttpClientFactory().getClass());
}

@Test
@DisplayName("With default and other implementation, should return Priority")
void withDefaultAndPriorityAndOther() {
factories.add(new OtherHttpClientFactory());
factories.add(new PriorityHttpClientFactory());
factories.add(new DefaultHttpClientFactory());
assertEquals(PriorityHttpClientFactory.class, HttpClientUtils.getHttpClientFactory().getClass());
}

private final class DefaultHttpClientFactory implements HttpClient.Factory {

@Override
public HttpClient.Builder newBuilder() {
return null;
}

@Override
public boolean isDefault() {
return true;
}
}

private final class OtherHttpClientFactory implements HttpClient.Factory {

@Override
public HttpClient.Builder newBuilder() {
return null;
}
}

private final class PriorityHttpClientFactory implements HttpClient.Factory {

@Override
public HttpClient.Builder newBuilder() {
return null;
}

@Override
public int priority() {
return Integer.MAX_VALUE;
}
}
}
}

0 comments on commit 5ffca33

Please sign in to comment.