Cleanup, update CHANGELOG, examples, deprecate additional_headers

ClickHouse · Mar 1, 2024 · ad604e1 · ad604e1
1 parent 6ff872b
commit ad604e1
Show file tree

Hide file tree

Showing 15 changed files with 109 additions and 55 deletions.
diff --git 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&param2=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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,22 +369,32 @@ 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(
         `Unknown URL parameters: ${Array.from(unknownParams).join(', ')}`
       )
     }
   }
+  // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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({