Skip to content

Use Basic auth for credentials instead of query params #548

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

Merged
zhicwu merged 1 commit into from
Feb 6, 2021
Merged

Use Basic auth for credentials instead of query params #548

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
7 changes: 7 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
<testcontainers.version>1.15.1</testcontainers.version>
<testng.version>6.14.3</testng.version>
<mockito.version>1.10.19</mockito.version>
<wiremock.version>2.27.2</wiremock.version>
<helper-plugin.version>3.2.0</helper-plugin.version>
<deploy-plugin.version>3.0.0-M1</deploy-plugin.version>
<staging-plugin.version>1.6.8</staging-plugin.version>
Expand Down Expand Up @@ -151,6 +152,12 @@
<version>${mockito.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.github.tomakehurst</groupId>
<artifactId>wiremock-jre8</artifactId>
<version>${wiremock.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<distributionManagement>
Expand Down
44 changes: 39 additions & 5 deletions src/main/java/ru/yandex/clickhouse/ClickHouseConnectionImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,13 @@ public ClickHouseStatement createStatement() throws SQLException {
}

public ClickHouseStatement createStatement(int resultSetType) throws SQLException {
return LogProxy.wrap(ClickHouseStatement.class, new ClickHouseStatementImpl(httpclient, this, properties, resultSetType));
return LogProxy.wrap(
ClickHouseStatement.class,
new ClickHouseStatementImpl(
httpclient,
this,
properties,
resultSetType));
}

@Deprecated
Expand All @@ -126,15 +132,37 @@ public TimeZone getTimeZone() {
}

private ClickHouseStatement createClickHouseStatement(CloseableHttpClient httpClient) throws SQLException {
return LogProxy.wrap(ClickHouseStatement.class, new ClickHouseStatementImpl(httpClient, this, properties, DEFAULT_RESULTSET_TYPE));
return LogProxy.wrap(
ClickHouseStatement.class,
new ClickHouseStatementImpl(
httpClient,
this,
properties,
DEFAULT_RESULTSET_TYPE));
}

public PreparedStatement createPreparedStatement(String sql, int resultSetType) throws SQLException {
return LogProxy.wrap(PreparedStatement.class, new ClickHousePreparedStatementImpl(httpclient, this, properties, sql, getTimeZone(), resultSetType));
return LogProxy.wrap(
PreparedStatement.class,
new ClickHousePreparedStatementImpl(
httpclient,
this,
properties,
sql,
getTimeZone(),
resultSetType));
}

public ClickHousePreparedStatement createClickHousePreparedStatement(String sql, int resultSetType) throws SQLException {
return LogProxy.wrap(ClickHousePreparedStatement.class, new ClickHousePreparedStatementImpl(httpclient, this, properties, sql, getTimeZone(), resultSetType));
return LogProxy.wrap(
ClickHousePreparedStatement.class,
new ClickHousePreparedStatementImpl(
httpclient,
this,
properties,
sql,
getTimeZone(),
resultSetType));
}


Expand Down Expand Up @@ -408,12 +436,13 @@ public boolean isValid(int timeout) throws SQLException {

return false;
} finally {
if (isAnotherHttpClient)
if (isAnotherHttpClient) {
try {
closeableHttpClient.close();
} catch (IOException e) {
log.warn("Can't close a http client", e);
}
}
}
}

Expand Down Expand Up @@ -462,22 +491,27 @@ public boolean isWrapperFor(Class<?> iface) throws SQLException {
return iface.isAssignableFrom(getClass());
}

@Override
public void setSchema(String schema) throws SQLException {
properties.setDatabase(schema);
}

@Override
public String getSchema() throws SQLException {
return properties.getDatabase();
}

@Override
public void abort(Executor executor) throws SQLException {
this.close();
}

@Override
public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException {

}

@Override
public int getNetworkTimeout() throws SQLException {
return 0;
}
Expand Down
56 changes: 39 additions & 17 deletions src/main/java/ru/yandex/clickhouse/ClickHousePreparedStatementImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,48 @@
package ru.yandex.clickhouse;

import org.apache.http.entity.AbstractHttpEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import ru.yandex.clickhouse.response.ClickHouseResponse;
import ru.yandex.clickhouse.settings.ClickHouseProperties;
import ru.yandex.clickhouse.settings.ClickHouseQueryParam;
import ru.yandex.clickhouse.util.ClickHouseArrayUtil;
import ru.yandex.clickhouse.util.ClickHouseValueFormatter;
import ru.yandex.clickhouse.util.guava.StreamUtils;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.Reader;
import java.math.BigDecimal;
import java.net.URL;
import java.sql.Array;
import java.sql.Blob;
import java.sql.Clob;
import java.sql.Date;
import java.sql.*;
import java.util.*;
import java.sql.NClob;
import java.sql.ParameterMetaData;
import java.sql.Ref;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.RowId;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.sql.SQLSyntaxErrorException;
import java.sql.SQLXML;
import java.sql.Time;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.TimeZone;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.http.entity.AbstractHttpEntity;
import org.apache.http.impl.client.CloseableHttpClient;

import ru.yandex.clickhouse.response.ClickHouseResponse;
import ru.yandex.clickhouse.settings.ClickHouseProperties;
import ru.yandex.clickhouse.settings.ClickHouseQueryParam;
import ru.yandex.clickhouse.util.ClickHouseArrayUtil;
import ru.yandex.clickhouse.util.ClickHouseValueFormatter;
import ru.yandex.clickhouse.util.guava.StreamUtils;


public class ClickHousePreparedStatementImpl extends ClickHouseStatementImpl implements ClickHousePreparedStatement {

Expand All @@ -36,11 +58,11 @@ public class ClickHousePreparedStatementImpl extends ClickHouseStatementImpl imp
private final ClickHousePreparedStatementParameter[] binds;
private final List<List<String>> parameterList;
private final boolean insertBatchMode;
private List<byte[]> batchRows = new ArrayList<byte[]>();
private List<byte[]> batchRows = new ArrayList<>();

public ClickHousePreparedStatementImpl(CloseableHttpClient client, ClickHouseConnection connection,
ClickHouseProperties properties, String sql, TimeZone serverTimeZone,
int resultSetType) throws SQLException
public ClickHousePreparedStatementImpl(CloseableHttpClient client,
ClickHouseConnection connection, ClickHouseProperties properties, String sql,
TimeZone serverTimeZone, int resultSetType) throws SQLException
{
super(client, connection, properties, resultSetType);
this.sql = sql;
Expand Down Expand Up @@ -295,7 +317,7 @@ public void addBatch() throws SQLException {

private List<byte[]> buildBatch() throws SQLException {
checkBinded();
List<byte[]> newBatches = new ArrayList<byte[]>(parameterList.size());
List<byte[]> newBatches = new ArrayList<>(parameterList.size());
StringBuilder sb = new StringBuilder();
for (int i = 0, p = 0; i < parameterList.size(); i++) {
List<String> pList = parameterList.get(i);
Expand Down Expand Up @@ -338,7 +360,7 @@ public int[] executeBatch(Map<ClickHouseQueryParam, String> additionalDBParams)
sendStream(entity, insertSql, additionalDBParams);
int[] result = new int[batchRows.size()];
Arrays.fill(result, 1);
batchRows = new ArrayList<byte[]>();
batchRows = new ArrayList<>();
return result;
}

Expand Down
71 changes: 47 additions & 24 deletions src/main/java/ru/yandex/clickhouse/ClickHouseStatementImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
package ru.yandex.clickhouse;

import com.google.common.base.Strings;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URISyntaxException;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.SQLWarning;
import java.util.ArrayList;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;
import java.util.TimeZone;
import java.util.UUID;

import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.NameValuePair;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.protocol.HttpClientContext;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
Expand All @@ -16,35 +32,34 @@
import org.apache.http.util.EntityUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.base.Strings;

import ru.yandex.clickhouse.domain.ClickHouseFormat;
import ru.yandex.clickhouse.except.ClickHouseException;
import ru.yandex.clickhouse.except.ClickHouseExceptionSpecifier;
import ru.yandex.clickhouse.response.*;
import ru.yandex.clickhouse.response.ClickHouseLZ4Stream;
import ru.yandex.clickhouse.response.ClickHouseResponse;
import ru.yandex.clickhouse.response.ClickHouseResponseSummary;
import ru.yandex.clickhouse.response.ClickHouseResultSet;
import ru.yandex.clickhouse.response.ClickHouseScrollableResultSet;
import ru.yandex.clickhouse.response.FastByteArrayOutputStream;
import ru.yandex.clickhouse.settings.ClickHouseProperties;
import ru.yandex.clickhouse.settings.ClickHouseQueryParam;
import ru.yandex.clickhouse.util.ClickHouseHttpClientBuilder;
import ru.yandex.clickhouse.util.ClickHouseRowBinaryInputStream;
import ru.yandex.clickhouse.util.ClickHouseStreamCallback;
import ru.yandex.clickhouse.util.Utils;
import ru.yandex.clickhouse.util.guava.StreamUtils;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URISyntaxException;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.SQLWarning;
import java.util.*;


public class ClickHouseStatementImpl extends ConfigurableApi<ClickHouseStatement> implements ClickHouseStatement {

private static final Logger log = LoggerFactory.getLogger(ClickHouseStatementImpl.class);

private final CloseableHttpClient client;

private final HttpClientContext httpContext;
Copy link
Contributor

@zhicwu zhicwu Feb 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make it protected and create the context in constructor?

Copy link
Contributor Author

@enqueue enqueue Feb 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is analogous to the client. This class does not know how to construct these, but gets ready to use "connection stuff".

Copy link
Contributor Author

@enqueue enqueue Feb 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I would say that the statement should simply get everything it needs to do its work. It does not need to know how to create connection context, but this is my subjective judgement. I can understand that we want to try and keep the number of ties with Apache HTTP Client under control. I am going to follow your suggestion.

Copy link
Contributor

@zhicwu zhicwu Feb 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point. Perhaps it's too early to consider refactoring. I'm fine to keep HttpContext argument now, but I think we may use a new interface like ClickHouseClient to replace both CloseableHtppClient and HttpContext, in order to hide protocol implementation details.


protected ClickHouseProperties properties;

private ClickHouseConnection connection;
Expand Down Expand Up @@ -79,10 +94,12 @@ public class ClickHouseStatementImpl extends ConfigurableApi<ClickHouseStatement
private static final String[] selectKeywords = new String[]{"SELECT", "WITH", "SHOW", "DESC", "EXISTS", "EXPLAIN"};
private static final String databaseKeyword = "CREATE DATABASE";


public ClickHouseStatementImpl(CloseableHttpClient client, ClickHouseConnection connection,
ClickHouseProperties properties, int resultSetType) {
super(null);
this.client = client;
this.httpContext = ClickHouseHttpClientBuilder.createClientContext(properties);
this.connection = connection;
this.properties = properties == null ? new ClickHouseProperties() : properties;
this.initialDatabase = this.properties.getDatabase();
Expand Down Expand Up @@ -112,9 +129,9 @@ public ResultSet executeQuery(String sql,

// forcibly disable extremes for ResultSet queries
if (additionalDBParams == null || additionalDBParams.isEmpty()) {
additionalDBParams = new EnumMap<ClickHouseQueryParam, String>(ClickHouseQueryParam.class);
additionalDBParams = new EnumMap<>(ClickHouseQueryParam.class);
} else {
additionalDBParams = new EnumMap<ClickHouseQueryParam, String>(additionalDBParams);
additionalDBParams = new EnumMap<>(additionalDBParams);
}
additionalDBParams.put(ClickHouseQueryParam.EXTREMES, "0");

Expand Down Expand Up @@ -621,7 +638,7 @@ private InputStream getInputStream(
HttpPost post = new HttpPost(uri);
post.setEntity(requestEntity);

HttpResponse response = client.execute(post);
HttpResponse response = client.execute(post, httpContext);
entity = response.getEntity();
checkForErrorAndThrow(entity, response);

Expand Down Expand Up @@ -687,7 +704,7 @@ private List<NameValuePair> getUrlQueryParams(
Map<String, String> additionalRequestParams,
boolean ignoreDatabase
) {
List<NameValuePair> result = new ArrayList<NameValuePair>();
List<NameValuePair> result = new ArrayList<>();

if (sql != null) {
result.add(new BasicNameValuePair("query", sql));
Expand Down Expand Up @@ -755,17 +772,21 @@ private boolean isQueryParamSet(ClickHouseQueryParam param, Map<ClickHouseQueryP
}

private String getQueryParamValue(ClickHouseQueryParam param, Map<ClickHouseQueryParam, String> additionalClickHouseDBParams, Map<String, String> additionalRequestParams) {
if (additionalRequestParams != null && additionalRequestParams.containsKey(param.getKey()) && !Strings.isNullOrEmpty(additionalRequestParams.get(param.getKey())))
if (additionalRequestParams != null && additionalRequestParams.containsKey(param.getKey()) && !Strings.isNullOrEmpty(additionalRequestParams.get(param.getKey()))) {
return additionalRequestParams.get(param.getKey());
}

if (getRequestParams().containsKey(param.getKey()) && !Strings.isNullOrEmpty(getRequestParams().get(param.getKey())))
if (getRequestParams().containsKey(param.getKey()) && !Strings.isNullOrEmpty(getRequestParams().get(param.getKey()))) {
return getRequestParams().get(param.getKey());
}

if (additionalClickHouseDBParams != null && additionalClickHouseDBParams.containsKey(param) && !Strings.isNullOrEmpty(additionalClickHouseDBParams.get(param)))
if (additionalClickHouseDBParams != null && additionalClickHouseDBParams.containsKey(param) && !Strings.isNullOrEmpty(additionalClickHouseDBParams.get(param))) {
return additionalClickHouseDBParams.get(param);
}

if (getAdditionalDBParams().containsKey(param) && !Strings.isNullOrEmpty(getAdditionalDBParams().get(param)))
if (getAdditionalDBParams().containsKey(param) && !Strings.isNullOrEmpty(getAdditionalDBParams().get(param))) {
return getAdditionalDBParams().get(param);
}

return properties.asProperties().getProperty(param.getKey());
}
Expand All @@ -775,7 +796,7 @@ private URI followRedirects(URI uri) throws IOException, URISyntaxException {
int redirects = 0;
while (redirects < properties.getMaxRedirects()) {
HttpGet httpGet = new HttpGet(uri);
HttpResponse response = client.execute(httpGet);
HttpResponse response = client.execute(httpGet, httpContext);
if (response.getStatusLine().getStatusCode() == 307) {
uri = new URI(response.getHeaders("Location")[0].getValue());
redirects++;
Expand Down Expand Up @@ -896,7 +917,7 @@ void sendStream(Writer writer, HttpEntity content) throws ClickHouseException {
httpPost.addHeader("Content-Encoding", writer.getCompression().name());
}
httpPost.setEntity(content);
HttpResponse response = client.execute(httpPost);
HttpResponse response = client.execute(httpPost, httpContext);
entity = response.getEntity();
checkForErrorAndThrow(entity, response);

Expand Down Expand Up @@ -932,10 +953,12 @@ private void checkForErrorAndThrow(HttpEntity entity, HttpResponse response) thr
}
}

@Override
public void closeOnCompletion() throws SQLException {
closeOnCompletion = true;
}

@Override
public boolean isCloseOnCompletion() throws SQLException {
return closeOnCompletion;
}
Expand Down
Loading