Repository: rickbergfalk/sqlpad
Pull request #814 (2020-08-03 23:18:07)

Configurable query result store
Allows query results to be stored in the backing database (SQLite, Postgres, etc.), file system (default), redis, or an in-memory store via `SQLPAD_QUERY_RESULT_STORE`. 

The store settings are the same values as `SQLPAD_SESSION_STORE`. 

The in-memory option (as well as `database` when using an in-memory SQLite database) could be problematic depending on data volume and real-world use.


Patch:
diff --git a/docs/configuration.md b/docs/configuration.md
index 0de74bb8..98e540f6 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -55,6 +55,12 @@ SQLPAD_SESSION_MINUTES = 60
 # `database` will use whatever backend database is used (or SQLite if SQLPAD_DB_PATH is set)
 SQLPAD_SESSION_STORE = "file"
 
+# Similar to session storage, query result storage may also be configured.
+# Valid values are `file` (default), `database`, `redis`, `memory`
+# If set to memory, store is limited to 1000 entries with a max age of 1 hour
+# Other storage mechanisms fall back to SQLPAD_QUERY_HISTORY_RETENTION_PERIOD_IN_DAYS
+SQLPAD_QUERY_RESULT_STORE = "file"
+
 # Name used for cookie. If running multiple SQLPads on same domain, set to different values.
 SQLPAD_COOKIE_NAME = "sqlpad.sid"
 
