From 8379c11b2289179e46dcca983709c77bc0ce5fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Fri, 12 Nov 2021 19:51:59 -0500 Subject: [PATCH] refactor: setObjectBulk to match sortedSetAddBulk --- src/categories/update.js | 3 +- src/database/mongo/hash.js | 22 +++++++++------ src/database/postgres/hash.js | 27 +++++++++--------- src/database/redis/hash.js | 19 +++++++++---- src/meta/settings.js | 7 ++--- src/posts/queue.js | 3 +- src/topics/scheduled.js | 2 +- src/topics/tags.js | 17 ++++------- src/upgrades/1.15.0/topic_poster_count.js | 8 ++---- src/upgrades/1.17.0/topic_thumb_count.js | 3 +- src/upgrades/1.18.0/topic_tags_refactor.js | 3 +- src/widgets/index.js | 10 ++----- test/database/hash.js | 33 ++++++++++------------ 13 files changed, 75 insertions(+), 82 deletions(-) diff --git a/src/categories/update.js b/src/categories/update.js index 259dd58d06..63015684dd 100644 --- a/src/categories/update.js +++ b/src/categories/update.js @@ -121,8 +121,7 @@ module.exports = function (Categories) { ); await db.setObjectBulk( - childrenCids.map(cid => `category:${cid}`), - childrenCids.map((cid, index) => ({ order: index + 1 })) + childrenCids.map((cid, index) => [`category:${cid}`, { order: index + 1 }]) ); cache.del([ diff --git a/src/database/mongo/hash.js b/src/database/mongo/hash.js index 67daab7b16..bb6c4d488f 100644 --- a/src/database/mongo/hash.js +++ b/src/database/mongo/hash.js @@ -35,20 +35,26 @@ module.exports = function (module) { cache.del(key); }; - module.setObjectBulk = async function (keys, data) { - if (!keys.length || !data.length) { + module.setObjectBulk = async function (...args) { + let data = args[0]; + if (!Array.isArray(data) || !data.length) { return; } + if (Array.isArray(args[1])) { + console.warn('[deprecated] db.setObjectBulk(keys, data) usage is deprecated, please use db.setObjectBulk(data)'); + // conver old format to new format for backwards compatibility + data = args[0].map((key, i) => [key, args[1][i]]); + } - const writeData = data.map(helpers.serializeData); try { let bulk; - keys.forEach((key, i) => { - if (Object.keys(writeData[i]).length) { + data.forEach((item) => { + const writeData = helpers.serializeData(item[1]); + if (Object.keys(writeData).length) { if (!bulk) { bulk = module.client.collection('objects').initializeUnorderedBulkOp(); } - bulk.find({ _key: key }).upsert().updateOne({ $set: writeData[i] }); + bulk.find({ _key: item[0] }).upsert().updateOne({ $set: writeData }); } }); if (bulk) { @@ -56,12 +62,12 @@ module.exports = function (module) { } } catch (err) { if (err && err.message.startsWith('E11000 duplicate key error')) { - return await module.setObjectBulk(keys, data); + return await module.setObjectBulk(data); } throw err; } - cache.del(keys); + cache.del(data.map(item => item[0])); }; module.setObjectField = async function (key, field, value) { diff --git a/src/database/postgres/hash.js b/src/database/postgres/hash.js index 015154aa42..519a8e6c0e 100644 --- a/src/database/postgres/hash.js +++ b/src/database/postgres/hash.js @@ -44,29 +44,30 @@ module.exports = function (module) { }); }; - module.setObjectBulk = async function (keys, data) { - if (!keys.length || !data.length) { + module.setObjectBulk = async function (...args) { + let data = args[0]; + if (!Array.isArray(data) || !data.length) { return; } + if (Array.isArray(args[1])) { + console.warn('[deprecated] db.setObjectBulk(keys, data) usage is deprecated, please use db.setObjectBulk(data)'); + // conver old format to new format for backwards compatibility + data = args[0].map((key, i) => [key, args[1][i]]); + } await module.transaction(async (client) => { - keys = keys.slice(); - data = data.filter((d, i) => { - if (d.hasOwnProperty('')) { - delete d['']; - } - const keep = !!Object.keys(d).length; - if (!keep) { - keys.splice(i, 1); + data = data.filter((item) => { + if (item[1].hasOwnProperty('')) { + delete item[1]['']; } - return keep; + return !!Object.keys(item[1]).length; }); - + const keys = data.map(item => item[0]); if (!keys.length) { return; } await helpers.ensureLegacyObjectsType(client, keys, 'hash'); - const dataStrings = data.map(JSON.stringify); + const dataStrings = data.map(item => JSON.stringify(item[1])); await client.query({ name: 'setObjectBulk', text: ` diff --git a/src/database/redis/hash.js b/src/database/redis/hash.js index 966a36eddd..1afdccd3b1 100644 --- a/src/database/redis/hash.js +++ b/src/database/redis/hash.js @@ -36,18 +36,25 @@ module.exports = function (module) { cache.del(key); }; - module.setObjectBulk = async function (keys, data) { - if (!keys.length || !data.length) { + module.setObjectBulk = async function (...args) { + let data = args[0]; + if (!Array.isArray(data) || !data.length) { return; } + if (Array.isArray(args[1])) { + console.warn('[deprecated] db.setObjectBulk(keys, data) usage is deprecated, please use db.setObjectBulk(data)'); + // conver old format to new format for backwards compatibility + data = args[0].map((key, i) => [key, args[1][i]]); + } + const batch = module.client.batch(); - keys.forEach((k, i) => { - if (Object.keys(data[i]).length) { - batch.hmset(k, data[i]); + data.forEach((item) => { + if (Object.keys(item[1]).length) { + batch.hmset(item[0], item[1]); } }); await helpers.execBatch(batch); - cache.del(keys); + cache.del(data.map(item => item[0])); }; module.setObjectField = async function (key, field, value) { diff --git a/src/meta/settings.js b/src/meta/settings.js index e9ba356eba..badfa791bd 100644 --- a/src/meta/settings.js +++ b/src/meta/settings.js @@ -70,19 +70,18 @@ Settings.set = async function (hash, values, quiet) { })); const sortedSetData = []; - const objectData = { keys: [], data: [] }; + const objectData = []; sortedLists.forEach((list) => { const arr = sortedListData[list]; arr.forEach((data, order) => { sortedSetData.push([`settings:${hash}:sorted-list:${list}`, order, order]); - objectData.keys.push(`settings:${hash}:sorted-list:${list}:${order}`); - objectData.data.push(data); + objectData.push([`settings:${hash}:sorted-list:${list}:${order}`, data]); }); }); await Promise.all([ db.sortedSetAddBulk(sortedSetData), - db.setObjectBulk(objectData.keys, objectData.data), + db.setObjectBulk(objectData), ]); } diff --git a/src/posts/queue.js b/src/posts/queue.js index 6ed1af9c47..33a64e14be 100644 --- a/src/posts/queue.js +++ b/src/posts/queue.js @@ -343,8 +343,7 @@ module.exports = function (Posts) { post.data.tid = newTid; }); await db.setObjectBulk( - postData.map(p => `post:queue:${p.id}`), - postData.map(p => ({ data: JSON.stringify(p.data) })) + postData.map(p => [`post:queue:${p.id}`, { data: JSON.stringify(p.data) }]), ); cache.del('post-queue'); } diff --git a/src/topics/scheduled.js b/src/topics/scheduled.js index 5d99b1432b..56c363db39 100644 --- a/src/topics/scheduled.js +++ b/src/topics/scheduled.js @@ -124,5 +124,5 @@ async function updateUserLastposttimes(uids, topicsData) { async function shiftPostTimes(tid, timestamp) { const pids = (await posts.getPidsFromSet(`tid:${tid}:posts`, 0, -1, false)); // Leaving other related score values intact, since they reflect post order correctly, and it seems that's good enough - return db.setObjectBulk(pids.map(pid => `post:${pid}`), pids.map((_, idx) => ({ timestamp: timestamp + idx + 1 }))); + return db.setObjectBulk(pids.map((pid, idx) => [`post:${pid}`, { timestamp: timestamp + idx + 1 }])); } diff --git a/src/topics/tags.js b/src/topics/tags.js index b0be277a9d..f78eb8e09e 100644 --- a/src/topics/tags.js +++ b/src/topics/tags.js @@ -155,8 +155,7 @@ module.exports = function (Topics) { } }); await db.setObjectBulk( - topicData.map(t => `topic:${t.tid}`), - topicData.map(t => ({ tags: t.tags.join(',') })) + topicData.map(t => [`topic:${t.tid}`, { tags: t.tags.join(',') }]), ); }, {}); await Topics.deleteTag(tag); @@ -335,14 +334,11 @@ module.exports = function (Topics) { topicTags.push(tag); } }); - bulkSet.push({ tags: topicTags.join(',') }); + bulkSet.push([`topic:${t.tid}`, { tags: topicTags.join(',') }]); }); await Promise.all([ db.sortedSetAddBulk(bulkAdd), - db.setObjectBulk( - topicData.map(t => `topic:${t.tid}`), - bulkSet, - ), + db.setObjectBulk(bulkSet), ]); await Promise.all(tags.map(updateTagCount)); @@ -363,14 +359,11 @@ module.exports = function (Topics) { topicTags.splice(topicTags.indexOf(tag), 1); } }); - bulkSet.push({ tags: topicTags.join(',') }); + bulkSet.push([`topic:${t.tid}`, { tags: topicTags.join(',') }]); }); await Promise.all([ db.sortedSetRemoveBulk(bulkRemove), - db.setObjectBulk( - topicData.map(t => `topic:${t.tid}`), - bulkSet, - ), + db.setObjectBulk(bulkSet), ]); await Promise.all(tags.map(updateTagCount)); diff --git a/src/upgrades/1.15.0/topic_poster_count.js b/src/upgrades/1.15.0/topic_poster_count.js index 91a93ce788..f7d20d4c0d 100644 --- a/src/upgrades/1.15.0/topic_poster_count.js +++ b/src/upgrades/1.15.0/topic_poster_count.js @@ -15,15 +15,13 @@ module.exports = { const keys = tids.map(tid => `tid:${tid}:posters`); await db.sortedSetsRemoveRangeByScore(keys, '-inf', 0); const counts = await db.sortedSetsCard(keys); - const setKeys = []; - const data = []; + const bulkSet = []; for (let i = 0; i < tids.length; i++) { if (counts[i] > 0) { - setKeys.push(`topic:${tids[i]}`); - data.push({ postercount: counts[i] }); + bulkSet.push([`topic:${tids[i]}`, { postercount: counts[i] }]); } } - await db.setObjectBulk(setKeys, data); + await db.setObjectBulk(bulkSet); }, { progress: progress, batchSize: 500, diff --git a/src/upgrades/1.17.0/topic_thumb_count.js b/src/upgrades/1.17.0/topic_thumb_count.js index 7e39b195ea..83762612fa 100644 --- a/src/upgrades/1.17.0/topic_thumb_count.js +++ b/src/upgrades/1.17.0/topic_thumb_count.js @@ -16,8 +16,7 @@ module.exports = { const tidToCount = _.zipObject(tids, counts); const tidsWithThumbs = tids.filter((t, i) => counts[i] > 0); await db.setObjectBulk( - tidsWithThumbs.map(tid => `topic:${tid}`), - tidsWithThumbs.map(tid => ({ numThumbs: tidToCount[tid] })) + tidsWithThumbs.map(tid => [`topic:${tid}`, { numThumbs: tidToCount[tid] }]), ); progress.incr(tids.length); diff --git a/src/upgrades/1.18.0/topic_tags_refactor.js b/src/upgrades/1.18.0/topic_tags_refactor.js index 5fd2218c49..eb895e720b 100644 --- a/src/upgrades/1.18.0/topic_tags_refactor.js +++ b/src/upgrades/1.18.0/topic_tags_refactor.js @@ -25,8 +25,7 @@ module.exports = { }).filter(t => t && t.tags.length); await db.setObjectBulk( - topicsWithTags.map(t => `topic:${t.tid}`), - topicsWithTags.map(t => ({ tags: t.tags.join(',') })) + topicsWithTags.map(t => [`topic:${t.tid}`, { tags: t.tags.join(',') }]), ); await db.deleteAll(tids.map(tid => `topic:${tid}:tags`)); progress.incr(tids.length); diff --git a/src/widgets/index.js b/src/widgets/index.js index bda9682736..686c309e7b 100644 --- a/src/widgets/index.js +++ b/src/widgets/index.js @@ -177,13 +177,9 @@ widgets.setAreas = async function (areas) { templates[area.template][area.location] = JSON.stringify(area.widgets); }); - const keys = []; - const data = []; - Object.keys(templates).forEach((tpl) => { - keys.push(`widgets:${tpl}`); - data.push(templates[tpl]); - }); - await db.setObjectBulk(keys, data); + await db.setObjectBulk( + Object.keys(templates).map(tpl => [`widgets:${tpl}`, templates[tpl]]) + ); }; widgets.reset = async function () { diff --git a/test/database/hash.js b/test/database/hash.js index e2e2ee7364..294e9f765b 100644 --- a/test/database/hash.js +++ b/test/database/hash.js @@ -73,36 +73,33 @@ describe('Hash methods', () => { }); it('should set multiple keys to different objects', async () => { - const keys = ['bulkKey1', 'bulkKey2']; - const data = [{ foo: '1' }, { baz: 'baz' }]; - - await db.setObjectBulk(keys, data); - const result = await db.getObjects(keys); - assert.deepStrictEqual(result, data); + await db.setObjectBulk([ + ['bulkKey1', { foo: '1' }], + ['bulkKey2', { baz: 'baz' }], + ]); + const result = await db.getObjects(['bulkKey1', 'bulkKey2']); + assert.deepStrictEqual(result, [{ foo: '1' }, { baz: 'baz' }]); }); it('should not error if object is empty', async () => { - const keys = ['bulkKey3', 'bulkKey4']; - const data = [{ foo: '1' }, { }]; - - await db.setObjectBulk(keys, data); - const result = await db.getObjects(keys); + await db.setObjectBulk([ + ['bulkKey3', { foo: '1' }], + ['bulkKey4', { }], + ]); + const result = await db.getObjects(['bulkKey3', 'bulkKey4']); assert.deepStrictEqual(result, [{ foo: '1' }, null]); }); it('should update existing object on second call', async () => { - await db.setObjectBulk(['bulkKey3.5'], [{ foo: '1' }]); - await db.setObjectBulk(['bulkKey3.5'], [{ baz: '2' }]); + await db.setObjectBulk([['bulkKey3.5', { foo: '1' }]]); + await db.setObjectBulk([['bulkKey3.5', { baz: '2' }]]); const result = await db.getObject('bulkKey3.5'); assert.deepStrictEqual(result, { foo: '1', baz: '2' }); }); it('should not error if object is empty', async () => { - const keys = ['bulkKey5']; - const data = [{ }]; - - await db.setObjectBulk(keys, data); - const result = await db.getObjects(keys); + await db.setObjectBulk([['bulkKey5', {}]]); + const result = await db.getObjects(['bulkKey5']); assert.deepStrictEqual(result, [null]); });