From 12cfed5c170eab1e400241454a60bf874abcb504 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 26 Aug 2021 14:42:26 -0400 Subject: [PATCH] refactor: clean up Client and associated files (#1556) * remove _PRIVATE * make requestAll public * un-type connection.request * fix lodash imports * add comments * Rename files to camelCase --- package.json | 2 +- .../{broadcast.ts => broadcastClient.ts} | 0 src/client/connection.ts | 10 +++--- src/client/index.ts | 33 ++++++++----------- src/client/{rangeset.ts => rangeSet.ts} | 0 src/client/{wswrapper.ts => wsWrapper.ts} | 0 src/common/schema-validator.ts | 2 +- src/common/validate.ts | 2 +- src/index.ts | 2 +- src/ledger/orderbook.ts | 4 +-- src/ledger/parse/fields.ts | 2 +- src/ledger/parse/ledger.ts | 2 +- src/ledger/parse/orderbook-order.ts | 2 +- src/ledger/parse/pathfind.ts | 2 +- src/ledger/parse/settings.ts | 2 +- src/ledger/pathfind.ts | 2 +- src/ledger/trustlines.ts | 4 +-- src/ledger/utils.ts | 2 +- src/transaction/combine.ts | 2 +- src/transaction/payment.ts | 2 +- src/transaction/ticket.ts | 2 +- src/utils/index.ts | 12 +++---- test/client/preparePayment.ts | 3 +- test/client/prepareTicket.ts | 2 +- test/client/sign.ts | 3 +- test/connection.ts | 18 +++++----- test/rangeset.ts | 3 +- test/rippleClientPrivate.ts | 20 +++++++---- test/wallet/signTransaction.ts | 3 +- webpack.config.js | 2 +- 30 files changed, 70 insertions(+), 75 deletions(-) rename src/client/{broadcast.ts => broadcastClient.ts} (100%) rename src/client/{rangeset.ts => rangeSet.ts} (100%) rename src/client/{wswrapper.ts => wsWrapper.ts} (100%) diff --git a/package.json b/package.json index f07f89c1..76e1da41 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "jsdelivr": "build/ripple-latest-min.js", "types": "dist/npm/index.d.ts", "browser": { - "ws": "./dist/npm/client/wswrapper.js", + "ws": "./dist/npm/client/wsWrapper.js", "https-proxy-agent": false }, "directories": { diff --git a/src/client/broadcast.ts b/src/client/broadcastClient.ts similarity index 100% rename from src/client/broadcast.ts rename to src/client/broadcastClient.ts diff --git a/src/client/connection.ts b/src/client/connection.ts index e66c0acf..0d7a4b13 100644 --- a/src/client/connection.ts +++ b/src/client/connection.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import {EventEmitter} from 'events' import {parse as parseURL} from 'url' import WebSocket from 'ws' @@ -12,7 +12,7 @@ import { RippleError } from '../common/errors' import {ExponentialBackoff} from './backoff' -import { Request, Response } from '../models/methods' +import { Response } from '../models/methods' /** * ConnectionOptions is the configuration for the Connection class. @@ -186,7 +186,7 @@ class RequestManager { * hung responses, and a promise that will resolve with the response once * the response is seen & handled. */ - createRequest(data: Request, timeout: number): [string | number, string, Promise] { + createRequest(data: any, timeout: number): [string | number, string, Promise] { const newId = data.id ? data.id : this.nextId++ const newData = JSON.stringify({...data, id: newId}) const timer = setTimeout( @@ -473,7 +473,7 @@ export class Connection extends EventEmitter { await this.connect() } - async request(request: T, timeout?: number): Promise { + async request(request: T, timeout?: number): Promise { if (!this._shouldBeConnected) { throw new NotConnectedError() } @@ -486,7 +486,7 @@ export class Connection extends EventEmitter { this._requestManager.reject(id, error) }) - return responsePromise as any + return responsePromise } /** diff --git a/src/client/index.ts b/src/client/index.ts index bc6afcb8..ffdcc0e6 100644 --- a/src/client/index.ts +++ b/src/client/index.ts @@ -2,7 +2,6 @@ import {EventEmitter} from 'events' import { constants, errors, - validate, txFlags, } from '../common' import { Connection, ConnectionUserOptions } from './connection' @@ -98,8 +97,6 @@ import { RandomResponse } from '../models/methods' -import RangeSet from './rangeset' -import * as ledgerUtils from '../ledger/utils' import * as transactionUtils from '../transaction/utils' import * as schemaValidator from '../common/schema-validator' import {getFee} from '../common/fee' @@ -172,21 +169,17 @@ type MarkerResponse = AccountChannelsResponse | LedgerDataResponse class Client extends EventEmitter { + // Factor to multiply estimated fee by to provide a cushion in case the + // required fee rises during submission of a transaction. Defaults to 1.2. _feeCushion: number + // Maximum fee to use with transactions, in XRP. Must be a string-encoded + // number. Defaults to '2'. _maxFeeXRP: string // New in > 0.21.0 // non-validated ledger versions are allowed, and passed to rippled as-is. connection: Connection - // these are exposed only for use by unit tests; they are not part of the client. - static _PRIVATE = { - validate, - RangeSet, - ledgerUtils, - schemaValidator - } - constructor(server: string, options: ClientOptions = {}) { super() if (typeof server !== 'string' || !server.match("^(wss?|wss?\\+unix)://")) { @@ -312,7 +305,7 @@ class Client extends EventEmitter { /** * Makes multiple paged requests to the client to return a given number of - * resources. _requestAll() will make multiple requests until the `limit` + * resources. requestAll() will make multiple requests until the `limit` * number of resources is reached (if no `limit` is provided, a single request * will be made). * @@ -323,14 +316,14 @@ class Client extends EventEmitter { * general use. Instead, use rippled's built-in pagination and make multiple * requests as needed. */ - async _requestAll(req: AccountChannelsRequest): Promise - async _requestAll(req: AccountLinesRequest): Promise - async _requestAll(req: AccountObjectsRequest): Promise - async _requestAll(req: AccountOffersRequest): Promise - async _requestAll(req: AccountTxRequest): Promise - async _requestAll(req: BookOffersRequest): Promise - async _requestAll(req: LedgerDataRequest): Promise - async _requestAll(request: T, options: {collect?: string} = {}): Promise { + async requestAll(req: AccountChannelsRequest): Promise + async requestAll(req: AccountLinesRequest): Promise + async requestAll(req: AccountObjectsRequest): Promise + async requestAll(req: AccountOffersRequest): Promise + async requestAll(req: AccountTxRequest): Promise + async requestAll(req: BookOffersRequest): Promise + async requestAll(req: LedgerDataRequest): Promise + async requestAll(request: T, options: {collect?: string} = {}): Promise { // The data under collection is keyed based on the command. Fail if command // not recognized and collection key not provided. const collectKey = options.collect || getCollectKeyFromCommand(request.command) diff --git a/src/client/rangeset.ts b/src/client/rangeSet.ts similarity index 100% rename from src/client/rangeset.ts rename to src/client/rangeSet.ts diff --git a/src/client/wswrapper.ts b/src/client/wsWrapper.ts similarity index 100% rename from src/client/wswrapper.ts rename to src/client/wsWrapper.ts diff --git a/src/common/schema-validator.ts b/src/common/schema-validator.ts index 2c6f8211..23ab4f84 100644 --- a/src/common/schema-validator.ts +++ b/src/common/schema-validator.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import * as assert from 'assert' const {Validator} = require('jsonschema') import {ValidationError} from './errors' diff --git a/src/common/validate.ts b/src/common/validate.ts index e179f7a1..145e4d65 100644 --- a/src/common/validate.ts +++ b/src/common/validate.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import {ValidationError} from './errors' import {schemaValidate} from './schema-validator' diff --git a/src/index.ts b/src/index.ts index 6c030ef3..a5e87fa7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,6 @@ export * from './models/methods' export * from './utils' // Broadcast client is experimental -export {BroadcastClient} from './client/broadcast' +export {BroadcastClient} from './client/broadcastClient' export * from './Wallet' diff --git a/src/ledger/orderbook.ts b/src/ledger/orderbook.ts index 850ba7b6..8bde6fe5 100644 --- a/src/ledger/orderbook.ts +++ b/src/ledger/orderbook.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import * as utils from './utils' import { parseOrderbookOrder, @@ -81,7 +81,7 @@ async function makeRequest( taker_gets: takerGets, taker_pays: takerPays }) - return client._requestAll({command: 'book_offers', + return client.requestAll({command: 'book_offers', taker_gets: orderData.taker_gets, taker_pays: orderData.taker_pays, ledger_index: options.ledgerVersion || 'validated', diff --git a/src/ledger/parse/fields.ts b/src/ledger/parse/fields.ts index 80bc1070..05ec3130 100644 --- a/src/ledger/parse/fields.ts +++ b/src/ledger/parse/fields.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import BigNumber from 'bignumber.js' import {constants} from '../../common' const AccountFields = constants.AccountFields diff --git a/src/ledger/parse/ledger.ts b/src/ledger/parse/ledger.ts index c667c264..298f06d9 100644 --- a/src/ledger/parse/ledger.ts +++ b/src/ledger/parse/ledger.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import {removeUndefined, rippleTimeToISOTime} from '../../utils' import parseTransaction from './transaction' import { TransactionAndMetadata } from '../../models/transactions' diff --git a/src/ledger/parse/orderbook-order.ts b/src/ledger/parse/orderbook-order.ts index 7b5b4f42..9f0b5738 100644 --- a/src/ledger/parse/orderbook-order.ts +++ b/src/ledger/parse/orderbook-order.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import {parseTimestamp, adjustQualityForXRP} from './utils' import {removeUndefined} from '../../utils' diff --git a/src/ledger/parse/pathfind.ts b/src/ledger/parse/pathfind.ts index ea1f4dd4..0320389a 100644 --- a/src/ledger/parse/pathfind.ts +++ b/src/ledger/parse/pathfind.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import parseAmount from './amount' import {Amount, RippledAmount} from '../../common/types/objects' import {Path, GetPaths, RippledPathsResponse} from '../pathfind-types' diff --git a/src/ledger/parse/settings.ts b/src/ledger/parse/settings.ts index 564ff6ba..6cba47f7 100644 --- a/src/ledger/parse/settings.ts +++ b/src/ledger/parse/settings.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import * as assert from 'assert' import {constants} from '../../common' const AccountFlags = constants.AccountFlags diff --git a/src/ledger/pathfind.ts b/src/ledger/pathfind.ts index 4a9084ce..ba5dfbfa 100644 --- a/src/ledger/pathfind.ts +++ b/src/ledger/pathfind.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import BigNumber from 'bignumber.js' import {getXRPBalance, renameCounterpartyToIssuer} from './utils' import { diff --git a/src/ledger/trustlines.ts b/src/ledger/trustlines.ts index b1a22e00..e3ec5888 100644 --- a/src/ledger/trustlines.ts +++ b/src/ledger/trustlines.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import {validate, ensureClassicAddress} from '../common' import parseAccountTrustline from './parse/account-trustline' import {Client} from '..' @@ -29,7 +29,7 @@ async function getTrustlines( address = ensureClassicAddress(address) // 2. Make Request - const responses = await this._requestAll({command: 'account_lines', + const responses = await this.requestAll({command: 'account_lines', account: address, ledger_index: options.ledgerVersion ?? 'validated', limit: options.limit, diff --git a/src/ledger/utils.ts b/src/ledger/utils.ts index 49d872c9..52b48e08 100644 --- a/src/ledger/utils.ts +++ b/src/ledger/utils.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import * as assert from 'assert' import * as common from '../common' import {Connection} from '../client' diff --git a/src/transaction/combine.ts b/src/transaction/combine.ts index bb19fe68..e0a5d0c3 100644 --- a/src/transaction/combine.ts +++ b/src/transaction/combine.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import binary from 'ripple-binary-codec' import BigNumber from 'bignumber.js' import {ValidationError} from '../common/errors' diff --git a/src/transaction/payment.ts b/src/transaction/payment.ts index cd8aca6a..bd87490a 100644 --- a/src/transaction/payment.ts +++ b/src/transaction/payment.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import * as utils from './utils' const validate = utils.common.validate const paymentFlags = utils.common.txFlags.Payment diff --git a/src/transaction/ticket.ts b/src/transaction/ticket.ts index 89e18baf..127ef359 100644 --- a/src/transaction/ticket.ts +++ b/src/transaction/ticket.ts @@ -1,4 +1,4 @@ -import * as _ from 'lodash' +import _ from 'lodash' import * as utils from './utils' import {Prepare, TransactionJSON, Instructions} from './types' import {Client} from '..' diff --git a/src/utils/index.ts b/src/utils/index.ts index 7dc3dd2a..69a23e35 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,3 +1,9 @@ +import _ from 'lodash' +import BigNumber from 'bignumber.js' +import {RippledAmount} from '../common/types/objects' +import {ValidationError} from '../common/errors' +import {xAddressToClassicAddress} from 'ripple-address-codec' + import { deriveKeypair, deriveAddress, deriveXAddress } from './derive' import computeLedgerHeaderHash from './ledgerHash' import signPaymentChannelClaim from './signPaymentChannelClaim' @@ -18,12 +24,6 @@ import { } from './hashes' import { generateXAddress } from './generateAddress' -import _ from 'lodash' -import BigNumber from 'bignumber.js' -import {RippledAmount} from '../common/types/objects' -import {ValidationError} from '../common/errors' -import {xAddressToClassicAddress} from 'ripple-address-codec' - function isValidSecret(secret: string): boolean { try { deriveKeypair(secret) diff --git a/test/client/preparePayment.ts b/test/client/preparePayment.ts index ddc68ba0..12194ae8 100644 --- a/test/client/preparePayment.ts +++ b/test/client/preparePayment.ts @@ -5,9 +5,8 @@ import requests from '../fixtures/requests' import {ValidationError} from 'xrpl-local/common/errors' import binary from 'ripple-binary-codec' import assert from 'assert-diff' -import {Client} from 'xrpl-local' +import * as schemaValidator from 'xrpl-local/common/schema-validator' -const {schemaValidator} = Client._PRIVATE const instructionsWithMaxLedgerVersionOffset = {maxLedgerVersionOffset: 100} const {preparePayment: REQUEST_FIXTURES} = requests const {preparePayment: RESPONSE_FIXTURES} = responses diff --git a/test/client/prepareTicket.ts b/test/client/prepareTicket.ts index 3f98cf77..58a4843e 100644 --- a/test/client/prepareTicket.ts +++ b/test/client/prepareTicket.ts @@ -6,8 +6,8 @@ import rippled from '../fixtures/rippled' // import binary from 'ripple-binary-codec' // import assert from 'assert-diff' // import {Client} from 'xrpl-local' +// import * as schemaValidator from 'xrpl-local/common/schema-validator' -// const {schemaValidator} = Client._PRIVATE // const instructionsWithMaxLedgerVersionOffset = {maxLedgerVersionOffset: 100} // const {preparePayment: REQUEST_FIXTURES} = requests // const {preparePayment: RESPONSE_FIXTURES} = responses diff --git a/test/client/sign.ts b/test/client/sign.ts index a70b5a2a..ee8f6cef 100644 --- a/test/client/sign.ts +++ b/test/client/sign.ts @@ -1,12 +1,11 @@ import assert from 'assert-diff' -import {Client} from 'xrpl-local' import binary from 'ripple-binary-codec' import requests from '../fixtures/requests' import responses from '../fixtures/responses' import rippled from '../fixtures/rippled' import {TestSuite} from '../testUtils' +import * as schemaValidator from 'xrpl-local/common/schema-validator' -const {schemaValidator} = Client._PRIVATE const {sign: REQUEST_FIXTURES} = requests const {sign: RESPONSE_FIXTURES} = responses diff --git a/test/connection.ts b/test/connection.ts index 9907e5b1..0ee5e8d4 100644 --- a/test/connection.ts +++ b/test/connection.ts @@ -4,8 +4,8 @@ import assert from 'assert-diff' import setupClient from './setupClient' import {Client} from 'xrpl-local' import {ignoreWebSocketDisconnect} from './testUtils' +import {Connection} from 'xrpl-local/client' import rippled from './fixtures/rippled' -const utils = Client._PRIVATE.ledgerUtils const TIMEOUT = 200000 // how long before each test case times out const isBrowser = (process as any).browser @@ -29,7 +29,7 @@ describe('Connection', function () { afterEach(setupClient.teardown) it('default options', function () { - const connection: any = new utils.Connection('url') + const connection: any = new Connection('url') assert.strictEqual(connection._url, 'url') assert(connection._config.proxy == null) assert(connection._config.authorization == null) @@ -52,7 +52,7 @@ describe('Connection', function () { it('as false', function () { const messages = [] console.log = (id, message) => messages.push([id, message]) - const connection: any = new utils.Connection('url', {trace: false}) + const connection: any = new Connection('url', {trace: false}) connection._ws = {send: function () {}} connection.request(mockedRequestData) connection._onMessage(mockedResponse) @@ -62,7 +62,7 @@ describe('Connection', function () { it('as true', function () { const messages = [] console.log = (id, message) => messages.push([id, message]) - const connection: any = new utils.Connection('url', {trace: true}) + const connection: any = new Connection('url', {trace: true}) connection._ws = {send: function () {}} connection.request(mockedRequestData) connection._onMessage(mockedResponse) @@ -71,7 +71,7 @@ describe('Connection', function () { it('as a function', function () { const messages = [] - const connection: any = new utils.Connection('url', { + const connection: any = new Connection('url', { trace: (id, message) => messages.push([id, message]) }) connection._ws = {send: function () {}} @@ -104,7 +104,7 @@ describe('Connection', function () { authorization: 'authorization', trustedCertificates: ['path/to/pem'] } - const connection = new utils.Connection( + const connection = new Connection( this.client.connection._url, options ) @@ -124,7 +124,7 @@ describe('Connection', function () { }) it('NotConnectedError', function () { - const connection = new utils.Connection('url') + const connection = new Connection('url') return connection.request({ command: 'ledger', ledger_index: 'validated' @@ -148,7 +148,7 @@ describe('Connection', function () { } // Address where no one listens - const connection = new utils.Connection( + const connection = new Connection( 'ws://testripple.circleci.com:129' ) connection.on('error', done) @@ -411,7 +411,7 @@ describe('Connection', function () { }) it('Cannot connect because no server', function () { - const connection = new utils.Connection(undefined as string) + const connection = new Connection(undefined as string) return connection .connect() .then(() => { diff --git a/test/rangeset.ts b/test/rangeset.ts index 9df4e9d0..fae13571 100644 --- a/test/rangeset.ts +++ b/test/rangeset.ts @@ -1,6 +1,5 @@ import assert from 'assert' -import {Client} from 'xrpl-local' -const RangeSet = Client._PRIVATE.RangeSet +import RangeSet from 'xrpl-local/client/rangeSet' describe('RangeSet', function () { it('addRange()/addValue()', function () { diff --git a/test/rippleClientPrivate.ts b/test/rippleClientPrivate.ts index b69d1e0f..e6a3e959 100644 --- a/test/rippleClientPrivate.ts +++ b/test/rippleClientPrivate.ts @@ -6,8 +6,14 @@ import {assertRejects} from './testUtils' import addresses from './fixtures/addresses.json' import setupClient from './setupClient' import {toRippledAmount} from '../src' +import * as schemaValidator from 'xrpl-local/common/schema-validator' +import {validate} from 'xrpl-local/common' +import { + renameCounterpartyToIssuerInOrder, + compareTransactions, + getRecursive +} from 'xrpl-local/ledger/utils' -const {validate, schemaValidator, ledgerUtils} = Client._PRIVATE const address = addresses.ACCOUNT assert.options.strict = true @@ -141,23 +147,23 @@ describe('Client', function () { taker_pays: {issuer: '1', currency: 'XRP'} } assert.deepEqual( - ledgerUtils.renameCounterpartyToIssuerInOrder(order), + renameCounterpartyToIssuerInOrder(order), expected ) }) it('ledger utils - compareTransactions', async () => { // @ts-ignore - assert.strictEqual(ledgerUtils.compareTransactions({}, {}), 0) + assert.strictEqual(compareTransactions({}, {}), 0) let first: any = {outcome: {ledgerVersion: 1, indexInLedger: 100}} let second: any = {outcome: {ledgerVersion: 1, indexInLedger: 200}} - assert.strictEqual(ledgerUtils.compareTransactions(first, second), -1) + assert.strictEqual(compareTransactions(first, second), -1) first = {outcome: {ledgerVersion: 1, indexInLedger: 100}} second = {outcome: {ledgerVersion: 1, indexInLedger: 100}} - assert.strictEqual(ledgerUtils.compareTransactions(first, second), 0) + assert.strictEqual(compareTransactions(first, second), 0) first = {outcome: {ledgerVersion: 1, indexInLedger: 200}} second = {outcome: {ledgerVersion: 1, indexInLedger: 100}} - assert.strictEqual(ledgerUtils.compareTransactions(first, second), 1) + assert.strictEqual(compareTransactions(first, second), 1) }) it('ledger utils - getRecursive', async () => { @@ -170,6 +176,6 @@ describe('Client', function () { resolve({marker: 'A', results: [1]}) }) } - await assertRejects(ledgerUtils.getRecursive(getter, 10), Error) + await assertRejects(getRecursive(getter, 10), Error) }) }) diff --git a/test/wallet/signTransaction.ts b/test/wallet/signTransaction.ts index 4fb4e5fd..19cef892 100644 --- a/test/wallet/signTransaction.ts +++ b/test/wallet/signTransaction.ts @@ -1,8 +1,7 @@ -import {Client} from 'xrpl-local' import {TestSuite} from '../testUtils' import Wallet from '../../src/Wallet' +import * as schemaValidator from 'xrpl-local/common/schema-validator' -const {schemaValidator} = Client._PRIVATE const publicKey = '030E58CDD076E798C84755590AAF6237CA8FAE821070A59F648B517A30DC6F589D' const privateKey = diff --git a/webpack.config.js b/webpack.config.js index 5ba3c50e..0b5215b0 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -16,7 +16,7 @@ function getDefaultConfiguration() { filename: `ripple-lib.default.js`, }, plugins: [ - new webpack.NormalModuleReplacementPlugin(/^ws$/, './wswrapper'), + new webpack.NormalModuleReplacementPlugin(/^ws$/, './wsWrapper'), new webpack.NormalModuleReplacementPlugin(/^\.\/wallet$/, './wallet-web'), new webpack.NormalModuleReplacementPlugin(/^.*setup-api$/, './setup-api-web'), new webpack.ProvidePlugin({ process: 'process/browser' }),