From 5cc58874b49c97221696a2673b748385f31e2138 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Wed, 24 Jul 2019 10:28:54 -0700 Subject: [PATCH 01/17] feat: batching --- src/RequestManager.ts | 48 +++++++++++++++++++++++++++++++++++++++++-- src/index.ts | 27 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/RequestManager.ts b/src/RequestManager.ts index a81026a..f620886 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -1,6 +1,13 @@ import ITransport from "./transports/Transport"; let id = 1; +interface IJSONRPCRequest { + jsonrpc: "2.0"; + id: number; + method: string; + params: any[] | object; +} + /* ** Naive Request Manager, only use 1st transport. * A more complex request manager could try each transport. @@ -10,11 +17,15 @@ class RequestManager { public transports: ITransport[]; private requests: any; private connectPromise: Promise; + private batchStarted: boolean = false; + private batch: IJSONRPCRequest[] = []; + constructor(transports: ITransport[]) { this.transports = transports; this.requests = {}; this.connectPromise = this.connect(); } + public connect(): Promise { const promises = this.transports.map((transport) => { return new Promise(async (resolve, reject) => { @@ -27,6 +38,7 @@ class RequestManager { }); return Promise.all(promises); } + public async request(method: string, params: any): Promise { await this.connectPromise; return new Promise((resolve, reject) => { @@ -37,19 +49,51 @@ class RequestManager { resolve, reject, }; - transport.sendData(JSON.stringify({ + const payload: IJSONRPCRequest = { jsonrpc: "2.0", id: i, method, params, - })); + }; + if (this.batchStarted) { + this.batch.push(payload); // could dedupe + } else { + transport.sendData(JSON.stringify(payload)); + } }); } + public close(): void { this.transports.forEach((transport) => { transport.close(); }); } + + /** + * + */ + public startBatch(): void { + if (this.batchStarted) { return; } + this.batchStarted = true; + } + + /** + * + */ + public endBatch(): void { + if (this.batchStarted === false) { + throw new Error("cannot end that which has never started"); + } + + if (this.batch.length === 0) { + return; + } + + const batch = JSON.stringify(this.batch); + this.batch = []; + this.transports[0].sendData(batch); + } + private onData(data: string): void { const parsedData = JSON.parse(data); if (typeof parsedData.result === "undefined" && typeof parsedData.error === "undefined") { diff --git a/src/index.ts b/src/index.ts index ff88926..9f098e1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ interface IClient { /** * OpenRPC Client JS is a browser-compatible JSON-RPC client with multiple transports and * multiple request managers to enable features like round-robin or fallback-by-position. + * * @example * ```typescript * import { RequestManager, HTTPTransport, Client } from '@open-rpc/client-js'; @@ -18,6 +19,7 @@ interface IClient { * const result = await client.request(‘addition’, [2, 2]); * // => { jsonrpc: '2.0', id: 1, result: 4 } * ``` + * */ class Client implements IClient { @@ -26,6 +28,31 @@ class Client implements IClient { this.requestManager = requestManager; } + /** + * A JSON-RPC call is represented by sending a Request object to a Server. + * + * @param method A String containing the name of the method to be invoked. + * Method names that begin with the word rpc followed by a + * period character (U+002E or ASCII 46) are reserved for rpc-internal + * methods and extensions and MUST NOT be used for anything else. + * @param params A Structured value that holds the parameter values to be used during the invocation of the method. + */ + public startBatch(): void { + return this.requestManager.startBatch(); + } + + /** + * A JSON-RPC call is represented by sending a Request object to a Server. See [[RequestManager#endBatch]] + * + * @param method A String containing the name of the method to be invoked. + * Method names that begin with the word rpc followed by a + * period character (U+002E or ASCII 46) are reserved for rpc-internal + * methods and extensions and MUST NOT be used for anything else. + * @param params A Structured value that holds the parameter values to be used during the invocation of the method. + */ + public endBatch(): void { + return this.requestManager.endBatch(); + } /** * A JSON-RPC call is represented by sending a Request object to a Server. * From c867faa680090a77a3ec9c931a46420100c32d23 Mon Sep 17 00:00:00 2001 From: shanejonas Date: Wed, 24 Jul 2019 12:32:05 -0700 Subject: [PATCH 02/17] fix(batching): add tests --- src/RequestManager.test.ts | 37 ++++++++++++++++++++++++++++++++++++- src/RequestManager.ts | 38 ++++++++++++++++++++++++++++++++++---- src/index.ts | 21 ++++++--------------- 3 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 6450bd8..5504f0c 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -39,7 +39,42 @@ describe("client-js", () => { c.request("foo", []); }); - it("can send a request and error", () => { + it("can batch a request", async () => { + const transport = new EventEmitterTransport("foo://unique-uri"); + transport.sendData = (data) => { + const result = JSON.stringify([ + { + jsonrpc: "2.0", + id: 3, + result: "foo", + }, + { + jsonrpc: "2.0", + id: 4, + result: "bar", + }, + ]); + transport.connection.emit("message", result); + }; + + const c = new RequestManager([transport]); + return c.connect().then(() => { + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + c.endBatch(); + return Promise.all(requests).then((results) => { + expect(results[0]).toEqual("foo"); + expect(results[1]).toEqual("bar"); + c.close(); + }); + + }); + }); + + it("can send a request and error", async () => { const transport = new EventEmitterTransport("foo://unique-uri"); const c = new RequestManager([transport]); transport.onData = (fn) => { diff --git a/src/RequestManager.ts b/src/RequestManager.ts index f620886..a86ccaa 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -1,4 +1,5 @@ import ITransport from "./transports/Transport"; + let id = 1; interface IJSONRPCRequest { @@ -7,6 +8,24 @@ interface IJSONRPCRequest { method: string; params: any[] | object; } +interface IJSONRPCError { + code: number; + message: string; + data: any; +} + +interface IJSONRPCResponse { + jsonrpc: "2.0"; + id: number; + result?: any; + error?: IJSONRPCError; +} + +interface IJSONRPCNotification { + jsonrpc: "2.0"; + method: string; + params: any[] | object; +} /* ** Naive Request Manager, only use 1st transport. @@ -15,8 +34,8 @@ interface IJSONRPCRequest { */ class RequestManager { public transports: ITransport[]; + public connectPromise: Promise; private requests: any; - private connectPromise: Promise; private batchStarted: boolean = false; private batch: IJSONRPCRequest[] = []; @@ -40,7 +59,6 @@ class RequestManager { } public async request(method: string, params: any): Promise { - await this.connectPromise; return new Promise((resolve, reject) => { const i = id++; // naively grab first transport and use it @@ -56,7 +74,7 @@ class RequestManager { params, }; if (this.batchStarted) { - this.batch.push(payload); // could dedupe + this.batch.push(payload); } else { transport.sendData(JSON.stringify(payload)); } @@ -95,7 +113,19 @@ class RequestManager { } private onData(data: string): void { - const parsedData = JSON.parse(data); + const parsedData: IJSONRPCResponse[] | IJSONRPCResponse = JSON.parse(data); + if (Array.isArray(parsedData)) { + parsedData.forEach((response) => { + if (this.requests[response.id]) { + if (response.error) { + this.requests[response.id].reject(new Error(response.error.message)); + } else { + this.requests[response.id].resolve(response.result); + } + } + }); + return; + } if (typeof parsedData.result === "undefined" && typeof parsedData.error === "undefined") { return; } diff --git a/src/index.ts b/src/index.ts index 9f098e1..01d46e8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -29,26 +29,16 @@ class Client implements IClient { } /** - * A JSON-RPC call is represented by sending a Request object to a Server. - * - * @param method A String containing the name of the method to be invoked. - * Method names that begin with the word rpc followed by a - * period character (U+002E or ASCII 46) are reserved for rpc-internal - * methods and extensions and MUST NOT be used for anything else. - * @param params A Structured value that holds the parameter values to be used during the invocation of the method. + * Starts a JSON-RPC Batch call. The client will batch following requests. + * This method REQUIRES endBatch be called when finished calling requests. */ public startBatch(): void { return this.requestManager.startBatch(); } /** - * A JSON-RPC call is represented by sending a Request object to a Server. See [[RequestManager#endBatch]] - * - * @param method A String containing the name of the method to be invoked. - * Method names that begin with the word rpc followed by a - * period character (U+002E or ASCII 46) are reserved for rpc-internal - * methods and extensions and MUST NOT be used for anything else. - * @param params A Structured value that holds the parameter values to be used during the invocation of the method. + * Ends a JSON-RPC Batch call. This client will batch requests from when startBatch + * was called until endBatch. This REQUIRES startBatch to be called first. */ public endBatch(): void { return this.requestManager.endBatch(); @@ -62,7 +52,8 @@ class Client implements IClient { * methods and extensions and MUST NOT be used for anything else. * @param params A Structured value that holds the parameter values to be used during the invocation of the method. */ - public request(method: string, params: any) { + public async request(method: string, params: any) { + await this.requestManager.connectPromise; return this.requestManager.request(method, params); } } From 46e8eecc2b4b2f40eedd1861a8955459e1ce0e75 Mon Sep 17 00:00:00 2001 From: shanejonas Date: Thu, 25 Jul 2019 10:03:06 -0700 Subject: [PATCH 03/17] fix(batching): add error tests --- src/RequestManager.test.ts | 57 ++++++++++++++++++++++++++++++++++++-- src/RequestManager.ts | 14 ++++++---- 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 5504f0c..e65cc4e 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -39,14 +39,28 @@ describe("client-js", () => { c.request("foo", []); }); - it("can batch a request", async () => { + it("can error on batchng a request", async () => { + const transport = new EventEmitterTransport("foo://unique-uri"); + const c = new RequestManager([transport]); + return c.connect().then(() => { + expect(() => c.endBatch()).toThrow(); + }); + }); + + it("can return errors on batchng requests", async () => { const transport = new EventEmitterTransport("foo://unique-uri"); transport.sendData = (data) => { const result = JSON.stringify([ { jsonrpc: "2.0", id: 3, - result: "foo", + error: { + code: 509, + message: "too much 509", + data: { + test: "data", + }, + }, }, { jsonrpc: "2.0", @@ -57,6 +71,43 @@ describe("client-js", () => { transport.connection.emit("message", result); }; + const c = new RequestManager([transport]); + return c.connect().then(() => { + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + c.endBatch(); + expect(Promise.all(requests)).rejects.toEqual({ + code: 509, + message: "too much 509", + data: { + test: "data", + }, + }); + c.close(); + }); + }); + + it("can batch a request", async () => { + const transport = new EventEmitterTransport("foo://unique-uri"); + transport.sendData = (data) => { + const result = JSON.stringify([ + { + jsonrpc: "2.0", + id: 5, + result: "foo", + }, + { + jsonrpc: "2.0", + id: 6, + result: "bar", + }, + ]); + transport.connection.emit("message", result); + }; + const c = new RequestManager([transport]); return c.connect().then(() => { c.startBatch(); @@ -81,7 +132,7 @@ describe("client-js", () => { transport.connection.on("message", () => { fn(JSON.stringify({ jsonrpc: "2.0", - id: 3, + id: 7, error: { code: 0, message: "out of order", diff --git a/src/RequestManager.ts b/src/RequestManager.ts index a86ccaa..6a819c8 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -114,14 +114,16 @@ class RequestManager { private onData(data: string): void { const parsedData: IJSONRPCResponse[] | IJSONRPCResponse = JSON.parse(data); + // handle batch requests if (Array.isArray(parsedData)) { parsedData.forEach((response) => { - if (this.requests[response.id]) { - if (response.error) { - this.requests[response.id].reject(new Error(response.error.message)); - } else { - this.requests[response.id].resolve(response.result); - } + if (!this.requests[response.id]) { + return; + } + if (response.error) { + this.requests[response.id].reject(response.error); + } else { + this.requests[response.id].resolve(response.result); } }); return; From 7423e52a27e6d0a4bd63cb3875f74cdcce8fa064 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Thu, 25 Jul 2019 10:37:54 -0700 Subject: [PATCH 04/17] fix: add more docs --- src/RequestManager.ts | 3 +++ src/index.ts | 34 +++++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 6a819c8..73b51f7 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -88,6 +88,9 @@ class RequestManager { } /** + * Begins a batch call by setting the [[RequestManager.batchStarted]] flag to `true`. + * + * [[RequestManager.batch]] is a singleton - only one batch can exist at a given time, per [[RequestManager]]. * */ public startBatch(): void { diff --git a/src/index.ts b/src/index.ts index 01d46e8..033065a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -21,7 +21,6 @@ interface IClient { * ``` * */ - class Client implements IClient { public requestManager: RequestManager; constructor(requestManager: RequestManager) { @@ -29,27 +28,44 @@ class Client implements IClient { } /** - * Starts a JSON-RPC Batch call. The client will batch following requests. - * This method REQUIRES endBatch be called when finished calling requests. + * Initiates [[RequestManager.startBatch]] in order to build a batch call. + * + * Subsequent calls to [[Client.request]] will be added to the batch. Once endBatch is called, the promises for the + * [[Client.request]] will then be resolved. If the request manager already has a batch in progress, this method + * is a noop. + * + * @example + * myClient.startBatch(); + * myClient.request("foo", ["bar"]).then(() => console.log('foobar')); + * myClient.request("foo", ["baz"]).then(() => console.log('foobaz')); + * myClient.endBatch(); */ public startBatch(): void { return this.requestManager.startBatch(); } /** - * Ends a JSON-RPC Batch call. This client will batch requests from when startBatch - * was called until endBatch. This REQUIRES startBatch to be called first. + * Initiates [[RequestManager.endBatch]] in order to finalize and send the batch to the underlying transport. + * + * [[Client.endBatch]] will send the [[Client.request]] calls made since the last [[Client.startBatch]] call. For that + * reason, [[Client.startBatch]] MUST be called before [[Client.endBatch]]. + * + * @example + * myClient.startBatch(); + * myClient.request("foo", ["bar"]).then(() => console.log('foobar')); + * myClient.request("foo", ["baz"]).then(() => console.log('foobaz')); + * myClient.endBatch(); */ public endBatch(): void { return this.requestManager.endBatch(); } + /** * A JSON-RPC call is represented by sending a Request object to a Server. * - * @param method A String containing the name of the method to be invoked. - * Method names that begin with the word rpc followed by a - * period character (U+002E or ASCII 46) are reserved for rpc-internal - * methods and extensions and MUST NOT be used for anything else. + * @param method A String containing the name of the method to be invoked. Method names that begin with the word rpc + * followed by a period character (U+002E or ASCII 46) are reserved for rpc-internal methods and extensions and + * MUST NOT be used for anything else. * @param params A Structured value that holds the parameter values to be used during the invocation of the method. */ public async request(method: string, params: any) { From 097c960bd859cb953d0493453af449cde2bceea3 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Thu, 25 Jul 2019 12:05:13 -0700 Subject: [PATCH 05/17] fix: deterministic id and cleanup --- src/RequestManager.test.ts | 65 ++++++++++++++++++-------------------- src/RequestManager.ts | 61 ++++++++++++++--------------------- 2 files changed, 55 insertions(+), 71 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index e65cc4e..041c8d0 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -47,13 +47,13 @@ describe("client-js", () => { }); }); - it("can return errors on batchng requests", async () => { + it("can return errors on batch requests", async () => { const transport = new EventEmitterTransport("foo://unique-uri"); transport.sendData = (data) => { const result = JSON.stringify([ { jsonrpc: "2.0", - id: 3, + id: "0", error: { code: 509, message: "too much 509", @@ -64,7 +64,7 @@ describe("client-js", () => { }, { jsonrpc: "2.0", - id: 4, + id: "1", result: "bar", }, ]); @@ -72,22 +72,22 @@ describe("client-js", () => { }; const c = new RequestManager([transport]); - return c.connect().then(() => { - c.startBatch(); - const requests = [ - c.request("foo", []), - c.request("foo", []), - ]; - c.endBatch(); - expect(Promise.all(requests)).rejects.toEqual({ - code: 509, - message: "too much 509", - data: { - test: "data", - }, - }); - c.close(); + await c.connect(); + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + expect(requests[0]).rejects.toEqual({ + code: 509, + message: "too much 509", + data: { + test: "data", + }, }); + expect(requests[1]).resolves.toEqual("bar"); + c.endBatch(); + c.close(); }); it("can batch a request", async () => { @@ -96,12 +96,12 @@ describe("client-js", () => { const result = JSON.stringify([ { jsonrpc: "2.0", - id: 5, + id: "0", result: "foo", }, { jsonrpc: "2.0", - id: 6, + id: "1", result: "bar", }, ]); @@ -109,20 +109,17 @@ describe("client-js", () => { }; const c = new RequestManager([transport]); - return c.connect().then(() => { - c.startBatch(); - const requests = [ - c.request("foo", []), - c.request("foo", []), - ]; - c.endBatch(); - return Promise.all(requests).then((results) => { - expect(results[0]).toEqual("foo"); - expect(results[1]).toEqual("bar"); - c.close(); - }); - - }); + await c.connect(); + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + c.endBatch(); + const [a, b] = await Promise.all(requests); + expect(a).toEqual("foo"); + expect(b).toEqual("bar"); + c.close(); }); it("can send a request and error", async () => { diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 73b51f7..433d129 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -1,10 +1,8 @@ import ITransport from "./transports/Transport"; -let id = 1; - interface IJSONRPCRequest { jsonrpc: "2.0"; - id: number; + id: string; method: string; params: any[] | object; } @@ -16,7 +14,7 @@ interface IJSONRPCError { interface IJSONRPCResponse { jsonrpc: "2.0"; - id: number; + id: string; // can also be null result?: any; error?: IJSONRPCError; } @@ -38,6 +36,7 @@ class RequestManager { private requests: any; private batchStarted: boolean = false; private batch: IJSONRPCRequest[] = []; + private lastId: number = -1; constructor(transports: ITransport[]) { this.transports = transports; @@ -59,14 +58,15 @@ class RequestManager { } public async request(method: string, params: any): Promise { + const i = (++this.lastId).toString(); return new Promise((resolve, reject) => { - const i = id++; // naively grab first transport and use it const transport = this.transports[0]; this.requests[i] = { resolve, reject, }; + const payload: IJSONRPCRequest = { jsonrpc: "2.0", id: i, @@ -78,7 +78,7 @@ class RequestManager { } else { transport.sendData(JSON.stringify(payload)); } - }); + }).finally(() => this.requests[i] = undefined); } public close(): void { @@ -98,9 +98,6 @@ class RequestManager { this.batchStarted = true; } - /** - * - */ public endBatch(): void { if (this.batchStarted === false) { throw new Error("cannot end that which has never started"); @@ -117,34 +114,24 @@ class RequestManager { private onData(data: string): void { const parsedData: IJSONRPCResponse[] | IJSONRPCResponse = JSON.parse(data); - // handle batch requests - if (Array.isArray(parsedData)) { - parsedData.forEach((response) => { - if (!this.requests[response.id]) { - return; - } - if (response.error) { - this.requests[response.id].reject(response.error); - } else { - this.requests[response.id].resolve(response.result); - } - }); - return; - } - if (typeof parsedData.result === "undefined" && typeof parsedData.error === "undefined") { - return; - } - const req = this.requests[parsedData.id]; - if (req === undefined) { - return; - } - // resolve promise for id - if (parsedData.error) { - req.reject(parsedData.error); - } else { - req.resolve(parsedData.result); - } - delete this.requests[parsedData.id]; + const results = parsedData instanceof Array ? parsedData : [parsedData]; + + results.forEach((response) => { + const promiseForResult = this.requests[response.id]; + if (promiseForResult === undefined) { + throw new Error( + `Received an unrecognized response id: ${response.id}. Valid ids are: ${Object.keys(this.requests)}`, + ); + } + + if (response.error) { + promiseForResult.reject(response.error); + } else if (response.result) { + promiseForResult.resolve(response.result); + } else { + throw new Error(`Malformed JSON-RPC response object: ${response}`); + } + }); } } From 2042f72aea02ea457647da2ab51a1aee2a9b12fd Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Thu, 25 Jul 2019 12:10:01 -0700 Subject: [PATCH 06/17] fix: allow using integers as id --- src/RequestManager.test.ts | 4 ++-- src/RequestManager.ts | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 041c8d0..330b4f5 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -96,12 +96,12 @@ describe("client-js", () => { const result = JSON.stringify([ { jsonrpc: "2.0", - id: "0", + id: 0, result: "foo", }, { jsonrpc: "2.0", - id: "1", + id: 1, result: "bar", }, ]); diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 433d129..d51aa44 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -2,7 +2,7 @@ import ITransport from "./transports/Transport"; interface IJSONRPCRequest { jsonrpc: "2.0"; - id: string; + id: string | number; method: string; params: any[] | object; } @@ -14,7 +14,7 @@ interface IJSONRPCError { interface IJSONRPCResponse { jsonrpc: "2.0"; - id: string; // can also be null + id: string | number; // can also be null result?: any; error?: IJSONRPCError; } @@ -117,6 +117,7 @@ class RequestManager { const results = parsedData instanceof Array ? parsedData : [parsedData]; results.forEach((response) => { + const id = typeof response.id === "string" ? response.id : response.id.toString(); const promiseForResult = this.requests[response.id]; if (promiseForResult === undefined) { throw new Error( From 72debcfdcd98913a51d5d3e85a22c5c72247c396 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Thu, 25 Jul 2019 12:15:23 -0700 Subject: [PATCH 07/17] fix: cleanup vars Co-Authored-By: Shane --- src/RequestManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RequestManager.ts b/src/RequestManager.ts index d51aa44..978a042 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -118,7 +118,7 @@ class RequestManager { results.forEach((response) => { const id = typeof response.id === "string" ? response.id : response.id.toString(); - const promiseForResult = this.requests[response.id]; + const promiseForResult = this.requests[id]; if (promiseForResult === undefined) { throw new Error( `Received an unrecognized response id: ${response.id}. Valid ids are: ${Object.keys(this.requests)}`, From 60c48caedc3fb66e5052b4266f87ff2a6425f9e8 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Thu, 25 Jul 2019 15:18:59 -0700 Subject: [PATCH 08/17] fix: change name of endBatch to stopBatch --- src/RequestManager.test.ts | 6 +++--- src/RequestManager.ts | 2 +- src/index.ts | 20 ++++++++++---------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 330b4f5..5667f32 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -43,7 +43,7 @@ describe("client-js", () => { const transport = new EventEmitterTransport("foo://unique-uri"); const c = new RequestManager([transport]); return c.connect().then(() => { - expect(() => c.endBatch()).toThrow(); + expect(() => c.stopBatch()).toThrow(); }); }); @@ -86,7 +86,7 @@ describe("client-js", () => { }, }); expect(requests[1]).resolves.toEqual("bar"); - c.endBatch(); + c.stopBatch(); c.close(); }); @@ -115,7 +115,7 @@ describe("client-js", () => { c.request("foo", []), c.request("foo", []), ]; - c.endBatch(); + c.stopBatch(); const [a, b] = await Promise.all(requests); expect(a).toEqual("foo"); expect(b).toEqual("bar"); diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 978a042..6435f8d 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -98,7 +98,7 @@ class RequestManager { this.batchStarted = true; } - public endBatch(): void { + public stopBatch(): void { if (this.batchStarted === false) { throw new Error("cannot end that which has never started"); } diff --git a/src/index.ts b/src/index.ts index 033065a..8b4eaf9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,34 +30,34 @@ class Client implements IClient { /** * Initiates [[RequestManager.startBatch]] in order to build a batch call. * - * Subsequent calls to [[Client.request]] will be added to the batch. Once endBatch is called, the promises for the - * [[Client.request]] will then be resolved. If the request manager already has a batch in progress, this method - * is a noop. + * Subsequent calls to [[Client.request]] will be added to the batch. Once [[Client.stopBatch]] is called, the + * promises for the [[Client.request]] will then be resolved. If the [[RequestManager]] already has a batch in + * progress, this method is a noop. * * @example * myClient.startBatch(); * myClient.request("foo", ["bar"]).then(() => console.log('foobar')); * myClient.request("foo", ["baz"]).then(() => console.log('foobaz')); - * myClient.endBatch(); + * myClient.stopBatch(); */ public startBatch(): void { return this.requestManager.startBatch(); } /** - * Initiates [[RequestManager.endBatch]] in order to finalize and send the batch to the underlying transport. + * Initiates [[RequestManager.stopBatch]] in order to finalize and send the batch to the underlying transport. * - * [[Client.endBatch]] will send the [[Client.request]] calls made since the last [[Client.startBatch]] call. For that - * reason, [[Client.startBatch]] MUST be called before [[Client.endBatch]]. + * [[Client.stopBatch]] will send the [[Client.request]] calls made since the last [[Client.startBatch]] call. For + * that reason, [[Client.startBatch]] MUST be called before [[Client.stopBatch]]. * * @example * myClient.startBatch(); * myClient.request("foo", ["bar"]).then(() => console.log('foobar')); * myClient.request("foo", ["baz"]).then(() => console.log('foobaz')); - * myClient.endBatch(); + * myClient.stopBatch(); */ - public endBatch(): void { - return this.requestManager.endBatch(); + public stopBatch(): void { + return this.requestManager.stopBatch(); } /** From 7e5930118ca5746df50d9367b3837d8e2fad35be Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Fri, 26 Jul 2019 12:32:51 -0700 Subject: [PATCH 09/17] fix: improve test coverage --- src/RequestManager.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 5667f32..dcd0652 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -150,4 +150,14 @@ describe("client-js", () => { }); }); + describe("stopBatch", () => { + it("does nothing if the batch is empty", () => { + const transport = new EventEmitterTransport("foo://unique-uri"); + transport.sendData = jest.fn(); + const c = new RequestManager([transport]); + c.startBatch(); + c.stopBatch(); + expect(transport.sendData).not.toHaveBeenCalled(); + }); + }); }); From 8bcf352b9770e3126a02680954d2337d84dc37e5 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Fri, 26 Jul 2019 12:47:29 -0700 Subject: [PATCH 10/17] fix: improve coverage on index --- src/index.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/index.test.ts b/src/index.test.ts index 40deb68..d78b6ec 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -2,6 +2,9 @@ import Client from "."; import RequestManager from "./RequestManager"; import EventEmitterTransport from "./transports/EventEmitterTransport"; +jest.mock("./RequestManager"); + +const mockedRequestManager = RequestManager as jest.Mock; describe("client-js", () => { it("can be constructed", () => { const c = new Client(new RequestManager([new EventEmitterTransport("foo://unique")])); @@ -14,4 +17,20 @@ describe("client-js", () => { expect(typeof c.request("my_method", null).then).toEqual("function"); }); + describe("startBatch", () => { + it("calls the requestManager.startBatch", () => { + const rm = new mockedRequestManager([new EventEmitterTransport("foo://unique")]); + const c = new Client(rm); + c.startBatch(); + expect(mockedRequestManager.mock.instances[0].startBatch).toHaveBeenCalled(); + }); + }); + + describe("stopBatch", () => { + const rm = new RequestManager([new EventEmitterTransport("foo://unique")]); + const c = new Client(rm); + c.startBatch(); + c.stopBatch(); + expect(mockedRequestManager.mock.instances[0].startBatch).toHaveBeenCalled(); + }); }); From ea19ecf74403228e267bbbf57ba3d3ba98be4fa7 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Fri, 26 Jul 2019 12:52:18 -0700 Subject: [PATCH 11/17] fix: refactor request manager connect --- src/RequestManager.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 6435f8d..f0d704e 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -45,16 +45,10 @@ class RequestManager { } public connect(): Promise { - const promises = this.transports.map((transport) => { - return new Promise(async (resolve, reject) => { - await transport.connect(); - transport.onData((data: any) => { - this.onData(data); - }); - resolve(); - }); - }); - return Promise.all(promises); + return Promise.all(this.transports.map(async (transport) => { + await transport.connect(); + transport.onData(this.onData.bind(this)); + })); } public async request(method: string, params: any): Promise { From ba1e23bb08a3ff6a23f5931c4de8c24f111cc710 Mon Sep 17 00:00:00 2001 From: shanejonas Date: Mon, 29 Jul 2019 09:52:56 -0700 Subject: [PATCH 12/17] fix: event emitter transport --- src/RequestManager.test.ts | 114 ++++++++++--------- src/index.test.ts | 13 ++- src/transports/EventEmitterTransport.test.ts | 16 ++- src/transports/EventEmitterTransport.ts | 12 +- 4 files changed, 92 insertions(+), 63 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index dcd0652..8767db1 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -1,46 +1,55 @@ import RequestManager from "./RequestManager"; import EventEmitterTransport from "./transports/EventEmitterTransport"; +import { EventEmitter } from "events"; describe("client-js", () => { it("can be constructed", () => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); expect(!!c).toEqual(true); }); it("has a request method that returns a promise", () => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); expect(typeof c.request).toEqual("function"); expect(typeof c.request("my_method", null).then).toEqual("function"); }); it("can connect", () => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); return c.connect(); }); it("can close", () => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); c.close(); }); it("can send a request", (done) => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); + const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); const c = new RequestManager([transport]); c.connect(); transport.onData((data: any) => { const d = JSON.parse(data); - expect(d.method).toEqual("foo"); + expect(d.foo).toEqual("bar"); done(); }); c.request("foo", []); + serverTransport.sendData(JSON.stringify({ foo: "bar" })); }); it("can error on batchng a request", async () => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); return c.connect().then(() => { expect(() => c.stopBatch()).toThrow(); @@ -48,28 +57,9 @@ describe("client-js", () => { }); it("can return errors on batch requests", async () => { - const transport = new EventEmitterTransport("foo://unique-uri"); - transport.sendData = (data) => { - const result = JSON.stringify([ - { - jsonrpc: "2.0", - id: "0", - error: { - code: 509, - message: "too much 509", - data: { - test: "data", - }, - }, - }, - { - jsonrpc: "2.0", - id: "1", - result: "bar", - }, - ]); - transport.connection.emit("message", result); - }; + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); + const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); const c = new RequestManager([transport]); await c.connect(); @@ -85,15 +75,49 @@ describe("client-js", () => { test: "data", }, }); + serverTransport.sendData(JSON.stringify([ + { + jsonrpc: "2.0", + id: "0", + error: { + code: 509, + message: "too much 509", + data: { + test: "data", + }, + }, + }, + { + jsonrpc: "2.0", + id: "1", + result: "bar", + }, + ])); expect(requests[1]).resolves.toEqual("bar"); c.stopBatch(); c.close(); }); - it("can batch a request", async () => { - const transport = new EventEmitterTransport("foo://unique-uri"); - transport.sendData = (data) => { - const result = JSON.stringify([ + it("can batch a request", (done) => { + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); + const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); + + const c = new RequestManager([transport]); + c.connect().then(() => { + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + c.stopBatch(); + Promise.all(requests).then(([a, b]) => { + expect(a).toEqual("foo"); + expect(b).toEqual("bar"); + c.close(); + done(); + }); + serverTransport.sendData(JSON.stringify([ { jsonrpc: "2.0", id: 0, @@ -104,26 +128,13 @@ describe("client-js", () => { id: 1, result: "bar", }, - ]); - transport.connection.emit("message", result); - }; - - const c = new RequestManager([transport]); - await c.connect(); - c.startBatch(); - const requests = [ - c.request("foo", []), - c.request("foo", []), - ]; - c.stopBatch(); - const [a, b] = await Promise.all(requests); - expect(a).toEqual("foo"); - expect(b).toEqual("bar"); - c.close(); + ])); + }); }); it("can send a request and error", async () => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); transport.onData = (fn) => { transport.connection.on("message", () => { @@ -152,7 +163,8 @@ describe("client-js", () => { describe("stopBatch", () => { it("does nothing if the batch is empty", () => { - const transport = new EventEmitterTransport("foo://unique-uri"); + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); transport.sendData = jest.fn(); const c = new RequestManager([transport]); c.startBatch(); diff --git a/src/index.test.ts b/src/index.test.ts index d78b6ec..e32cd7e 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -1,25 +1,29 @@ import Client from "."; import RequestManager from "./RequestManager"; import EventEmitterTransport from "./transports/EventEmitterTransport"; +import { EventEmitter } from "events"; jest.mock("./RequestManager"); const mockedRequestManager = RequestManager as jest.Mock; describe("client-js", () => { it("can be constructed", () => { - const c = new Client(new RequestManager([new EventEmitterTransport("foo://unique")])); + const emitter = new EventEmitter(); + const c = new Client(new RequestManager([new EventEmitterTransport(emitter, "from1", "to1")])); expect(!!c).toEqual(true); }); it("has a request method that returns a promise", () => { - const c = new Client(new RequestManager([new EventEmitterTransport("foo://unique")])); + const emitter = new EventEmitter(); + const c = new Client(new RequestManager([new EventEmitterTransport(emitter, "from1", "to1")])); expect(typeof c.request).toEqual("function"); expect(typeof c.request("my_method", null).then).toEqual("function"); }); describe("startBatch", () => { it("calls the requestManager.startBatch", () => { - const rm = new mockedRequestManager([new EventEmitterTransport("foo://unique")]); + const emitter = new EventEmitter(); + const rm = new mockedRequestManager([new EventEmitterTransport(emitter, "from1", "to1")]); const c = new Client(rm); c.startBatch(); expect(mockedRequestManager.mock.instances[0].startBatch).toHaveBeenCalled(); @@ -27,7 +31,8 @@ describe("client-js", () => { }); describe("stopBatch", () => { - const rm = new RequestManager([new EventEmitterTransport("foo://unique")]); + const emitter = new EventEmitter(); + const rm = new mockedRequestManager([new EventEmitterTransport(emitter, "from1", "to1")]); const c = new Client(rm); c.startBatch(); c.stopBatch(); diff --git a/src/transports/EventEmitterTransport.test.ts b/src/transports/EventEmitterTransport.test.ts index 5875836..4417126 100644 --- a/src/transports/EventEmitterTransport.test.ts +++ b/src/transports/EventEmitterTransport.test.ts @@ -1,21 +1,29 @@ import EventEmitterTransport from "./EventEmitterTransport"; +import { EventEmitter } from "events"; describe("EventEmitterTransport", () => { it("can connect", () => { - const eventEmitterTransport = new EventEmitterTransport("foo://bar"); + const emitter = new EventEmitter(); + const eventEmitterTransport = new EventEmitterTransport(emitter, "foo://in", "foo://out"); eventEmitterTransport.connect(); }); it("can close", () => { - const eventEmitterTransport = new EventEmitterTransport("foo://bar"); + const emitter = new EventEmitter(); + const reqUri = "from"; + const resUri = "to"; + const eventEmitterTransport = new EventEmitterTransport(emitter, reqUri, resUri); eventEmitterTransport.close(); }); it("can send and receive data", (done) => { - const eventEmitterTransport = new EventEmitterTransport("foo://bar"); + const emitter = new EventEmitter(); + const eventEmitterTransport = new EventEmitterTransport(emitter, "from1", "to1"); eventEmitterTransport.onData((data: any) => { const d = JSON.parse(data); expect(d.foo).toEqual("bar"); done(); }); - eventEmitterTransport.sendData(JSON.stringify({foo: "bar"})); + + const eventEmitterServerTransport = new EventEmitterTransport(emitter, "to1", "from1"); + eventEmitterServerTransport.sendData(JSON.stringify({foo: "bar"})); }); }); diff --git a/src/transports/EventEmitterTransport.ts b/src/transports/EventEmitterTransport.ts index 47c2702..87bf5ff 100644 --- a/src/transports/EventEmitterTransport.ts +++ b/src/transports/EventEmitterTransport.ts @@ -3,19 +3,23 @@ import ITransport from "./Transport"; class EventEmitterTransport implements ITransport { public connection: EventEmitter; - constructor(uri: string) { - this.connection = new EventEmitter(); + private reqUri: string; + private resUri: string; + constructor(emitter: EventEmitter, reqUri: string, resUri: string) { + this.connection = emitter; + this.reqUri = reqUri; + this.resUri = resUri; } public connect(): Promise { return Promise.resolve(); } public onData(callback: (data: string) => any) { - this.connection.addListener("message", (data: any) => { + this.connection.on(this.reqUri, (data: any) => { callback(data); }); } public sendData(data: string) { - this.connection.emit("message", data); + this.connection.emit(this.resUri, data); } public close() { this.connection.removeAllListeners(); From 8ba714ecc7fc6a95de7419da795cf5264363c210 Mon Sep 17 00:00:00 2001 From: shanejonas Date: Mon, 29 Jul 2019 10:56:38 -0700 Subject: [PATCH 13/17] fix: request manager tests --- src/RequestManager.test.ts | 131 ++++++++++++++++++++----------------- src/RequestManager.ts | 2 +- 2 files changed, 73 insertions(+), 60 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 8767db1..1910d4e 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -1,6 +1,7 @@ import RequestManager from "./RequestManager"; import EventEmitterTransport from "./transports/EventEmitterTransport"; import { EventEmitter } from "events"; +import { doesNotReject } from "assert"; describe("client-js", () => { it("can be constructed", () => { @@ -18,7 +19,7 @@ describe("client-js", () => { expect(typeof c.request("my_method", null).then).toEqual("function"); }); - it("can connect", () => { + it("can connect", async () => { const emitter = new EventEmitter(); const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); @@ -37,14 +38,26 @@ describe("client-js", () => { const transport = new EventEmitterTransport(emitter, "from1", "to1"); const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); const c = new RequestManager([transport]); - c.connect(); - transport.onData((data: any) => { - const d = JSON.parse(data); - expect(d.foo).toEqual("bar"); - done(); + c.connect().then(() => { + c.request("foo", []).then(() => { + done(); + }); + serverTransport.sendData(JSON.stringify({ id: 0, result: { foo: "foofoo" } })); + }); + }); + + it("can error on malformed response", (done) => { + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); + const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); + const c = new RequestManager([transport]); + c.connect().then(() => { + c.request("foo", []).catch((e) => { + expect(e.message).toContain("Malformed"); + done(); + }); + serverTransport.sendData(JSON.stringify({ id: 0, foo: "bar" })); }); - c.request("foo", []); - serverTransport.sendData(JSON.stringify({ foo: "bar" })); }); it("can error on batchng a request", async () => { @@ -56,46 +69,50 @@ describe("client-js", () => { }); }); - it("can return errors on batch requests", async () => { + it("can return errors on batch requests", (done) => { const emitter = new EventEmitter(); const transport = new EventEmitterTransport(emitter, "from1", "to1"); const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); const c = new RequestManager([transport]); - await c.connect(); - c.startBatch(); - const requests = [ - c.request("foo", []), - c.request("foo", []), - ]; - expect(requests[0]).rejects.toEqual({ - code: 509, - message: "too much 509", - data: { - test: "data", - }, - }); - serverTransport.sendData(JSON.stringify([ - { - jsonrpc: "2.0", - id: "0", - error: { + c.connect().then(() => { + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + Promise.all(requests).catch((e) => { + expect(e).toEqual({ code: 509, message: "too much 509", data: { test: "data", }, + }); + c.close(); + done(); + }); + c.stopBatch(); + serverTransport.sendData(JSON.stringify([ + { + jsonrpc: "2.0", + id: "0", + error: { + code: 509, + message: "too much 509", + data: { + test: "data", + }, + }, }, - }, - { - jsonrpc: "2.0", - id: "1", - result: "bar", - }, - ])); - expect(requests[1]).resolves.toEqual("bar"); - c.stopBatch(); - c.close(); + { + jsonrpc: "2.0", + id: "1", + result: "bar", + }, + ])); + + }); }); it("can batch a request", (done) => { @@ -132,32 +149,28 @@ describe("client-js", () => { }); }); - it("can send a request and error", async () => { + it("can send a request and error", (done) => { const emitter = new EventEmitter(); const transport = new EventEmitterTransport(emitter, "from1", "to1"); + const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); const c = new RequestManager([transport]); - transport.onData = (fn) => { - transport.connection.on("message", () => { - fn(JSON.stringify({ - jsonrpc: "2.0", - id: 7, - error: { - code: 0, - message: "out of order", - data: { - foo: "bar", - }, + c.connect().then(() => { + c.request("foo", []) + .catch((e) => { + expect(e.message).toEqual("out of order"); + done(); + }); + serverTransport.sendData(JSON.stringify({ + jsonrpc: "2.0", + id: 0, + error: { + code: 0, + message: "out of order", + data: { + foo: "bar", }, - })); - }); - }; - c.connect(); - expect(c.request("foo", [])).rejects.toBe({ - code: 0, - message: "out of order", - data: { - foo: "bar", - }, + }, + })); }); }); diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 6435f8d..f8c3974 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -130,7 +130,7 @@ class RequestManager { } else if (response.result) { promiseForResult.resolve(response.result); } else { - throw new Error(`Malformed JSON-RPC response object: ${response}`); + promiseForResult.reject(new Error(`Malformed JSON-RPC response object: ${response}`)); } }); } From 617e2ede7303522dae50a816928699ad0b870b05 Mon Sep 17 00:00:00 2001 From: Shane Date: Mon, 29 Jul 2019 12:51:36 -0700 Subject: [PATCH 14/17] fix: update src/RequestManager.test.ts --- src/RequestManager.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 1910d4e..9132638 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -1,7 +1,6 @@ import RequestManager from "./RequestManager"; import EventEmitterTransport from "./transports/EventEmitterTransport"; import { EventEmitter } from "events"; -import { doesNotReject } from "assert"; describe("client-js", () => { it("can be constructed", () => { From 586ef40856412efe5af91ad743660843cbbff177 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Mon, 29 Jul 2019 08:50:36 -0700 Subject: [PATCH 15/17] fix: mocking --- src/RequestManager.test.ts | 18 +++++++-------- src/RequestManager.ts | 30 ++++++++++++------------- src/transports/EventEmitterTransport.ts | 4 ++++ 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 9132638..5eb4803 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -3,6 +3,7 @@ import EventEmitterTransport from "./transports/EventEmitterTransport"; import { EventEmitter } from "events"; describe("client-js", () => { + it("can be constructed", () => { const emitter = new EventEmitter(); const transport = new EventEmitterTransport(emitter, "from1", "to1"); @@ -32,17 +33,15 @@ describe("client-js", () => { c.close(); }); - it("can send a request", (done) => { + it("can send a request", async () => { const emitter = new EventEmitter(); const transport = new EventEmitterTransport(emitter, "from1", "to1"); const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); const c = new RequestManager([transport]); - c.connect().then(() => { - c.request("foo", []).then(() => { - done(); - }); - serverTransport.sendData(JSON.stringify({ id: 0, result: { foo: "foofoo" } })); - }); + await c.connect(); + const reqPromise = c.request("foo", []); + serverTransport.sendData(JSON.stringify({ id: 0, result: { foo: "foofoo" } })); + await expect(reqPromise).resolves.toEqual({ foo: "foofoo" }); }); it("can error on malformed response", (done) => { @@ -63,9 +62,8 @@ describe("client-js", () => { const emitter = new EventEmitter(); const transport = new EventEmitterTransport(emitter, "from1", "to1"); const c = new RequestManager([transport]); - return c.connect().then(() => { - expect(() => c.stopBatch()).toThrow(); - }); + await c.connect(); + expect(() => c.stopBatch()).toThrow(); }); it("can return errors on batch requests", (done) => { diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 682df25..76ab368 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -46,27 +46,27 @@ class RequestManager { public connect(): Promise { return Promise.all(this.transports.map(async (transport) => { - await transport.connect(); transport.onData(this.onData.bind(this)); + await transport.connect(); })); } public async request(method: string, params: any): Promise { const i = (++this.lastId).toString(); + + // naively grab first transport and use it + const transport = this.transports[0]; + + const payload: IJSONRPCRequest = { + jsonrpc: "2.0", + id: i, + method, + params, + }; + return new Promise((resolve, reject) => { - // naively grab first transport and use it - const transport = this.transports[0]; - this.requests[i] = { - resolve, - reject, - }; - - const payload: IJSONRPCRequest = { - jsonrpc: "2.0", - id: i, - method, - params, - }; + this.requests[i] = { resolve, reject }; + if (this.batchStarted) { this.batch.push(payload); } else { @@ -124,7 +124,7 @@ class RequestManager { } else if (response.result) { promiseForResult.resolve(response.result); } else { - promiseForResult.reject(new Error(`Malformed JSON-RPC response object: ${response}`)); + promiseForResult.reject(new Error(`Malformed JSON-RPC response object: ${JSON.stringify(response)}`)); } }); } diff --git a/src/transports/EventEmitterTransport.ts b/src/transports/EventEmitterTransport.ts index 87bf5ff..fd0521e 100644 --- a/src/transports/EventEmitterTransport.ts +++ b/src/transports/EventEmitterTransport.ts @@ -10,17 +10,21 @@ class EventEmitterTransport implements ITransport { this.reqUri = reqUri; this.resUri = resUri; } + public connect(): Promise { return Promise.resolve(); } + public onData(callback: (data: string) => any) { this.connection.on(this.reqUri, (data: any) => { callback(data); }); } + public sendData(data: string) { this.connection.emit(this.resUri, data); } + public close() { this.connection.removeAllListeners(); } From 5496caff4dd8f5aeb67d99bc7cb9bb14dff56564 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Mon, 29 Jul 2019 16:21:01 -0700 Subject: [PATCH 16/17] fix: improve coverage --- src/RequestManager.test.ts | 34 ++++++++++++++++++++++++++++++++++ src/RequestManager.ts | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 5eb4803..05edcf0 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -171,6 +171,25 @@ describe("client-js", () => { }); }); + it("onData throws if the ID is not found", async () => { + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); + const serverTransport = new EventEmitterTransport(emitter, "to1", "from1"); + const c = new RequestManager([transport]); + await c.connect(); + expect(() => serverTransport.sendData(JSON.stringify({ + jsonrpc: "2.0", + id: 10, + error: { + code: 0, + message: "out of order", + data: { + foo: "bar", + }, + }, + }))).toThrow(); + }); + describe("stopBatch", () => { it("does nothing if the batch is empty", () => { const emitter = new EventEmitter(); @@ -182,4 +201,19 @@ describe("client-js", () => { expect(transport.sendData).not.toHaveBeenCalled(); }); }); + + describe("startBatch", () => { + it("it does nothing if a batch is already started", async () => { + const emitter = new EventEmitter(); + const transport = new EventEmitterTransport(emitter, "from1", "to1"); + const c = new RequestManager([transport]); + await c.connect(); + c.startBatch(); + c.request("foo", []); + expect(c.batch.length).toBe(1); + c.startBatch(); + c.request("foo", []); + expect(c.batch.length).toBe(2); + }); + }); }); diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 76ab368..59d785d 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -33,9 +33,9 @@ interface IJSONRPCNotification { class RequestManager { public transports: ITransport[]; public connectPromise: Promise; + public batch: IJSONRPCRequest[] = []; private requests: any; private batchStarted: boolean = false; - private batch: IJSONRPCRequest[] = []; private lastId: number = -1; constructor(transports: ITransport[]) { From 374beb3a535990b8adcfb14c1b97686e92dcac7c Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Mon, 29 Jul 2019 16:26:37 -0700 Subject: [PATCH 17/17] fix: more specific assertion --- src/RequestManager.test.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index 05edcf0..1b2d5f1 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -180,14 +180,8 @@ describe("client-js", () => { expect(() => serverTransport.sendData(JSON.stringify({ jsonrpc: "2.0", id: 10, - error: { - code: 0, - message: "out of order", - data: { - foo: "bar", - }, - }, - }))).toThrow(); + result: 123, + }))).toThrow("Received an unrecognized response id: 10. Valid ids are: "); }); describe("stopBatch", () => {