Skip to content

fix: Avoid Optional in fields/params - intended solely for return types #342

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Avoid Optional in fields/params - intended solely for return types #342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 6 additions & 7 deletions databend-client/src/main/java/com/databend/client/DatabendClientV1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.time.Duration;
import java.util.List;
import java.util.Map;
import java.util.OptionalLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.logging.Level;
Expand Down Expand Up @@ -57,7 +56,7 @@ public class DatabendClientV1

public static final String QUERY_PATH = "/v1/query";
public static final String DISCOVERY_PATH = "/v1/discovery_nodes";
private static final long MAX_MATERIALIZED_JSON_RESPONSE_SIZE = 128 * 1024;
private static final Long MAX_MATERIALIZED_JSON_RESPONSE_SIZE = 128 * 1024L;
private final OkHttpClient httpClient;
private final String query;
private final String host;
Expand Down Expand Up @@ -109,7 +108,7 @@ public static List<DiscoveryNode> discoverNodes(OkHttpClient httpClient, ClientS
requireNonNull(settings, "settings is null");
requireNonNull(settings.getHost(), "settings.host is null");
Request request = buildDiscoveryRequest(settings);
DiscoveryResponseCodec.DiscoveryResponse response = getDiscoveryResponse(httpClient, request, OptionalLong.empty(), settings.getQueryTimeoutSecs());
DiscoveryResponseCodec.DiscoveryResponse response = getDiscoveryResponse(httpClient, request, null, settings.getQueryTimeoutSecs());
return response.getNodes();
}

Expand Down Expand Up @@ -169,7 +168,7 @@ public String getQuery() {
return query;
}

private static DiscoveryResponseCodec.DiscoveryResponse getDiscoveryResponse(OkHttpClient httpClient, Request request, OptionalLong materializedJsonSizeLimit, int requestTimeoutSecs) {
private static DiscoveryResponseCodec.DiscoveryResponse getDiscoveryResponse(OkHttpClient httpClient, Request request, Long materializedJsonSizeLimit, int requestTimeoutSecs) {
requireNonNull(request, "request is null");

long start = System.nanoTime();
Expand Down Expand Up @@ -237,7 +236,7 @@ private static DiscoveryResponseCodec.DiscoveryResponse getDiscoveryResponse(OkH
}
}

private boolean executeInternal(Request request, OptionalLong materializedJsonSizeLimit) {
private boolean executeInternal(Request request, Long materializedJsonSizeLimit) {
requireNonNull(request, "request is null");

long start = System.nanoTime();
Expand Down Expand Up @@ -328,7 +327,7 @@ private String requestBodyToString(Request request) {

@Override
public boolean execute(Request request) {
return executeInternal(request, OptionalLong.empty());
return executeInternal(request, null);
}

private void processResponse(Headers headers, QueryResults results) {
Expand Down Expand Up @@ -378,7 +377,7 @@ public boolean advance() {
Request.Builder builder = prepareRequest(url, this.additonalHeaders);
builder.addHeader(ClientSettings.X_DATABEND_STICKY_NODE, this.nodeID);
Request request = builder.get().build();
return executeInternal(request, OptionalLong.of(MAX_MATERIALIZED_JSON_RESPONSE_SIZE));
return executeInternal(request, MAX_MATERIALIZED_JSON_RESPONSE_SIZE);
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions databend-client/src/main/java/com/databend/client/JsonResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import javax.annotation.Nullable;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Objects;
import java.util.Optional;
import java.util.OptionalLong;

import static com.google.common.base.MoreObjects.toStringHelper;
import static com.google.common.net.HttpHeaders.LOCATION;
Expand Down Expand Up @@ -59,7 +59,7 @@ private JsonResponse(int statusCode, String statusMessage, Headers headers, @Nul
this.hasValue = (exception == null);
}

public static <T> JsonResponse<T> execute(JsonCodec<T> codec, OkHttpClient client, Request request, OptionalLong materializedJsonSizeLimit) throws RuntimeException {
public static <T> JsonResponse<T> execute(JsonCodec<T> codec, OkHttpClient client, Request request, Long materializedJsonSizeLimit) throws RuntimeException {
try (Response response = client.newCall(request).execute()) {
// TODO: fix in OkHttp: https://github.com/square/okhttp/issues/3111
if ((response.code() == 307) || (response.code() == 308)) {
Expand All @@ -76,7 +76,7 @@ public static <T> JsonResponse<T> execute(JsonCodec<T> codec, OkHttpClient clien
T value = null;
IllegalArgumentException exception = null;
try {
if (materializedJsonSizeLimit.isPresent() && (responseBody.contentLength() < 0 || responseBody.contentLength() > materializedJsonSizeLimit.getAsLong())) {
if (!Objects.isNull(materializedJsonSizeLimit) && (responseBody.contentLength() < 0 || responseBody.contentLength() > materializedJsonSizeLimit)) {
// Parse from input stream, response is either of unknown size or too large to materialize. Raw response body
// will not be available if parsing fails
value = codec.fromJson(responseBody.byteStream());
Expand Down
Loading