refactor: use postgres connection pool (#2106)

* refactor: use postgres connection pool * enable modern build * use spread operator for postgres query args * bump server size limit --------- Co-authored-by: Lutz Biedinger <[email protected]>
europeana · Oct 26, 2023 · 0be7a6b · 0be7a6b
1 parent 54fc84f
commit 0be7a6b
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 98 deletions.
diff --git a/packages/portal/nuxt.config.js b/packages/portal/nuxt.config.js
@@ -42,7 +42,8 @@ const redisConfig = () => {
 const postgresConfig = () => {
   const postgresOptions = {
     enabled: featureIsEnabled('eventLogging'),
-    connectionString: process.env.POSTGRES_URL
+    connectionString: process.env.POSTGRES_URL,
+    max: Number(process.env.POSTGRES_MAX || 10)
   };
 
   if (process.env.POSTGRES_SSL_CA) {

diff --git a/packages/portal/src/server-middleware/api/events/log.js b/packages/portal/src/server-middleware/api/events/log.js
@@ -1,67 +1,57 @@
-import { Client } from 'pg';
 import isbot from 'isbot';
+import pg from './pg.js';
 
 // TODO: use `next` for error handling
-// TODO: end pg conn when done?
 // TODO: accept multiple uris for the same action
 // TODO: log user agent?
 // TODO: validate action_types
-export default (options = {}) => {
-  let client;
+export default (config = {}) => {
+  pg.config = config;
 
   return async(req, res) => {
     try {
       // Respond early as clients don't need to wait for the results of this logging
       res.sendStatus(204);
 
-      if (!options.enabled || isbot(req.get('user-agent'))) {
+      if (!pg.enabled || isbot(req.get('user-agent'))) {
         return;
       }
 
-      if (!client) {
-        client = new Client(options);
-        client.on('error', (err) => {
-          console.error('PostgreSQL client error', err);
-          client = null;
-        });
-        await client.connect();
-      }
-
       const { actionType, objectUri, sessionId } = req.body;
 
       let objectRow;
-      const selectObjectResult = await client.query(
+      const selectObjectResult = await pg.query(
         'SELECT id FROM events.objects WHERE uri=$1',
         [objectUri]
       );
 
       if (selectObjectResult.rowCount > 0) {
         objectRow = selectObjectResult.rows[0];
       } else {
-        const insertObjectResult = await client.query(
+        const insertObjectResult = await pg.query(
           'INSERT INTO events.objects (uri) VALUES($1) RETURNING id',
           [objectUri]
         );
         objectRow = insertObjectResult.rows[0];
       }
 
       let sessionRow;
-      const selectSessionResult = await client.query(
+      const selectSessionResult = await pg.query(
         'SELECT id FROM events.sessions WHERE uuid=$1',
         [sessionId]
       );
 
       if (selectSessionResult.rowCount > 0) {
         sessionRow = selectSessionResult.rows[0];
       } else {
-        const insertSessionResult = await client.query(
+        const insertSessionResult = await pg.query(
           'INSERT INTO events.sessions (uuid) VALUES($1) RETURNING id',
           [sessionId]
         );
         sessionRow = insertSessionResult.rows[0];
       }
 
-      const selectActionResult = await client.query(`
+      const selectActionResult = await pg.query(`
         SELECT a.id FROM events.actions a LEFT JOIN events.action_types at
           ON a.action_type_id=at.id
         WHERE a.object_id=$1
@@ -77,7 +67,7 @@ export default (options = {}) => {
         return;
       }
 
-      await client.query(`
+      await pg.query(`
         INSERT INTO events.actions (object_id, action_type_id, session_id, occurred_at)
         SELECT $1, at.id, $2, current_timestamp
         FROM events.action_types at

diff --git a/packages/portal/src/server-middleware/api/events/pg.js b/packages/portal/src/server-middleware/api/events/pg.js
@@ -0,0 +1,26 @@
+import { Pool } from 'pg';
+
+let pool;
+
+export default {
+  config: {},
+
+  get enabled() {
+    return this.config.enabled;
+  },
+
+  get pool() {
+    if (!pool) {
+      pool = new Pool(this.config);
+      pool.on('error', (err) => {
+        console.error('PostgreSQL pool error', err);
+        pool = null;
+      });
+    }
+    return pool;
+  },
+
+  query() {
+    return this.pool.query(...arguments);
+  }
+};
diff --git a/packages/portal/src/server-middleware/api/events/trending.js b/packages/portal/src/server-middleware/api/events/trending.js
@@ -1,27 +1,17 @@
-import { Client } from 'pg';
+import pg from './pg.js';
 
 // TODO: use `next` for error handling
-// TODO: end pg conn when done?
-export default (options = {}) => {
-  let client;
+export default (config = {}) => {
+  pg.config = config;
 
   return async(req, res) => {
     try {
-      if (!options.enabled) {
+      if (!pg.enabled) {
         res.json({ items: [] });
         return;
       }
 
-      if (!client) {
-        client = new Client(options);
-        client.on('error', (err) => {
-          console.error('PostgreSQL client error', err);
-          client = null;
-        });
-        await client.connect();
-      }
-
-      const selectObjectResult = await client.query(`
+      const selectObjectResult = await pg.query(`
         SELECT o.uri, COUNT(a.id) AS num
         FROM events.actions a
         LEFT JOIN events.objects o ON a.object_id=o.id

diff --git a/packages/portal/src/server-middleware/api/utils.js b/packages/portal/src/server-middleware/api/utils.js
@@ -1,3 +1,7 @@
+import defu  from 'defu';
+
+import nuxtConfig from '../../../nuxt.config.js';
+
 export const errorHandler = (res, error) => {
   let status = error.status || 500;
   let message = error.message;
@@ -9,3 +13,12 @@ export const errorHandler = (res, error) => {
 
   res.status(status).set('Content-Type', 'text/plain').send(message);
 };
+
+export const nuxtRuntimeConfig = (key) => {
+  const runtimeConfig = defu(nuxtConfig.privateRuntimeConfig, nuxtConfig.publicRuntimeConfig);
+  if (key) {
+    return runtimeConfig[key];
+  } else {
+    return runtimeConfig;
+  }
+};
diff --git a/packages/portal/tests/size/.size-limit.json b/packages/portal/tests/size/.size-limit.json
@@ -19,7 +19,7 @@
   {
     "name": "portal.js server",
     "running": false,
-    "limit": "845 KB",
+    "limit": "850 KB",
     "path": [
       ".nuxt/dist/server/*.js"
     ]

diff --git a/packages/portal/tests/unit/server-middleware/api/events/log.spec.js b/packages/portal/tests/unit/server-middleware/api/events/log.spec.js
@@ -1,7 +1,7 @@
 import pg from 'pg';
 import sinon from 'sinon';
 
-import logEventsMiddleware from '@/server-middleware/api/events/log';
+import logEventsHandler from '@/server-middleware/api/events/log';
 
 const fixtures = {
   db: {
@@ -15,35 +15,33 @@ const fixtures = {
   }
 };
 
-const pgClientConnect = sinon.stub();
-const pgClientOn = sinon.stub();
-const pgClientQuery = sinon.stub();
-pgClientQuery.withArgs(
+const pgPoolQuery = sinon.stub();
+pgPoolQuery.withArgs(
   sinon.match((sql) => sql.startsWith('SELECT id FROM events.objects ')),
   [fixtures.reqBody.objectUri]
 )
   .resolves({ rowCount: 0 });
-pgClientQuery.withArgs(
+pgPoolQuery.withArgs(
   sinon.match((sql) => sql.startsWith('INSERT INTO events.objects ')),
   [fixtures.reqBody.objectUri]
 )
   .resolves({ rows: [{ id: fixtures.db.objectId }] });
-pgClientQuery.withArgs(
+pgPoolQuery.withArgs(
   sinon.match((sql) => sql.startsWith('SELECT id FROM events.sessions ')),
   [fixtures.reqBody.sessionId]
 )
   .resolves({ rowCount: 0 });
-pgClientQuery.withArgs(
+pgPoolQuery.withArgs(
   sinon.match((sql) => sql.startsWith('INSERT INTO events.sessions ')),
   [fixtures.reqBody.sessionId]
 )
   .resolves({ rows: [{ id: fixtures.db.sessionId }] });
-pgClientQuery.withArgs(
+pgPoolQuery.withArgs(
   sinon.match((sql) => sql.trim().startsWith('SELECT a.id FROM events.actions a LEFT JOIN events.action_types at')),
   [fixtures.db.objectId, fixtures.reqBody.actionType, fixtures.db.sessionId]
 )
   .resolves({ rowCount: 0 });
-pgClientQuery.withArgs(
+pgPoolQuery.withArgs(
   sinon.match((sql) => sql.trim().startsWith('INSERT INTO events.actions ')),
   [fixtures.db.objectId, fixtures.db.sessionId, fixtures.reqBody.actionType]
 )
@@ -60,24 +58,22 @@ const expressResStub = {
 
 describe('@/server-middleware/api/events/log', () => {
   beforeAll(() => {
-    sinon.replace(pg.Client.prototype, 'connect', pgClientConnect);
-    sinon.replace(pg.Client.prototype, 'on', pgClientOn);
-    sinon.replace(pg.Client.prototype, 'query', pgClientQuery);
+    sinon.replace(pg.Pool.prototype, 'query', pgPoolQuery);
   });
   afterEach(sinon.resetHistory);
   afterAll(sinon.resetBehavior);
 
   describe('when not explicitly enabled', () => {
     const options = {};
 
-    it('does not connect to postgres', async() => {
-      await logEventsMiddleware(options)(expressReqStub, expressResStub);
+    it('does not query postgres', async() => {
+      await logEventsHandler(options)(expressReqStub, expressResStub);
 
-      expect(pgClientConnect.called).toBe(false);
+      expect(pgPoolQuery.called).toBe(false);
     });
 
     it('responds with 204 status', async() => {
-      await logEventsMiddleware(options)(expressReqStub, expressResStub);
+      await logEventsHandler(options)(expressReqStub, expressResStub);
 
       expect(expressResStub.sendStatus.calledWith(204)).toBe(true);
     });
@@ -91,14 +87,14 @@ describe('@/server-middleware/api/events/log', () => {
         get: sinon.stub().withArgs('user-agent').returns('search engine bot')
       };
 
-      it('does not connect to postgres', async() => {
-        await logEventsMiddleware(options)(expressReqStub, expressResStub);
+      it('does not query postgres', async() => {
+        await logEventsHandler(options)(expressReqStub, expressResStub);
 
-        expect(pgClientConnect.called).toBe(false);
+        expect(pgPoolQuery.called).toBe(false);
       });
 
       it('responds with 204 status', async() => {
-        await logEventsMiddleware(options)(expressReqStub, expressResStub);
+        await logEventsHandler(options)(expressReqStub, expressResStub);
 
         expect(expressResStub.sendStatus.calledWith(204)).toBe(true);
       });
@@ -109,26 +105,15 @@ describe('@/server-middleware/api/events/log', () => {
         body: fixtures.reqBody,
         get: sinon.spy()
       };
-      it('connects to postgres', async() => {
-        await logEventsMiddleware(options)(expressReqStub, expressResStub);
-
-        expect(pgClientConnect.called).toBe(true);
-      });
-
-      it('registers postgres error handler', async() => {
-        await logEventsMiddleware(options)(expressReqStub, expressResStub);
-
-        expect(pgClientOn.calledWith('error', sinon.match.func)).toBe(true);
-      });
 
       it('runs all postgres queries to log event', async() => {
-        await logEventsMiddleware(options)(expressReqStub, expressResStub);
+        await logEventsHandler(options)(expressReqStub, expressResStub);
 
-        expect(pgClientQuery.getCalls().length).toBe(6);
+        expect(pgPoolQuery.getCalls().length).toBe(6);
       });
 
       it('responds with 204 status', async() => {
-        await logEventsMiddleware(options)(expressReqStub, expressResStub);
+        await logEventsHandler(options)(expressReqStub, expressResStub);
 
         expect(expressResStub.sendStatus.calledWith(204)).toBe(true);
       });

diff --git a/packages/portal/tests/unit/server-middleware/api/events/pg.spec.js b/packages/portal/tests/unit/server-middleware/api/events/pg.spec.js
@@ -0,0 +1,36 @@
+import pg from 'pg';
+import sinon from 'sinon';
+
+import pgEvents from '@/server-middleware/api/events/pg';
+
+const pgPoolOn = sinon.stub();
+const pgPoolQuery = sinon.stub();
+
+describe('@/server-middleware/api/events/pg', () => {
+  beforeAll(() => {
+    sinon.replace(pg.Pool.prototype, 'on', pgPoolOn);
+    sinon.replace(pg.Pool.prototype, 'query', pgPoolQuery);
+  });
+  afterEach(sinon.resetHistory);
+  afterAll(sinon.resetBehavior);
+
+  describe('pool', () => {
+    it('creates and returns a pg pool, with error handler', () => {
+      const pool = pgEvents.pool;
+
+      expect(pool instanceof pg.Pool).toBe(true);
+      expect(pgPoolOn.calledWith('error', sinon.match.func)).toBe(true);
+    });
+  });
+
+  describe('query', () => {
+    it('delegates with all args to pg pool', () => {
+      const sql = 'SELECT * FROM table WHERE type=$1';
+      const params = ['like'];
+
+      pgEvents.query(sql, params);
+
+      expect(pgPoolQuery.calledWith(sql, params)).toBe(true);
+    });
+  });
+});