fix: typo in HttpClient.Factory scoring system logic

Signed-off-by: Marc Nuri <[email protected]>

Author: manusa

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

kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/HttpClientUtils.java

@@ -157,7 +157,7 @@ private static HttpClient.Factory getFactory(ServiceLoader<HttpClient.Factory> 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;
       }
     }

kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/HttpClientUtilsTest.java

@@ -247,52 +247,69 @@ 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() {
         return null;
       }
 
       @Override
-      public int priority() {
-        return Integer.MAX_VALUE;
+      public boolean isDefault() {
+        return true;
       }
     }
+
   }
 }