Fix/json_passing (#34)

* Cleanup multi job pool test Signed-off-by: worksofliam <[email protected]> * Remove logs and bad test case Signed-off-by: worksofliam <[email protected]> * Less JSON parsing Signed-off-by: worksofliam <[email protected]> * Add type to ServerRequest Signed-off-by: worksofliam <[email protected]> * Use T instead of any for explain Signed-off-by: worksofliam <[email protected]> --------- Signed-off-by: worksofliam <[email protected]>
Mapepire-IBMi · Sep 3, 2024 · d63780d · d63780d
1 parent 3e3e043
commit d63780d
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 45 deletions.
diff --git a/src/query.ts b/src/query.ts
@@ -1,5 +1,5 @@
 import { SQLJob } from "./sqlJob";
-import { QueryOptions, QueryResult } from "./types";
+import { QueryOptions, QueryResult, ServerResponse } from "./types";
 
 /**
  * Represents the possible states of a query execution.
@@ -197,8 +197,7 @@ export class Query<T> {
       };
     }
     this.rowsToFetch = rowsToFetch;
-    let result = await this.job.send(JSON.stringify(queryObject));
-    let queryResult: QueryResult<T> = JSON.parse(result);
+    let queryResult = await this.job.send<QueryResult<T>>(queryObject);
 
     this.state = queryResult.is_done
       ? QueryState.RUN_DONE
@@ -248,9 +247,8 @@ export class Query<T> {
     };
 
     this.rowsToFetch = rowsToFetch;
-    let result = await this.job.send(JSON.stringify(queryObject));
+    let queryResult = await this.job.send<QueryResult<T>>(queryObject);
 
-    let queryResult: QueryResult<T> = JSON.parse(result);
     this.state = queryResult.is_done
       ? QueryState.RUN_DONE
       : QueryState.RUN_MORE_DATA_AVAILABLE;
@@ -278,7 +276,7 @@ export class Query<T> {
         type: `sqlclose`,
       };
 
-      return this.job.send(JSON.stringify(queryObject));
+      return this.job.send<ServerResponse>(queryObject);
     } else if (undefined === this.correlationId) {
       this.state = QueryState.RUN_DONE;
     }

diff --git a/src/sqlJob.ts b/src/sqlJob.ts
@@ -15,6 +15,7 @@ import {
   ServerTraceLevel,
   SetConfigResult,
   TransactionEndType,
+  ServerRequest,
   VersionCheckResult
 } from "./types";
 
@@ -110,7 +111,7 @@ export class SQLJob {
         }
         try {
           let response: ReqRespFmt = JSON.parse(asString);
-          this.responseEmitter.emit(response.id, asString);
+          this.responseEmitter.emit(response.id, response);
         } catch (e: any) {
           console.log(`Error: ` + e);
         }
@@ -128,17 +129,15 @@ export class SQLJob {
    * @param content - The message content to send.
    * @returns A promise that resolves to the server's response.
    */
-  async send(content: string): Promise<string> {
+  async send<T>(content: ServerRequest): Promise<T> {
     if (this.isTracingChannelData) console.log(content);
 
-    let req: ReqRespFmt = JSON.parse(content);
-    this.socket.send(content);
+    this.socket.send(JSON.stringify(content));
     return new Promise((resolve, reject) => {
       this.status = JobStatus.Busy;
-      this.responseEmitter.on(req.id, (x: string) => {
-        this.responseEmitter.removeAllListeners(req.id);
-        this.status =
-          this.getRunningCount() === 0 ? JobStatus.Ready : JobStatus.Busy;
+      this.responseEmitter.on(content.id, (x: T) => {
+        this.responseEmitter.removeAllListeners(content.id);
+        this.status = this.getRunningCount() === 0 ? JobStatus.Ready : JobStatus.Busy;
         resolve(x);
       });
     });
@@ -199,9 +198,7 @@ export class SQLJob {
       props: props.length > 0 ? props : undefined,
     };
 
-    const result = await this.send(JSON.stringify(connectionObject));
-
-    const connectResult: ConnectionResult = JSON.parse(result);
+    const connectResult = await this.send<ConnectionResult>(connectionObject);
 
     if (connectResult.success === true) {
       this.status = JobStatus.Ready;
@@ -258,9 +255,7 @@ export class SQLJob {
       type: `getversion`,
     };
 
-    const result = await this.send(JSON.stringify(verObj));
-
-    const version: VersionCheckResult = JSON.parse(result);
+    const version = await this.send<VersionCheckResult>(verObj);
 
     if (version.success !== true) {
       throw new Error(version.error || `Failed to get version from backend`);
@@ -275,20 +270,18 @@ export class SQLJob {
    * @param type - The type of explain to perform (default is ExplainType.Run).
    * @returns A promise that resolves to the explain results.
    */
-  async explain(
+  async explain<T>(
     statement: string,
     type: ExplainType = ExplainType.Run
-  ): Promise<ExplainResults<any>> {
+  ): Promise<ExplainResults<T>> {
     const explainRequest = {
       id: SQLJob.getNewUniqueId(),
       type: `dove`,
       sql: statement,
       run: type === ExplainType.Run,
     };
 
-    const result = await this.send(JSON.stringify(explainRequest));
-
-    const explainResult: ExplainResults<any> = JSON.parse(result);
+    const explainResult = await this.send<ExplainResults<T>>(explainRequest);
 
     if (explainResult.success !== true) {
       throw new Error(explainResult.error || `Failed to explain.`);
@@ -317,9 +310,7 @@ export class SQLJob {
       type: `gettracedata`,
     };
 
-    const result = await this.send(JSON.stringify(tracedataReqObj));
-
-    const rpy: GetTraceDataResult = JSON.parse(result);
+    const rpy = await this.send<GetTraceDataResult>(tracedataReqObj);
 
     if (rpy.success !== true) {
       throw new Error(rpy.error || `Failed to get trace data from backend`);
@@ -348,9 +339,7 @@ export class SQLJob {
 
     this.isTracingChannelData = true;
 
-    const result = await this.send(JSON.stringify(reqObj));
-
-    const rpy: SetConfigResult = JSON.parse(result);
+    const rpy = await this.send<SetConfigResult>(reqObj);
 
     if (rpy.success !== true) {
       throw new Error(rpy.error || `Failed to set trace options on backend`);

diff --git a/src/types.ts b/src/types.ts
@@ -75,6 +75,11 @@ export interface ServerResponse {
   sql_state: string;
 }
 
+export interface ServerRequest {
+  id: string;
+  type: string;
+}
+
 /** Interface representing the result of a connection request. */
 export interface ConnectionResult extends ServerResponse {
   /** Unique job identifier for the connection. */

diff --git a/test/pool.test.ts b/test/pool.test.ts
@@ -82,6 +82,8 @@ test("Starting size of 0", async () => {
 
 test("Performance test", async () => {
   let pool = new Pool({ creds, maxSize: 5, startingSize: 5 });
+
+  // Pool 1
   await pool.init();
   const startPool1 = Date.now();
   let queries = [];
@@ -90,9 +92,11 @@ test("Performance test", async () => {
   }
   let results: QueryResult<any>[] = await Promise.all(queries);
   const endPool1 = Date.now();
-  await pool.end();
+  pool.end();
+
   results.forEach((res) => expect(res.has_results).toBe(true));
 
+  // Pool 2
   pool = new Pool({ creds, maxSize: 1, startingSize: 1 });
   await pool.init();
   const startPool2 = Date.now();
@@ -101,22 +105,18 @@ test("Performance test", async () => {
     queries.push(pool.execute("select * FROM SAMPLE.SYSCOLUMNS"));
   }
   results = await Promise.all(queries);
+
   const endPool2 = Date.now();
 
-  await pool.end();
+  pool.end();
   results.forEach((res) => expect(res.has_results).toBe(true));
 
-  const noPoolStart = Date.now();
-  for (let i = 0; i < 20; i++) {
-    const job = new SQLJob();
-    await job.connect(creds);
-    await job.execute("select * FROM SAMPLE.SYSCOLUMNS");
-    await job.close();
-  }
-  const noPoolEnd = Date.now();
+  // Compare
+  const multiJobPoolTime = endPool1 - startPool1;
+  const singleJobPoolTime = endPool2 - startPool2;
 
-  expect(endPool2 - startPool2).toBeGreaterThan(endPool1 - startPool1);
-  expect(noPoolEnd - noPoolStart).toBeGreaterThan(endPool2 - startPool2);
+  // Expect singlejob to be slower than multi job
+  expect(singleJobPoolTime).toBeGreaterThan(multiJobPoolTime);
 }, 30000);
 
 test("Pop jobs returns free job", async () => {

diff --git a/test/sql.test.ts b/test/sql.test.ts
@@ -315,8 +315,8 @@ test("Prepare SQL with Edge Case Inputs", async () => {
   //   error = err;
   // }
 
-  expect(error).toBeDefined();
-  expect(error.message).toEqual("Not a JSON Array: 99");
+  // expect(error).toBeDefined();
+  // expect(error.message).toEqual("Not a JSON Array: 99");
 
   try {
     query = await job.query<any>(