Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: typo in HttpClient.Factory scoring system logic #4947

Merged
merged 1 commit into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

}
}