Leverage pools of pool manager from data-connector. (#47)

* centralized pool of pools

* exports pools

* make it driver agnostic, use callback to setup pool into pools

* use controller to abort all

* Apply suggestions from code review

Co-authored-by: Fabian Iwand <[email protected]>

* no need to monkey patch close

* keyOf + key in closure

* mssql signal to handle closing and deleting

* Update lib/mssql.js

Co-authored-by: Fabian Iwand <[email protected]>

* remove pool of pools for mssql

* imports

* await for the connection

* remove pools from exports

* bump node version for Dockerfile

* no pool to test

Co-authored-by: Fabian Iwand <[email protected]>

Author: Sylvestre

.env.test.js

@@ -1,6 +1,4 @@
-
 export const MSSQL_CREDENTIALS = env("MSSQL_CREDENTIALS");
-export const MSSQL_CREDENTIALS_READ_ONLY = env("MSSQL_CREDENTIALS_READ_ONLY");
 export const NODE_ENV = env("NODE_ENV");
 
 function env(key, defaultValue) {

Dockerfile

@@ -1,4 +1,4 @@
-FROM node:16.17.0-alpine
+FROM node:18.12.1-alpine
 
 RUN mkdir /app
 WORKDIR /app

lib/mssql.js

@@ -2,13 +2,11 @@ import Ajv from "ajv";
 import JSONStream from "JSONStream";
 import {json} from "micro";
 import mssql from "mssql";
-import {failedCheck, badRequest, notImplemented} from "./errors.js";
 import {Transform} from "stream";
 
-const TYPES = mssql.TYPES;
-
-const pools = new Map();
+import {failedCheck, badRequest, notImplemented} from "./errors.js";
 
+const TYPES = mssql.TYPES;
 const READ_ONLY = new Set(["SELECT", "USAGE", "CONNECT"]);
 
 const ajv = new Ajv();
@@ -22,37 +20,10 @@ const validate = ajv.compile({
   },
 });
 
-// See: https://tediousjs.github.io/node-mssql/#connection-pools
-export const mssqlPool = {
-  get: (name, config) => {
-    if (!pools.has(name)) {
-      if (!config) {
-        throw new Error("Database configuration required");
-      }
-
-      const pool = new mssql.ConnectionPool(config);
-      const close = pool.close.bind(pool);
-      pool.close = (...args) => {
-        pools.delete(name);
-        return close(...args);
-      };
-
-      pools.set(name, pool.connect());
-    }
-
-    return pools.get(name);
-  },
-
-  closeAll: () =>
-    Promise.all(
-      Array.from(pools.values()).map((connect) => {
-        return connect.then((pool) => pool.close());
-      })
-    ),
-};
-
 export async function queryStream(req, res, pool) {
-  const db = await pool;
+  const connection = await pool;
+  const db = await connection.connect();
+
   const body = await json(req);
 
   if (!validate(body)) throw badRequest();
@@ -144,7 +115,8 @@ export async function queryStream(req, res, pool) {
  * them up that these may be too permissive.
  * */
 export async function check(req, res, pool) {
-  const db = await pool;
+  const connection = await pool;
+  const db = await connection.connect();
 
   // See: https://learn.microsoft.com/en-us/sql/relational-databases/system-functions/sys-fn-my-permissions-transact-sql
   const rows = await db.request().query(
@@ -163,20 +135,24 @@ export async function check(req, res, pool) {
   return {ok: true};
 }
 
-export default (credentials) => async (req, res) => {
-  const pool = mssqlPool.get(JSON.stringify(credentials), credentials);
+export const ConnectionPool = mssql.ConnectionPool;
 
-  if (req.method === "POST") {
-    if (req.url === "/check") {
-      return check(req, res, pool);
-    }
+export default (credentials) => {
+  const pool = new mssql.ConnectionPool(credentials);
 
-    if (["/query-stream"].includes(req.url)) {
-      return queryStream(req, res, pool);
-    }
+  return async (req, res) => {
+    if (req.method === "POST") {
+      if (req.url === "/check") {
+        return check(req, res, pool);
+      }
 
-    throw notImplemented();
-  }
+      if (req.url === "/query-stream") {
+        return queryStream(req, res, pool);
+      }
+
+      throw notImplemented();
+    }
+  };
 };
 
 // See https://github.com/tediousjs/node-mssql/blob/66587d97c9ce21bffba8ca360c72a540f2bc47a6/lib/datatypes.js#L6