[CCI] Type improvement and optional callback on empty method (#451)

* refactor: addConnection

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* refactor: improve types

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* fix: connection type tests

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* feat: addConnection test with only URL provided

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* refactor: optimization

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: use vars

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* refactor: make callback optional

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: string type

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: update changelog

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* fix: update index.d.ts empty method args

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: user guide on empty method use

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* fix: remove optional chaining

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* refactor: remove else block

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* fix: make cb arg optional

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* fix: remove jsdoc return type

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: update types and tests

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: update changelog

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* refactor: swap to expecttype

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* feat: connection empty tests

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: remove log from user guide

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: format and update toc

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* fix: remove passing connection instance to update

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: test for createConnection with a Connection instance

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: remove jsdoc return

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: update changelog

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* chore: revert changelog

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

* feat: added call empty twice test

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

---------

Signed-off-by: Timur Bolotov <bolotovtmr@gmail.com>

CHANGELOG.md

@@ -38,6 +38,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Remove guidance on using npm and switch completely to yarn in developer_guide ([#439](https://github.com/opensearch-project/opensearch-js/issues/435))
 - Change coverage, compatability, integration, integration with unreleased Open Search, node ci, bundler tests not to run on documentation change ([441](https://github.com/opensearch-project/opensearch-js/pull/441))
 - Change the Windows yarn installation troubleshoot steps ([455](https://github.com/opensearch-project/opensearch-js/issues/455))
+- Make `callback` arg in `BaseConnectionPool`, `CloudConnectionPool` and `ConnectionPool` optional ([#451](https://github.com/opensearch-project/opensearch-js/pull/451))
+
 ### Deprecated
 
 - Remove deprecation warnings in bulk.test.js ([#434](https://github.com/opensearch-project/opensearch-js/issues/434))

USER_GUIDE.md

@@ -10,6 +10,7 @@
   - [Search for the Document](#search-for-the-document)
   - [Delete the document](#delete-the-document)
   - [Delete the index](#delete-the-index)
+  - [Empty all Pool Connections](#empty-all-pool-connections)
 
 ## Initializing a Client
 
@@ -87,7 +88,7 @@ const { AwsSigv4Signer } = require('@opensearch-project/opensearch/aws');
 const client = new Client({
   ...AwsSigv4Signer({
     region: 'us-east-1',
-    service: 'es',  // 'aoss' for OpenSearch Serverless
+    service: 'es', // 'aoss' for OpenSearch Serverless
     // Must return a Promise that resolve to an AWS.Credentials object.
     // This function is used to acquire the credentials when the client start and
     // when the credentials are expired.
@@ -249,4 +250,18 @@ console.log('Deleting all PITs:');
 var response = await client.deleteAllPits();
 
 console.log(response.body);
-```
+```
+
+## Empty all Pool Connections
+
+```javascript
+var pool = new ConnectionPool({ Connection });
+pool.addConnection('http://localhost:9200/');
+pool.addConnection('http://localhost:9201/');
+
+pool.empty();
+// OR
+pool.empty(() => {
+  // Do something after emptying the pool
+});
+```

lib/pool/BaseConnectionPool.js

@@ -32,6 +32,7 @@
 const { URL } = require('url');
 const debug = require('debug')('opensearch');
 const Connection = require('../Connection');
+const { ConfigurationError } = require('../errors');
 const noop = () => {};
 
 class BaseConnectionPool {
@@ -64,6 +65,9 @@ class BaseConnectionPool {
    * Creates a new connection instance.
    */
   createConnection(opts) {
+    if (opts instanceof Connection) {
+      throw new ConfigurationError('The argument provided is already a Connection instance.');
+    }
     if (typeof opts === 'string') {
       opts = this.urlToHost(opts);
     }
@@ -102,18 +106,25 @@ class BaseConnectionPool {
    */
   addConnection(opts) {
     if (Array.isArray(opts)) {
-      return opts.forEach((o) => this.addConnection(o));
+      opts.forEach((o) => this.addConnection(o));
+      return;
     }
-
     if (typeof opts === 'string') {
       opts = this.urlToHost(opts);
     }
 
-    const connectionById = this.connections.find((c) => c.id === opts.id);
-    const connectionByUrl = this.connections.find((c) => c.id === opts.url.href);
+    const connectionId = opts.id;
+    const connectionUrl = opts.url.href;
+
+    if (connectionId || connectionUrl) {
+      const connectionById = this.connections.find((c) => c.id === connectionId);
+      const connectionByUrl = this.connections.find((c) => c.id === connectionUrl);
 
-    if (connectionById || connectionByUrl) {
-      throw new Error(`Connection with id '${opts.id || opts.url.href}' is already present`);
+      if (connectionById || connectionByUrl) {
+        throw new ConfigurationError(
+          `Connection with id '${connectionId || connectionUrl}' is already present`
+        );
+      }
     }
 
     this.update([...this.connections, opts]);
@@ -133,10 +144,8 @@ class BaseConnectionPool {
 
   /**
    * Empties the connection pool.
-   *
-   * @returns {ConnectionPool}
    */
-  empty(callback) {
+  empty(callback = noop) {
     debug('Emptying the connection pool');
     let openConnections = this.size;
     this.connections.forEach((connection) => {

lib/pool/CloudConnectionPool.js

@@ -30,6 +30,7 @@
 'use strict';
 
 const BaseConnectionPool = require('./BaseConnectionPool');
+const noop = () => {};
 
 class CloudConnectionPool extends BaseConnectionPool {
   constructor(opts) {
@@ -49,9 +50,8 @@ class CloudConnectionPool extends BaseConnectionPool {
   /**
    * Empties the connection pool.
    *
-   * @returns {ConnectionPool}
    */
-  empty(callback) {
+  empty(callback = noop) {
     super.empty(() => {
       this.cloudConnection = null;
       callback();

lib/pool/ConnectionPool.js

@@ -227,7 +227,7 @@ class ConnectionPool extends BaseConnectionPool {
    *
    * @returns {ConnectionPool}
    */
-  empty(callback) {
+  empty(callback = noop) {
     super.empty(() => {
       this.dead = [];
       callback();

lib/pool/index.d.ts

@@ -31,7 +31,7 @@
 
 import { URL } from 'url';
 import { SecureContextOptions } from 'tls';
-import Connection, { AgentOptions } from '../Connection';
+import Connection, { AgentOptions, ConnectionOptions } from '../Connection';
 import { nodeFilterFn, nodeSelectorFn } from '../Transport';
 
 interface BaseConnectionPoolOptions {
@@ -121,7 +121,7 @@ declare class BaseConnectionPool {
    * @param {object|string} host
    * @returns {ConnectionPool}
    */
-  addConnection(opts: any): Connection;
+  addConnection(opts: string | ConnectionOptions | ConnectionOptions[]): Connection;
   /**
    * Removes a new connection to the pool.
    *
@@ -132,16 +132,15 @@ declare class BaseConnectionPool {
   /**
    * Empties the connection pool.
    *
-   * @returns {ConnectionPool}
    */
-  empty(): this;
+  empty(callback?: () => void): void;
   /**
    * Update the ConnectionPool with new connections.
    *
    * @param {array} array of connections
    * @returns {ConnectionPool}
    */
-  update(connections: any[]): this;
+  update(connections: Connection[]): this;
   /**
    * Transforms the nodes objects to a host object.
    *
@@ -155,7 +154,7 @@ declare class BaseConnectionPool {
    * @param {string} url
    * @returns {object} host
    */
-  urlToHost(url: string): { url: URL };
+  urlToHost(url: string): ConnectionOptions;
 }
 
 declare class ConnectionPool extends BaseConnectionPool {
@@ -183,12 +182,35 @@ declare class ConnectionPool extends BaseConnectionPool {
     opts: resurrectOptions,
     callback?: (isAlive: boolean | null, connection: Connection | null) => void
   ): void;
+
+  /**
+   * Empties the connection pool.
+   */
+  empty(callback?: () => void): void;
+  /**
+   * Update the ConnectionPool with new connections.
+   *
+   * @param {array} array of connections
+   * @returns {ConnectionPool}
+   */
+  update(connections: Connection[]): this;
 }
 
 declare class CloudConnectionPool extends BaseConnectionPool {
   cloudConnection: Connection | null;
   constructor(opts?: BaseConnectionPoolOptions);
   getConnection(): Connection | null;
+  /**
+   * Empties the connection pool.
+   */
+  empty(callback?: () => void): void;
+  /**
+   * Update the ConnectionPool with new connections.
+   *
+   * @param {array} array of connections
+   * @returns {ConnectionPool}
+   */
+  update(connections: Connection[]): this;
 }
 
 declare function defaultNodeFilter(node: Connection): boolean;

test/types/connection-pool.test-d.ts

@@ -30,6 +30,7 @@
 import { expectType, expectAssignable } from 'tsd';
 import { URL } from 'url';
 import { BaseConnectionPool, ConnectionPool, CloudConnectionPool, Connection } from '../../';
+import { ConnectionOptions } from '../../lib/Connection';
 
 {
   const pool = new BaseConnectionPool({
@@ -59,12 +60,12 @@ import { BaseConnectionPool, ConnectionPool, CloudConnectionPool, Connection } f
       now: Date.now(),
     })
   );
-  expectType<Connection>(pool.addConnection({}));
+  expectType<Connection>(pool.addConnection({ url: new URL('url') }));
   expectType<BaseConnectionPool>(pool.removeConnection(new Connection()));
-  expectType<BaseConnectionPool>(pool.empty());
+  expectType<void>(pool.empty());
   expectType<BaseConnectionPool>(pool.update([]));
   expectType<any[]>(pool.nodesToHost([], 'https'));
-  expectType<{ url: URL }>(pool.urlToHost('url'));
+  expectType<ConnectionOptions>(pool.urlToHost('url'));
 }
 
 {
@@ -99,12 +100,12 @@ import { BaseConnectionPool, ConnectionPool, CloudConnectionPool, Connection } f
       now: Date.now(),
     })
   );
-  expectType<Connection>(pool.addConnection({}));
+  expectType<Connection>(pool.addConnection({ url: new URL('url') }));
   expectAssignable<ConnectionPool>(pool.removeConnection(new Connection()));
-  expectAssignable<ConnectionPool>(pool.empty());
+  expectType<void>(pool.empty());
   expectAssignable<ConnectionPool>(pool.update([]));
   expectType<any[]>(pool.nodesToHost([], 'https'));
-  expectType<{ url: URL }>(pool.urlToHost('url'));
+  expectType<ConnectionOptions>(pool.urlToHost('url'));
   expectType<void>(
     pool.resurrect({
       now: Date.now(),

test/unit/base-connection-pool.test.js

@@ -70,6 +70,15 @@ test('API', (t) => {
     t.end();
   });
 
+  t.test('addConnection with only URL', (t) => {
+    const pool = new BaseConnectionPool({ Connection });
+    const href = 'http://localhost:9200/';
+    pool.addConnection({ url: new URL(href) });
+    t.ok(pool.connections.find((c) => c.id === href) instanceof Connection);
+    t.equal(pool.connections.find((c) => c.id === href).status, Connection.statuses.ALIVE);
+    t.end();
+  });
+
   t.test('markDead', (t) => {
     const pool = new BaseConnectionPool({ Connection, sniffEnabled: true });
     const href = 'http://localhost:9200/';
@@ -122,6 +131,31 @@ test('API', (t) => {
     });
   });
 
+  t.test('empty with no callback', (t) => {
+    const pool = new BaseConnectionPool({ Connection });
+    pool.addConnection('http://localhost:9200/');
+    pool.addConnection('http://localhost:9201/');
+    pool.empty();
+    t.equal(pool.size, 0);
+    t.end();
+  });
+
+  t.test('call empty twice', (t) => {
+    const pool = new BaseConnectionPool({ Connection });
+    pool.addConnection('http://localhost:9200/');
+    pool.addConnection('http://localhost:9201/');
+    try {
+      pool.empty();
+      pool.empty(() => {
+        t.equal(pool.size, 0);
+        t.pass();
+      });
+    } catch (error) {
+      t.fail('Should not throw');
+    }
+    t.end();
+  });
+
   t.test('urlToHost', (t) => {
     const pool = new BaseConnectionPool({ Connection });
     const url = 'http://localhost:9200';
@@ -581,5 +615,18 @@ test('API', (t) => {
     }
   });
 
+  t.test('Create Connection with a Connection instance', (t) => {
+    t.plan(1);
+    const pool = new BaseConnectionPool({ Connection });
+    const conn = pool.createConnection('http://localhost:9200');
+    pool.connections.push(conn);
+    try {
+      pool.createConnection(conn);
+      t.fail('Should throw');
+    } catch (err) {
+      t.pass();
+    }
+  });
+
   t.end();
 });

test/unit/cloud-connection-pool.test.js

@@ -56,3 +56,12 @@ test('pool.empty should reset cloudConnection', (t) => {
     t.end();
   });
 });
+
+test('empty with no callback', (t) => {
+  const pool = new CloudConnectionPool({ Connection });
+  pool.addConnection('http://localhost:9200/');
+  t.ok(pool.cloudConnection instanceof Connection);
+  pool.empty();
+  t.equal(pool.cloudConnection, null);
+  t.end();
+});

test/unit/connection-pool.test.js

@@ -325,6 +325,16 @@ test('API', (t) => {
     });
   });
 
+  t.test('empty with no callback', (t) => {
+    const pool = new ConnectionPool({ Connection });
+    pool.addConnection('http://localhost:9200/');
+    pool.addConnection('http://localhost:9201/');
+    pool.empty();
+    t.equal(pool.size, 0);
+    t.same(pool.dead, []);
+    t.end();
+  });
+
   t.test('urlToHost', (t) => {
     const pool = new ConnectionPool({ Connection });
     const url = 'http://localhost:9200';