diff --git a/server/app.js b/server/app.js
index 0e8df5e2..5fe3c6f3 100644
--- a/server/app.js
+++ b/server/app.js
@@ -142,13 +142,7 @@ async function makeApp(config, models) {
       break;
     }
     case 'redis': {
-      const redisUri = config.get('redisUri');
-      if (!redisUri) {
-        throw new Error(
-          `Redis session store requires SQLPAD_REDIS_URI to be set`
-        );
-      }
-      const redisClient = redis.createClient(redisUri);
+      const redisClient = redis.createClient(config.get('redisUri'));
       sessionOptions.store = new RedisStore({ client: redisClient });
       sessionOptions.resave = false;
       break;
diff --git a/server/lib/config/config-items.js b/server/lib/config/config-items.js
index 5892f26b..b9507c32 100644
--- a/server/lib/config/config-items.js
+++ b/server/lib/config/config-items.js
@@ -267,6 +267,11 @@ const configItems = [
     envVar: 'SQLPAD_QUERY_RESULT_MAX_ROWS',
     default: 50000,
   },
+  {
+    key: 'queryResultStore',
+    envVar: 'SQLPAD_QUERY_RESULT_STORE',
+    default: 'file', // allowed values file, memory, database
+  },
   {
     key: 'slackWebhook',
     envVar: 'SQLPAD_SLACK_WEBHOOK',
diff --git a/server/lib/config/index.js b/server/lib/config/index.js
index b130aed4..cb6d3abc 100644
--- a/server/lib/config/index.js
+++ b/server/lib/config/index.js
@@ -70,6 +70,29 @@ class Config {
       errors.push(getOldConfigWarning());
     }
 
+    const allowedStores = ['redis', 'database', 'file', 'memory'];
+    if (!allowedStores.includes(this.all.sessionStore)) {
+      errors.push(
+        `SQLPAD_SESSION_STORE must be one of ${allowedStores.join(', ')}`
+      );
+    }
+
+    if (!allowedStores.includes(this.all.queryResultStore)) {
+      errors.push(
+        `SQLPAD_QUERY_RESULT_STORE must be one of ${allowedStores.join(', ')}`
+      );
+    }
+
+    if (this.all.queryResultStore === 'redis' && !this.all.redisUri) {
+      errors.push(
+        `Redis query result store requires SQLPAD_REDIS_URI to be set`
+      );
+    }
+
+    if (this.all.sessionStore === 'redis' && !this.all.redisUri) {
+      errors.push(`Redis session store requires SQLPAD_REDIS_URI to be set`);
+    }
+
     // If fileConfig has something in it, an INI or JSON file was read
     // These file formats are deprecated, replaced by .env file and env vars
     if (this.configFilePath && Object.keys(this.fileConfig).length > 0) {
diff --git a/server/models/statements.js b/server/models/statements.js
index 3d54cbc0..ffcfed09 100644
--- a/server/models/statements.js
+++ b/server/models/statements.js
@@ -3,6 +3,9 @@ const util = require('util');
 const path = require('path');
 const fs = require('fs');
 const mkdirp = require('mkdirp');
+const { promisify } = require('util');
+const LRU = require('lru-cache');
+const redis = require('redis');
 const { Op } = require('sequelize');
 const ensureJson = require('./ensure-json');
 const writeFile = util.promisify(fs.writeFile);
@@ -10,6 +13,10 @@ const readFile = util.promisify(fs.readFile);
 const unlink = util.promisify(fs.unlink);
 const access = util.promisify(fs.access);
 
+function redisDbKey(id) {
+  return `statement-result/${id}`;
+}
+
 class Statements {
   /**
    * @param {import('../sequelize-db')} sequelizeDb
@@ -18,6 +25,35 @@ class Statements {
   constructor(sequelizeDb, config) {
     this.sequelizeDb = sequelizeDb;
     this.config = config;
+    this.queryResultStore = config.get('queryResultStore');
+
+    if (this.queryResultStore === 'redis') {
+      const client = redis.createClient(config.get('redisUri'));
+      this.redisClient = client;
+      this.redisGetAsync = promisify(client.get).bind(client);
+      this.redisSetexAsync = promisify(client.setex).bind(client);
+      this.redisDelAsync = promisify(client.del).bind(client);
+    }
+
+    if (this.queryResultStore === 'memory') {
+      this.memoryCache = new LRU({ max: 1000, maxAge: 1000 * 60 * 60 });
+    }
+  }
+
+  isFileStore() {
+    return this.queryResultStore === 'file';
+  }
+
+  isDatabaseStore() {
+    return this.queryResultStore === 'database';
+  }
+
+  isRedisStore() {
+    return this.queryResultStore === 'redis';
+  }
+
+  isMemoryStore() {
+    return this.queryResultStore === 'memory';
   }
 
   async findOneById(id) {
@@ -46,11 +82,15 @@ class Statements {
     return items;
   }
 
+  /**
+   * Remove statement by id
+   * @param {string} id - statement id
+   */
   async removeById(id) {
     const statement = await this.findOneById(id);
     const { resultsPath } = statement;
 
-    if (resultsPath) {
+    if (this.isFileStore() && resultsPath) {
       const dbPath = this.config.get('dbPath');
       const fullPath = path.join(dbPath, resultsPath);
 
@@ -66,6 +106,18 @@ class Statements {
       }
     }
 
+    if (this.isRedisStore()) {
+      await this.redisDelAsync(redisDbKey(id));
+    }
+
+    if (this.isMemoryStore()) {
+      this.memoryCache.del(id);
+    }
+
+    if (this.isDatabaseStore()) {
+      await this.sequelizeDb.Cache.destroy({ where: { id: redisDbKey(id) } });
+    }
+
     return this.sequelizeDb.Statements.destroy({ where: { id } });
   }
 
@@ -90,21 +142,57 @@ class Statements {
   }
 
   async updateFinished(id, queryResult, stopTime, durationMs) {
-    const dbPath = this.config.get('dbPath');
+    const { config } = this;
+    const dbPath = config.get('dbPath');
     const rowCount = queryResult.rows.length;
 
     let resultsPath;
 
     // If rows returned write results csv
     if (rowCount > 0) {
-      const dir = id.slice(0, 3);
-      await mkdirp(path.join(dbPath, 'results', dir));
-      resultsPath = path.join('results', dir, `${id}.json`);
-      const fullPath = path.join(dbPath, resultsPath);
       const arrOfArr = queryResult.rows.map((row) => {
         return queryResult.columns.map((col) => row[col.name]);
       });
-      await writeFile(fullPath, JSON.stringify(arrOfArr));
+
+      if (this.isFileStore()) {
+        const dir = id.slice(0, 3);
+        await mkdirp(path.join(dbPath, 'results', dir));
+        resultsPath = path.join('results', dir, `${id}.json`);
+        const fullPath = path.join(dbPath, resultsPath);
+        await writeFile(fullPath, JSON.stringify(arrOfArr));
+      }
+
+      if (this.isRedisStore()) {
+        // Redis results can be removed by redis itself
+        // In the event seconds does not exist or is zero, default to 1 hour
+        let seconds =
+          parseInt(config.get('queryHistoryRetentionTimeInDays'), 10) * 86400;
+        if (!seconds || seconds <= 0) {
+          seconds = 60 * 60;
+        }
+        await this.redisSetexAsync(
+          redisDbKey(id),
+          seconds,
+          JSON.stringify(arrOfArr)
+        );
+      }
+
+      if (this.isDatabaseStore()) {
+        const ONE_DAY = 1000 * 60 * 60 * 24;
+        const daysMs =
+          parseInt(config.get('queryHistoryRetentionTimeInDays'), 10) * ONE_DAY;
+        const expiryDate = new Date(Date.now() + daysMs);
+        await this.sequelizeDb.Cache.create({
+          id: redisDbKey(id),
+          data: arrOfArr,
+          expiryDate,
+          name: 'statement results',
+        });
+      }
+
+      if (this.isMemoryStore()) {
+        this.memoryCache.set(id, arrOfArr);
+      }
     }
 
     const update = {
@@ -125,29 +213,66 @@ class Statements {
     if (!statement) {
       throw new Error('Statement not found');
     }
-    const { resultsPath } = statement;
 
-    // If no result path the query had no rows.
-    // Return empty array
-    if (!resultsPath) {
+    const { config } = this;
+
+    if (this.isFileStore()) {
+      const { resultsPath } = statement;
+
+      // If no result path the query had no rows.
+      // Return empty array
+      if (!resultsPath) {
+        return [];
+      }
+
+      const fullPath = path.join(config.get('dbPath'), resultsPath);
+
+      let exists = true;
+      try {
+        await access(fullPath);
+      } catch (error) {
+        exists = false;
+      }
+
+      if (exists) {
+        const fileData = await readFile(fullPath, 'utf8');
+        return JSON.parse(fileData);
+      }
+
       return [];
     }
 
-    const fullPath = path.join(this.config.get('dbPath'), resultsPath);
-
-    let exists = true;
-    try {
-      await access(fullPath);
-    } catch (error) {
-      exists = false;
+    if (this.isRedisStore()) {
+      const json = await this.redisGetAsync(redisDbKey(statement.id));
+      if (json) {
+        const parsed = JSON.parse(json);
+        if (Array.isArray(parsed)) {
+          return parsed;
+        }
+      }
+      return [];
     }
 
-    if (exists) {
-      const fileData = await readFile(fullPath, 'utf8');
-      return JSON.parse(fileData);
+    if (this.isDatabaseStore()) {
+      const doc = await this.sequelizeDb.Cache.findOne({
+        where: { id: redisDbKey(statement.id) },
+      });
+      if (doc) {
+        const result = ensureJson(doc.data);
+        if (Array.isArray(result)) {
+          return result;
+        }
+      }
+      return [];
     }
 
-    return [];
+    if (this.isMemoryStore()) {
+      const result = this.memoryCache.get(statement.id);
+      if (Array.isArray(result)) {
+        return result;
+      }
+      return [];
+    }
   }
 
   async removeOldEntries() {
diff --git a/server/package-lock.json b/server/package-lock.json
index 12de3519..3f213c34 100644
--- a/server/package-lock.json
+++ b/server/package-lock.json
@@ -2367,6 +2367,16 @@
         "gtoken": "^4.1.0",
         "jws": "^4.0.0",
         "lru-cache": "^5.0.0"
+      },
+      "dependencies": {
+        "lru-cache": {
+          "version": "5.1.1",
+          "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-5.1.1.tgz",
+          "integrity": "sha512-KpNARQA3Iwv+jTA0utUVVbrh+Jlrr1Fv0e56GGzAFOXN7dk/FviaDW8LHmK52DlcH4WP2n6gI8vN1aesBFgo9w==",
+          "requires": {
+            "yallist": "^3.0.2"
+          }
+        }
       }
     },
     "google-p12-pem": {
@@ -3195,11 +3205,18 @@
       "integrity": "sha1-n6GAux2VAM3CnEFWdmoZleH0Uk8="
     },
     "lru-cache": {
-      "version": "5.1.1",
-      "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-5.1.1.tgz",
-      "integrity": "sha512-KpNARQA3Iwv+jTA0utUVVbrh+Jlrr1Fv0e56GGzAFOXN7dk/FviaDW8LHmK52DlcH4WP2n6gI8vN1aesBFgo9w==",
+      "version": "6.0.0",
+      "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz",
+      "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==",
       "requires": {
-        "yallist": "^3.0.2"
+        "yallist": "^4.0.0"
+      },
+      "dependencies": {
+        "yallist": {
+          "version": "4.0.0",
+          "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz",
+          "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A=="
+        }
       }
     },
     "mariadb": {
@@ -3609,6 +3626,14 @@
           "version": "4.0.0",
           "resolved": "https://registry.npmjs.org/long/-/long-4.0.0.tgz",
           "integrity": "sha512-XsP+KhQif4bjX1kbuSiySJFNAehNxgLb6hPRGJ9QsUr8ajHkuXGdrHmFUTUUXhDwVX2R5bY4JNZEwbUiMhV+MA=="
+        },
+        "lru-cache": {
+          "version": "5.1.1",
+          "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-5.1.1.tgz",
+          "integrity": "sha512-KpNARQA3Iwv+jTA0utUVVbrh+Jlrr1Fv0e56GGzAFOXN7dk/FviaDW8LHmK52DlcH4WP2n6gI8vN1aesBFgo9w==",
+          "requires": {
+            "yallist": "^3.0.2"
+          }
         }
       }
     },
diff --git a/server/package.json b/server/package.json
index 6ab1ee5d..15d6c0a3 100644
--- a/server/package.json
+++ b/server/package.json
@@ -59,6 +59,7 @@
     "ini": "^1.3.5",
     "jsonwebtoken": "^8.5.1",
     "lodash": "^4.17.19",
+    "lru-cache": "^6.0.0",
     "mariadb": "^2.4.2",
     "memorystore": "^1.6.2",
     "minimist": "^1.2.5",


Test patch:
diff --git a/server/test/api/batches.js b/server/test/api/batches.js
index 08bf8221..ad52f569 100644
--- a/server/test/api/batches.js
+++ b/server/test/api/batches.js
@@ -39,6 +39,7 @@ describe('api/batches', function () {
     utils = new TestUtils({
       queryResultMaxRows: 3,
       queryHistoryRetentionTimeInDays: 0,
+      queryResultStore: 'file',
     });
     await utils.init(true);
 
diff --git a/server/test/api/query-result-stores.js b/server/test/api/query-result-stores.js
new file mode 100644
index 00000000..7179c483
--- /dev/null
+++ b/server/test/api/query-result-stores.js
@@ -0,0 +1,77 @@
+/* eslint-disable no-await-in-loop */
+const assert = require('assert');
+const TestUtils = require('../utils');
+
+const query1 = `SELECT 1 AS id, 'blue' AS color`;
+
+function wait(ms) {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
+async function testBatchToCompletion(config) {
+  const utils = new TestUtils(config);
+  await utils.init(true);
+
+  const connection = await utils.post('admin', '/api/connections', {
+    name: 'test connection',
+    driver: 'sqlite',
+    data: {
+      filename: './test/fixtures/sales.sqlite',
+    },
+  });
+
+  let batch = await utils.post('admin', `/api/batches`, {
+    connectionId: connection.id,
+    batchText: query1,
+  });
+  while (batch.status !== 'finished' && batch.status !== 'error') {
+    await wait(25);
+    batch = await utils.get('admin', `/api/batches/${batch.id}`);
+  }
+
+  const statements = await utils.get(
+    'admin',
+    `/api/batches/${batch.id}/statements`
+  );
+
+  const statement1 = statements[0];
+
+  let result1 = await utils.get(
+    'admin',
+    `/api/statements/${statement1.id}/results`
+  );
+  assert.deepEqual(result1, [[1, 'blue']], 'results as expected');
+
+  // remove should succeed
+  await utils.models.statements.removeById(statement1.id);
+  await utils.get('admin', `/api/statements/${statement1.id}/results`, 404);
+}
+
+describe('api/query-result-stores', function () {
+  it('file', async function () {
+    return testBatchToCompletion({ queryResultStore: 'file' });
+  });
+
+  it('redis', async function () {
+    const available = await TestUtils.redisAvailable('redis://localhost:6379');
+    if (!available) {
+      return this.skip();
+    }
+    return testBatchToCompletion({
+      queryResultStore: 'redis',
+      redisUri: 'redis://localhost:6379',
+    });
+  });
+
+  it('database', async function () {
+    return testBatchToCompletion({
+      queryResultStore: 'database',
+    });
+  });
+
+  it('memory', async function () {
+    return testBatchToCompletion({
+      queryResultStore: 'memory',
+    });
+  });
+});
diff --git a/server/test/auth/session-stores.js b/server/test/auth/session-stores.js
index 4e9f7eec..a7dbe61b 100644
--- a/server/test/auth/session-stores.js
+++ b/server/test/auth/session-stores.js
@@ -1,9 +1,9 @@
 const assert = require('assert');
 const request = require('supertest');
-const TestUtil = require('../utils');
+const TestUtils = require('../utils');
 
 async function testSessionStore(sessionStore, additionalOpts = {}) {
-  const utils = new TestUtil({
+  const utils = new TestUtils({
     authProxyEnabled: true,
     authProxyAutoSignUp: true,
     authProxyDefaultRole: 'admin',
@@ -40,14 +40,10 @@ describe('auth/session-stores', function () {
   });
 
   it('redis', async function () {
+    const available = await TestUtils.redisAvailable('redis://localhost:6379');
+    if (!available) {
+      return this.skip();
+    }
     return testSessionStore('redis', { redisUri: 'redis://localhost:6379' });
   });
-
-  it('redis - Error throws if no URI', async function () {
-    assert.rejects(() => testSessionStore('redis'));
-  });
-
-  it('throws for unknown sessionStore', async function () {
-    assert.rejects(() => testSessionStore('not-real-option'));
-  });
 });
diff --git a/server/test/lib/config.js b/server/test/lib/config.js
index bdd72e6c..9641e0e3 100644
--- a/server/test/lib/config.js
+++ b/server/test/lib/config.js
@@ -6,6 +6,14 @@ const fromCli = require('../../lib/config/from-cli');
 const fromFile = require('../../lib/config/from-file');
 const Config = require('../../lib/config');
 
+function configHasError(args, errorFindFunction) {
+  const config = new Config(args, {});
+  const validations = config.getValidations();
+  assert(validations.errors);
+  const found = validations.errors.find(errorFindFunction);
+  assert(found);
+}
+
 describe('lib/config/from-default', function () {
   it('provides expected values', function () {
     const conf = fromDefault();
@@ -70,17 +78,34 @@ describe('lib/config/fromFile', function () {
   });
 
   it('Errors for old config file key', function () {
-    const config = new Config(
+    configHasError(
       { config: path.join(__dirname, '../fixtures/old-config.json') },
-      {}
-    );
-    const validations = config.getValidations();
-    assert(validations.errors);
-    const found = validations.errors.find(
       (error) =>
         error.includes('cert-passphrase') && error.includes('NOT RECOGNIZED')
     );
-    assert(found, 'has error about old key');
+  });
+
+  it('Error: Unknown session store', function () {
+    configHasError({ sessionStore: 'not-real-store' }, (error) =>
+      error.includes('SQLPAD_SESSION_STORE must be one of')
+    );
+  });
+
+  it('Error: Unknown query result store', function () {
+    configHasError({ queryResultStore: 'not-real-store' }, (error) =>
+      error.includes('SQLPAD_QUERY_RESULT_STORE must be one of')
+    );
+  });
+
+  it('Error: redis store requires redis URI', function () {
+    configHasError({ sessionStore: 'redis' }, (error) =>
+      error.includes('Redis session store requires SQLPAD_REDIS_URI to be set')
+    );
+    configHasError({ queryResultStore: 'redis' }, (error) =>
+      error.includes(
+        'Redis query result store requires SQLPAD_REDIS_URI to be set'
+      )
+    );
   });
 
   it('Deprecated warning for json/ini config file', function () {
diff --git a/server/test/utils.js b/server/test/utils.js
index 48d8fb28..044355bd 100644
--- a/server/test/utils.js
+++ b/server/test/utils.js
@@ -3,6 +3,7 @@ const { v4: uuidv4 } = require('uuid');
 const rimraf = require('rimraf');
 const mkdirp = require('mkdirp');
 const path = require('path');
+const redis = require('redis');
 const request = require('supertest');
 const detectPort = require('detect-port');
 const bodyParser = require('body-parser');
@@ -31,6 +32,7 @@ class TestUtils {
         dbPath: path.join(__dirname, '/artifacts/defaultdb'),
         dbInMemory: true,
         sessionStore: 'memory',
+        queryResultStore: 'memory',
         appLogLevel: 'error',
         backendDatabaseUri: TestUtils.randomize_dbname(
           process.env.SQLPAD_BACKEND_DB_URI
@@ -75,6 +77,20 @@ class TestUtils {
     };
   }
 
+  static redisAvailable(redisUri) {
+    return new Promise((resolve) => {
+      const client = redis.createClient(redisUri);
+      client.on('error', () => {
+        resolve(false);
+        client.end(true);
+      });
+      client.on('connect', () => {
+        resolve(true);
+        client.end(true);
+      });
+    });
+  }
+
   static async makeHookServer() {
     const responses = [];
     const port = await detectPort(4000);