Edit Client constructor to take a server URI (#1544)

* edit Client

* fix TS issues + tests

* minor edits

* rename ClientBroadcast -> BroadcastClient

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()
 

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()
 

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()
 

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++) {

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}

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,23 +223,29 @@ 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 = 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
@@ -249,11 +255,6 @@ class Client extends EventEmitter {
       }
       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)
-    }
   }
 
   /**

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'

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)

test/client/computeLedgerHash/index.ts

@@ -156,7 +156,6 @@ export default <TestSuite>{
   },
 
   '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 <TestSuite>{
   },
 
   'computeLedgerHash - with transactions': async (client, address) => {
-    // const client = new Client()
     const header = {
       ...REQUEST_FIXTURES.header,
       transactionHash: undefined,
@@ -180,7 +178,6 @@ export default <TestSuite>{
   },
 
   'computeLedgerHash - incorrent transaction_hash': async (client, address) => {
-    // const client = new Client()
     const header = Object.assign({}, REQUEST_FIXTURES.header, {
       transactionHash:
         '325EACC5271322539EEEC2D6A5292471EF1B3E72AE7180533EFC3B8F0AD435C9'

test/client/constructor/index.ts

@@ -9,7 +9,7 @@ import {Client} from 'xrpl-local'
  */
 export default <TestSuite>{
   '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 <TestSuite>{
   },
 
   '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'))
   }
 }

test/client/prepareSettings/index.ts

@@ -236,7 +236,6 @@ export default <TestSuite>{
     }
   },
   'offline': async (client, address) => {
-    // const client = new Client()
     const secret = 'shsWGZcmZz6YsWWmcnpfr6fLTdtFV'
 
     const settings = requests.prepareSettings.domain

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!')

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(
     () => {

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.
   })

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:

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<void>((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<void>((resolve, reject) => {
     this.client
       .connect()

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<void>((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<void>((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(() => {