Repository: rickbergfalk/sqlpad
Pull request #543 (2020-02-20 21:16:02)

Add config to model classes
Adds `config` property to models, and cleans up initialization of express app to take models object instead of nedb.

Both `nedb` and `models` instances are created during `makeDb` call. Instead of returning a promise, `makeDb` now only returns a boolean and `getDb` should used to keep things a bit more clear that making the db is not a synchronous action, and that it is not yet ready.


Patch:
diff --git a/server/app.js b/server/app.js
index 58864b0c..0de4b94e 100644
--- a/server/app.js
+++ b/server/app.js
@@ -5,18 +5,17 @@ const helmet = require('helmet');
 const session = require('express-session');
 const FileStore = require('session-file-store')(session);
 const logger = require('./lib/logger');
-const getModels = require('./models');
 
 /**
  * Create an express app using config
  * @param {object} config
  */
-function makeApp(config, nedb) {
+function makeApp(config, models) {
   if (typeof config.get !== 'function') {
     throw new Error('config is required to create app');
   }
-  if (!nedb) {
-    throw new Error('nedb is required to create app');
+  if (!models) {
+    throw new Error('models is required to create app');
   }
 
   const baseUrl = config.get('baseUrl');
@@ -64,7 +63,7 @@ function makeApp(config, nedb) {
   // Decorate req with app things
   app.use(function(req, res, next) {
     req.config = config;
-    req.models = getModels(nedb);
+    req.models = models;
     next();
   });
 
diff --git a/server/lib/db.js b/server/lib/db.js
index 76984fb1..7f52500f 100644
--- a/server/lib/db.js
+++ b/server/lib/db.js
@@ -4,7 +4,7 @@ const mkdirp = require('mkdirp');
 const logger = require('./logger');
 const ensureAdmin = require('./ensureAdmin');
 const consts = require('./consts');
-const getModels = require('../models');
+const Models = require('../models');
 
 const TEN_MINUTES = 1000 * 60 * 10;
 const FIVE_MINUTES = 1000 * 60 * 5;
@@ -22,13 +22,14 @@ let instances = {};
  * Returns promise of nedb instance
  * @param {string} [instanceAlias]
  */
-function getNedb(instanceAlias = 'default') {
-  const nedb = instances[instanceAlias];
-  if (!nedb) {
-    throw new Error('nedb instance must be created first');
+async function getDb(instanceAlias = 'default') {
+  const instancePromise = instances[instanceAlias];
+  if (!instancePromise) {
+    throw new Error('db instance must be created first');
   }
   // nedb will already be a promise -- this just makes it explicit
-  return Promise.resolve(nedb);
+  const { nedb, models } = await instancePromise;
+  return { nedb, models };
 }
 
 /**
@@ -111,7 +112,7 @@ async function initNedb(config) {
   });
 
   // Schedule cleanups
-  const models = getModels(nedb);
+  const models = new Models(nedb, config);
   setInterval(async () => {
     await models.resultCache.removeExpired();
     await models.queryHistory.removeOldEntries();
@@ -120,7 +121,7 @@ async function initNedb(config) {
   // Ensure admin is set as specified if provided
   await ensureAdmin(nedb, admin, adminPassword);
 
-  return nedb;
+  return { nedb, models };
 }
 
 /**
@@ -130,17 +131,17 @@ async function initNedb(config) {
  * @param {object} config
  * @param {string} instanceAlias
  */
-function makeNedb(config, instanceAlias = 'default') {
-  // makeNedb should only be called once for a given alias
+function makeDb(config, instanceAlias = 'default') {
+  // makeDb should only be called once for a given alias
   if (instances[instanceAlias]) {
     throw new Error(`db instance ${instanceAlias} already made`);
   }
   const dbPromise = initNedb(config);
   instances[instanceAlias] = dbPromise;
-  return dbPromise;
+  return true;
 }
 
 module.exports = {
-  makeNedb,
-  getNedb
+  makeDb,
+  getDb
 };
diff --git a/server/models/connectionAccesses.js b/server/models/connectionAccesses.js
index 825a2a33..44efd360 100644
--- a/server/models/connectionAccesses.js
+++ b/server/models/connectionAccesses.js
@@ -18,8 +18,9 @@ const schema = Joi.object({
 });
 
 class ConnectionAccesses {
-  constructor(nedb) {
+  constructor(nedb, config) {
     this.nedb = nedb;
+    this.config = config;
   }
 
   async save(data) {
diff --git a/server/models/connections.js b/server/models/connections.js
index 3dfd35d4..906ca8ae 100644
--- a/server/models/connections.js
+++ b/server/models/connections.js
@@ -1,32 +1,33 @@
 const _ = require('lodash');
 const drivers = require('../drivers');
 const makeCipher = require('../lib/makeCipher');
-const config = require('../lib/config');
 // TODO: during app init upsert these into db?
 const { getConnectionsFromConfig } = require('../lib/connectionsFromConfig');
 
-const { cipher, decipher } = makeCipher(config.get('passphrase'));
-
-function decipherConnection(connection) {
-  if (connection.username) {
-    connection.username = decipher(connection.username);
-  }
-  if (connection.password) {
-    connection.password = decipher(connection.password);
-  }
-  return connection;
-}
-
 class Connections {
-  constructor(nedb) {
+  constructor(nedb, config) {
     this.nedb = nedb;
+    this.config = config;
+    const { cipher, decipher } = makeCipher(config.get('passphrase'));
+    this.cipher = cipher;
+    this.decipher = decipher;
+  }
+
+  decipherConnection(connection) {
+    if (connection.username) {
+      connection.username = this.decipher(connection.username);
+    }
+    if (connection.password) {
+      connection.password = this.decipher(connection.password);
+    }
+    return connection;
   }
 
   async findAll() {
     let connectionsFromDb = await this.nedb.connections.find({});
     connectionsFromDb = connectionsFromDb.map(conn => {
       conn.editable = true;
-      return decipherConnection(conn);
+      return this.decipherConnection(conn);
     });
 
     const allConnections = connectionsFromDb.concat(getConnectionsFromConfig());
@@ -37,7 +38,7 @@ class Connections {
     const connection = await this.nedb.connections.findOne({ _id: id });
     if (connection) {
       connection.editable = true;
-      return decipherConnection(connection);
+      return this.decipherConnection(connection);
     }
 
     // If connection was not found in db try env
@@ -57,8 +58,8 @@ class Connections {
       throw new Error('connections.save() requires a connection');
     }
 
-    connection.username = cipher(connection.username || '');
-    connection.password = cipher(connection.password || '');
+    connection.username = this.cipher(connection.username || '');
+    connection.password = this.cipher(connection.password || '');
 
     if (!connection.createdDate) {
       connection.createdDate = new Date();
diff --git a/server/models/index.js b/server/models/index.js
index b3cd2d54..5a8d8d9f 100644
--- a/server/models/index.js
+++ b/server/models/index.js
@@ -6,14 +6,16 @@ const Queries = require('./queries');
 const Connections = require('./connections');
 const ConnectionAccesses = require('./connectionAccesses');
 
-module.exports = function(nedb) {
-  return {
-    users: new Users(nedb),
-    schemaInfo: new SchemaInfo(nedb),
-    resultCache: new ResultCache(nedb),
-    queryHistory: new QueryHistory(nedb),
-    queries: new Queries(nedb),
-    connections: new Connections(nedb),
-    connectionAccesses: new ConnectionAccesses(nedb)
-  };
-};
+class Models {
+  constructor(nedb, config) {
+    this.users = new Users(nedb, config);
+    this.schemaInfo = new SchemaInfo(nedb, config);
+    this.resultCache = new ResultCache(nedb, config);
+    this.queryHistory = new QueryHistory(nedb, config);
+    this.queries = new Queries(nedb, config);
+    this.connections = new Connections(nedb, config);
+    this.connectionAccesses = new ConnectionAccesses(nedb, config);
+  }
+}
+
+module.exports = Models;
diff --git a/server/models/queries.js b/server/models/queries.js
index 9d4ed9fd..1a6823d6 100644
--- a/server/models/queries.js
+++ b/server/models/queries.js
@@ -44,8 +44,9 @@ const schema = Joi.object({
 });
 
 class Queries {
-  constructor(nedb) {
+  constructor(nedb, config) {
     this.nedb = nedb;
+    this.config = config;
   }
 
   findOneById(id) {
diff --git a/server/models/queryHistory.js b/server/models/queryHistory.js
index 7924facf..61bb8954 100644
--- a/server/models/queryHistory.js
+++ b/server/models/queryHistory.js
@@ -1,5 +1,4 @@
 const Joi = require('@hapi/joi');
-const config = require('../lib/config');
 
 const schema = Joi.object({
   _id: Joi.string().optional(), // generated by nedb
@@ -19,8 +18,9 @@ const schema = Joi.object({
 });
 
 class QueryHistory {
-  constructor(nedb) {
+  constructor(nedb, config) {
     this.nedb = nedb;
+    this.config = config;
   }
 
   findOneById(id) {
@@ -38,12 +38,13 @@ class QueryHistory {
     return this.nedb.queryHistory
       .cfind(filter || {}, {})
       .sort({ startTime: -1 })
-      .limit(config.get('queryHistoryResultMaxRows'))
+      .limit(this.config.get('queryHistoryResultMaxRows'))
       .exec();
   }
 
   async removeOldEntries() {
-    const days = config.get('queryHistoryRetentionTimeInDays') * 86400 * 1000;
+    const days =
+      this.config.get('queryHistoryRetentionTimeInDays') * 86400 * 1000;
     const retentionPeriodStartTime = new Date(new Date().getTime() - days);
 
     return this.nedb.queryHistory.remove(
diff --git a/server/models/resultCache.js b/server/models/resultCache.js
index 6e6783cf..767da940 100644
--- a/server/models/resultCache.js
+++ b/server/models/resultCache.js
@@ -1,4 +1,3 @@
-/* eslint-disable class-methods-use-this */
 const fs = require('fs');
 const path = require('path');
 const moment = require('moment');
@@ -6,8 +5,6 @@ const sanitize = require('sanitize-filename');
 const logger = require('../lib/logger');
 const xlsx = require('node-xlsx');
 const { parse } = require('json2csv');
-const config = require('../lib/config');
-const dbPath = config.get('dbPath');
 
 // This is a workaround till BigInt is fully supported by the standard
 // See https://tc39.es/ecma262/#sec-ecmascript-language-types-bigint-type
@@ -21,20 +18,22 @@ BigInt.prototype.toJSON = function() {
 };
 
 class ResultCache {
-  constructor(nedb) {
+  constructor(nedb, config) {
     this.nedb = nedb;
+    this.config = config;
+    this.dbPath = config.get('dbPath');
   }
 
   xlsxFilePath(cacheKey) {
-    return path.join(dbPath, '/cache/', cacheKey + '.xlsx');
+    return path.join(this.dbPath, '/cache/', cacheKey + '.xlsx');
   }
 
   csvFilePath(cacheKey) {
-    return path.join(dbPath, '/cache/', cacheKey + '.csv');
+    return path.join(this.dbPath, '/cache/', cacheKey + '.csv');
   }
 
   jsonFilePath(cacheKey) {
-    return path.join(dbPath, '/cache/', cacheKey + '.json');
+    return path.join(this.dbPath, '/cache/', cacheKey + '.json');
   }
 
   async findOneByCacheKey(cacheKey) {
diff --git a/server/models/schemaInfo.js b/server/models/schemaInfo.js
index e286f205..13f96daa 100644
--- a/server/models/schemaInfo.js
+++ b/server/models/schemaInfo.js
@@ -3,8 +3,9 @@ function getCacheKey(connectionId) {
 }
 
 class SchemaInfo {
-  constructor(nedb) {
+  constructor(nedb, config) {
     this.nedb = nedb;
+    this.config = config;
   }
 
   /**
diff --git a/server/models/users.js b/server/models/users.js
index bd7fafc0..cda32213 100644
--- a/server/models/users.js
+++ b/server/models/users.js
@@ -21,8 +21,9 @@ const schema = Joi.object({
 });
 
 class Users {
-  constructor(nedb) {
+  constructor(nedb, config) {
     this.nedb = nedb;
+    this.config = config;
   }
 
   async save(data) {
diff --git a/server/server.js b/server/server.js
index f9da527f..cd5f1e0c 100755
--- a/server/server.js
+++ b/server/server.js
@@ -11,9 +11,9 @@ require('./lib/cli-flow.js');
 
 const logger = require('./lib/logger');
 const config = require('./lib/config');
-const { makeNedb } = require('./lib/db');
+const { makeDb, getDb } = require('./lib/db');
 
-const dbPromise = makeNedb(config);
+makeDb(config);
 
 const configValidations = config.getValidations();
 configValidations.warnings.map(warning => logger.warn(warning));
@@ -70,8 +70,8 @@ function detectPortOrSystemd(port) {
 ============================================================================= */
 let server;
 
-async function startServer(nedb) {
-  const app = makeApp(config, nedb);
+async function startServer(models) {
+  const app = makeApp(config, models);
 
   // determine if key pair exists for certs
   if (keyPath && certPath) {
@@ -124,8 +124,8 @@ async function startServer(nedb) {
   server.setTimeout(timeoutSeconds * 1000);
 }
 
-dbPromise
-  .then(nedb => startServer(nedb))
+getDb()
+  .then(db => startServer(db.models))
   .catch(error => {
     logger.error(error, 'Error starting SQLPad');
     process.exit(1);
diff --git a/server/typedefs.js b/server/typedefs.js
index 61cc1f0e..f3dcab33 100644
--- a/server/typedefs.js
+++ b/server/typedefs.js
@@ -1,17 +1,5 @@
-/**
- * A collection of data access objects
- * @typedef {Object} Models
- * @property {import('./models/connectionAccesses')} connectionAccesses - connection accesses DAO
- * @property {import('./models/connections')} connections - connections DAO
- * @property {import('./models/queries')} queries - queries DAO
- * @property {import('./models/queryHistory')} queryHistory - queryHistory DAO
- * @property {import('./models/resultCache')} resultCache - resultCache DAO
- * @property {import('./models/schemaInfo')} schemaInfo - schemaInfo DAO
- * @property {import('./models/users')} users - users DAO
- */
-
 /**
  * The expressjs Request object plus decorations
  * @typedef {Object} Req
- * @property {Models} models - Collection of data access objects
+ * @property {import('./models')} models - Collection of data access objects
  */


Test patch:
diff --git a/server/test/api/password-reset.js b/server/test/api/password-reset.js
index 9c2e9521..f25471bf 100644
--- a/server/test/api/password-reset.js
+++ b/server/test/api/password-reset.js
@@ -1,12 +1,10 @@
 const assert = require('assert');
 const utils = require('../utils');
 const uuid = require('uuid');
-const { getNedb } = require('../../lib/db');
-const getModels = require('../../models');
+const { getDb } = require('../../lib/db');
 
 async function setReset() {
-  const nedb = await getNedb();
-  const models = getModels(nedb);
+  const { models } = await getDb();
   const user = await models.users.findOneByEmail('admin@test.com');
   const passwordResetId = uuid.v4();
   user.passwordResetId = passwordResetId;
diff --git a/server/test/models/connections.js b/server/test/models/connections.js
index ec47afee..a7edc11e 100644
--- a/server/test/models/connections.js
+++ b/server/test/models/connections.js
@@ -1,15 +1,13 @@
 const assert = require('assert');
-const { getNedb } = require('../../lib/db');
+const { getDb } = require('../../lib/db');
 const { getConnectionsFromConfig } = require('../../lib/connectionsFromConfig');
-const getModels = require('../../models');
 
 describe('getConnectionsFromConfig', function() {
-  let nedb;
   let models;
 
   before(async function() {
-    nedb = await getNedb();
-    models = getModels(nedb);
+    const db = await getDb();
+    models = db.models;
   });
 
   it('handles empty object', function() {
diff --git a/server/test/utils.js b/server/test/utils.js
index c9cdf17d..30b62a3c 100644
--- a/server/test/utils.js
+++ b/server/test/utils.js
@@ -1,12 +1,11 @@
 const assert = require('assert');
 const request = require('supertest');
-const getModels = require('../models');
 const consts = require('../lib/consts');
 const config = require('../lib/config');
-const { makeNedb } = require('../lib/db');
+const { makeDb, getDb } = require('../lib/db');
 const makeApp = require('../app');
 
-const dbPromise = makeNedb(config);
+makeDb(config);
 let app;
 
 const users = {
@@ -29,7 +28,7 @@ function expectKeys(data, expectedKeys) {
 }
 
 async function reset() {
-  const nedb = await dbPromise;
+  const { nedb } = await getDb();
   return Promise.all([
     nedb.users.remove({}, { multi: true }),
     nedb.queries.remove({}, { multi: true }),
@@ -51,8 +50,7 @@ async function reset() {
 
 async function resetWithUser() {
   await reset();
-  const nedb = await dbPromise;
-  const models = getModels(nedb);
+  const { models } = await getDb();
   const saves = Object.keys(users).map(key => {
     return models.users.save(users[key]);
   });
@@ -69,7 +67,6 @@ function addAuth(req, role) {
 }
 
 async function del(role, url, statusCode = 200) {
-  await dbPromise;
   let req = request(app).delete(url);
   req = addAuth(req, role);
   const response = await req.expect(statusCode);
@@ -77,7 +74,6 @@ async function del(role, url, statusCode = 200) {
 }
 
 async function get(role, url, statusCode = 200) {
-  await dbPromise;
   let req = request(app).get(url);
   req = addAuth(req, role);
   const response = await req.expect(statusCode);
@@ -85,7 +81,6 @@ async function get(role, url, statusCode = 200) {
 }
 
 async function post(role, url, body, statusCode = 200) {
-  await dbPromise;
   let req = request(app).post(url);
   req = addAuth(req, role);
   const response = await req.send(body).expect(statusCode);
@@ -93,7 +88,6 @@ async function post(role, url, body, statusCode = 200) {
 }
 
 async function put(role, url, body, statusCode = 200) {
-  await dbPromise;
   let req = request(app).put(url);
   req = addAuth(req, role);
   const response = await req.send(body).expect(statusCode);
@@ -101,11 +95,11 @@ async function put(role, url, body, statusCode = 200) {
 }
 
 before(async function() {
-  const nedb = await dbPromise;
-  app = makeApp(config, nedb);
+  const { models } = await getDb();
+  app = makeApp(config, models);
 
   assert.throws(() => {
-    makeNedb(config);
+    makeDb(config);
   }, 'ensure nedb can be made once');
 
   return resetWithUser();