Skip to content
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.

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

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

Merged
merged 28 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e2d2a2a
refactor: addConnection
timursaurus Mar 24, 2023
1478ac4
refactor: improve types
timursaurus Mar 24, 2023
974a81c
fix: connection type tests
timursaurus Mar 24, 2023
e5dbcea
feat: addConnection test with only URL provided
timursaurus Mar 24, 2023
7450723
refactor: optimization
timursaurus Mar 24, 2023
fc2b6f8
chore: use vars
timursaurus Mar 24, 2023
c045560
refactor: make callback optional
timursaurus Mar 24, 2023
e9676ed
chore: string type
timursaurus Mar 24, 2023
116c372
chore: update changelog
timursaurus Mar 24, 2023
68c1300
fix: update index.d.ts empty method args
timursaurus Mar 24, 2023
544e506
chore: user guide on empty method use
timursaurus Mar 24, 2023
dc1d6b9
fix: remove optional chaining
timursaurus Mar 24, 2023
2002850
refactor: remove else block
timursaurus Mar 24, 2023
8282711
fix: make cb arg optional
timursaurus Mar 24, 2023
cc57aec
fix: remove jsdoc return type
timursaurus Mar 24, 2023
caa82c6
chore: update types and tests
timursaurus Mar 24, 2023
5a82af5
chore: update changelog
timursaurus Mar 24, 2023
abfb2cb
refactor: swap to expecttype
timursaurus Mar 24, 2023
ed1178d
feat: connection empty tests
timursaurus Mar 24, 2023
39934c7
chore: remove log from user guide
timursaurus Mar 24, 2023
1b480e1
chore: format and update toc
timursaurus Mar 24, 2023
4b088a6
Merge branch 'main' into pool-cons
timursaurus Mar 27, 2023
16c454a
fix: remove passing connection instance to update
timursaurus Mar 27, 2023
94557df
chore: test for createConnection with a Connection instance
timursaurus Mar 27, 2023
c5de3d1
chore: remove jsdoc return
timursaurus Mar 27, 2023
9b38bdb
chore: update changelog
timursaurus Mar 27, 2023
1922e37
chore: revert changelog
timursaurus Mar 28, 2023
6c18ce1
feat: added call empty twice test
timursaurus Mar 28, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Make fields in `BulkOperation` optional to match OpenSearch Bulk API requirements ([#378](https://github.com/opensearch-project/opensearch-js/pull/378))
- 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))
- Make `callback` arg in `BaseConnectionPool`, `CloudConnectionPool` and `ConnectionPool` optional ([#451](https://github.com/opensearch-project/opensearch-js/pull/451))

### Deprecated

Expand Down
15 changes: 15 additions & 0 deletions USER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,4 +249,19 @@ console.log('Deleting all PITs:');
var response = await client.deleteAllPits();

console.log(response.body);
```

## Empty all pool connections
timursaurus marked this conversation as resolved.
Show resolved Hide resolved
```javascript
console.log('Emptying all pool connections:');
timursaurus marked this conversation as resolved.
Show resolved Hide resolved

var pool = new ConnectionPool({ Connection });
pool.addConnection('http://localhost:9200/');
pool.addConnection('http://localhost:9201/');

pool.empty()
// OR
pool.empty(() => {
console.log('All connections are empty');
})
```
27 changes: 17 additions & 10 deletions lib/pool/BaseConnectionPool.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -102,21 +103,29 @@ class BaseConnectionPool {
*/
addConnection(opts) {
if (Array.isArray(opts)) {
return opts.forEach((o) => this.addConnection(o));
opts.forEach((o) => this.addConnection(o));
nhtruong marked this conversation as resolved.
Show resolved Hide resolved
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]);
const connection = this.createConnection(opts);
this.update([...this.connections, connection]);
timursaurus marked this conversation as resolved.
Show resolved Hide resolved
return this.connections[this.size - 1];
}

Expand All @@ -133,10 +142,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) => {
Expand Down
4 changes: 2 additions & 2 deletions lib/pool/CloudConnectionPool.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
'use strict';

const BaseConnectionPool = require('./BaseConnectionPool');
const noop = () => {};

class CloudConnectionPool extends BaseConnectionPool {
constructor(opts) {
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion lib/pool/ConnectionPool.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ class ConnectionPool extends BaseConnectionPool {
*
* @returns {ConnectionPool}
*/
empty(callback) {
empty(callback = noop) {
super.empty(() => {
this.dead = [];
callback();
Expand Down
33 changes: 28 additions & 5 deletions lib/pool/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
*
Expand All @@ -134,14 +134,14 @@ declare class BaseConnectionPool {
*
* @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.
*
Expand All @@ -155,7 +155,7 @@ declare class BaseConnectionPool {
* @param {string} url
* @returns {object} host
*/
urlToHost(url: string): { url: URL };
urlToHost(url: string): ConnectionOptions;
}

declare class ConnectionPool extends BaseConnectionPool {
Expand Down Expand Up @@ -183,12 +183,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;
Expand Down
13 changes: 7 additions & 6 deletions test/types/connection-pool.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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'));
}

{
Expand Down Expand Up @@ -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(),
Expand Down
18 changes: 18 additions & 0 deletions test/unit/base-connection-pool.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/';
Expand Down Expand Up @@ -122,6 +131,15 @@ 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('urlToHost', (t) => {
const pool = new BaseConnectionPool({ Connection });
const url = 'http://localhost:9200';
Expand Down
9 changes: 9 additions & 0 deletions test/unit/cloud-connection-pool.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
10 changes: 10 additions & 0 deletions test/unit/connection-pool.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down