Object cache refactor (#7558)

* fix: cache refactor

db.getObjectField no longer loads entire object
db.getObjectsFields only clones data once
more tests

* feat: add back cache to redis

db.getObjectField no longer loads entire object

src/database/mongo/hash.js

@@ -39,57 +39,29 @@ module.exports = function (db, module) {
 
 	module.getObject = function (key, callback) {
 		if (!key) {
-			return callback();
+			return setImmediate(callback, null, null);
 		}
 
 		module.getObjects([key], function (err, data) {
-			if (err) {
+			callback(err, data && data.length ? data[0] : null);
-				return callback(err);
-			}
-			callback(null, data && data.length ? data[0] : null);
 		});
 	};
 
 	module.getObjects = function (keys, callback) {
-		var cachedData = {};
+		module.getObjectsFields(keys, [], callback);
-		function getFromCache() {
-			process.nextTick(callback, null, keys.map(key => _.clone(cachedData[key])));
-		}
-
-		if (!Array.isArray(keys) || !keys.length) {
-			return callback(null, []);
-		}
-
-		const unCachedKeys = cache.getUnCachedKeys(keys, cachedData);
-
-		if (!unCachedKeys.length) {
-			return getFromCache();
-		}
-
-		var query = { _key: { $in: unCachedKeys } };
-		if (unCachedKeys.length === 1) {
-			query._key = unCachedKeys[0];
-		}
-		db.collection('objects').find(query, { projection: { _id: 0 } }).toArray(function (err, data) {
-			if (err) {
-				return callback(err);
-			}
-			data = data.map(helpers.deserializeData);
-			var map = helpers.toMap(data);
-			unCachedKeys.forEach(function (key) {
-				cachedData[key] = map[key] || null;
-				cache.set(key, cachedData[key]);
-			});
-
-			getFromCache();
-		});
 	};
 
 	module.getObjectField = function (key, field, callback) {
 		if (!key) {
-			return callback();
+			return setImmediate(callback, null, null);
+		}
+		const cachedData = {};
+		cache.getUnCachedKeys([key], cachedData);
+		if (cachedData[key]) {
+			return setImmediate(callback, null, cachedData[key].hasOwnProperty(field) ? cachedData[key][field] : null);
 		}
-		module.getObject(key, function (err, item) {
+		field = helpers.fieldToString(field);
+		db.collection('objects').findOne({ _key: key }, { projection: { _id: 0, [field]: 1 } }, function (err, item) {
 			if (err || !item) {
 				return callback(err, null);
 			}
@@ -99,7 +71,7 @@ module.exports = function (db, module) {
 
 	module.getObjectFields = function (key, fields, callback) {
 		if (!key) {
-			return callback();
+			return setImmediate(callback, null, null);
 		}
 		module.getObjectsFields([key], fields, function (err, data) {
 			callback(err, data ? data[0] : null);
@@ -108,18 +80,16 @@ module.exports = function (db, module) {
 
 	module.getObjectsFields = function (keys, fields, callback) {
 		if (!Array.isArray(keys) || !keys.length) {
-			return callback(null, []);
+			return setImmediate(callback, null, []);
 		}
-		module.getObjects(keys, function (err, items) {
+		const cachedData = {};
-			if (err) {
+		function returnData() {
-				return callback(err);
+			var mapped = keys.map(function (key) {
-			}
+				if (!fields.length) {
-			if (items === null) {
+					return _.clone(cachedData[key]);
-				items = [];
+				}
-			}
 
-			const returnData = items.map((item) => {
+				const item = cachedData[key] || {};
-				item = item || {};
 				const result = {};
 				fields.forEach((field) => {
 					result[field] = item[field] !== undefined ? item[field] : null;
@@ -127,7 +97,30 @@ module.exports = function (db, module) {
 				return result;
 			});
 
-			callback(null, returnData);
+			callback(null, mapped);
+		}
+
+		const unCachedKeys = cache.getUnCachedKeys(keys, cachedData);
+		if (!unCachedKeys.length) {
+			return process.nextTick(returnData);
+		}
+
+		var query = { _key: { $in: unCachedKeys } };
+		if (unCachedKeys.length === 1) {
+			query._key = unCachedKeys[0];
+		}
+		db.collection('objects').find(query, { projection: { _id: 0 } }).toArray(function (err, data) {
+			if (err) {
+				return callback(err);
+			}
+			data = data.map(helpers.deserializeData);
+			var map = helpers.toMap(data);
+			unCachedKeys.forEach(function (key) {
+				cachedData[key] = map[key] || null;
+				cache.set(key, cachedData[key]);
+			});
+
+			returnData();
 		});
 	};
 

src/database/postgres/hash.js

@@ -65,7 +65,7 @@ VALUES ($1::TEXT, jsonb_build_object($2::TEXT, $3::TEXT::JSONB))
 
 	module.getObject = function (key, callback) {
 		if (!key) {
-			return callback();
+			return callback(null, null);
 		}
 
 		db.query({
@@ -122,7 +122,7 @@ SELECT h."data"
 
 	module.getObjectField = function (key, field, callback) {
 		if (!key) {
-			return callback();
+			return setImmediate(callback, null, null);
 		}
 
 		db.query({
@@ -151,7 +151,7 @@ SELECT h."data"->>$2::TEXT f
 
 	module.getObjectFields = function (key, fields, callback) {
 		if (!key) {
-			return callback();
+			return setImmediate(callback, null, null);
 		}
 
 		db.query({

src/database/redis/hash.js

@@ -3,6 +3,13 @@
 module.exports = function (redisClient, module) {
 	var helpers = module.helpers.redis;
 
+	const async = require('async');
+	const _ = require('lodash');
+
+	const cache = require('../cache').create('redis');
+
+	module.objectCache = cache;
+
 	module.setObject = function (key, data, callback) {
 		callback = callback || function () {};
 		if (!key || !data) {
@@ -23,7 +30,11 @@ module.exports = function (redisClient, module) {
 			return callback();
 		}
 		redisClient.hmset(key, data, function (err) {
-			callback(err);
+			if (err) {
+				return callback(err);
+			}
+			cache.delObjectCache(key);
+			callback();
 		});
 	};
 
@@ -33,61 +44,90 @@ module.exports = function (redisClient, module) {
 			return callback();
 		}
 		redisClient.hset(key, field, value, function (err) {
-			callback(err);
+			if (err) {
+				return callback(err);
+			}
+			cache.delObjectCache(key);
+			callback();
 		});
 	};
 
 	module.getObject = function (key, callback) {
-		redisClient.hgetall(key, callback);
+		if (!key) {
+			return setImmediate(callback, null, null);
+		}
+
+		module.getObjectsFields([key], [], function (err, data) {
+			callback(err, data && data.length ? data[0] : null);
+		});
 	};
 
 	module.getObjects = function (keys, callback) {
-		if (!Array.isArray(keys) || !keys.length) {
+		module.getObjectsFields(keys, [], callback);
-			return setImmediate(callback, null, []);
-		}
-		if (keys.length > 1) {
-			helpers.execKeys(redisClient, 'batch', 'hgetall', keys, callback);
-		} else {
-			redisClient.hgetall(keys[0], (err, data) => callback(err, [data]));
-		}
 	};
 
 	module.getObjectField = function (key, field, callback) {
-		module.getObjectFields(key, [field], function (err, data) {
+		if (!key) {
-			callback(err, data ? data[field] : null);
+			return setImmediate(callback, null, null);
-		});
+		}
+		const cachedData = {};
+		cache.getUnCachedKeys([key], cachedData);
+		if (cachedData[key]) {
+			return setImmediate(callback, null, cachedData[key].hasOwnProperty(field) ? cachedData[key][field] : null);
+		}
+		redisClient.hget(key, field, callback);
 	};
 
 	module.getObjectFields = function (key, fields, callback) {
+		if (!key) {
+			return setImmediate(callback, null, null);
+		}
 		module.getObjectsFields([key], fields, function (err, results) {
 			callback(err, results ? results[0] : null);
 		});
 	};
 
 	module.getObjectsFields = function (keys, fields, callback) {
-		if (!Array.isArray(fields) || !fields.length) {
+		if (!Array.isArray(keys) || !keys.length) {
-			return callback(null, keys.map(function () { return {}; }));
+			return setImmediate(callback, null, []);
 		}
-		var batch = redisClient.batch();
+		if (!Array.isArray(fields)) {
-
+			return callback(null, keys.map(function () { return {}; }));
-		for (var x = 0; x < keys.length; x += 1) {
-			batch.hmget.apply(batch, [keys[x]].concat(fields));
 		}
-
+		const cachedData = {};
-		batch.exec(function (err, results) {
+		const unCachedKeys = cache.getUnCachedKeys(keys, cachedData);
-			if (err) {
+
-				return callback(err);
+		async.waterfall([
-			}
+			function (next) {
-
+				if (unCachedKeys.length > 1) {
-			results = results.map(function makeObject(array) {
+					helpers.execKeys(redisClient, 'batch', 'hgetall', unCachedKeys, next);
-				var obj = {};
+				} else if (unCachedKeys.length === 1) {
-				for (var i = 0, ii = fields.length; i < ii; i += 1) {
+					redisClient.hgetall(unCachedKeys[0], (err, data) => next(err, [data]));
-					obj[fields[i]] = array[i];
+				} else {
+					next(null, []);
 				}
-				return obj;
+			},
-			});
+			function (data, next) {
-			callback(null, results);
+				unCachedKeys.forEach(function (key, i) {
-		});
+					cachedData[key] = data[i] || null;
+					cache.set(key, cachedData[key]);
+				});
+
+				var mapped = keys.map(function (key) {
+					if (!fields.length) {
+						return _.clone(cachedData[key]);
+					}
+
+					const item = cachedData[key] || {};
+					const result = {};
+					fields.forEach((field) => {
+						result[field] = item[field] !== undefined ? item[field] : null;
+					});
+					return result;
+				});
+				next(null, mapped);
+			},
+		], callback);
 	};
 
 	module.getObjectKeys = function (key, callback) {
@@ -116,12 +156,14 @@ module.exports = function (redisClient, module) {
 			return setImmediate(callback);
 		}
 		redisClient.hdel(key, field, function (err) {
+			cache.delObjectCache(key);
 			callback(err);
 		});
 	};
 
 	module.deleteObjectFields = function (key, fields, callback) {
 		helpers.execKeyValues(redisClient, 'batch', 'hdel', key, fields, function (err) {
+			cache.delObjectCache(key);
 			callback(err);
 		});
 	};
@@ -140,6 +182,7 @@ module.exports = function (redisClient, module) {
 			if (err) {
 				return callback(err);
 			}
+			cache.delObjectCache(key);
 			callback(null, Array.isArray(result) ? result.map(value => parseInt(value, 10)) : parseInt(result, 10));
 		}
 		value = parseInt(value, 10);

src/database/redis/main.js

@@ -16,6 +16,7 @@ module.exports = function (redisClient, module) {
 			if (err) {
 				return callback(err);
 			}
+			module.objectCache.resetObjectCache();
 			callback();
 		});
 	};
@@ -35,6 +36,7 @@ module.exports = function (redisClient, module) {
 	module.delete = function (key, callback) {
 		callback = callback || function () {};
 		redisClient.del(key, function (err) {
+			module.objectCache.delObjectCache(key);
 			callback(err);
 		});
 	};
@@ -46,6 +48,7 @@ module.exports = function (redisClient, module) {
 			batch.del(keys[i]);
 		}
 		batch.exec(function (err) {
+			module.objectCache.delObjectCache(keys);
 			callback(err);
 		});
 	};
@@ -72,6 +75,8 @@ module.exports = function (redisClient, module) {
 			if (err && err.message !== 'ERR no such key') {
 				return callback(err);
 			}
+			module.objectCache.delObjectCache(oldKey);
+			module.objectCache.delObjectCache(newKey);
 			callback();
 		});
 	};

test/database/hash.js

@@ -92,6 +92,20 @@ describe('Hash methods', function () {
 				});
 			});
 		});
+
+		it('should work for field names with "." in them when they are cached', function (done) {
+			db.setObjectField('dotObject3', 'my.dot.field', 'foo2', function (err) {
+				assert.ifError(err);
+				db.getObject('dotObject3', function (err, data) {
+					assert.ifError(err);
+					db.getObjectField('dotObject3', 'my.dot.field', function (err, value) {
+						assert.ifError(err);
+						assert.equal(value, 'foo2');
+						done();
+					});
+				});
+			});
+		});
 	});
 
 	describe('getObject()', function () {
@@ -113,6 +127,15 @@ describe('Hash methods', function () {
 				done();
 			});
 		});
+
+		it('should return null if key is falsy', function (done) {
+			db.getObject(null, function (err, data) {
+				assert.ifError(err);
+				assert.equal(arguments.length, 2);
+				assert.equal(data, null);
+				done();
+			});
+		});
 	});
 
 	describe('getObjects()', function () {
@@ -163,6 +186,15 @@ describe('Hash methods', function () {
 				done();
 			});
 		});
+
+		it('should return null if key is falsy', function (done) {
+			db.getObjectField(null, 'test', function (err, data) {
+				assert.ifError(err);
+				assert.equal(arguments.length, 2);
+				assert.equal(data, null);
+				done();
+			});
+		});
 	});
 
 	describe('getObjectFields()', function () {
@@ -188,6 +220,15 @@ describe('Hash methods', function () {
 				done();
 			});
 		});
+
+		it('should return null if key is falsy', function (done) {
+			db.getObjectFields(null, ['test', 'foo'], function (err, data) {
+				assert.ifError(err);
+				assert.equal(arguments.length, 2);
+				assert.equal(data, null);
+				done();
+			});
+		});
 	});
 
 	describe('getObjectsFields()', function () {