From 7de68695febea6fb97df58585b6efe1a915c6075 Mon Sep 17 00:00:00 2001 From: zneix <44851575+zneix@users.noreply.github.com> Date: Wed, 2 Sep 2020 16:16:54 +0200 Subject: [PATCH] Replaced all callbacks with promises in document stores --- lib/document_handler.js | 131 ++++++++++++++------------------- lib/document_stores/file.js | 18 ++--- lib/document_stores/mongodb.js | 89 +++++++++++----------- lib/document_stores/redis.js | 38 +++++----- 4 files changed, 130 insertions(+), 146 deletions(-) diff --git a/lib/document_handler.js b/lib/document_handler.js index 44a46de..c6ba21d 100644 --- a/lib/document_handler.js +++ b/lib/document_handler.js @@ -15,34 +15,30 @@ DocumentHandler.defaultKeyLength = 10; // Handle retrieving a document DocumentHandler.prototype.handleGet = async function(key, res, skipExpire){ - await this.store.get(key, function(ret){ - if (ret){ - winston.verbose('retrieved document', { key: key }); - res.writeHead(200, { 'content-type': 'application/json' }); - res.end(JSON.stringify({ data: ret, key: key })); - } - else { - winston.warn('document not found', { key: key }); - res.writeHead(404, { 'content-type': 'application/json' }); - res.end(JSON.stringify({ message: 'Document not found.' })); - } - }, skipExpire); + const data = await this.store.get(key, skipExpire); + + //when data is null it means there was either no data or an error + if (!data){ + winston.warn('document not found', { key: key }); + res.status(404).json({ message: 'Document not found.' }); + return; + } + winston.verbose('retrieved document', { key: key }); + res.status(200).json({ data: data, key: key }); + return; }; // Handle retrieving the raw version of a document DocumentHandler.prototype.handleGetRaw = async function(key, res, skipExpire){ - await this.store.get(key, function(ret){ - if (ret){ - winston.verbose('retrieved raw document', { key: key }); - res.writeHead(200, { 'content-type': 'text/plain; charset=UTF-8' }); - res.end(ret); - } - else { - winston.warn('raw document not found', { key: key }); - res.writeHead(404, { 'content-type': 'application/json' }); - res.end(JSON.stringify({ message: 'Document not found.' })); - } - }, skipExpire); + const data = await this.store.get(key, skipExpire); + + if (!data){ + winston.warn('raw document not found', { key: key }); + res.status(404).json({ message: 'Document not found.' }); + return; + } + winston.verbose('retrieved raw document', { key: key }); + res.status(200).end(data); }; // Handle adding a new Document @@ -51,40 +47,12 @@ DocumentHandler.prototype.handlePost = function (req, res){ let buffer = ''; let cancelled = false; - // What to do when done - let onSuccess = async function (){ - // Check length - if (_this.maxLength && buffer.length > _this.maxLength){ - cancelled = true; - winston.warn('document >maxLength', { maxLength: _this.maxLength }); - res.writeHead(400, { 'content-type': 'application/json' }); - res.end( - JSON.stringify({ message: 'Document exceeds maximum length.' }) - ); - return; - } - // And then save if we should - await _this.chooseKey(async function (key){ - await _this.store.set(key, buffer, function (resp){ - if (resp){ - winston.verbose('added document', { key: key }); - res.writeHead(200, { 'content-type': 'application/json' }); - res.end(JSON.stringify({ key: key })); - } - else { - winston.verbose('error adding document'); - res.writeHead(500, { 'content-type': 'application/json' }); - res.end(JSON.stringify({ message: 'Error adding document.' })); - } - }); - }); - }; - - // If we should, parse a form to grab the data + //parse a form to grab the data let ct = req.headers['content-type']; if (ct && ct.split(';')[0] == 'multipart/form-data'){ + winston.debug('okayge'); let busboy = new Busboy({ headers: req.headers }); - busboy.on('field', function (fieldname, val){ + busboy.on('field', (fieldname, val) => { if (fieldname == 'data'){ buffer = val; } @@ -93,35 +61,50 @@ DocumentHandler.prototype.handlePost = function (req, res){ await onSuccess(); }); req.pipe(busboy); - // Otherwise, use our own and just grab flat data from POST body - } else { - req.on('data', function (data){ + //otherwise, grab flat data from POST body + } + else { + req.on('data', data => { buffer += data.toString(); }); - req.on('end', async function (){ - if (cancelled){ return; } + req.on('end', async () => { + if (cancelled) return; await onSuccess(); }); - req.on('error', function (error){ - winston.error('connection error: ' + error.message); - res.writeHead(500, { 'content-type': 'application/json' }); - res.end(JSON.stringify({ message: 'Connection error.' })); + req.on('error', error => { + winston.error(`busboy connection error: ${error.message}`); + res.status(500).json({ message: 'Internal server error occured while adding document.' }); cancelled = true; }); } + + let onSuccess = async function (){ + //check length + if (_this.maxLength && buffer.length > _this.maxLength){ + cancelled = true; + winston.warn('document >maxLength', { maxLength: _this.maxLength }); + res.status(413).json({ message: 'Document exceeds maximum length.' }); + return; + } + //and save + const key = await _this.chooseKey(); + const success = await _this.store.set(key, buffer); + if (!success){ + winston.verbose('error adding document'); + res.status(500).json({ message: 'Internal server error occured while adding document.' }); + return; + } + winston.verbose('added document', { key: key }); + res.status(200).json({ key: key }); + }; }; -// Keep choosing keys until one isn't taken -DocumentHandler.prototype.chooseKey = async function(callback){ +//keep choosing keys until one isn't taken +DocumentHandler.prototype.chooseKey = async function(){ let key = this.acceptableKey(); - let _this = this; - await this.store.get(key, function(ret){ - if (ret){ - _this.chooseKey(callback); - } else { - callback(key); - } - }, true); // Don't bump expirations when key searching + let data = await this.store.get(key, true); //don't bump expirations on key searching + if (data) return this.chooseKey(); + return key; }; DocumentHandler.prototype.acceptableKey = function(){ diff --git a/lib/document_stores/file.js b/lib/document_stores/file.js index 7e22be9..15270be 100644 --- a/lib/document_stores/file.js +++ b/lib/document_stores/file.js @@ -10,7 +10,7 @@ class FileDocumentStore { } //save data in a file, key as md5 - since we don't know what we can passed here - async set(key, data, callback, skipExpire){ + async set(key, data, skipExpire){ const _this = this; const filePath = this.getPath(key); @@ -23,35 +23,35 @@ class FileDocumentStore { } winston.silly('set key', { type: 'file', filename: filePath }); - await fs.promises.writeFile(filePath, data, {mode: '600'}) + return await fs.promises.writeFile(filePath, data, {mode: '600'}) .then(() => { - callback(true); if (_this.expire && !skipExpire){ - winston.warn('file store cannot set expirations on keys', { file: filePath }); + winston.warn('file store doesn\'t support expiration', { file: filePath }); } + return true; }) .catch(err => { winston.error('error while writing document to file', { file: filePath, error: err }); - callback(false); + return false; }); } //get data from a file - async get(key, callback, skipExpire){ + async get(key, skipExpire){ const _this = this; const filePath = this.getPath(key); winston.silly('get key', { type: 'file', filename: filePath }); - await fs.promises.readFile(filePath, {encoding: 'utf8'}) + return await fs.promises.readFile(filePath, {encoding: 'utf8'}) .then(data => { - callback(data); if (_this.expire && !skipExpire){ winston.warn('file store cannot set expirations on keys', { file: filePath }); } + return data; }) .catch(err => { winston.debug('error while reading document', { file: filePath, error: err }); - return callback(false); + return null; }); } diff --git a/lib/document_stores/mongodb.js b/lib/document_stores/mongodb.js index b685a52..9c64f68 100644 --- a/lib/document_stores/mongodb.js +++ b/lib/document_stores/mongodb.js @@ -1,20 +1,20 @@ const winston = require('winston'); const mongodb = require('mongodb'); -const MongoDocumentStore = function (config){ - this.expire = config.expire; - this.MongoClient = new mongodb.MongoClient(config.connectionUri, config.clientOptions); -}; +class MongoDocumentStore { + constructor(config){ + this.expire = config.expire; + this.MongoClient = new mongodb.MongoClient(config.connectionUri, config.clientOptions); + } -MongoDocumentStore.prototype.set = async function (key, data, callback, skipExpire){ - winston.silly(`mongo set ${key}`); - const now = Math.floor(Date.now() / 1000); - const that = this; + async set(key, data, callback, skipExpire){ + winston.silly(`mongo set ${key}`); + const now = Math.floor(Date.now() / 1000); + const that = this; - await this.safeConnect(async ( {error} = {} ) => { - if (error) return callback(false); + if ((await this.safeConnect()).error) return null; - await this.MongoClient.db().collection('entries').updateOne( + return await this.MongoClient.db().collection('entries').updateOne( { 'entry_id': key, $or: [ @@ -34,22 +34,19 @@ MongoDocumentStore.prototype.set = async function (key, data, callback, skipExpi } ) .then((err, result) => { - return callback(true); + return true; }) .catch((err, result) => { winston.error('error updating mongodb document', { error: err }); - return callback(false); + return false; }); - }); -}; + } + async get(key, skipExpire){ + winston.silly(`mongo get ${key}`); + const now = Math.floor(Date.now() / 1000); + const that = this; -MongoDocumentStore.prototype.get = async function (key, callback, skipExpire){ - winston.silly(`mongo get ${key}`); - const now = Math.floor(Date.now() / 1000); - const that = this; - - await this.safeConnect(async ( {error} = {} ) => { - if (error) return callback(false); + if ((await this.safeConnect()).error) return null; let document = await this.MongoClient.db().collection('entries').findOne({ 'entry_id': key, @@ -57,15 +54,12 @@ MongoDocumentStore.prototype.get = async function (key, callback, skipExpire){ { expiration: -1 }, { expiration: { $gt: now } } ] - }) - .catch(err => { - winston.error('error finding mongodb document', { error: err }); - return callback(false); - }); + }).catch(err => { + winston.error('error finding mongodb document', { error: err }); + return null; + }); - callback(document ? document.value : false); - - if (document && document.expiration != -1 && that.expire && !skipExpire){ + if (document && document.expiration != -1 && that.expire && !skipExpire) { await this.MongoClient.db().collection('entries').updateOne( { 'entry_id': key }, { $set: { expiration: that.expire + now } } @@ -74,22 +68,25 @@ MongoDocumentStore.prototype.get = async function (key, callback, skipExpire){ }); winston.silly('extended expiry of mongodb document', { key: key, timestamp: that.expire + now }); } - }); -}; + return document ? document.value : null; + } + safeConnect(){ + //don't try connecting again if already connected + //https://jira.mongodb.org/browse/NODE-1868 + if (this.MongoClient.isConnected()) return { error: null }; + return this.MongoClient.connect() + .then(client => { + winston.debug('connected to mongodb', { success: true }); + return { error: null }; + }) + .catch(err => { + winston.error('error connecting to mongodb', { error: err }); + return { error: err }; + }); + } +} + + -MongoDocumentStore.prototype.safeConnect = function(cb){ - //don't try connecting again if already connected - //https://jira.mongodb.org/browse/NODE-1868 - if (this.MongoClient.isConnected()) return cb({error: null}); - this.MongoClient.connect() - .then(client => { - winston.debug('connected to mongodb', { success: true }); - cb({error: null}); - }) - .catch(err => { - winston.error('error connecting to mongodb', { error: err }); - cb({error: err}); - }); -}; module.exports = MongoDocumentStore; \ No newline at end of file diff --git a/lib/document_stores/redis.js b/lib/document_stores/redis.js index 16f1a8a..85e26db 100644 --- a/lib/document_stores/redis.js +++ b/lib/document_stores/redis.js @@ -18,30 +18,34 @@ class RedisDocumentStore { winston.info('initialized redis client'); } - async set(key, data, callback, skipExpire){ - await this.client.set(key, data).catch(err => { - winston.error('failed to call redisClient.set', {error: err}); - callback(false); - return; - }); - if (!skipExpire) this.setExpiration(key); - callback(true); + async set(key, data, skipExpire){ + return await this.client.set(key, data) + .then(() => { + if (!skipExpire) this.setExpiration(key); + return true; + }) + .catch(err => { + winston.error('failed to set redis document', {error: err}); + return false; + }); } - async get(key, callback, skipExpire){ - let data = await this.client.get(key).catch(err => { - winston.error('failed to get document from redis', {key: key, error: err}); - callback(false); - return; - }); - if (!skipExpire) this.setExpiration(key); - callback(data); + async get(key, skipExpire){ + return await this.client.get(key) + .then(data => { + if (!skipExpire) this.setExpiration(key); + return data; + }) + .catch(err => { + winston.error('failed to get redis document', {key: key, error: err}); + return null; + }); } async setExpiration(key){ if (!this.expire) return; await this.client.expire(key, this.expire).catch(err => { - winston.warn('failed to set expiry on key', {key: key, error: err}); + winston.warn('failed to set redis key expiry', {key: key, error: err}); }); } }