From ad604e121edf74c5f57480cdc18797afcd4aeaba Mon Sep 17 00:00:00 2001 From: Serge Klochkov <hypnoash@gmail.com> Date: Fri, 1 Mar 2024 15:14:45 +0100 Subject: [PATCH] Cleanup, update CHANGELOG, examples, deprecate additional_headers --- CHANGELOG.md | 44 ++++++----- examples/README.md | 2 +- examples/create_table_cloud.ts | 2 +- examples/insert_cloud.ts | 2 +- examples/node/basic_tls.ts | 2 +- examples/node/mutual_tls.ts | 2 +- examples/ping_cloud.ts | 2 +- packages/client-common/src/config.ts | 76 +++++++++++++++---- packages/client-common/src/connection.ts | 2 +- .../__tests__/integration/node_client.test.ts | 8 +- .../__tests__/unit/node_client.test.ts | 4 +- .../src/connection/node_base_connection.ts | 6 +- .../__tests__/integration/web_client.test.ts | 6 +- .../__tests__/unit/web_client.test.ts | 4 +- .../src/connection/web_connection.ts | 2 +- 15 files changed, 109 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08db0da7..04173aa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ Formal stable release milestone. The client will follow the [official semantic v ### Deprecated API - `host` configuration parameter is deprecated; use `url` instead. +- `additional_headers` configuration parameter is deprecated; use `http_headers` instead. + +The client will log a warning if these deprecated configuration parameters are used. + +See "New features" section for more details. ### Breaking changes @@ -40,26 +45,29 @@ See also: [readonly documentation](https://clickhouse.com/docs/en/operations/set ### New features -- Added `url` configuration parameter. It is intended to replace the deprecated `host`, which was already supposed to be passed a valid URL. +- Added `url` configuration parameter. It is intended to replace the deprecated `host`, which was already supposed to be passed as a valid URL. +- Added `http_headers` configuration parameter as a direct replacement for `additional_headers`. Functionally, it is the same, and the change is purely cosmetic, as we'd like to leave an option to implement TCP connection in the future open. - It is now possible to configure most of the client instance parameters with a URL. The URL format is `http[s]://[username:password@]hostname:port[/database][?param1=value1¶m2=value2]`. The name of a particular parameter is supposed to reflect its path in the config options interface. The following parameters are supported: - - `readonly` - boolean. See below [1]. - - `application_id` - non-empty string. - - `session_id` - non-empty string. - - `request_timeout` - non-negative number. - - `max_open_connections` - non-negative number, greater than zero. - - `compression_request` - boolean. - - `compression_response` - boolean. - - `log_level` - allowed values: `OFF`, `TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`. - - `keep_alive_enabled` - boolean. - - `clickhouse_settings_*` - see below [2]. - - `additional_headers_*` - see below [3]. - - (Node.js only) `keep_alive_socket_ttl` - non-negative number. - - (Node.js only) `keep_alive_retry_on_expired_socket` - boolean. +| Parameter | Type | +| --------------------------------------------------- | ----------------------------------------------------------------- | +| `readonly` | boolean. See below [1]. | +| `application_id` | non-empty string. | +| `session_id` | non-empty string. | +| `request_timeout` | non-negative number. | +| `max_open_connections` | non-negative number, greater than zero. | +| `compression_request` | boolean. | +| `compression_response` | boolean. | +| `log_level` | allowed values: `OFF`, `TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`. | +| `keep_alive_enabled` | boolean. | +| `clickhouse_setting_*` or `ch_*` | see below [2]. | +| `http_header_*` | see below [3]. | +| (Node.js only) `keep_alive_socket_ttl` | non-negative number. | +| (Node.js only) `keep_alive_retry_on_expired_socket` | boolean. | [1] For booleans, valid values will be `true`/`1` and `false`/`0`. -[2] Any parameter prefixed with `clickhouse_settings_` will have this prefix removed and the rest added to client's `clickhouse_settings`. For example, `?clickhouse_settings_async_insert=1&clickhouse_settings_wait_for_async_insert=1` will be the same as: +[2] Any parameter prefixed with `clickhouse_setting_` or `ch_` will have this prefix removed and the rest added to client's `clickhouse_settings`. For example, `?ch_async_insert=1&ch_wait_for_async_insert=1` will be the same as: ```ts createClient({ @@ -70,17 +78,17 @@ createClient({ }) ``` -[3] Similar to [2], but for `additional_headers` configuration. For example, `?additional_headers_x-clickhouse-auth=foobar` will be an equivalent of: +[3] Similar to [2], but for `http_header` configuration. For example, `?http_header_x-clickhouse-auth=foobar` will be an equivalent of: ```ts createClient({ - additional_headers: { + http_headers: { 'x-clickhouse-auth': 'foobar', }, }) ``` -URL will _always_ overwrite the hardcoded values and a warning will be logged in this case. +**Important: URL will _always_ overwrite the hardcoded values and a warning will be logged in this case.** Currently not supported via URL: diff --git a/examples/README.md b/examples/README.md index 6d9aca86..9daae153 100644 --- a/examples/README.md +++ b/examples/README.md @@ -62,7 +62,7 @@ ts-node --transpile-only create_table_local_cluster.ts - for `create_table_cloud.ts`, Docker containers are not required, but you need to set some environment variables first: ```sh -export CLICKHOUSE_HOST=https://<your-clickhouse-cloud-hostname>:8443 +export CLICKHOUSE_URL=https://<your-clickhouse-cloud-hostname>:8443 export CLICKHOUSE_PASSWORD=<your-clickhouse-cloud-password> ``` diff --git a/examples/create_table_cloud.ts b/examples/create_table_cloud.ts index fb3b6840..26355fcd 100644 --- a/examples/create_table_cloud.ts +++ b/examples/create_table_cloud.ts @@ -2,7 +2,7 @@ import { createClient } from '@clickhouse/client' // or '@clickhouse/client-web' void (async () => { const client = createClient({ - host: getFromEnv('CLICKHOUSE_HOST'), + url: getFromEnv('CLICKHOUSE_URL'), password: getFromEnv('CLICKHOUSE_PASSWORD'), }) // Note that ENGINE and ON CLUSTER clauses can be omitted entirely here. diff --git a/examples/insert_cloud.ts b/examples/insert_cloud.ts index e25d462a..4ec25700 100644 --- a/examples/insert_cloud.ts +++ b/examples/insert_cloud.ts @@ -2,7 +2,7 @@ import { createClient } from '@clickhouse/client' // or '@clickhouse/client-web' void (async () => { const client = createClient({ - host: getFromEnv('CLICKHOUSE_HOST'), + url: getFromEnv('CLICKHOUSE_URL'), password: getFromEnv('CLICKHOUSE_PASSWORD'), // See https://clickhouse.com/docs/en/optimize/asynchronous-inserts clickhouse_settings: { diff --git a/examples/node/basic_tls.ts b/examples/node/basic_tls.ts index be79c111..b9a6b171 100644 --- a/examples/node/basic_tls.ts +++ b/examples/node/basic_tls.ts @@ -3,7 +3,7 @@ import fs from 'fs' void (async () => { const client = createClient({ - host: 'https://server.clickhouseconnect.test:8443', + url: 'https://server.clickhouseconnect.test:8443', tls: { ca_cert: fs.readFileSync( '../.docker/clickhouse/single_node_tls/certificates/ca.crt' diff --git a/examples/node/mutual_tls.ts b/examples/node/mutual_tls.ts index b18e08c9..e7ca43fb 100644 --- a/examples/node/mutual_tls.ts +++ b/examples/node/mutual_tls.ts @@ -4,7 +4,7 @@ import fs from 'fs' void (async () => { const certsPath = '../.docker/clickhouse/single_node_tls/certificates' const client = createClient({ - host: 'https://server.clickhouseconnect.test:8443', + url: 'https://server.clickhouseconnect.test:8443', username: 'cert_user', tls: { ca_cert: fs.readFileSync(`${certsPath}/ca.crt`), diff --git a/examples/ping_cloud.ts b/examples/ping_cloud.ts index b7534587..fbf83cfb 100644 --- a/examples/ping_cloud.ts +++ b/examples/ping_cloud.ts @@ -2,7 +2,7 @@ import { createClient } from '@clickhouse/client' // or '@clickhouse/client-web' void (async () => { const client = createClient({ - host: getFromEnv('CLICKHOUSE_HOST'), + url: getFromEnv('CLICKHOUSE_URL'), password: getFromEnv('CLICKHOUSE_PASSWORD'), }) console.info(await client.ping()) diff --git a/packages/client-common/src/config.ts b/packages/client-common/src/config.ts index f382c5ba..581a862e 100644 --- a/packages/client-common/src/config.ts +++ b/packages/client-common/src/config.ts @@ -24,7 +24,7 @@ const DefaultClickHouseSettings: ClickHouseSettings = { } export interface BaseClickHouseClientConfigOptions { - /** @deprecated since version 0.3.0. Use {@link url} instead. <br/> + /** @deprecated since version 1.0.0. Use {@link url} instead. <br/> * A ClickHouse instance URL. Default value: `http://localhost:8123`. */ host?: string /** A ClickHouse instance URL. Default value: `http://localhost:8123`. */ @@ -66,9 +66,13 @@ export interface BaseClickHouseClientConfigOptions { /** ClickHouse Session id to attach to the outgoing requests. * Default: empty. */ session_id?: string - /** Additional HTTP headers to attach to the outgoing requests. + /** @deprecated since version 1.0.0. Use {@link http_headers} instead. <br/> + * Additional HTTP headers to attach to the outgoing requests. * Default: empty. */ additional_headers?: Record<string, string> + /** Additional HTTP headers to attach to the outgoing requests. + * Default: empty. */ + http_headers?: Record<string, string> /** If the client instance created for a user with `READONLY = 1` mode, * some settings, such as {@link compression}, `send_progress_in_http_headers`, * and `http_headers_progress_interval_ms` can't be modified, @@ -120,13 +124,19 @@ export type CloseStream<Stream> = (stream: Stream) => Promise<void> /** * An implementation might have extra config parameters that we can parse from the connection URL. * These are supposed to be processed after we finish parsing the base configuration. + * URL params handled in the common package will be deleted from the URL object. + * This way we ensure that only implementation-specific params are passed there. */ export type HandleExtraURLParams = ( config: BaseClickHouseClientConfigOptions, url: URL ) => { config: BaseClickHouseClientConfigOptions + // params that were handled in the implementation; used to calculate final "unknown" URL params + // i.e. common package does not know about Node.js-specific ones, + // but after handling we will be able to remove them from the final unknown set (and not throw). handled_params: Set<string> + // params that are still unknown even in the implementation unknown_params: Set<string> } @@ -151,6 +161,15 @@ export function getConnectionParams( const logger = baseConfig?.log?.LoggerClass ? new baseConfig.log.LoggerClass() : new DefaultLogger() + let httpHeaders = baseConfig.http_headers + if (baseConfig.additional_headers !== undefined) { + logger.warn({ + module: 'Config', + message: + 'Configuration parameter "additional_headers" is deprecated. Use "http_headers" instead.', + }) + httpHeaders = baseConfig.additional_headers + } let configURL if (baseConfig.host !== undefined) { logger.warn({ @@ -198,10 +217,14 @@ export function getConnectionParams( database: config.database ?? 'default', clickhouse_settings: clickHouseSettings, log_writer: new LogWriter(logger, config.log?.level), - additional_headers: config.additional_headers, + http_headers: httpHeaders, } } +/** + * Merge two versions of the config: base (hardcoded) from the instance creation and the URL parsed one. + * NB: currently, merges only the top level keys to simplify the flow. + */ export function mergeConfigs( baseConfig: BaseClickHouseClientConfigOptions, configFromURL: BaseClickHouseClientConfigOptions, @@ -232,7 +255,7 @@ export function createUrl(configURL: string | URL | undefined): URL { return new URL('http://localhost:8123') } } catch (err) { - throw new Error('Client URL is malformed.') + throw new Error('ClickHouse URL is malformed.') } if (url.protocol !== 'http:' && url.protocol !== 'https:') { throw new Error( @@ -242,6 +265,11 @@ export function createUrl(configURL: string | URL | undefined): URL { return url } +/** + * @param url potentially contains auth, database and URL params to parse the configuration from + * @param handleExtraURLParams some platform-specific URL params might be unknown by the common package; + * use this function defined in the implementation to handle them. + */ export function loadConfigOptionsFromURL( url: URL, handleExtraURLParams: HandleExtraURLParams | null @@ -259,23 +287,31 @@ export function loadConfigOptionsFromURL( } if (url.searchParams.size > 0) { const unknownParams = new Set<string>() - const settingsPrefix = 'clickhouse_settings_' - const additionalHeadersPrefix = 'additional_headers_' + const settingPrefix = 'clickhouse_setting_' + const settingShortPrefix = 'ch_' + const httpHeaderPrefix = 'http_header_' for (const key of url.searchParams.keys()) { + let paramWasProcessed = true const value = url.searchParams.get(key) as string // clickhouse_settings_* - if (key.startsWith(settingsPrefix)) { - const settingKey = key.slice(settingsPrefix.length) + if (key.startsWith(settingPrefix)) { + const settingKey = key.slice(settingPrefix.length) if (config.clickhouse_settings === undefined) { config.clickhouse_settings = {} } config.clickhouse_settings[settingKey] = value - } else if (key.startsWith(additionalHeadersPrefix)) { - const headerKey = key.slice(additionalHeadersPrefix.length) - if (config.additional_headers === undefined) { - config.additional_headers = {} + } else if (key.startsWith(settingShortPrefix)) { + const settingKey = key.slice(settingShortPrefix.length) + if (config.clickhouse_settings === undefined) { + config.clickhouse_settings = {} } - config.additional_headers[headerKey] = value + config.clickhouse_settings[settingKey] = value + } else if (key.startsWith(httpHeaderPrefix)) { + const headerKey = key.slice(httpHeaderPrefix.length) + if (config.http_headers === undefined) { + config.http_headers = {} + } + config.http_headers[headerKey] = value } else { switch (key) { case 'readonly': @@ -333,15 +369,24 @@ export function loadConfigOptionsFromURL( config.keep_alive.enabled = booleanConfigURLValue({ key, value }) break default: + paramWasProcessed = false unknownParams.add(key) } } - url.searchParams.delete(key) + if (paramWasProcessed) { + // so it won't be passed to the impl URL params handler + url.searchParams.delete(key) + } } if (handleExtraURLParams !== null) { const res = handleExtraURLParams(config, url) config = res.config - res.handled_params.forEach((k) => unknownParams.delete(k)) + if (unknownParams.size > 0) { + res.handled_params.forEach((k) => unknownParams.delete(k)) + } + if (res.unknown_params.size > 0) { + res.unknown_params.forEach((k) => unknownParams.add(k)) + } } if (unknownParams.size > 0) { throw new Error( @@ -349,6 +394,7 @@ export function loadConfigOptionsFromURL( ) } } + // clean up the final ClickHouse URL to be used in the connection const clickHouseURL = new URL(`${url.protocol}//${url.host}`) return [clickHouseURL, config] } diff --git a/packages/client-common/src/connection.ts b/packages/client-common/src/connection.ts index 49adabad..22c9ad91 100644 --- a/packages/client-common/src/connection.ts +++ b/packages/client-common/src/connection.ts @@ -13,7 +13,7 @@ export interface ConnectionParams { clickhouse_settings: ClickHouseSettings log_writer: LogWriter application_id?: string - additional_headers?: Record<string, string> + http_headers?: Record<string, string> } export interface CompressionSettings { diff --git a/packages/client-node/__tests__/integration/node_client.test.ts b/packages/client-node/__tests__/integration/node_client.test.ts index c579cf19..115638bc 100644 --- a/packages/client-node/__tests__/integration/node_client.test.ts +++ b/packages/client-node/__tests__/integration/node_client.test.ts @@ -12,10 +12,10 @@ describe('[Node.js] Client', () => { httpRequestStub = spyOn(Http, 'request').and.returnValue(clientRequest) }) - describe('Additional headers', () => { - it('should be possible to set additional_headers', async () => { + describe('HTTP headers', () => { + it('should be possible to set http_headers', async () => { const client = createClient({ - additional_headers: { + http_headers: { 'Test-Header': 'foobar', }, }) @@ -32,7 +32,7 @@ describe('[Node.js] Client', () => { assertSearchParams(callURL) }) - it('should work without additional headers', async () => { + it('should work without additional HTTP headers', async () => { const client = createClient({}) await query(client) diff --git a/packages/client-node/__tests__/unit/node_client.test.ts b/packages/client-node/__tests__/unit/node_client.test.ts index 09ca19f4..9f36bd72 100644 --- a/packages/client-node/__tests__/unit/node_client.test.ts +++ b/packages/client-node/__tests__/unit/node_client.test.ts @@ -2,9 +2,9 @@ import type { BaseClickHouseClientConfigOptions } from '@clickhouse/client-commo import { createClient } from '../../src' describe('[Node.js] createClient', () => { - it('throws on incorrect "host" config value', () => { + it('throws on incorrect "url" config value', () => { expect(() => createClient({ url: 'foo' })).toThrowError( - 'Client URL is malformed.' + 'ClickHouse URL is malformed.' ) }) diff --git a/packages/client-node/src/connection/node_base_connection.ts b/packages/client-node/src/connection/node_base_connection.ts index 81a2204e..c006df22 100644 --- a/packages/client-node/src/connection/node_base_connection.ts +++ b/packages/client-node/src/connection/node_base_connection.ts @@ -85,21 +85,21 @@ export abstract class NodeBaseConnection this.headers = this.buildDefaultHeaders( params.username, params.password, - params.additional_headers + params.http_headers ) } protected buildDefaultHeaders( username: string, password: string, - additional_headers?: Record<string, string> + http_headers?: Record<string, string> ): Http.OutgoingHttpHeaders { return { Authorization: `Basic ${Buffer.from(`${username}:${password}`).toString( 'base64' )}`, 'User-Agent': getUserAgent(this.params.application_id), - ...additional_headers, + ...http_headers, } } diff --git a/packages/client-web/__tests__/integration/web_client.test.ts b/packages/client-web/__tests__/integration/web_client.test.ts index eb89b7da..8ee83429 100644 --- a/packages/client-web/__tests__/integration/web_client.test.ts +++ b/packages/client-web/__tests__/integration/web_client.test.ts @@ -9,10 +9,10 @@ describe('[Web] Client', () => { ) }) - describe('Additional headers', () => { + describe('HTTP headers', () => { it('should be possible to set', async () => { const client = createClient({ - additional_headers: { + http_headers: { 'Test-Header': 'foobar', }, }) @@ -24,7 +24,7 @@ describe('[Web] Client', () => { }) }) - it('should work with no additional headers provided', async () => { + it('should work with no additional HTTP headers provided', async () => { const client = createClient({}) const fetchParams = await pingAndGetRequestInit(client) expect(fetchParams!.headers).toEqual({ diff --git a/packages/client-web/__tests__/unit/web_client.test.ts b/packages/client-web/__tests__/unit/web_client.test.ts index 9d9151f1..23b6fab0 100644 --- a/packages/client-web/__tests__/unit/web_client.test.ts +++ b/packages/client-web/__tests__/unit/web_client.test.ts @@ -2,9 +2,9 @@ import type { BaseClickHouseClientConfigOptions } from '@clickhouse/client-commo import { createClient } from '../../src' describe('[Web] createClient', () => { - it('throws on incorrect "host" config value', () => { + it('throws on incorrect "url" config value', () => { expect(() => createClient({ url: 'foo' })).toThrowError( - 'Configuration parameter "host" contains malformed url.' + 'ClickHouse URL is malformed.' ) }) diff --git a/packages/client-web/src/connection/web_connection.ts b/packages/client-web/src/connection/web_connection.ts index a0fc1cbb..208aad51 100644 --- a/packages/client-web/src/connection/web_connection.ts +++ b/packages/client-web/src/connection/web_connection.ts @@ -35,7 +35,7 @@ export class WebConnection implements Connection<ReadableStream> { constructor(private readonly params: WebConnectionParams) { this.defaultHeaders = { Authorization: `Basic ${btoa(`${params.username}:${params.password}`)}`, - ...params?.additional_headers, + ...params?.http_headers, } }