From 140656c0f974e73c071b030ecaa6e9e48688532c Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Fri, 3 Mar 2023 11:16:41 +0100 Subject: [PATCH] fix: typo in HttpClient.Factory scoring system logic Signed-off-by: Marc Nuri --- CHANGELOG.md | 1 + .../client/utils/HttpClientUtils.java | 2 +- .../client/utils/HttpClientUtilsTest.java | 57 ++++++++++++------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dd852c81f3..2938d1b5f3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ * Fix #4891: address vertx not completely reading exec streams * Fix #4899: BuildConfigs.instantiateBinary().fromFile() does not time out * Fix #4908: using the response headers in the vertx response +* Fix #4947: typo in HttpClient.Factory scoring system logic #### Improvements * Fix #4675: adding a fully client side timeout for informer watches diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java index e65aa49a560..8f6741b62d3 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java @@ -157,7 +157,7 @@ private static HttpClient.Factory getFactory(ServiceLoader l + "You should exclude dependencies that aren't needed or use an explicit association of the HttpClient.Factory."); } if (factory == null || (factory.isDefault() && !possible.isDefault()) - || (!factory.isDefault()) && factory.priority() < possible.priority()) { + || (!factory.isDefault() && factory.priority() < possible.priority())) { factory = possible; } } diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/HttpClientUtilsTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/HttpClientUtilsTest.java index 3043b6d898d..155b0229f6b 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/HttpClientUtilsTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/HttpClientUtilsTest.java @@ -247,42 +247,58 @@ void withDefault() { @Test @DisplayName("With default and other implementation, should return other") void withDefaultAndOther() { - factories.add(new OtherHttpClientFactory()); + factories.add(new ScoredHttpClientFactory("other", 0)); factories.add(new DefaultHttpClientFactory()); - assertEquals(OtherHttpClientFactory.class, HttpClientUtils.getHttpClientFactory().getClass()); + assertEquals(ScoredHttpClientFactory.class, HttpClientUtils.getHttpClientFactory().getClass()); } @Test - @DisplayName("With default and other implementation, should return Priority") + @DisplayName("With default, other, and priority implementations; should return Priority") void withDefaultAndPriorityAndOther() { - factories.add(new OtherHttpClientFactory()); - factories.add(new PriorityHttpClientFactory()); + factories.add(new ScoredHttpClientFactory("other", 0)); + factories.add(new ScoredHttpClientFactory("priority", Integer.MAX_VALUE)); factories.add(new DefaultHttpClientFactory()); - assertEquals(PriorityHttpClientFactory.class, HttpClientUtils.getHttpClientFactory().getClass()); + final HttpClient.Factory result = HttpClientUtils.getHttpClientFactory(); + assertEquals(ScoredHttpClientFactory.class, result.getClass()); + assertEquals(Integer.MAX_VALUE, result.priority()); + assertEquals("priority", ((ScoredHttpClientFactory) result).id); } - private final class DefaultHttpClientFactory implements HttpClient.Factory { + @Test + @DisplayName("With multiple implementations and several with max priority, should return first of max priority") + void withMultipleAndCollision() { + factories.add(new DefaultHttpClientFactory()); + factories.add(new ScoredHttpClientFactory("other", 0)); + factories.add(new ScoredHttpClientFactory("priority-1", Integer.MAX_VALUE)); + factories.add(new ScoredHttpClientFactory("priority-2", Integer.MAX_VALUE)); + factories.add(new DefaultHttpClientFactory()); + final HttpClient.Factory result = HttpClientUtils.getHttpClientFactory(); + assertEquals(ScoredHttpClientFactory.class, result.getClass()); + assertEquals(Integer.MAX_VALUE, result.priority()); + assertEquals("priority-1", ((ScoredHttpClientFactory) result).id); + } - @Override - public HttpClient.Builder newBuilder() { - return null; - } + private final class ScoredHttpClientFactory implements HttpClient.Factory { + private final String id; + private final int priority; - @Override - public boolean isDefault() { - return true; + public ScoredHttpClientFactory(String id, int priority) { + this.id = id; + this.priority = priority; } - } - - private final class OtherHttpClientFactory implements HttpClient.Factory { @Override public HttpClient.Builder newBuilder() { return null; } + + @Override + public int priority() { + return priority; + } } - private final class PriorityHttpClientFactory implements HttpClient.Factory { + private final class DefaultHttpClientFactory implements HttpClient.Factory { @Override public HttpClient.Builder newBuilder() { @@ -290,9 +306,10 @@ public HttpClient.Builder newBuilder() { } @Override - public int priority() { - return Integer.MAX_VALUE; + public boolean isDefault() { + return true; } } + } }