Skip to content

Commit 6feafbd

Browse files
committed
Fixed issue with mutating config
1 parent 1ed667d commit 6feafbd

4 files changed

Lines changed: 27 additions & 23 deletions

File tree

client-v2/src/main/java/com/clickhouse/client/api/http/ClickHouseHttpProto.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ public class ClickHouseHttpProto {
8484
*/
8585
public static final String QPARAM_QUERY_STMT = "query";
8686

87+
public static final String QPARAM_ENABLE_HTTP_COMPRESSION = "enable_http_compression";
88+
89+
public static final String QPARAM_COMPRESS = "compress";
90+
91+
public static final String QPARAM_DECOMPRESS = "decompress";
92+
8793
public static final int DEFAULT_HTTP_PORT = 8123;
8894

8995
public static final int DEFAULT_HTTPS_PORT = 8443;

client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -508,14 +508,16 @@ private HttpContext createRequestHttpContext(Map<String, Object> requestConfig)
508508
return context;
509509
}
510510

511-
private URI createRequestURI(Endpoint server, Map<String,Object> requestConfig, boolean addParameters) {
511+
private URI createRequestURI(Endpoint server, Map<String,Object> requestConfig, boolean isMultipartRequest) {
512512
URI uri;
513513
try {
514514
URIBuilder uriBuilder = new URIBuilder(server.getURI());
515515
addRequestParams(requestConfig, uriBuilder::addParameter);
516516

517-
if (addParameters) {
517+
if (!isMultipartRequest) {
518518
addStatementParams(requestConfig, uriBuilder::addParameter);
519+
} else {
520+
uriBuilder.removeParameter(ClickHouseHttpProto.QPARAM_DECOMPRESS); // multipart request doesn't support compression yet
519521
}
520522

521523
uri = uriBuilder.optimize().build();
@@ -573,35 +575,32 @@ public TransportRequest createRequest(Endpoint server, Map<String, Object> reque
573575
boolean useMultipart = ClientConfigProperties.HTTP_SEND_PARAMS_IN_BODY.<Boolean>getOrDefault(requestConfig) &&
574576
requestConfig.containsKey(HttpAPIClientHelper.KEY_STATEMENT_PARAMS);
575577

576-
// adjust configuration
577-
if (useMultipart) {
578-
requestConfig.put(ClientConfigProperties.COMPRESS_CLIENT_REQUEST.getKey(), false); // turn-off client-req compression
579-
}
580-
581578
// create configuration dependent objects
582-
final URI uri = createRequestURI(server, requestConfig, !useMultipart);
579+
final URI uri = createRequestURI(server, requestConfig, useMultipart);
583580
final HttpPost req = createPostRequest(uri, requestConfig);
584581

582+
final HttpEntity httpEntity;
585583
if (useMultipart) {
586584
MultipartEntityBuilder multipartEntityBuilder = MultipartEntityBuilder.create();
587585
addStatementParams(requestConfig, multipartEntityBuilder::addTextBody);
588586
multipartEntityBuilder.addTextBody(ClickHouseHttpProto.QPARAM_QUERY_STMT, body);
589587

590-
HttpEntity httpEntity = multipartEntityBuilder.build();
588+
httpEntity = multipartEntityBuilder.build();
591589
req.setHeader(HttpHeaders.CONTENT_TYPE, httpEntity.getContentType()); // set proper content type with generated boundary value
592-
req.setEntity(wrapRequestEntity(httpEntity, requestConfig));
593-
594590
} else {
595-
final HttpEntity httpEntity;
596591
try {
597592
final String contentEncoding = req.containsHeader(HttpHeaders.CONTENT_ENCODING) ? req.getHeader(HttpHeaders.CONTENT_ENCODING).getValue() : null;
598593
httpEntity = new ByteArrayEntity(body.getBytes(StandardCharsets.UTF_8.name()), CONTENT_TYPE, contentEncoding);
599594
} catch (UnsupportedEncodingException | ProtocolException e) {
600595
throw new ClientException("failed to create request body entity", e);
601596
}
597+
}
598+
// adjust configuration
599+
if (useMultipart) {
600+
req.setEntity(httpEntity); // multipart doesn't support compression right now
601+
} else {
602602
req.setEntity(wrapRequestEntity(httpEntity, requestConfig));
603603
}
604-
605604
return new TransportRequestImpl(req, requestConfig);
606605
}
607606

@@ -723,7 +722,7 @@ public TransportResponse executeRequest(TransportRequest transportRequest) throw
723722
}
724723

725724
public TransportRequest createRequest(Endpoint server, Map<String, Object> requestConfig, IOCallback<OutputStream> writeCallback) {
726-
final URI uri = createRequestURI(server, requestConfig, true);
725+
final URI uri = createRequestURI(server, requestConfig, false);
727726
final HttpPost req = createPostRequest(uri, requestConfig);
728727
try {
729728
String contentEncoding = req.containsHeader(HttpHeaders.CONTENT_ENCODING) ? req.getHeader(HttpHeaders.CONTENT_ENCODING).getValue() : null;
@@ -852,13 +851,13 @@ private void addRequestParams(Map<String, Object> requestConfig, BiConsumer<Stri
852851
// enable_http_compression make server react on http header
853852
// for client side compression Content-Encoding should be set
854853
// for server side compression Accept-Encoding should be set
855-
consumer.accept("enable_http_compression", "1");
854+
consumer.accept(ClickHouseHttpProto.QPARAM_ENABLE_HTTP_COMPRESSION, "1");
856855
} else {
857856
if (serverCompression) {
858-
consumer.accept("compress", "1");
857+
consumer.accept(ClickHouseHttpProto.QPARAM_COMPRESS, "1");
859858
}
860859
if (clientCompression) {
861-
consumer.accept("decompress", "1");
860+
consumer.accept(ClickHouseHttpProto.QPARAM_DECOMPRESS, "1");
862861
}
863862
}
864863

client-v2/src/test/java/com/clickhouse/client/api/transport/TransportBaseTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,6 @@ public void testCancelRequest(String name, boolean async, boolean isInsert) thro
363363
new InsertSettings().setQueryId(queryId)).get(35, TimeUnit.SECONDS)) {
364364
opFinished.set(true);
365365
} catch (Throwable t) {
366-
t.printStackTrace();
367366
opError.set(t);
368367
}
369368
};

docs/releases/0_10_0.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ This property (not a feature) is deprecated. Please use `com.clickhouse.jdbc.Dri
8585

8686
## CLIENT-V2: Service Unavailable Retry Behavior Changed
8787

88-
Http status code `503 Service Unavailable` is handled now as `ConnectionException`. It means that if
89-
retry is needed on this type of error than `client_retry_on_failures` should contain `ConnectTimeout`.
90-
Previously it was under `ServerRetryable` failure cause.
88+
HTTP status code `503 Service Unavailable` is now surfaced as a connection-initiation failure (`ConnectionInitiationException`). It means that if
89+
retry is needed on this type of error then `client_retry_on_failures` should contain `ConnectTimeout`.
90+
Previously it was categorized under the `ServerRetryable` failure cause.
9191

9292
## CLIENT-V2: Handling unknown status codes
9393

94-
New version will throw `ClientException` instead of `ServerException` on any unhandled status code. Previously
95-
any such case was wrapped as `ServerException` what is not correct because it is client fails to process the code.
94+
New version will throw `ClientException` instead of `ServerException` on any unhandled status code. Previously
95+
any such case was wrapped as `ServerException`, which is not correct because it is the client that fails to process the code.

0 commit comments

Comments
 (0)