From e90257be2f03ab5e981926803fe847f5f375e8fc Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 20 Aug 2021 13:16:15 -0400 Subject: [PATCH] Edit Client constructor to take a server URI (#1544) * edit Client * fix TS issues + tests * minor edits * rename ClientBroadcast -> BroadcastClient --- snippets/src/getTransaction.ts | 4 +- snippets/src/parseAccountFlags.ts | 2 +- snippets/src/paths.ts | 10 ++-- snippets/src/reliableTransactionSubmission.ts | 2 +- src/{ => client}/broadcast.ts | 10 ++-- src/client/index.ts | 59 ++++++++++--------- src/index.ts | 2 +- test/broadcast-client-test.ts | 2 +- test/client/computeLedgerHash/index.ts | 3 - test/client/constructor/index.ts | 6 +- test/client/prepareSettings/index.ts | 1 - test/connection-test.ts | 2 +- test/integration/integration-test.ts | 2 +- test/ripple-client-test-private.ts | 8 +-- test/ripple-client-test.ts | 4 +- test/setup-client-web.ts | 8 +-- test/setup-client.ts | 6 +- 17 files changed, 64 insertions(+), 67 deletions(-) rename src/{ => client}/broadcast.ts (90%) diff --git a/snippets/src/getTransaction.ts b/snippets/src/getTransaction.ts index ea75f8e6..fa5314ec 100644 --- a/snippets/src/getTransaction.ts +++ b/snippets/src/getTransaction.ts @@ -1,8 +1,6 @@ import {Client} from '../../dist/npm' -const client = new Client({ - server: 'wss://s.altnet.rippletest.net:51233' -}) +const client = new Client('wss://s.altnet.rippletest.net:51233') getTransaction() diff --git a/snippets/src/parseAccountFlags.ts b/snippets/src/parseAccountFlags.ts index 52740aa0..49206fec 100644 --- a/snippets/src/parseAccountFlags.ts +++ b/snippets/src/parseAccountFlags.ts @@ -1,6 +1,6 @@ import {Client} from '../../dist/npm' -const client = new Client({server: 'wss://s.altnet.rippletest.net:51233'}) +const client = new Client('wss://s.altnet.rippletest.net:51233') parseAccountFlags() diff --git a/snippets/src/paths.ts b/snippets/src/paths.ts index c663ad28..4ad4acff 100644 --- a/snippets/src/paths.ts +++ b/snippets/src/paths.ts @@ -1,10 +1,10 @@ import {Client} from '../../dist/npm' -const client = new Client({ - // server: 'wss://s.altnet.rippletest.net:51233' - // server: 'ws://35.158.96.209:51233' - server: 'ws://34.210.87.206:51233' -}) +const client = new Client( + // 'wss://s.altnet.rippletest.net:51233' + // 'ws://35.158.96.209:51233' + 'ws://34.210.87.206:51233' +) sign() diff --git a/snippets/src/reliableTransactionSubmission.ts b/snippets/src/reliableTransactionSubmission.ts index f42bd459..d6bd3300 100644 --- a/snippets/src/reliableTransactionSubmission.ts +++ b/snippets/src/reliableTransactionSubmission.ts @@ -68,7 +68,7 @@ async function reliableTransactionSubmissionExample() { async function performPayments(payments) { const finalResults = [] const txFinalizedPromises = [] - const client = new Client({server: 'wss://s.altnet.rippletest.net:51233'}) + const client = new Client('wss://s.altnet.rippletest.net:51233') await client.connect() for (let i = 0; i < payments.length; i++) { diff --git a/src/broadcast.ts b/src/client/broadcast.ts similarity index 90% rename from src/broadcast.ts rename to src/client/broadcast.ts index a319490a..6cea231c 100644 --- a/src/broadcast.ts +++ b/src/client/broadcast.ts @@ -1,14 +1,14 @@ -import {Client, ClientOptions} from './client' +import {Client, ClientOptions} from './' -class ClientBroadcast extends Client { +class BroadcastClient extends Client { ledgerVersion: number | undefined = undefined private _clients: Client[] constructor(servers, options: ClientOptions = {}) { - super(options) + super(servers[0], options) const clients: Client[] = servers.map( - (server) => new Client(Object.assign({}, options, {server})) + (server) => new Client(server, options) ) // exposed for testing @@ -69,4 +69,4 @@ class ClientBroadcast extends Client { } } -export {ClientBroadcast} +export {BroadcastClient} diff --git a/src/client/index.ts b/src/client/index.ts index 11aaa6db..5575c46e 100644 --- a/src/client/index.ts +++ b/src/client/index.ts @@ -159,9 +159,9 @@ import { computePaymentChannelHash } from '../common/hashes' import generateFaucetWallet from '../wallet/wallet-generation' +import { ValidationError } from '../common/errors' export interface ClientOptions extends ConnectionUserOptions { - server?: string feeCushion?: number maxFeeXRP?: string proxy?: string @@ -223,37 +223,38 @@ class Client extends EventEmitter { schemaValidator } - constructor(options: ClientOptions = {}) { + constructor(server: string, options: ClientOptions = {}) { super() - validate.apiOptions(options) + if (typeof server !== 'string' || !server.match("^(wss?|wss?\\+unix)://")) { + throw new ValidationError("server URI must start with `wss://`, `ws://`, `wss+unix://`, or `ws+unix://`.") + } + this._feeCushion = options.feeCushion || 1.2 this._maxFeeXRP = options.maxFeeXRP || '2' - const serverURL = options.server - if (serverURL != null) { - this.connection = new Connection(serverURL, options) - this.connection.on('ledgerClosed', (message: LedgerStream) => { - this.emit('ledger', formatLedgerClose(message)) - }) - this.connection.on('error', (errorCode, errorMessage, data) => { - this.emit('error', errorCode, errorMessage, data) - }) - this.connection.on('connected', () => { - this.emit('connected') - }) - this.connection.on('disconnected', (code) => { - let finalCode = code - // 4000: Connection uses a 4000 code internally to indicate a manual disconnect/close - // Since 4000 is a normal disconnect reason, we convert this to the standard exit code 1000 - if (finalCode === 4000) { - finalCode = 1000 - } - this.emit('disconnected', finalCode) - }) - } else { - // use null object pattern to provide better error message if user - // tries to call a method that requires a connection - this.connection = new Connection(null, options) - } + + this.connection = new Connection(server, options) + + this.connection.on('ledgerClosed', (message: LedgerStream) => { + this.emit('ledger', formatLedgerClose(message)) + }) + + this.connection.on('error', (errorCode, errorMessage, data) => { + this.emit('error', errorCode, errorMessage, data) + }) + + this.connection.on('connected', () => { + this.emit('connected') + }) + + this.connection.on('disconnected', (code) => { + let finalCode = code + // 4000: Connection uses a 4000 code internally to indicate a manual disconnect/close + // Since 4000 is a normal disconnect reason, we convert this to the standard exit code 1000 + if (finalCode === 4000) { + finalCode = 1000 + } + this.emit('disconnected', finalCode) + }) } /** diff --git a/src/index.ts b/src/index.ts index e2aa66d8..eec2303a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,6 @@ export * from './models/methods' export * from './offline/utils' // Broadcast client is experimental -export {ClientBroadcast} from './broadcast' +export {BroadcastClient} from './client/broadcast' export * from './Wallet' diff --git a/test/broadcast-client-test.ts b/test/broadcast-client-test.ts index 83738dd9..fee79de0 100644 --- a/test/broadcast-client-test.ts +++ b/test/broadcast-client-test.ts @@ -16,7 +16,7 @@ function checkResult(expected, response) { return response } -describe('ClientBroadcast', function () { +describe('BroadcastClient', function () { this.timeout(TIMEOUT) beforeEach(setupClient.setupBroadcast) afterEach(setupClient.teardown) diff --git a/test/client/computeLedgerHash/index.ts b/test/client/computeLedgerHash/index.ts index 73548b5f..70376c61 100644 --- a/test/client/computeLedgerHash/index.ts +++ b/test/client/computeLedgerHash/index.ts @@ -156,7 +156,6 @@ export default { }, 'computeLedgerHash': async (client, address) => { - // const client = new Client() const header = REQUEST_FIXTURES.header const ledgerHash = client.computeLedgerHash(header) assert.strictEqual( @@ -166,7 +165,6 @@ export default { }, 'computeLedgerHash - with transactions': async (client, address) => { - // const client = new Client() const header = { ...REQUEST_FIXTURES.header, transactionHash: undefined, @@ -180,7 +178,6 @@ export default { }, 'computeLedgerHash - incorrent transaction_hash': async (client, address) => { - // const client = new Client() const header = Object.assign({}, REQUEST_FIXTURES.header, { transactionHash: '325EACC5271322539EEEC2D6A5292471EF1B3E72AE7180533EFC3B8F0AD435C9' diff --git a/test/client/constructor/index.ts b/test/client/constructor/index.ts index 72cf4c15..ac30cebb 100644 --- a/test/client/constructor/index.ts +++ b/test/client/constructor/index.ts @@ -9,7 +9,7 @@ import {Client} from 'xrpl-local' */ export default { 'Client - implicit server port': () => { - new Client({server: 'wss://s1.ripple.com'}) + new Client('wss://s1.ripple.com') }, 'Client invalid options': () => { @@ -18,12 +18,12 @@ export default { }, 'Client valid options': () => { - const client = new Client({server: 'wss://s:1'}) + const client = new Client('wss://s:1') const privateConnectionUrl = (client.connection as any)._url assert.deepEqual(privateConnectionUrl, 'wss://s:1') }, 'Client invalid server uri': () => { - assert.throws(() => new Client({server: 'wss//s:1'})) + assert.throws(() => new Client('wss//s:1')) } } diff --git a/test/client/prepareSettings/index.ts b/test/client/prepareSettings/index.ts index 6299a555..5e8a3d1f 100644 --- a/test/client/prepareSettings/index.ts +++ b/test/client/prepareSettings/index.ts @@ -236,7 +236,6 @@ export default { } }, 'offline': async (client, address) => { - // const client = new Client() const secret = 'shsWGZcmZz6YsWWmcnpfr6fLTdtFV' const settings = requests.prepareSettings.domain diff --git a/test/connection-test.ts b/test/connection-test.ts index 97c28b79..93dd716c 100644 --- a/test/connection-test.ts +++ b/test/connection-test.ts @@ -578,7 +578,7 @@ describe('Connection', function () { data: {returnEmptySubscribeRequest: 1} }) - const client = new Client({server: this.client.connection._url}) + const client = new Client(this.client.connection._url) return client.connect().then( () => { assert(false, 'Must have thrown!') diff --git a/test/integration/integration-test.ts b/test/integration/integration-test.ts index 9e9cabf4..50453957 100644 --- a/test/integration/integration-test.ts +++ b/test/integration/integration-test.ts @@ -106,7 +106,7 @@ function testTransaction( } function setup(this: any, server = serverUrl) { - this.client = new Client({server}) + this.client = new Client(server) console.log('CONNECTING...') return this.client.connect().then( () => { diff --git a/test/ripple-client-test-private.ts b/test/ripple-client-test-private.ts index adf28087..e0444b06 100644 --- a/test/ripple-client-test-private.ts +++ b/test/ripple-client-test-private.ts @@ -21,7 +21,7 @@ describe('Client', function () { afterEach(setupClient.teardown) it('Client - implicit server port', function () { - new Client({server: 'wss://s1.ripple.com'}) + new Client('wss://s1.ripple.com') }) it('Client invalid options', function () { @@ -30,16 +30,16 @@ describe('Client', function () { }) it('Client valid options', function () { - const client = new Client({server: 'wss://s:1'}) + const client = new Client('wss://s:1') const privateConnectionUrl = (client.connection as any)._url assert.deepEqual(privateConnectionUrl, 'wss://s:1') }) it('Client invalid server uri', function () { - assert.throws(() => new Client({server: 'wss//s:1'})) + assert.throws(() => new Client('wss//s:1')) }) - xit('Client connect() times out after 2 seconds', function () { + it('Client connect() times out after 2 seconds', function () { // TODO: Use a timer mock like https://jestjs.io/docs/en/timer-mocks // to test that connect() times out after 2 seconds. }) diff --git a/test/ripple-client-test.ts b/test/ripple-client-test.ts index c0042a66..522cdf42 100644 --- a/test/ripple-client-test.ts +++ b/test/ripple-client-test.ts @@ -26,7 +26,9 @@ describe('Client [Test Runner]', function () { afterEach(setupClient.teardown) // Collect all the tests: - const allPublicMethods = getAllPublicMethods(new Client()) + const allPublicMethods = getAllPublicMethods(new Client("wss://")) + // doesn't need the client, just needs to instantiate to get public methods + const allTestSuites = loadTestSuites() // Run all the tests: diff --git a/test/setup-client-web.ts b/test/setup-client-web.ts index 0c91f34b..54f593b7 100644 --- a/test/setup-client-web.ts +++ b/test/setup-client-web.ts @@ -1,11 +1,11 @@ -import {Client, ClientBroadcast} from 'xrpl-local' +import {Client, BroadcastClient} from 'xrpl-local' import ledgerClosed from './fixtures/rippled/ledger-close.json' const port = 34371 const baseUrl = 'ws://testripple.circleci.com:' function setup(this: any, port_ = port) { - const tclient = new Client({server: baseUrl + port_}) + const tclient = new Client(baseUrl + port_) return tclient .connect() .then(() => { @@ -19,7 +19,7 @@ function setup(this: any, port_ = port) { .then((got) => { return new Promise((resolve, reject) => { // @ts-ignore - this.client = new Client({server: baseUrl + got.port}) + this.client = new Client(baseUrl + got.port) this.client .connect() .then(() => { @@ -39,7 +39,7 @@ function setup(this: any, port_ = port) { function setupBroadcast(this: any) { const servers = [port, port + 1].map((port_) => baseUrl + port_) - this.client = new ClientBroadcast(servers) + this.client = new BroadcastClient(servers) return new Promise((resolve, reject) => { this.client .connect() diff --git a/test/setup-client.ts b/test/setup-client.ts index c53b4c99..304b261b 100644 --- a/test/setup-client.ts +++ b/test/setup-client.ts @@ -1,4 +1,4 @@ -import {Client, ClientBroadcast} from 'xrpl-local' +import {Client, BroadcastClient} from 'xrpl-local' import ledgerClosed from './fixtures/rippled/ledger-close.json' import {createMockRippled} from './mock-rippled' import {getFreePort} from './utils' @@ -7,7 +7,7 @@ function setupMockRippledConnection(testcase, port) { return new Promise((resolve, reject) => { testcase.mockRippled = createMockRippled(port) testcase._mockedServerPort = port - testcase.client = new Client({server: 'ws://localhost:' + port}) + testcase.client = new Client('ws://localhost:' + port) testcase.client .connect() .then(() => { @@ -25,7 +25,7 @@ function setupMockRippledConnectionForBroadcast(testcase, ports) { return new Promise((resolve, reject) => { const servers = ports.map((port) => 'ws://localhost:' + port) testcase.mocks = ports.map((port) => createMockRippled(port)) - testcase.client = new ClientBroadcast(servers) + testcase.client = new BroadcastClient(servers) testcase.client .connect() .then(() => {