From 0b092f605fab1d407d4571cba15ac62fb8091a34 Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 20 Sep 2019 16:41:05 +0600 Subject: [PATCH 01/12] feat: update user and organization metadata * REWORK --- .../user_and_organization_meta_update.md | 30 +++ src/actions/alias.js | 12 +- src/actions/ban.js | 35 ++-- .../organization/members/permission.js | 5 +- src/actions/organization/members/remove.js | 4 +- src/actions/register.js | 4 +- src/actions/updateMetadata.js | 15 +- src/auth/oauth/utils/attach.js | 30 +-- src/auth/oauth/utils/detach.js | 30 +-- src/constants.js | 2 + src/custom/cappasity-users-activate.js | 7 +- src/custom/rfx-create-room-on-activate.js | 8 +- src/utils/metadata/organization.js | 27 +++ src/utils/metadata/redis/audience.js | 30 +++ src/utils/metadata/redis/update-metadata.js | 173 ++++++++++++++++++ src/utils/metadata/user.js | 58 ++++++ .../organization/add-organization-members.js | 7 +- .../register-organization-members.js | 5 +- src/utils/set-organization-metadata.js | 32 +--- test/suites/ban.js | 3 +- test/suites/update-metadata.js | 78 ++++++-- 21 files changed, 481 insertions(+), 114 deletions(-) create mode 100644 rfcs/inactive_users/user_and_organization_meta_update.md create mode 100644 src/utils/metadata/organization.js create mode 100644 src/utils/metadata/redis/audience.js create mode 100644 src/utils/metadata/redis/update-metadata.js create mode 100644 src/utils/metadata/user.js diff --git a/rfcs/inactive_users/user_and_organization_meta_update.md b/rfcs/inactive_users/user_and_organization_meta_update.md new file mode 100644 index 000000000..b30104076 --- /dev/null +++ b/rfcs/inactive_users/user_and_organization_meta_update.md @@ -0,0 +1,30 @@ +# User/Organization metadata update rework +## Overview and Motivation +When user or organization metadata needs to be updated, the Service uses the Redis pipeline javascript code. +For each assigned meta hash always exists a single `audience`, but there is no list of `audiences` assigned to the user or company. +To achieve easier audience tracking and a combined metadata update, I advise using a Lua based script. + +## Audience lists +Audiences stored in sets formed from `USERS_AUDIENCE` or `ORGANISATION_AUDIENCE` constants and `Id` +(eg: `{ms-users}10110110111!audiences`). Both keys contain `audience` names that are currently have assigned values. + +## DEL utils/updateMetadata.js +All logic from this file moved to separate class `utils/metadata/redis/update-metadata.js`. + +## utils/metadata/{user|organization}.js +Classes that perform user or organization metadata update and +audience list tracking. + +Available methods: + * update + * updateMulti + * batchUpdate + * delete + +**TODO** Fill up + + +## global updates +`ms-users` source code now use new `UserMetadata` when any update operation happens. + + diff --git a/src/actions/alias.js b/src/actions/alias.js index 9fb4855b0..15d030816 100644 --- a/src/actions/alias.js +++ b/src/actions/alias.js @@ -6,9 +6,10 @@ const isActive = require('../utils/is-active'); const isBanned = require('../utils/is-banned'); const key = require('../utils/key'); const handlePipeline = require('../utils/pipeline-error'); +const UserMetadata = require('../utils/metadata/user'); + const { USERS_DATA, - USERS_METADATA, USERS_ALIAS_TO_ID, USERS_ID_FIELD, USERS_ALIAS_FIELD, @@ -69,10 +70,11 @@ async function assignAlias({ params }) { return Promise.reject(err); } - const pipeline = redis.pipeline([ - ['hset', key(userId, USERS_DATA), USERS_ALIAS_FIELD, alias], - ['hset', key(userId, USERS_METADATA, defaultAudience), USERS_ALIAS_FIELD, JSON.stringify(alias)], - ]); + const pipeline = redis.pipeline(); + const userMetaData = new UserMetadata(pipeline); + + pipeline.hset(key(userId, USERS_DATA), USERS_ALIAS_FIELD, alias); + userMetaData.update(userId, USERS_ALIAS_FIELD, JSON.stringify(alias), defaultAudience); if (activeUser) { pipeline.sadd(USERS_PUBLIC_INDEX, username); diff --git a/src/actions/ban.js b/src/actions/ban.js index 91e8355ce..15c0c198e 100644 --- a/src/actions/ban.js +++ b/src/actions/ban.js @@ -4,9 +4,10 @@ const mapValues = require('lodash/mapValues'); const redisKey = require('../utils/key.js'); const { getInternalData } = require('../utils/userData'); const handlePipeline = require('../utils/pipeline-error'); +const UserMetadata = require('../utils/metadata/user'); + const { - USERS_DATA, USERS_METADATA, - USERS_BANNED_FLAG, USERS_TOKENS, USERS_BANNED_DATA, + USERS_DATA, USERS_BANNED_FLAG, USERS_TOKENS, USERS_BANNED_DATA, } = require('../constants.js'); // helper @@ -25,26 +26,30 @@ function lockUser({ remoteip: remoteip || '', }, }; + const pipeline = redis.pipeline(); + const userMetadata = new UserMetadata(pipeline); + + pipeline.hset(redisKey(id, USERS_DATA), USERS_BANNED_FLAG, 'true'); + // set .banned on metadata for filtering & sorting users by that field + userMetadata.updateMulti(id, mapValues(data, stringify), defaultAudience); + pipeline.del(redisKey(id, USERS_TOKENS)); - return redis - .pipeline() - .hset(redisKey(id, USERS_DATA), USERS_BANNED_FLAG, 'true') - // set .banned on metadata for filtering & sorting users by that field - .hmset(redisKey(id, USERS_METADATA, defaultAudience), mapValues(data, stringify)) - .del(redisKey(id, USERS_TOKENS)) - .exec(); + return pipeline.exec(); } function unlockUser({ id }) { const { redis, config } = this; const { jwt: { defaultAudience } } = config; + const pipeline = redis.pipeline(); + const userMetadata = new UserMetadata(pipeline); - return redis - .pipeline() - .hdel(redisKey(id, USERS_DATA), USERS_BANNED_FLAG) - // remove .banned on metadata for filtering & sorting users by that field - .hdel(redisKey(id, USERS_METADATA, defaultAudience), 'banned', USERS_BANNED_DATA) - .exec(); + pipeline.hdel(redisKey(id, USERS_DATA), USERS_BANNED_FLAG); + // remove .banned on metadata for filtering & sorting users by that field + userMetadata.delete(id, [ + 'banned', + USERS_BANNED_DATA, + ], defaultAudience); + return pipeline.exec(); } /** diff --git a/src/actions/organization/members/permission.js b/src/actions/organization/members/permission.js index 4e7e96eb7..00e094292 100644 --- a/src/actions/organization/members/permission.js +++ b/src/actions/organization/members/permission.js @@ -5,6 +5,7 @@ const { checkOrganizationExists } = require('../../../utils/organization'); const redisKey = require('../../../utils/key'); const handlePipeline = require('../../../utils/pipeline-error'); const getUserId = require('../../../utils/userData/get-user-id'); +const UserMetadata = require('../../../utils/metadata/user'); const { ErrorUserNotMember, USERS_METADATA, ORGANIZATIONS_MEMBERS } = require('../../../constants'); /** @@ -41,7 +42,9 @@ async function setOrganizationMemberPermission({ params }) { permissions = JSON.stringify(permissions); const pipeline = redis.pipeline(); - pipeline.hset(memberMetadataKey, organizationId, permissions); + const userMetadata = new UserMetadata(pipeline); + + userMetadata.update(userId, organizationId, permissions); pipeline.hset(redisKey(organizationId, ORGANIZATIONS_MEMBERS, userId), 'permissions', permissions); return pipeline.exec().then(handlePipeline); diff --git a/src/actions/organization/members/remove.js b/src/actions/organization/members/remove.js index 61437cd59..142964b05 100644 --- a/src/actions/organization/members/remove.js +++ b/src/actions/organization/members/remove.js @@ -3,6 +3,7 @@ const redisKey = require('../../../utils/key'); const getUserId = require('../../../utils/userData/get-user-id'); const handlePipeline = require('../../../utils/pipeline-error'); const { checkOrganizationExists } = require('../../../utils/organization'); +const UserMetadata = require('../../../utils/metadata/user'); const { ORGANIZATIONS_MEMBERS, USERS_METADATA, @@ -34,9 +35,10 @@ async function removeMember({ params }) { } const pipeline = redis.pipeline(); + const userMetadata = new UserMetadata(pipeline); pipeline.del(memberKey); pipeline.zrem(redisKey(organizationId, ORGANIZATIONS_MEMBERS), memberKey); - pipeline.hdel(memberMetadataKey, organizationId); + userMetadata.delete(userId, organizationId, audience); return pipeline.exec().then(handlePipeline); } diff --git a/src/actions/register.js b/src/actions/register.js index e7f5347fc..a13ead175 100644 --- a/src/actions/register.js +++ b/src/actions/register.js @@ -9,7 +9,7 @@ const reduce = require('lodash/reduce'); const last = require('lodash/last'); // internal deps -const setMetadata = require('../utils/update-metadata'); +const UserMetadata = require('../utils/metadata/user'); const redisKey = require('../utils/key'); const jwt = require('../utils/jwt'); const isDisposable = require('../utils/is-disposable'); @@ -213,7 +213,7 @@ async function performRegistration({ service, params }) { await pipeline.exec().then(handlePipeline); - await setMetadata.call(service, { + await new UserMetadata(service.redis).batchUpdate({ userId, audience, metadata: audience.map((metaAudience) => ({ diff --git a/src/actions/updateMetadata.js b/src/actions/updateMetadata.js index e6ede60b7..024779d32 100644 --- a/src/actions/updateMetadata.js +++ b/src/actions/updateMetadata.js @@ -1,6 +1,6 @@ const omit = require('lodash/omit'); const Promise = require('bluebird'); -const updateMetadata = require('../utils/update-metadata'); +const UserMetadata = require('../utils/metadata/user'); const { getUserId } = require('../utils/userData'); /** @@ -19,12 +19,15 @@ const { getUserId } = require('../utils/userData'); * @apiParam (Payload) {Object} [script] - if present will be called with passed metadata keys & username, provides direct scripting access. * Be careful with granting access to this function. */ -module.exports = function updateMetadataAction(request) { - return Promise +module.exports = async function updateMetadataAction(request) { + const userId = await Promise .bind(this, request.params.username) - .then(getUserId) - .then((userId) => ({ ...omit(request.params, 'username'), userId })) - .then(updateMetadata); + .then(getUserId); + + const userMetadata = new UserMetadata(this.redis); + const updateParams = { ...omit(request.params, 'username'), userId }; + + return userMetadata.batchUpdate(updateParams); }; module.exports.transports = [require('@microfleet/core').ActionTransport.amqp]; diff --git a/src/auth/oauth/utils/attach.js b/src/auth/oauth/utils/attach.js index 6028ca7e1..88a2d8aba 100644 --- a/src/auth/oauth/utils/attach.js +++ b/src/auth/oauth/utils/attach.js @@ -1,13 +1,13 @@ const get = require('lodash/get'); const redisKey = require('../../../utils/key'); -const updateMetadata = require('../../../utils/update-metadata'); +const UserMetadata = require('../../../utils/metadata/user'); const handlePipeline = require('../../../utils/pipeline-error'); const { USERS_SSO_TO_ID, USERS_DATA, } = require('../../../constants'); -module.exports = function attach(account, user) { +module.exports = async function attach(account, user) { const { redis, config } = this; const { id: userId } = user; const { @@ -23,17 +23,19 @@ module.exports = function attach(account, user) { // link uid to user id pipeline.hset(USERS_SSO_TO_ID, uid, userId); - return pipeline.exec().then(handlePipeline) - .bind(this) - .return({ - userId, - audience, - metadata: { - $set: { - [provider]: profile, - }, + await pipeline.exec().then(handlePipeline); + + const userMetadata = new UserMetadata(redis); + const updateParams = { + userId, + audience, + metadata: { + $set: { + [provider]: profile, }, - }) - .then(updateMetadata) - .return(profile); + }, + }; + await userMetadata.batchUpdate(updateParams); + + return profile; }; diff --git a/src/auth/oauth/utils/detach.js b/src/auth/oauth/utils/detach.js index 8c4ea8530..3ddbbf3d4 100644 --- a/src/auth/oauth/utils/detach.js +++ b/src/auth/oauth/utils/detach.js @@ -2,7 +2,7 @@ const Errors = require('common-errors'); const get = require('../../../utils/get-value'); const redisKey = require('../../../utils/key'); -const updateMetadata = require('../../../utils/update-metadata'); +const UserMetadata = require('../../../utils/metadata/user'); const handlePipeline = require('../../../utils/pipeline-error'); const { @@ -10,7 +10,7 @@ const { USERS_DATA, } = require('../../../constants'); -module.exports = function detach(provider, userData) { +module.exports = async function detach(provider, userData) { const { id: userId } = userData; const { redis, config } = this; const audience = get(config, 'jwt.defaultAudience'); @@ -28,16 +28,18 @@ module.exports = function detach(provider, userData) { // delete account reference pipeline.hdel(USERS_SSO_TO_ID, uid); - return pipeline.exec().then(handlePipeline) - .bind(this) - .return({ - userId, - audience, - metadata: { - $remove: [ - provider, - ], - }, - }) - .then(updateMetadata); + await pipeline.exec().then(handlePipeline); + + const userMetadata = new UserMetadata(redis); + const updateParams = { + userId, + audience, + metadata: { + $remove: [ + provider, + ], + }, + }; + + return userMetadata.batchUpdate(updateParams); }; diff --git a/src/constants.js b/src/constants.js index 525284ecc..10f2033a2 100644 --- a/src/constants.js +++ b/src/constants.js @@ -18,6 +18,7 @@ module.exports = exports = { // hashes USERS_DATA: 'data', USERS_METADATA: 'metadata', + USERS_AUDIENCE: 'users-audiences', USERS_TOKENS: 'tokens', USERS_API_TOKENS: 'api-tokens', USERS_API_TOKENS_ZSET: 'api-tokens-set', @@ -26,6 +27,7 @@ module.exports = exports = { USERS_ORGANIZATIONS: 'user-organizations', ORGANIZATIONS_DATA: 'data', ORGANIZATIONS_METADATA: 'metadata', + ORGANIZATIONS_AUDIENCE: 'organization-audiences', ORGANIZATIONS_MEMBERS: 'members', // standard JWT with TTL diff --git a/src/custom/cappasity-users-activate.js b/src/custom/cappasity-users-activate.js index 94ac4faa4..0c1edb930 100644 --- a/src/custom/cappasity-users-activate.js +++ b/src/custom/cappasity-users-activate.js @@ -1,6 +1,6 @@ const find = require('lodash/find'); const moment = require('moment'); -const setMetadata = require('../utils/update-metadata'); +const UserMetadata = require('../utils/metadata/user'); /** * Adds metadata from billing into usermix @@ -13,6 +13,7 @@ module.exports = function mixPlan(userId, params) { const { payments } = config; const route = [payments.prefix, payments.routes.planGet].join('.'); const id = 'free'; + const userMetadata = new UserMetadata(this.redis); return amqp .publishAndWait(route, id, { timeout: 5000 }) @@ -20,7 +21,7 @@ module.exports = function mixPlan(userId, params) { .then(function mix(plan) { const subscription = find(plan.subs, ['name', 'month']); const nextCycle = moment().add(1, 'month').valueOf(); - const update = { + const updateParams = { userId, audience, metadata: { @@ -36,6 +37,6 @@ module.exports = function mixPlan(userId, params) { }, }; - return setMetadata.call(this, update); + return userMetadata.batchUpdate(updateParams); }); }; diff --git a/src/custom/rfx-create-room-on-activate.js b/src/custom/rfx-create-room-on-activate.js index 81bcfa779..c48f4d345 100644 --- a/src/custom/rfx-create-room-on-activate.js +++ b/src/custom/rfx-create-room-on-activate.js @@ -1,6 +1,6 @@ const is = require('is'); const Promise = require('bluebird'); -const setMetadata = require('../utils/update-metadata'); +const UserMetadata = require('../utils/metadata/user'); /** * @param {String} username @@ -22,10 +22,12 @@ function createRoom(userId, params, metadata) { name: `${metadata[audience].stationName} | ${metadata[audience].stationSchool}`, }; + const userMetadata = new UserMetadata(this.redis); + return amqp.publishAndWait(route, roomParams, { timeout: 5000 }) .bind(this) .then((room) => { - const update = { + const updateParams = { userId, audience, metadata: { @@ -35,7 +37,7 @@ function createRoom(userId, params, metadata) { }, }; - return setMetadata.call(this, update); + return userMetadata.batchUpdate(updateParams); }); } diff --git a/src/utils/metadata/organization.js b/src/utils/metadata/organization.js new file mode 100644 index 000000000..48ba54b01 --- /dev/null +++ b/src/utils/metadata/organization.js @@ -0,0 +1,27 @@ +const Promise = require('bluebird'); +const Audience = require('./redis/audience'); +const MetaUpdate = require('./redis/update-metadata'); +const { ORGANIZATIONS_METADATA, ORGANIZATIONS_AUDIENCE } = require('../../constants'); + +class Organization { + constructor(redis) { + this.redis = redis; + this.meta = new MetaUpdate(this.redis, ORGANIZATIONS_METADATA); + this.audience = new Audience(this.redis, ORGANIZATIONS_AUDIENCE); + } + + /** + * Updates metadata on a organization object + * @param {Object} opts + * @return {Promise} + */ + async batchUpdate(opts) { + const { organizationId, ...restOpts } = opts; + const audienceWork = this.audience.batchAdd(organizationId, restOpts.audience); + + await Promise.all(audienceWork); + return this.meta.batchUpdate({ id: organizationId, ...restOpts }); + } +} + +module.exports = Organization; diff --git a/src/utils/metadata/redis/audience.js b/src/utils/metadata/redis/audience.js new file mode 100644 index 000000000..c8a6da38d --- /dev/null +++ b/src/utils/metadata/redis/audience.js @@ -0,0 +1,30 @@ +class Audience { + constructor(redis, audienceKeyBase) { + this.redis = redis; + this.audienceKeyBase = audienceKeyBase; + } + + getAudienceKey(id) { + return `${id}!${this.audienceKeyBase}`; + } + + add(id, audience, redis = this.redis) { + return redis.sadd(this.getAudienceKey(id), audience); + } + + batchAdd(id, audiences, redis = this.redis) { + const audienceWork = []; + for (const audience of Array.isArray(audiences) ? audiences : [audiences]) { + audienceWork.push( + redis.sadd(this.getAudienceKey(id), audience) + ); + } + return audienceWork; + } + + delete(id, audience, redis = this.redis) { + return redis.srem(this.getAudienceKey(id), audience); + } +} + +module.exports = Audience; diff --git a/src/utils/metadata/redis/update-metadata.js b/src/utils/metadata/redis/update-metadata.js new file mode 100644 index 000000000..168181fb0 --- /dev/null +++ b/src/utils/metadata/redis/update-metadata.js @@ -0,0 +1,173 @@ +const Promise = require('bluebird'); +const mapValues = require('lodash/mapValues'); +const assert = require('assert'); +const { Pipeline } = require('ioredis'); +const { HttpStatusError } = require('common-errors'); +const handlePipeline = require('../../pipelineError'); +const sha256 = require('../../sha256'); + +const JSONStringify = (data) => JSON.stringify(data); + +/** + * Class wraps User/Organization metadata update using atomic LUA script + */ +class UpdateMetadata { + /** + * @param redis + * @param metadataKeyTemplate + * @param audienceKeyTemplate + */ + constructor(redis, metadataKeyBase) { + this.redis = redis; + this.metadataKeyBase = metadataKeyBase; + } + + getMetadataKey(id, audience) { + return `${id}!${this.metadataKeyBase}!${audience}`; + } + + update(id, audience, key, value, redis = this.redis) { + return redis.hset(this.getMetadataKey(id, audience), key, value); + } + + updateMulti(id, audience, values, redis = this.redis) { + return redis.hmset(this.getMetadataKey(id, audience), values); + } + + delete(id, audience, key, redis = this.redis) { + return redis.hdel(this.getMetadataKey(id, audience), key); + } + + /** + * Updates metadata on a user object + * @param {Object} opts + * @return {Promise} + */ + async batchUpdate(opts) { + const { redis } = this; + assert(!(redis instanceof Pipeline), 'impossible to use with pipeline'); + const { + id, audience, metadata, script, + } = opts; + const audiences = Array.isArray(audience) ? audience : [audience]; + + // keys + const keys = audiences.map((aud) => this.getMetadataKey(id, aud)); + + // if we have meta, then we can + if (metadata) { + const pipe = redis.pipeline(); + const metaOps = Array.isArray(metadata) ? metadata : [metadata]; + + if (metaOps.length !== audiences.length) { + return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); + } + + const operations = metaOps.map((meta, idx) => UpdateMetadata.handleAudience(pipe, keys[idx], meta)); + return pipe.exec() + .then(handlePipeline) + .then((res) => UpdateMetadata.mapMetaResponse(operations, res)); + } + + // dynamic scripts + const $scriptKeys = Object.keys(script); + const scripts = $scriptKeys.map((scriptName) => { + const { lua, argv = [] } = script[scriptName]; + const sha = sha256(lua); + const name = `ms_users_${sha}`; + if (typeof redis[name] !== 'function') { + redis.defineCommand(name, { lua }); + } + return redis[name](keys.length, keys, argv); + }); + + return Promise.all(scripts).then((res) => UpdateMetadata.mapScriptResponse($scriptKeys, res)); + } + + /** + * Process metadata update operation for a passed audience + * @param {Object} pipeline + * @param {String} audience + * @param {Object} metadata + */ + static handleAudience(pipeline, key, metadata) { + const { $remove } = metadata; + const $removeOps = $remove ? $remove.length : 0; + if ($removeOps > 0) { + pipeline.hdel(key, $remove); + } + + const { $set } = metadata; + const $setKeys = $set && Object.keys($set); + const $setLength = $setKeys ? $setKeys.length : 0; + if ($setLength > 0) { + pipeline.hmset(key, mapValues($set, JSONStringify)); + } + + const { $incr } = metadata; + const $incrFields = $incr && Object.keys($incr); + const $incrLength = $incrFields ? $incrFields.length : 0; + if ($incrLength > 0) { + $incrFields.forEach((fieldName) => { + pipeline.hincrby(key, fieldName, $incr[fieldName]); + }); + } + + return { + $removeOps, $setLength, $incrLength, $incrFields, + }; + } + + /** + * Maps updateMetadata ops + * @param {Array} responses + * @param {Array} operations + * @return {Object|Array} + */ + static mapMetaResponse(operations, responses) { + let cursor = 0; + return Promise + .map(operations, (props) => { + const { + $removeOps, $setLength, $incrLength, $incrFields, + } = props; + const output = {}; + + if ($removeOps > 0) { + output.$remove = responses[cursor]; + cursor += 1; + } + + if ($setLength > 0) { + output.$set = responses[cursor]; + cursor += 1; + } + + if ($incrLength > 0) { + const $incrResponse = output.$incr = {}; + $incrFields.forEach((fieldName) => { + $incrResponse[fieldName] = responses[cursor]; + cursor += 1; + }); + } + + return output; + }) + .then((ops) => (ops.length > 1 ? ops : ops[0])); + } + + /** + * Handle script, mutually exclusive with metadata + * @param {Array} scriptKeys + * @param {Array} responses + */ + static mapScriptResponse(scriptKeys, responses) { + const output = {}; + scriptKeys.forEach((fieldName, idx) => { + output[fieldName] = responses[idx]; + }); + return output; + } +} + +module.exports = UpdateMetadata; diff --git a/src/utils/metadata/user.js b/src/utils/metadata/user.js new file mode 100644 index 000000000..fc106ba61 --- /dev/null +++ b/src/utils/metadata/user.js @@ -0,0 +1,58 @@ +const Promise = require('bluebird'); +const { Pipeline } = require('ioredis'); +const MetaUpdate = require('./redis/update-metadata'); +const Audience = require('./redis/audience'); + +const { USERS_METADATA, USERS_AUDIENCE } = require('../../constants'); + +class User { + constructor(redis) { + this.pipeline = redis instanceof Pipeline; + this.redis = redis; + this.metadata = new MetaUpdate(this.redis, USERS_METADATA); + this.audience = new Audience(this.redis, USERS_AUDIENCE); + } + + /** + * + * @param id - User id + * @param hashKey - Key in metadata + * @param value + * @param audience + * @returns {Promise} + */ + update(id, hashKey, value, audience) { + const work = [ + this.audience.add(id, audience), + this.metadata.update(id, audience, hashKey, value), + ]; + return this.pipeline ? Promise.all(work) : work; + } + + updateMulti(id, values, audience) { + const work = [ + this.audience.add(id, audience), + this.metadata.updateMulti(id, audience, values), + ]; + return this.pipeline ? Promise.all(work) : work; + } + + delete(id, hashKey, audience) { + return this.metadata.delete(id, audience, hashKey); + } + + /** + * Updates metadata on a user object using batch operations + * @param {Object} opts + * @return {Promise} + */ + async batchUpdate(opts) { + const { userId, ...restOpts } = opts; + const audienceWork = this.audience.batchAdd(userId, restOpts.audience); + + await Promise.all(audienceWork); + return this.metadata.batchUpdate({ id: userId, ...restOpts }); + } +} + +module.exports = User; diff --git a/src/utils/organization/add-organization-members.js b/src/utils/organization/add-organization-members.js index fa98eb300..5e397b079 100644 --- a/src/utils/organization/add-organization-members.js +++ b/src/utils/organization/add-organization-members.js @@ -7,9 +7,10 @@ const sendInviteMail = require('./send-invite-email'); const getInternalData = require('./get-internal-data'); const registerOrganizationMembers = require('./register-organization-members'); const handlePipeline = require('../pipeline-error'); +const UserMetadata = require('../metadata/user'); + const { ORGANIZATIONS_MEMBERS, - USERS_METADATA, ORGANIZATIONS_NAME_FIELD, USERS_ACTION_ORGANIZATION_INVITE, USERS_ACTION_ORGANIZATION_REGISTER, @@ -42,18 +43,18 @@ async function addOrganizationMembers(opts) { const createdMembers = await registerOrganizationMembers.call(this, notRegisteredMembers); const pipe = redis.pipeline(); + const userMetadata = new UserMetadata(pipe); const membersKey = redisKey(organizationId, ORGANIZATIONS_MEMBERS); const organizationMembers = registeredMembers.concat(createdMembers); organizationMembers.forEach(({ password, ...member }) => { const memberKey = redisKey(organizationId, ORGANIZATIONS_MEMBERS, member.id); - const memberOrganizations = redisKey(member.id, USERS_METADATA, audience); member.username = member.email; member.invited = Date.now(); member.accepted = password ? Date.now() : null; member.permissions = member.permissions || []; const stringifyMember = mapValues(member, JSONStringify); pipe.hmset(memberKey, stringifyMember); - pipe.hset(memberOrganizations, organizationId, stringifyMember.permissions); + userMetadata.update(member.id, organizationId, stringifyMember.permissions, audience); pipe.zadd(membersKey, stringifyMember.invited, memberKey); }); diff --git a/src/utils/organization/register-organization-members.js b/src/utils/organization/register-organization-members.js index 788a08e7f..0006cc061 100644 --- a/src/utils/organization/register-organization-members.js +++ b/src/utils/organization/register-organization-members.js @@ -14,7 +14,7 @@ const { USERS_ID_FIELD, } = require('../../constants.js'); const scrypt = require('../scrypt'); -const setMetadata = require('../update-metadata'); +const UserMetadata = require('../metadata/user'); async function registerOrganizationMember(member) { const { redis, config } = this; @@ -36,7 +36,8 @@ async function registerOrganizationMember(member) { pipeline.hset(USERS_USERNAME_TO_ID, email, userId); await pipeline.exec().then(handlePipeline); - await setMetadata.call(this, { + const userMetadata = new UserMetadata(redis); + await userMetadata.batchUpdate({ userId, audience, metadata: [{ diff --git a/src/utils/set-organization-metadata.js b/src/utils/set-organization-metadata.js index fce6659cf..6480c9417 100644 --- a/src/utils/set-organization-metadata.js +++ b/src/utils/set-organization-metadata.js @@ -1,11 +1,5 @@ /* eslint-disable no-mixed-operators */ -const Promise = require('bluebird'); -const is = require('is'); -const { HttpStatusError } = require('common-errors'); -const redisKey = require('../utils/key'); -const handlePipeline = require('../utils/pipeline-error'); -const { handleAudience } = require('../utils/update-metadata'); -const { ORGANIZATIONS_METADATA } = require('../constants'); +const OrganizationMetadata = require('../utils/metadata/organization'); /** * Updates metadata on a organization object @@ -13,29 +7,7 @@ const { ORGANIZATIONS_METADATA } = require('../constants'); * @return {Promise} */ async function setOrganizationMetadata(opts) { - const { redis } = this; - const { - organizationId, audience, metadata, - } = opts; - const audiences = is.array(audience) ? audience : [audience]; - - // keys - const keys = audiences.map((aud) => redisKey(organizationId, ORGANIZATIONS_METADATA, aud)); - - // if we have meta, then we can - if (metadata) { - const pipe = redis.pipeline(); - const metaOps = is.array(metadata) ? metadata : [metadata]; - - if (metaOps.length !== audiences.length) { - return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); - } - - metaOps.forEach((meta, idx) => handleAudience(pipe, keys[idx], meta)); - return pipe.exec().then(handlePipeline); - } - - return true; + return new OrganizationMetadata(this.redis).batchUpdate(opts); } module.exports = setOrganizationMetadata; diff --git a/test/suites/ban.js b/test/suites/ban.js index 3ab61b30f..24bb863b5 100644 --- a/test/suites/ban.js +++ b/test/suites/ban.js @@ -34,9 +34,8 @@ describe('#ban', function banSuite() { it('must be able to ban an existing user', async function test() { const response = await this.dispatch('users.ban', { username, ban: true }); - assert.equal(response[0], 1); - assert.equal(response[1], 'OK'); + assert.equal(response[2], 'OK'); }); it('requesting metadata with a special flag verifies ban state and throws', async function test() { diff --git a/test/suites/update-metadata.js b/test/suites/update-metadata.js index b03ba7492..d312e23d1 100644 --- a/test/suites/update-metadata.js +++ b/test/suites/update-metadata.js @@ -64,6 +64,7 @@ describe('#updateMetadata', function getMetadataSuite() { $incr: { b: 2, }, + $remove: ['c'], }, { $incr: { @@ -75,15 +76,13 @@ describe('#updateMetadata', function getMetadataSuite() { .then(inspectPromise()) .then((data) => { const [mainData, extraData] = data; - expect(mainData.$set).to.be.eq('OK'); expect(mainData.$incr.b).to.be.eq(2); expect(extraData.$incr.b).to.be.eq(3); }); }); - it('must be able to run dynamic scripts', function test() { - const dispatch = simpleDispatcher(this.users.router); + it('must be able to run dynamic scripts', async function test() { const params = { username, audience: [audience, extra], @@ -95,15 +94,68 @@ describe('#updateMetadata', function getMetadataSuite() { }, }; - return dispatch('users.updateMetadata', params) - .reflect() - .then(inspectPromise()) - .then((data) => { - expect(data.balance).to.be.deep.eq([ - `{ms-users}${this.userId}!metadata!${audience}`, - `{ms-users}${this.userId}!metadata!${extra}`, - 'nom-nom', - ]); - }); + const updated = await this.dispatch('users.updateMetadata', params); + + expect(updated.balance).to.be.deep.eq([ + `{ms-users}${this.userId}!metadata!${audience}`, + `{ms-users}${this.userId}!metadata!${extra}`, + 'nom-nom', + ]); + }); + + it('tracks audienceList', async function test() { + const params = { + username, + audience: [ + audience, + '*.extra', + ], + metadata: [ + { + $set: { + x: 10, + b: 12, + c: 'cval', + }, + }, { + $set: { + x: 20, + b: 22, + c: 'xval', + }, + }, + ], + }; + + await this.dispatch('users.updateMetadata', params); + const audiencesList = await this.users.redis.smembers(`${this.userId}!users-audiences`); + expect(audiencesList).to.include.members(['*.localhost', '*.extra']); + }); + + it('must be able to run dynamic scripts / default namespace available', async function test() { + const lua = ` + local t = {} + table.insert(t, "foo") + local jsonDec = cjson.decode('{"bar": 1}') + local typeCheck = type(t) + redis.call("SET", "fookey", 777); + return {jsonDec.bar, redis.call("TIME"), redis.call("GET", "fookey"), typeCheck, unpack(t)} + `; + + const params = { + username, + audience: [audience], + script: { + check: { + lua, + argv: ['nom-nom'], + }, + }, + }; + const updated = await this.dispatch('users.updateMetadata', params); + const [jsonVal, redisTime, keyValue] = updated.check; + expect(jsonVal).to.be.eq(1); + expect(redisTime).to.be.an('array'); + expect(keyValue).to.be.eq('777'); }); }); From ca15da540ec833d5b833efdbb9917e83852b4d22 Mon Sep 17 00:00:00 2001 From: pajgo Date: Thu, 31 Oct 2019 18:14:41 +0600 Subject: [PATCH 02/12] feat: update user and organization metadata * asserts and rf update --- .../user_and_organization_meta_update.md | 38 ++++----- src/actions/organization/delete.js | 5 ++ src/actions/remove.js | 4 + src/utils/asserts/id.js | 3 + src/utils/asserts/integer.js | 3 + src/utils/asserts/redis.js | 14 ++++ src/utils/asserts/string-not-empty.js | 3 + src/utils/metadata/organization.js | 15 ++-- src/utils/metadata/redis/audience.js | 49 +++++++++--- src/utils/metadata/redis/update-metadata.js | 78 +++++++++++++++---- src/utils/metadata/user.js | 39 +++++++--- 11 files changed, 187 insertions(+), 64 deletions(-) create mode 100644 src/utils/asserts/id.js create mode 100644 src/utils/asserts/integer.js create mode 100644 src/utils/asserts/redis.js create mode 100644 src/utils/asserts/string-not-empty.js diff --git a/rfcs/inactive_users/user_and_organization_meta_update.md b/rfcs/inactive_users/user_and_organization_meta_update.md index b30104076..f94d20ba0 100644 --- a/rfcs/inactive_users/user_and_organization_meta_update.md +++ b/rfcs/inactive_users/user_and_organization_meta_update.md @@ -1,30 +1,24 @@ -# User/Organization metadata update rework +# User/Organization metadata update ## Overview and Motivation -When user or organization metadata needs to be updated, the Service uses the Redis pipeline javascript code. -For each assigned meta hash always exists a single `audience`, but there is no list of `audiences` assigned to the user or company. -To achieve easier audience tracking and a combined metadata update, I advise using a Lua based script. +When user or organization metadata is updated, the Service should track audiences with assigned metadata. +For each assigned meta hash always exists a single `audience`, but there is no list of `audiences` assigned to the user or organization. -## Audience lists -Audiences stored in sets formed from `USERS_AUDIENCE` or `ORGANISATION_AUDIENCE` constants and `Id` -(eg: `{ms-users}10110110111!audiences`). Both keys contain `audience` names that are currently have assigned values. - -## DEL utils/updateMetadata.js -All logic from this file moved to separate class `utils/metadata/redis/update-metadata.js`. +To achieve this ability, I advise these updates: -## utils/metadata/{user|organization}.js -Classes that perform user or organization metadata update and -audience list tracking. - -Available methods: - * update - * updateMulti - * batchUpdate - * delete +## Audience lists +Audiences stored in sets with names created from `USERS_AUDIENCE` or `ORGANISATION_AUDIENCE` constants and `Id` +(e.g.: `{ms-users}10110110111!audiences`). Both keys contain `audience` names that are currently have assigned values. -**TODO** Fill up +The `audience` list will be updated on each update of the metadata. +## Metadata Handling classes +Service logic is updated to use 2 specific classes that will perform all CRUD operations on User or Organization metadata. -## global updates -`ms-users` source code now use new `UserMetadata` when any update operation happens. +* Classes located in: `utils/metadata/{user|organization}.js`. +* Both classes use same [Redis backend](#redis-metadata-backend-class). +## Redis Metadata Backend class +The class performs all work on metadata using Redis DB as a backend. +## Notice +* All User or Organization metadata operations should be performed using Provided classes otherwise, audiences won't be tracked. diff --git a/src/actions/organization/delete.js b/src/actions/organization/delete.js index 76065b4bf..f4e087da0 100644 --- a/src/actions/organization/delete.js +++ b/src/actions/organization/delete.js @@ -3,6 +3,7 @@ const snakeCase = require('lodash/snakeCase'); const redisKey = require('../../utils/key'); const handlePipeline = require('../../utils/pipeline-error'); const { checkOrganizationExists, getInternalData } = require('../../utils/organization'); +const OrganizationMetadata = require('../../utils/metadata/organization'); const { ORGANIZATIONS_DATA, ORGANIZATIONS_METADATA, @@ -32,11 +33,15 @@ async function deleteOrganization({ params }) { const organizationMembersListKey = redisKey(organizationId, ORGANIZATIONS_MEMBERS); const organizationMembersIds = await redis.zrange(organizationMembersListKey, 0, -1); const organization = await getInternalData.call(this, organizationId); + const organizationMetadata = new OrganizationMetadata(redis); const pipeline = redis.pipeline(); pipeline.del(redisKey(organizationId, ORGANIZATIONS_DATA)); pipeline.del(redisKey(organizationId, ORGANIZATIONS_METADATA, audience)); + // delete organization audiences index + pipeline.del(organizationMetadata.audience.getAudienceKey(organizationId)); + pipeline.srem(ORGANIZATIONS_INDEX, organizationId); if (organizationMembersIds) { organizationMembersIds.forEach((memberId) => { diff --git a/src/actions/remove.js b/src/actions/remove.js index 0951c03bf..f89f87ba2 100644 --- a/src/actions/remove.js +++ b/src/actions/remove.js @@ -7,6 +7,7 @@ const key = require('../utils/key'); const { getInternalData } = require('../utils/userData'); const getMetadata = require('../utils/get-metadata'); const handlePipeline = require('../utils/pipeline-error'); +const UserMetadata = require('../utils/metadata/user'); const { USERS_INDEX, USERS_PUBLIC_INDEX, @@ -69,6 +70,7 @@ async function removeUser({ params }) { } const transaction = redis.pipeline(); + const userMetadata = new UserMetadata(transaction); const alias = internal[USERS_ALIAS_FIELD]; const userId = internal[USERS_ID_FIELD]; const resolvedUsername = internal[USERS_USERNAME_FIELD]; @@ -94,7 +96,9 @@ async function removeUser({ params }) { // remove metadata & internal data transaction.del(key(userId, USERS_DATA)); + // TODO fix if user has multiple audiences some data will remain transaction.del(key(userId, USERS_METADATA, audience)); + transaction.del(userMetadata.audience.getAudienceKey(userId)); // remove auth tokens transaction.del(key(userId, USERS_TOKENS)); diff --git a/src/utils/asserts/id.js b/src/utils/asserts/id.js new file mode 100644 index 000000000..5ea248bee --- /dev/null +++ b/src/utils/asserts/id.js @@ -0,0 +1,3 @@ +module.exports = function isId(value) { + return Number.isInteger(value) || (typeof value === 'string' && value.length > 0); +}; diff --git a/src/utils/asserts/integer.js b/src/utils/asserts/integer.js new file mode 100644 index 000000000..56b6477c6 --- /dev/null +++ b/src/utils/asserts/integer.js @@ -0,0 +1,3 @@ +module.exports = function isInteger(value) { + return Number.isInteger(value); +}; diff --git a/src/utils/asserts/redis.js b/src/utils/asserts/redis.js new file mode 100644 index 000000000..2dd184513 --- /dev/null +++ b/src/utils/asserts/redis.js @@ -0,0 +1,14 @@ +const Redis = require('ioredis'); + +function isRedisPipeline(value) { + return value instanceof Redis.Pipeline; +} + +function isRedis(value) { + return value instanceof Redis || value instanceof Redis.Cluster || value instanceof Redis.Pipeline; +} + +module.exports = { + isRedisPipeline, + isRedis, +}; diff --git a/src/utils/asserts/string-not-empty.js b/src/utils/asserts/string-not-empty.js new file mode 100644 index 000000000..d6a2af0ba --- /dev/null +++ b/src/utils/asserts/string-not-empty.js @@ -0,0 +1,3 @@ +module.exports = function stringNotEmpty(value) { + return typeof value === 'string' && value.length !== 0; +}; diff --git a/src/utils/metadata/organization.js b/src/utils/metadata/organization.js index 48ba54b01..df8af93fd 100644 --- a/src/utils/metadata/organization.js +++ b/src/utils/metadata/organization.js @@ -1,9 +1,14 @@ -const Promise = require('bluebird'); const Audience = require('./redis/audience'); const MetaUpdate = require('./redis/update-metadata'); const { ORGANIZATIONS_METADATA, ORGANIZATIONS_AUDIENCE } = require('../../constants'); -class Organization { +/** + * Class handling Organization Metadata operations + */ +class OrganizationMetadata { + /** + * @param {ioredis|Pipeline} redis + */ constructor(redis) { this.redis = redis; this.meta = new MetaUpdate(this.redis, ORGANIZATIONS_METADATA); @@ -17,11 +22,9 @@ class Organization { */ async batchUpdate(opts) { const { organizationId, ...restOpts } = opts; - const audienceWork = this.audience.batchAdd(organizationId, restOpts.audience); - - await Promise.all(audienceWork); + await this.audience.add(organizationId, restOpts.audience); return this.meta.batchUpdate({ id: organizationId, ...restOpts }); } } -module.exports = Organization; +module.exports = OrganizationMetadata; diff --git a/src/utils/metadata/redis/audience.js b/src/utils/metadata/redis/audience.js index c8a6da38d..395a236bc 100644 --- a/src/utils/metadata/redis/audience.js +++ b/src/utils/metadata/redis/audience.js @@ -1,28 +1,57 @@ +const assert = require('assert'); +const { isRedis } = require('../../asserts/redis'); +const isNotEmptyString = require('../../asserts/string-not-empty'); +const isValidId = require('../../asserts/id'); + +/** + * Class handling Audience tracking using Redis backend + */ class Audience { + /** + * @param {ioredis|Pipeline} redis + * @param {string} audienceKeyBase + */ constructor(redis, audienceKeyBase) { + assert(isRedis(redis), 'must be ioredis instance'); + assert(isNotEmptyString(audienceKeyBase), 'must be not empty string'); this.redis = redis; this.audienceKeyBase = audienceKeyBase; } + /** + * Generates Redis key + * Template `{id}!{metadataKeyBase}` + * @param {String|Number} id + * @returns {string} + */ getAudienceKey(id) { + assert(isValidId(id), 'must be valid Id'); return `${id}!${this.audienceKeyBase}`; } + /** + * Adds audience + * @param {String|Number} id + * @param {String|Array} audience + * @param {ioredis|Pipeline} [redis] + * @returns {Promise|Pipeline} + */ add(id, audience, redis = this.redis) { + assert(isRedis(redis), 'must be ioredis instance'); + assert(isNotEmptyString(audience) || Array.isArray(audience), 'must be not empty string or Array'); return redis.sadd(this.getAudienceKey(id), audience); } - batchAdd(id, audiences, redis = this.redis) { - const audienceWork = []; - for (const audience of Array.isArray(audiences) ? audiences : [audiences]) { - audienceWork.push( - redis.sadd(this.getAudienceKey(id), audience) - ); - } - return audienceWork; - } - + /** + * Deletes audience + * @param {String|Number} id + * @param {String} audience + * @param {ioredis|Pipeline} [redis] + * @returns {Promise|Pipeline} + */ delete(id, audience, redis = this.redis) { + assert(isRedis(redis), 'must be ioredis instance'); + assert(isNotEmptyString(audience) || Array.isArray(audience), 'must be not empty string or Array'); return redis.srem(this.getAudienceKey(id), audience); } } diff --git a/src/utils/metadata/redis/update-metadata.js b/src/utils/metadata/redis/update-metadata.js index 168181fb0..41d82bfbd 100644 --- a/src/utils/metadata/redis/update-metadata.js +++ b/src/utils/metadata/redis/update-metadata.js @@ -1,57 +1,103 @@ const Promise = require('bluebird'); const mapValues = require('lodash/mapValues'); const assert = require('assert'); -const { Pipeline } = require('ioredis'); const { HttpStatusError } = require('common-errors'); -const handlePipeline = require('../../pipelineError'); +const handlePipeline = require('../../pipeline-error'); const sha256 = require('../../sha256'); +const { isRedis, isRedisPipeline } = require('../../asserts/redis'); +const isNotEmptyString = require('../../asserts/string-not-empty'); +const isValidId = require('../../asserts/id'); const JSONStringify = (data) => JSON.stringify(data); /** - * Class wraps User/Organization metadata update using atomic LUA script + * Class handling metadata update operations using Redis backend */ class UpdateMetadata { /** - * @param redis - * @param metadataKeyTemplate - * @param audienceKeyTemplate + * @param {ioredis} redis or pipeline instance + * @param metadataKeyTemplate template base for Metadata key */ constructor(redis, metadataKeyBase) { + assert(isRedis(redis), 'must be ioredis instance'); + assert(isNotEmptyString(metadataKeyBase), 'must be not empty string'); this.redis = redis; this.metadataKeyBase = metadataKeyBase; } + /** + * Generates Redis key + * Template `{id}!{metadataKeyBase}!{audience}` + * @param {String|integer} id + * @param {String} audience + * @returns {String} + */ getMetadataKey(id, audience) { + assert(isValidId(id), 'must be valid Id'); + assert(isNotEmptyString(audience), 'must be not empty string'); return `${id}!${this.metadataKeyBase}!${audience}`; } + /** + * Updates metadata hash key + * @param {String} id + * @param {String} audience + * @param {String} key - Hash key + * @param {*} value + * @param {ioredis} [redis] + * @returns {Promise|Pipeline} + */ update(id, audience, key, value, redis = this.redis) { + assert(isNotEmptyString(key), 'must be not empty string'); + assert(isRedis(redis), 'must be ioredis instance'); + assert(value, 'must not be empty'); + return redis.hset(this.getMetadataKey(id, audience), key, value); } + /** + * Updates metadata hash keys + * @param {String} id + * @param {String} audience + * @param {Object} values - Object with keys and values + * @param {ioredis} [redis] + * @returns {Promise|Pipeline} + */ updateMulti(id, audience, values, redis = this.redis) { + assert(values !== null && typeof values === 'object', 'must be an object'); return redis.hmset(this.getMetadataKey(id, audience), values); } + /** + * Deletes metadata hash keys + * @param {String} id + * @param {String} audience + * @param {String} key - Hash key + * @param {ioredis} [redis] + * @returns {Promise|Pipeline} + */ delete(id, audience, key, redis = this.redis) { + assert(isNotEmptyString(key) || Array.isArray(key), 'must be not empty string or Array'); + assert(isRedis(redis), 'must be ioredis instance'); return redis.hdel(this.getMetadataKey(id, audience), key); } /** - * Updates metadata on a user object + * Updates metadata hash on provided Id * @param {Object} opts - * @return {Promise} + * @return {*} */ async batchUpdate(opts) { const { redis } = this; - assert(!(redis instanceof Pipeline), 'impossible to use with pipeline'); const { id, audience, metadata, script, } = opts; - const audiences = Array.isArray(audience) ? audience : [audience]; - // keys + // we use own pipeline or Promise here + assert(!isRedisPipeline(redis), 'impossible to use with pipeline'); + assert(isValidId(id), 'must be valid id'); + + const audiences = Array.isArray(audience) ? audience : [audience]; const keys = audiences.map((aud) => this.getMetadataKey(id, aud)); // if we have meta, then we can @@ -64,9 +110,9 @@ class UpdateMetadata { } const operations = metaOps.map((meta, idx) => UpdateMetadata.handleAudience(pipe, keys[idx], meta)); - return pipe.exec() - .then(handlePipeline) - .then((res) => UpdateMetadata.mapMetaResponse(operations, res)); + const result = handlePipeline(await pipe.exec()); + + return UpdateMetadata.mapMetaResponse(operations, result); } // dynamic scripts @@ -81,7 +127,9 @@ class UpdateMetadata { return redis[name](keys.length, keys, argv); }); - return Promise.all(scripts).then((res) => UpdateMetadata.mapScriptResponse($scriptKeys, res)); + const result = await Promise.all(scripts); + + return UpdateMetadata.mapScriptResponse($scriptKeys, result); } /** diff --git a/src/utils/metadata/user.js b/src/utils/metadata/user.js index fc106ba61..074bef656 100644 --- a/src/utils/metadata/user.js +++ b/src/utils/metadata/user.js @@ -5,7 +5,13 @@ const Audience = require('./redis/audience'); const { USERS_METADATA, USERS_AUDIENCE } = require('../../constants'); -class User { +/** + * Class handles User metadata operations + */ +class UserMetadata { + /** + * @param {ioredis|Pipeline} redis + */ constructor(redis) { this.pipeline = redis instanceof Pipeline; this.redis = redis; @@ -14,12 +20,11 @@ class User { } /** - * - * @param id - User id - * @param hashKey - Key in metadata - * @param value - * @param audience - * @returns {Promise} + * Updates metadata field on a user object + * @param {String|Number} id + * @param {Object} values + * @param {String} audience + * @returns {Promise|void} */ update(id, hashKey, value, audience) { const work = [ @@ -29,6 +34,13 @@ class User { return this.pipeline ? Promise.all(work) : work; } + /** + * Updates metadata on a user object using fields and values from provided Object + * @param {String|Number} id + * @param {Object} values + * @param {String} audience + * @returns {Promise|void} + */ updateMulti(id, values, audience) { const work = [ this.audience.add(id, audience), @@ -37,6 +49,13 @@ class User { return this.pipeline ? Promise.all(work) : work; } + /** + * Deletes key from user metadata object + * @param {String|Number} id + * @param {String} hashKey + * @param {String} audience + * @returns {Promise|void} + */ delete(id, hashKey, audience) { return this.metadata.delete(id, audience, hashKey); } @@ -48,11 +67,9 @@ class User { */ async batchUpdate(opts) { const { userId, ...restOpts } = opts; - const audienceWork = this.audience.batchAdd(userId, restOpts.audience); - - await Promise.all(audienceWork); + await this.audience.add(userId, restOpts.audience); return this.metadata.batchUpdate({ id: userId, ...restOpts }); } } -module.exports = User; +module.exports = UserMetadata; From db3902caf67bdf56371a3aacb5f382ab43f84915 Mon Sep 17 00:00:00 2001 From: pajgo Date: Thu, 31 Oct 2019 19:00:32 +0600 Subject: [PATCH 03/12] feat: update user and organization metadata * typo fix --- src/actions/organization/members/permission.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions/organization/members/permission.js b/src/actions/organization/members/permission.js index 00e094292..59ce7362d 100644 --- a/src/actions/organization/members/permission.js +++ b/src/actions/organization/members/permission.js @@ -44,7 +44,7 @@ async function setOrganizationMemberPermission({ params }) { const pipeline = redis.pipeline(); const userMetadata = new UserMetadata(pipeline); - userMetadata.update(userId, organizationId, permissions); + userMetadata.update(userId, organizationId, permissions, audience); pipeline.hset(redisKey(organizationId, ORGANIZATIONS_MEMBERS, userId), 'permissions', permissions); return pipeline.exec().then(handlePipeline); From 42f609b1bac276dc7a86594448284fb8a55020da Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 1 Nov 2019 15:12:22 +0600 Subject: [PATCH 04/12] feat: update user and organization metadata * slight api changes --- src/actions/alias.js | 5 +- src/actions/ban.js | 16 +- .../organization/members/permission.js | 5 +- src/actions/organization/members/remove.js | 5 +- src/actions/register.js | 23 ++- src/actions/remove.js | 2 +- src/actions/updateMetadata.js | 9 +- src/auth/oauth/utils/attach.js | 7 +- src/auth/oauth/utils/detach.js | 7 +- src/custom/cappasity-users-activate.js | 7 +- src/custom/rfx-create-room-on-activate.js | 6 +- src/utils/metadata/organization.js | 15 +- src/utils/metadata/redis/audience.js | 24 +++ src/utils/metadata/redis/update-metadata.js | 2 +- src/utils/metadata/user.js | 66 +++++--- .../organization/add-organization-members.js | 5 +- .../register-organization-members.js | 23 ++- src/utils/update-metadata.js | 144 ------------------ test/suites/update-metadata.js | 94 ++++++------ 19 files changed, 188 insertions(+), 277 deletions(-) delete mode 100644 src/utils/update-metadata.js diff --git a/src/actions/alias.js b/src/actions/alias.js index 15d030816..0393e9624 100644 --- a/src/actions/alias.js +++ b/src/actions/alias.js @@ -71,10 +71,11 @@ async function assignAlias({ params }) { } const pipeline = redis.pipeline(); - const userMetaData = new UserMetadata(pipeline); pipeline.hset(key(userId, USERS_DATA), USERS_ALIAS_FIELD, alias); - userMetaData.update(userId, USERS_ALIAS_FIELD, JSON.stringify(alias), defaultAudience); + UserMetadata + .for(userId, defaultAudience, pipeline) + .update(USERS_ALIAS_FIELD, JSON.stringify(alias)); if (activeUser) { pipeline.sadd(USERS_PUBLIC_INDEX, username); diff --git a/src/actions/ban.js b/src/actions/ban.js index 15c0c198e..d9ce7d207 100644 --- a/src/actions/ban.js +++ b/src/actions/ban.js @@ -27,11 +27,12 @@ function lockUser({ }, }; const pipeline = redis.pipeline(); - const userMetadata = new UserMetadata(pipeline); pipeline.hset(redisKey(id, USERS_DATA), USERS_BANNED_FLAG, 'true'); // set .banned on metadata for filtering & sorting users by that field - userMetadata.updateMulti(id, mapValues(data, stringify), defaultAudience); + UserMetadata + .for(id, defaultAudience, pipeline) + .updateMulti(mapValues(data, stringify)); pipeline.del(redisKey(id, USERS_TOKENS)); return pipeline.exec(); @@ -41,14 +42,15 @@ function unlockUser({ id }) { const { redis, config } = this; const { jwt: { defaultAudience } } = config; const pipeline = redis.pipeline(); - const userMetadata = new UserMetadata(pipeline); pipeline.hdel(redisKey(id, USERS_DATA), USERS_BANNED_FLAG); // remove .banned on metadata for filtering & sorting users by that field - userMetadata.delete(id, [ - 'banned', - USERS_BANNED_DATA, - ], defaultAudience); + UserMetadata + .for(id, defaultAudience, pipeline) + .delete([ + 'banned', + USERS_BANNED_DATA, + ]); return pipeline.exec(); } diff --git a/src/actions/organization/members/permission.js b/src/actions/organization/members/permission.js index 59ce7362d..ef8cb0169 100644 --- a/src/actions/organization/members/permission.js +++ b/src/actions/organization/members/permission.js @@ -42,9 +42,10 @@ async function setOrganizationMemberPermission({ params }) { permissions = JSON.stringify(permissions); const pipeline = redis.pipeline(); - const userMetadata = new UserMetadata(pipeline); - userMetadata.update(userId, organizationId, permissions, audience); + UserMetadata + .for(userId, audience, pipeline) + .update(organizationId, permissions); pipeline.hset(redisKey(organizationId, ORGANIZATIONS_MEMBERS, userId), 'permissions', permissions); return pipeline.exec().then(handlePipeline); diff --git a/src/actions/organization/members/remove.js b/src/actions/organization/members/remove.js index 142964b05..26d0381cb 100644 --- a/src/actions/organization/members/remove.js +++ b/src/actions/organization/members/remove.js @@ -35,10 +35,11 @@ async function removeMember({ params }) { } const pipeline = redis.pipeline(); - const userMetadata = new UserMetadata(pipeline); pipeline.del(memberKey); pipeline.zrem(redisKey(organizationId, ORGANIZATIONS_MEMBERS), memberKey); - userMetadata.delete(userId, organizationId, audience); + UserMetadata + .for(userId, audience, pipeline) + .delete(organizationId); return pipeline.exec().then(handlePipeline); } diff --git a/src/actions/register.js b/src/actions/register.js index a13ead175..c31efd0e8 100644 --- a/src/actions/register.js +++ b/src/actions/register.js @@ -212,18 +212,17 @@ async function performRegistration({ service, params }) { } await pipeline.exec().then(handlePipeline); - - await new UserMetadata(service.redis).batchUpdate({ - userId, - audience, - metadata: audience.map((metaAudience) => ({ - $set: Object.assign(metadata[metaAudience] || {}, metaAudience === defaultAudience && { - [USERS_ID_FIELD]: userId, - [USERS_USERNAME_FIELD]: username, - [USERS_CREATED_FIELD]: created, - }), - })), - }); + await UserMetadata + .for(userId, audience, service.redis) + .batchUpdate({ + metadata: audience.map((metaAudience) => ({ + $set: Object.assign(metadata[metaAudience] || {}, metaAudience === defaultAudience && { + [USERS_ID_FIELD]: userId, + [USERS_USERNAME_FIELD]: username, + [USERS_CREATED_FIELD]: created, + }), + })), + }); // assign alias if (alias) { diff --git a/src/actions/remove.js b/src/actions/remove.js index f89f87ba2..0cae563b6 100644 --- a/src/actions/remove.js +++ b/src/actions/remove.js @@ -70,7 +70,7 @@ async function removeUser({ params }) { } const transaction = redis.pipeline(); - const userMetadata = new UserMetadata(transaction); + const userMetadata = new UserMetadata(redis); const alias = internal[USERS_ALIAS_FIELD]; const userId = internal[USERS_ID_FIELD]; const resolvedUsername = internal[USERS_USERNAME_FIELD]; diff --git a/src/actions/updateMetadata.js b/src/actions/updateMetadata.js index 024779d32..a4a1b5e1b 100644 --- a/src/actions/updateMetadata.js +++ b/src/actions/updateMetadata.js @@ -1,4 +1,3 @@ -const omit = require('lodash/omit'); const Promise = require('bluebird'); const UserMetadata = require('../utils/metadata/user'); const { getUserId } = require('../utils/userData'); @@ -20,14 +19,14 @@ const { getUserId } = require('../utils/userData'); * Be careful with granting access to this function. */ module.exports = async function updateMetadataAction(request) { + const { username: _, audience, ...updateParams } = request.params; const userId = await Promise .bind(this, request.params.username) .then(getUserId); - const userMetadata = new UserMetadata(this.redis); - const updateParams = { ...omit(request.params, 'username'), userId }; - - return userMetadata.batchUpdate(updateParams); + return UserMetadata + .for(userId, audience, this.redis) + .batchUpdate(updateParams); }; module.exports.transports = [require('@microfleet/core').ActionTransport.amqp]; diff --git a/src/auth/oauth/utils/attach.js b/src/auth/oauth/utils/attach.js index 88a2d8aba..ebd90c7d4 100644 --- a/src/auth/oauth/utils/attach.js +++ b/src/auth/oauth/utils/attach.js @@ -25,17 +25,16 @@ module.exports = async function attach(account, user) { await pipeline.exec().then(handlePipeline); - const userMetadata = new UserMetadata(redis); const updateParams = { - userId, - audience, metadata: { $set: { [provider]: profile, }, }, }; - await userMetadata.batchUpdate(updateParams); + await UserMetadata + .for(userId, audience, redis) + .batchUpdate(updateParams); return profile; }; diff --git a/src/auth/oauth/utils/detach.js b/src/auth/oauth/utils/detach.js index 3ddbbf3d4..408423da0 100644 --- a/src/auth/oauth/utils/detach.js +++ b/src/auth/oauth/utils/detach.js @@ -30,10 +30,7 @@ module.exports = async function detach(provider, userData) { await pipeline.exec().then(handlePipeline); - const userMetadata = new UserMetadata(redis); const updateParams = { - userId, - audience, metadata: { $remove: [ provider, @@ -41,5 +38,7 @@ module.exports = async function detach(provider, userData) { }, }; - return userMetadata.batchUpdate(updateParams); + return UserMetadata + .for(userId, audience, redis) + .batchUpdate(updateParams); }; diff --git a/src/custom/cappasity-users-activate.js b/src/custom/cappasity-users-activate.js index 0c1edb930..5a24186d6 100644 --- a/src/custom/cappasity-users-activate.js +++ b/src/custom/cappasity-users-activate.js @@ -13,7 +13,6 @@ module.exports = function mixPlan(userId, params) { const { payments } = config; const route = [payments.prefix, payments.routes.planGet].join('.'); const id = 'free'; - const userMetadata = new UserMetadata(this.redis); return amqp .publishAndWait(route, id, { timeout: 5000 }) @@ -22,8 +21,6 @@ module.exports = function mixPlan(userId, params) { const subscription = find(plan.subs, ['name', 'month']); const nextCycle = moment().add(1, 'month').valueOf(); const updateParams = { - userId, - audience, metadata: { $set: { plan: id, @@ -37,6 +34,8 @@ module.exports = function mixPlan(userId, params) { }, }; - return userMetadata.batchUpdate(updateParams); + return UserMetadata + .for(userId, audience, this.redis) + .batchUpdate(updateParams); }); }; diff --git a/src/custom/rfx-create-room-on-activate.js b/src/custom/rfx-create-room-on-activate.js index c48f4d345..5a84db0aa 100644 --- a/src/custom/rfx-create-room-on-activate.js +++ b/src/custom/rfx-create-room-on-activate.js @@ -22,8 +22,6 @@ function createRoom(userId, params, metadata) { name: `${metadata[audience].stationName} | ${metadata[audience].stationSchool}`, }; - const userMetadata = new UserMetadata(this.redis); - return amqp.publishAndWait(route, roomParams, { timeout: 5000 }) .bind(this) .then((room) => { @@ -37,7 +35,9 @@ function createRoom(userId, params, metadata) { }, }; - return userMetadata.batchUpdate(updateParams); + return UserMetadata + .for(userId, audience, this.redis) + .batchUpdate(updateParams); }); } diff --git a/src/utils/metadata/organization.js b/src/utils/metadata/organization.js index df8af93fd..6acd5d58a 100644 --- a/src/utils/metadata/organization.js +++ b/src/utils/metadata/organization.js @@ -11,10 +11,15 @@ class OrganizationMetadata { */ constructor(redis) { this.redis = redis; - this.meta = new MetaUpdate(this.redis, ORGANIZATIONS_METADATA); + this.metadata = new MetaUpdate(this.redis, ORGANIZATIONS_METADATA); this.audience = new Audience(this.redis, ORGANIZATIONS_AUDIENCE); } + async syncAudience(organizationId) { + const metaKeyTemplate = this.metadata.getMetadataKey('{{ID}}', '{{AUDIENCE}}'); + return this.audience.resyncSet(organizationId, metaKeyTemplate); + } + /** * Updates metadata on a organization object * @param {Object} opts @@ -23,7 +28,13 @@ class OrganizationMetadata { async batchUpdate(opts) { const { organizationId, ...restOpts } = opts; await this.audience.add(organizationId, restOpts.audience); - return this.meta.batchUpdate({ id: organizationId, ...restOpts }); + const updateResult = await this.metadata + .batchUpdate({ + id: organizationId, + ...opts, + }); + await this.syncAudience(organizationId); + return updateResult; } } diff --git a/src/utils/metadata/redis/audience.js b/src/utils/metadata/redis/audience.js index 395a236bc..d207d84b7 100644 --- a/src/utils/metadata/redis/audience.js +++ b/src/utils/metadata/redis/audience.js @@ -54,6 +54,30 @@ class Audience { assert(isNotEmptyString(audience) || Array.isArray(audience), 'must be not empty string or Array'); return redis.srem(this.getAudienceKey(id), audience); } + + /** + * Synchronizes audience list with currently available metadata + * @param id + * @param metadataKeyTemplate - format '{{ID}}!yourMetadataClass!{{AUDIENCE}}' + * @param redis + * @returns {*} + */ + resyncSet(id, metadataKeyTemplate, redis = this.redis) { + assert(isRedis(this.redis), 'must be ioredis instance'); + assert(isNotEmptyString(metadataKeyTemplate), 'must be not empty string'); + const luaScript = ` + local audiences = redis.call("SMEMBERS", KEYS[1]) + for _, audience in pairs(audiences) do + local metaKey = string.gsub(KEYS[2], '{{ID}}', '${id}') + metaKey = string.gsub(metaKey, '{{AUDIENCE}}', audience) + local keyLen = redis.call("HLEN", metaKey) + if (keyLen < 1) then + redis.call('SREM', KEYS[1], audience) + end + end + `; + return redis.eval(luaScript, 2, this.getAudienceKey(id), metadataKeyTemplate); + } } module.exports = Audience; diff --git a/src/utils/metadata/redis/update-metadata.js b/src/utils/metadata/redis/update-metadata.js index 41d82bfbd..2fd1cf3f5 100644 --- a/src/utils/metadata/redis/update-metadata.js +++ b/src/utils/metadata/redis/update-metadata.js @@ -90,7 +90,7 @@ class UpdateMetadata { async batchUpdate(opts) { const { redis } = this; const { - id, audience, metadata, script, + id, metadata, audience, script, } = opts; // we use own pipeline or Promise here diff --git a/src/utils/metadata/user.js b/src/utils/metadata/user.js index 074bef656..33875282c 100644 --- a/src/utils/metadata/user.js +++ b/src/utils/metadata/user.js @@ -11,53 +11,71 @@ const { USERS_METADATA, USERS_AUDIENCE } = require('../../constants'); class UserMetadata { /** * @param {ioredis|Pipeline} redis + * @param {String|Number} userId + * @param {Staing|Array} audience */ - constructor(redis) { + constructor(redis, userId, audience) { this.pipeline = redis instanceof Pipeline; this.redis = redis; + this.userAudience = audience; + this.userId = userId; this.metadata = new MetaUpdate(this.redis, USERS_METADATA); this.audience = new Audience(this.redis, USERS_AUDIENCE); } /** * Updates metadata field on a user object - * @param {String|Number} id * @param {Object} values - * @param {String} audience + * @param {String} [audience] * @returns {Promise|void} */ - update(id, hashKey, value, audience) { + update(hashKey, value, audience = this.userAudience) { const work = [ - this.audience.add(id, audience), - this.metadata.update(id, audience, hashKey, value), + this.audience.add(this.userId, audience), + this.metadata.update(this.userId, audience, hashKey, value), ]; - return this.pipeline ? Promise.all(work) : work; + return this.pipeline ? work : Promise.all(work); } /** * Updates metadata on a user object using fields and values from provided Object - * @param {String|Number} id * @param {Object} values - * @param {String} audience + * @param {String} [audience] * @returns {Promise|void} */ - updateMulti(id, values, audience) { + updateMulti(values, audience = this.userAudience) { const work = [ - this.audience.add(id, audience), - this.metadata.updateMulti(id, audience, values), + this.audience.add(this.userId, audience), + this.metadata.updateMulti(this.userId, audience, values), ]; - return this.pipeline ? Promise.all(work) : work; + return this.pipeline ? work : Promise.all(work); } /** * Deletes key from user metadata object * @param {String|Number} id * @param {String} hashKey - * @param {String} audience + * @param {String} [audience] * @returns {Promise|void} */ - delete(id, hashKey, audience) { - return this.metadata.delete(id, audience, hashKey); + delete(hashKey, audience = this.userAudience) { + return this.pipeline ? this.deletePipeline(hashKey, audience) : this.deleteAsync(hashKey, audience); + } + + deletePipeline(hashKey, audience) { + this.metadata.delete(this.userId, audience, hashKey); + return this.syncAudience(); + } + + async deleteAsync(hashKey, audience) { + const result = await this.metadata.delete(this.userId, audience, hashKey); + await this.syncAudience(); + return result; + } + + async syncAudience() { + const metaKeyTemplate = this.metadata.getMetadataKey('{{ID}}', '{{AUDIENCE}}'); + return this.audience.resyncSet(this.userId, metaKeyTemplate); } /** @@ -66,9 +84,19 @@ class UserMetadata { * @return {Promise} */ async batchUpdate(opts) { - const { userId, ...restOpts } = opts; - await this.audience.add(userId, restOpts.audience); - return this.metadata.batchUpdate({ id: userId, ...restOpts }); + await this.audience.add(this.userId, this.userAudience); + const updateResult = await this.metadata + .batchUpdate({ + id: this.userId, + audience: this.userAudience, + ...opts, + }); + await this.syncAudience(); + return updateResult; + } + + static for(userId, audience, redis) { + return new UserMetadata(redis, userId, audience); } } diff --git a/src/utils/organization/add-organization-members.js b/src/utils/organization/add-organization-members.js index 5e397b079..7437013ab 100644 --- a/src/utils/organization/add-organization-members.js +++ b/src/utils/organization/add-organization-members.js @@ -43,7 +43,6 @@ async function addOrganizationMembers(opts) { const createdMembers = await registerOrganizationMembers.call(this, notRegisteredMembers); const pipe = redis.pipeline(); - const userMetadata = new UserMetadata(pipe); const membersKey = redisKey(organizationId, ORGANIZATIONS_MEMBERS); const organizationMembers = registeredMembers.concat(createdMembers); organizationMembers.forEach(({ password, ...member }) => { @@ -54,7 +53,9 @@ async function addOrganizationMembers(opts) { member.permissions = member.permissions || []; const stringifyMember = mapValues(member, JSONStringify); pipe.hmset(memberKey, stringifyMember); - userMetadata.update(member.id, organizationId, stringifyMember.permissions, audience); + UserMetadata + .for(member.id, audience, pipe) + .update(organizationId, stringifyMember.permissions); pipe.zadd(membersKey, stringifyMember.invited, memberKey); }); diff --git a/src/utils/organization/register-organization-members.js b/src/utils/organization/register-organization-members.js index 0006cc061..ebc39e2e8 100644 --- a/src/utils/organization/register-organization-members.js +++ b/src/utils/organization/register-organization-members.js @@ -36,18 +36,17 @@ async function registerOrganizationMember(member) { pipeline.hset(USERS_USERNAME_TO_ID, email, userId); await pipeline.exec().then(handlePipeline); - const userMetadata = new UserMetadata(redis); - await userMetadata.batchUpdate({ - userId, - audience, - metadata: [{ - $set: { - [USERS_ID_FIELD]: userId, - [USERS_USERNAME_FIELD]: email, - [USERS_CREATED_FIELD]: basicInfo[USERS_CREATED_FIELD], - }, - }], - }); + await UserMetadata + .for(userId, audience, redis) + .batchUpdate({ + metadata: [{ + $set: { + [USERS_ID_FIELD]: userId, + [USERS_USERNAME_FIELD]: email, + [USERS_CREATED_FIELD]: basicInfo[USERS_CREATED_FIELD], + }, + }], + }); // perform instant activation // internal username index const regPipeline = redis.pipeline().sadd(USERS_INDEX, userId); diff --git a/src/utils/update-metadata.js b/src/utils/update-metadata.js deleted file mode 100644 index f93ea9070..000000000 --- a/src/utils/update-metadata.js +++ /dev/null @@ -1,144 +0,0 @@ -/* eslint-disable no-mixed-operators */ -const Promise = require('bluebird'); -const mapValues = require('lodash/mapValues'); -const is = require('is'); -const { HttpStatusError } = require('common-errors'); -const redisKey = require('../utils/key'); -const sha256 = require('./sha256'); -const handlePipeline = require('../utils/pipeline-error'); -const { USERS_METADATA } = require('../constants'); - -const JSONStringify = (data) => JSON.stringify(data); - -/** - * Process metadata update operation for a passed audience - * @param {Object} pipeline - * @param {String} audience - * @param {Object} metadata - */ -function handleAudience(pipeline, key, metadata) { - const { $remove } = metadata; - const $removeOps = $remove && $remove.length || 0; - if ($removeOps > 0) { - pipeline.hdel(key, $remove); - } - - const { $set } = metadata; - const $setKeys = $set && Object.keys($set); - const $setLength = $setKeys && $setKeys.length || 0; - if ($setLength > 0) { - pipeline.hmset(key, mapValues($set, JSONStringify)); - } - - const { $incr } = metadata; - const $incrFields = $incr && Object.keys($incr); - const $incrLength = $incrFields && $incrFields.length || 0; - if ($incrLength > 0) { - $incrFields.forEach((fieldName) => { - pipeline.hincrby(key, fieldName, $incr[fieldName]); - }); - } - - return { - $removeOps, $setLength, $incrLength, $incrFields, - }; -} - -/** - * Maps updateMetadata ops - * @param {Array} responses - * @param {Array} operations - * @return {Object|Array} - */ -function mapMetaResponse(operations, responses) { - let cursor = 0; - return Promise - .map(operations, (props) => { - const { - $removeOps, $setLength, $incrLength, $incrFields, - } = props; - const output = {}; - - if ($removeOps > 0) { - output.$remove = responses[cursor]; - cursor += 1; - } - - if ($setLength > 0) { - output.$set = responses[cursor]; - cursor += 1; - } - - if ($incrLength > 0) { - const $incrResponse = output.$incr = {}; - $incrFields.forEach((fieldName) => { - $incrResponse[fieldName] = responses[cursor]; - cursor += 1; - }); - } - - return output; - }) - .then((ops) => (ops.length > 1 ? ops : ops[0])); -} - -/** - * Handle script, mutually exclusive with metadata - * @param {Array} scriptKeys - * @param {Array} responses - */ -function mapScriptResponse(scriptKeys, responses) { - const output = {}; - scriptKeys.forEach((fieldName, idx) => { - output[fieldName] = responses[idx]; - }); - return output; -} - -/** - * Updates metadata on a user object - * @param {Object} opts - * @return {Promise} - */ -function updateMetadata(opts) { - const { redis } = this; - const { - userId, audience, metadata, script, - } = opts; - const audiences = is.array(audience) ? audience : [audience]; - - // keys - const keys = audiences.map((aud) => redisKey(userId, USERS_METADATA, aud)); - - // if we have meta, then we can - if (metadata) { - const pipe = redis.pipeline(); - const metaOps = is.array(metadata) ? metadata : [metadata]; - - if (metaOps.length !== audiences.length) { - return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); - } - - const operations = metaOps.map((meta, idx) => handleAudience(pipe, keys[idx], meta)); - return pipe.exec() - .then(handlePipeline) - .then((res) => mapMetaResponse(operations, res)); - } - - // dynamic scripts - const $scriptKeys = Object.keys(script); - const scripts = $scriptKeys.map((scriptName) => { - const { lua, argv = [] } = script[scriptName]; - const sha = sha256(lua); - const name = `ms_users_${sha}`; - if (!is.fn(redis[name])) { - redis.defineCommand(name, { lua }); - } - return redis[name](keys.length, keys, argv); - }); - - return Promise.all(scripts).then((res) => mapScriptResponse($scriptKeys, res)); -} - -updateMetadata.handleAudience = handleAudience; -module.exports = updateMetadata; diff --git a/test/suites/update-metadata.js b/test/suites/update-metadata.js index d312e23d1..b1320b23c 100644 --- a/test/suites/update-metadata.js +++ b/test/suites/update-metadata.js @@ -103,59 +103,51 @@ describe('#updateMetadata', function getMetadataSuite() { ]); }); - it('tracks audienceList', async function test() { - const params = { - username, - audience: [ - audience, - '*.extra', - ], - metadata: [ - { - $set: { - x: 10, - b: 12, - c: 'cval', - }, - }, { - $set: { - x: 20, - b: 22, - c: 'xval', + describe('tracks audienceList', function audienceTrackSuite() { + beforeEach(async function updateUser() { + const params = { + username, + audience: [ + audience, + '*.extra', + ], + metadata: [ + { + $set: { + x: 10, + b: 12, + c: 'cval', + }, + }, { + $set: { + x: 20, + b: 22, + c: 'xval', + }, }, - }, - ], - }; - - await this.dispatch('users.updateMetadata', params); - const audiencesList = await this.users.redis.smembers(`${this.userId}!users-audiences`); - expect(audiencesList).to.include.members(['*.localhost', '*.extra']); - }); + ], + }; - it('must be able to run dynamic scripts / default namespace available', async function test() { - const lua = ` - local t = {} - table.insert(t, "foo") - local jsonDec = cjson.decode('{"bar": 1}') - local typeCheck = type(t) - redis.call("SET", "fookey", 777); - return {jsonDec.bar, redis.call("TIME"), redis.call("GET", "fookey"), typeCheck, unpack(t)} - `; + await this.dispatch('users.updateMetadata', params); + }); + it('adds audience', async function test() { + const audiencesList = await this.users.redis.smembers(`${this.userId}!users-audiences`); + expect(audiencesList).to.include.members(['*.localhost', '*.extra']); + }); - const params = { - username, - audience: [audience], - script: { - check: { - lua, - argv: ['nom-nom'], - }, - }, - }; - const updated = await this.dispatch('users.updateMetadata', params); - const [jsonVal, redisTime, keyValue] = updated.check; - expect(jsonVal).to.be.eq(1); - expect(redisTime).to.be.an('array'); - expect(keyValue).to.be.eq('777'); + it('deletes audience when no metadata left', async function test() { + const deleteParams = { + username, + audience: ['*.extra'], + metadata: [ + { + $rem: ['x', 'b', 'c'], + }, + ], + }; + await this.dispatch('users.updateMetadata', deleteParams); + const audiencesList = await this.users.redis.smembers(`${this.userId}!users-audiences`); + expect(audiencesList).to.include.members(['*.localhost']); + }); }); }); From bf27314d5740cfb78960386445544f7e59bee092 Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 1 Nov 2019 15:13:06 +0600 Subject: [PATCH 05/12] feat: update user and organization metadata * UserMetadata for->using --- src/actions/alias.js | 2 +- src/actions/ban.js | 4 ++-- src/actions/organization/members/permission.js | 2 +- src/actions/organization/members/remove.js | 2 +- src/actions/register.js | 2 +- src/actions/updateMetadata.js | 2 +- src/auth/oauth/utils/attach.js | 2 +- src/auth/oauth/utils/detach.js | 2 +- src/custom/cappasity-users-activate.js | 2 +- src/custom/rfx-create-room-on-activate.js | 2 +- src/utils/metadata/user.js | 2 +- src/utils/organization/add-organization-members.js | 2 +- src/utils/organization/register-organization-members.js | 2 +- 13 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/actions/alias.js b/src/actions/alias.js index 0393e9624..616785c78 100644 --- a/src/actions/alias.js +++ b/src/actions/alias.js @@ -74,7 +74,7 @@ async function assignAlias({ params }) { pipeline.hset(key(userId, USERS_DATA), USERS_ALIAS_FIELD, alias); UserMetadata - .for(userId, defaultAudience, pipeline) + .using(userId, defaultAudience, pipeline) .update(USERS_ALIAS_FIELD, JSON.stringify(alias)); if (activeUser) { diff --git a/src/actions/ban.js b/src/actions/ban.js index d9ce7d207..9ff92c1b5 100644 --- a/src/actions/ban.js +++ b/src/actions/ban.js @@ -31,7 +31,7 @@ function lockUser({ pipeline.hset(redisKey(id, USERS_DATA), USERS_BANNED_FLAG, 'true'); // set .banned on metadata for filtering & sorting users by that field UserMetadata - .for(id, defaultAudience, pipeline) + .using(id, defaultAudience, pipeline) .updateMulti(mapValues(data, stringify)); pipeline.del(redisKey(id, USERS_TOKENS)); @@ -46,7 +46,7 @@ function unlockUser({ id }) { pipeline.hdel(redisKey(id, USERS_DATA), USERS_BANNED_FLAG); // remove .banned on metadata for filtering & sorting users by that field UserMetadata - .for(id, defaultAudience, pipeline) + .using(id, defaultAudience, pipeline) .delete([ 'banned', USERS_BANNED_DATA, diff --git a/src/actions/organization/members/permission.js b/src/actions/organization/members/permission.js index ef8cb0169..bdc4aa1ef 100644 --- a/src/actions/organization/members/permission.js +++ b/src/actions/organization/members/permission.js @@ -44,7 +44,7 @@ async function setOrganizationMemberPermission({ params }) { const pipeline = redis.pipeline(); UserMetadata - .for(userId, audience, pipeline) + .using(userId, audience, pipeline) .update(organizationId, permissions); pipeline.hset(redisKey(organizationId, ORGANIZATIONS_MEMBERS, userId), 'permissions', permissions); diff --git a/src/actions/organization/members/remove.js b/src/actions/organization/members/remove.js index 26d0381cb..30292f7dd 100644 --- a/src/actions/organization/members/remove.js +++ b/src/actions/organization/members/remove.js @@ -38,7 +38,7 @@ async function removeMember({ params }) { pipeline.del(memberKey); pipeline.zrem(redisKey(organizationId, ORGANIZATIONS_MEMBERS), memberKey); UserMetadata - .for(userId, audience, pipeline) + .using(userId, audience, pipeline) .delete(organizationId); return pipeline.exec().then(handlePipeline); diff --git a/src/actions/register.js b/src/actions/register.js index c31efd0e8..6e81a5bd2 100644 --- a/src/actions/register.js +++ b/src/actions/register.js @@ -213,7 +213,7 @@ async function performRegistration({ service, params }) { await pipeline.exec().then(handlePipeline); await UserMetadata - .for(userId, audience, service.redis) + .using(userId, audience, service.redis) .batchUpdate({ metadata: audience.map((metaAudience) => ({ $set: Object.assign(metadata[metaAudience] || {}, metaAudience === defaultAudience && { diff --git a/src/actions/updateMetadata.js b/src/actions/updateMetadata.js index a4a1b5e1b..d60558ed1 100644 --- a/src/actions/updateMetadata.js +++ b/src/actions/updateMetadata.js @@ -25,7 +25,7 @@ module.exports = async function updateMetadataAction(request) { .then(getUserId); return UserMetadata - .for(userId, audience, this.redis) + .using(userId, audience, this.redis) .batchUpdate(updateParams); }; diff --git a/src/auth/oauth/utils/attach.js b/src/auth/oauth/utils/attach.js index ebd90c7d4..cc71642a3 100644 --- a/src/auth/oauth/utils/attach.js +++ b/src/auth/oauth/utils/attach.js @@ -33,7 +33,7 @@ module.exports = async function attach(account, user) { }, }; await UserMetadata - .for(userId, audience, redis) + .using(userId, audience, redis) .batchUpdate(updateParams); return profile; diff --git a/src/auth/oauth/utils/detach.js b/src/auth/oauth/utils/detach.js index 408423da0..c008ae1f1 100644 --- a/src/auth/oauth/utils/detach.js +++ b/src/auth/oauth/utils/detach.js @@ -39,6 +39,6 @@ module.exports = async function detach(provider, userData) { }; return UserMetadata - .for(userId, audience, redis) + .using(userId, audience, redis) .batchUpdate(updateParams); }; diff --git a/src/custom/cappasity-users-activate.js b/src/custom/cappasity-users-activate.js index 5a24186d6..8b9d105ab 100644 --- a/src/custom/cappasity-users-activate.js +++ b/src/custom/cappasity-users-activate.js @@ -35,7 +35,7 @@ module.exports = function mixPlan(userId, params) { }; return UserMetadata - .for(userId, audience, this.redis) + .using(userId, audience, this.redis) .batchUpdate(updateParams); }); }; diff --git a/src/custom/rfx-create-room-on-activate.js b/src/custom/rfx-create-room-on-activate.js index 5a84db0aa..cbfb95a2b 100644 --- a/src/custom/rfx-create-room-on-activate.js +++ b/src/custom/rfx-create-room-on-activate.js @@ -36,7 +36,7 @@ function createRoom(userId, params, metadata) { }; return UserMetadata - .for(userId, audience, this.redis) + .using(userId, audience, this.redis) .batchUpdate(updateParams); }); } diff --git a/src/utils/metadata/user.js b/src/utils/metadata/user.js index 33875282c..a0537db35 100644 --- a/src/utils/metadata/user.js +++ b/src/utils/metadata/user.js @@ -95,7 +95,7 @@ class UserMetadata { return updateResult; } - static for(userId, audience, redis) { + static using(userId, audience, redis) { return new UserMetadata(redis, userId, audience); } } diff --git a/src/utils/organization/add-organization-members.js b/src/utils/organization/add-organization-members.js index 7437013ab..ccde8ec0f 100644 --- a/src/utils/organization/add-organization-members.js +++ b/src/utils/organization/add-organization-members.js @@ -54,7 +54,7 @@ async function addOrganizationMembers(opts) { const stringifyMember = mapValues(member, JSONStringify); pipe.hmset(memberKey, stringifyMember); UserMetadata - .for(member.id, audience, pipe) + .using(member.id, audience, pipe) .update(organizationId, stringifyMember.permissions); pipe.zadd(membersKey, stringifyMember.invited, memberKey); }); diff --git a/src/utils/organization/register-organization-members.js b/src/utils/organization/register-organization-members.js index ebc39e2e8..5522f9ecc 100644 --- a/src/utils/organization/register-organization-members.js +++ b/src/utils/organization/register-organization-members.js @@ -37,7 +37,7 @@ async function registerOrganizationMember(member) { await pipeline.exec().then(handlePipeline); await UserMetadata - .for(userId, audience, redis) + .using(userId, audience, redis) .batchUpdate({ metadata: [{ $set: { From 62c759bd0252e57b4d8554c3e9f6007255d20c47 Mon Sep 17 00:00:00 2001 From: pajgo Date: Wed, 6 Nov 2019 17:50:29 +0600 Subject: [PATCH 06/12] feat: update user and organization metadata * delete all metadata on remove --- src/actions/remove.js | 10 +++---- src/auth/oauth/utils/attach.js | 2 +- src/auth/oauth/utils/detach.js | 2 +- src/utils/metadata/organization.js | 4 +-- src/utils/metadata/redis/audience.js | 10 +++++++ .../redis/{update-metadata.js => metadata.js} | 29 ++++++++++++++----- src/utils/metadata/user.js | 24 +++++++++++++-- .../register-organization-members.js | 2 +- 8 files changed, 63 insertions(+), 20 deletions(-) rename src/utils/metadata/redis/{update-metadata.js => metadata.js} (89%) diff --git a/src/actions/remove.js b/src/actions/remove.js index 0cae563b6..fb74b5966 100644 --- a/src/actions/remove.js +++ b/src/actions/remove.js @@ -16,7 +16,6 @@ const { USERS_USERNAME_TO_ID, USERS_USERNAME_FIELD, USERS_DATA, - USERS_METADATA, USERS_TOKENS, USERS_ID_FIELD, USERS_ALIAS_FIELD, @@ -70,10 +69,11 @@ async function removeUser({ params }) { } const transaction = redis.pipeline(); - const userMetadata = new UserMetadata(redis); const alias = internal[USERS_ALIAS_FIELD]; const userId = internal[USERS_ID_FIELD]; const resolvedUsername = internal[USERS_USERNAME_FIELD]; + const metaAudiences = await UserMetadata.using(userId, null, redis).getAudience(); + const userMetadata = UserMetadata.using(userId, null, transaction); if (alias) { transaction.hdel(USERS_ALIAS_TO_ID, alias.toLowerCase(), alias); @@ -96,9 +96,9 @@ async function removeUser({ params }) { // remove metadata & internal data transaction.del(key(userId, USERS_DATA)); - // TODO fix if user has multiple audiences some data will remain - transaction.del(key(userId, USERS_METADATA, audience)); - transaction.del(userMetadata.audience.getAudienceKey(userId)); + for (const metaAudience of metaAudiences) { + userMetadata.deleteMetadata(metaAudience); + } // remove auth tokens transaction.del(key(userId, USERS_TOKENS)); diff --git a/src/auth/oauth/utils/attach.js b/src/auth/oauth/utils/attach.js index cc71642a3..0344b607f 100644 --- a/src/auth/oauth/utils/attach.js +++ b/src/auth/oauth/utils/attach.js @@ -23,7 +23,7 @@ module.exports = async function attach(account, user) { // link uid to user id pipeline.hset(USERS_SSO_TO_ID, uid, userId); - await pipeline.exec().then(handlePipeline); + handlePipeline(await pipeline.exec()); const updateParams = { metadata: { diff --git a/src/auth/oauth/utils/detach.js b/src/auth/oauth/utils/detach.js index c008ae1f1..300a08468 100644 --- a/src/auth/oauth/utils/detach.js +++ b/src/auth/oauth/utils/detach.js @@ -28,7 +28,7 @@ module.exports = async function detach(provider, userData) { // delete account reference pipeline.hdel(USERS_SSO_TO_ID, uid); - await pipeline.exec().then(handlePipeline); + handlePipeline(await pipeline.exec()); const updateParams = { metadata: { diff --git a/src/utils/metadata/organization.js b/src/utils/metadata/organization.js index 6acd5d58a..dd566a7ee 100644 --- a/src/utils/metadata/organization.js +++ b/src/utils/metadata/organization.js @@ -1,5 +1,5 @@ const Audience = require('./redis/audience'); -const MetaUpdate = require('./redis/update-metadata'); +const Metadata = require('./redis/metadata'); const { ORGANIZATIONS_METADATA, ORGANIZATIONS_AUDIENCE } = require('../../constants'); /** @@ -11,7 +11,7 @@ class OrganizationMetadata { */ constructor(redis) { this.redis = redis; - this.metadata = new MetaUpdate(this.redis, ORGANIZATIONS_METADATA); + this.metadata = new Metadata(this.redis, ORGANIZATIONS_METADATA); this.audience = new Audience(this.redis, ORGANIZATIONS_AUDIENCE); } diff --git a/src/utils/metadata/redis/audience.js b/src/utils/metadata/redis/audience.js index d207d84b7..2e47b8d82 100644 --- a/src/utils/metadata/redis/audience.js +++ b/src/utils/metadata/redis/audience.js @@ -55,6 +55,16 @@ class Audience { return redis.srem(this.getAudienceKey(id), audience); } + /** + * Get list of assigned audiences + * @param {String|Number} id + * @param {ioredis|Pipeline}redis + * @returns {Promise|Pipeline} + */ + get(id, redis = this.redis) { + return redis.smembers(this.getAudienceKey(id)); + } + /** * Synchronizes audience list with currently available metadata * @param id diff --git a/src/utils/metadata/redis/update-metadata.js b/src/utils/metadata/redis/metadata.js similarity index 89% rename from src/utils/metadata/redis/update-metadata.js rename to src/utils/metadata/redis/metadata.js index 2fd1cf3f5..591bb4b1c 100644 --- a/src/utils/metadata/redis/update-metadata.js +++ b/src/utils/metadata/redis/metadata.js @@ -11,12 +11,12 @@ const isValidId = require('../../asserts/id'); const JSONStringify = (data) => JSON.stringify(data); /** - * Class handling metadata update operations using Redis backend + * Class handling metadata operations using Redis backend */ -class UpdateMetadata { +class Metadata { /** * @param {ioredis} redis or pipeline instance - * @param metadataKeyTemplate template base for Metadata key + * @param metadataKeyBase template base for Metadata key */ constructor(redis, metadataKeyBase) { assert(isRedis(redis), 'must be ioredis instance'); @@ -82,6 +82,19 @@ class UpdateMetadata { return redis.hdel(this.getMetadataKey(id, audience), key); } + /** + * Deletes metadata key + * @param id + * @param audience + * @param redis + * @returns {void|request.Request} + */ + deleteKey(id, audience, redis = this.redis) { + assert(isValidId(id), 'must be valid id'); + assert(isRedis(redis), 'must be ioredis instance'); + return redis.del(this.getMetadataKey(id, audience)); + } + /** * Updates metadata hash on provided Id * @param {Object} opts @@ -109,10 +122,10 @@ class UpdateMetadata { return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); } - const operations = metaOps.map((meta, idx) => UpdateMetadata.handleAudience(pipe, keys[idx], meta)); + const operations = metaOps.map((meta, idx) => Metadata.handleAudience(pipe, keys[idx], meta)); const result = handlePipeline(await pipe.exec()); - return UpdateMetadata.mapMetaResponse(operations, result); + return Metadata.mapMetaResponse(operations, result); } // dynamic scripts @@ -129,13 +142,13 @@ class UpdateMetadata { const result = await Promise.all(scripts); - return UpdateMetadata.mapScriptResponse($scriptKeys, result); + return Metadata.mapScriptResponse($scriptKeys, result); } /** * Process metadata update operation for a passed audience * @param {Object} pipeline - * @param {String} audience + * @param {String} key * @param {Object} metadata */ static handleAudience(pipeline, key, metadata) { @@ -218,4 +231,4 @@ class UpdateMetadata { } } -module.exports = UpdateMetadata; +module.exports = Metadata; diff --git a/src/utils/metadata/user.js b/src/utils/metadata/user.js index a0537db35..37b524607 100644 --- a/src/utils/metadata/user.js +++ b/src/utils/metadata/user.js @@ -1,6 +1,6 @@ const Promise = require('bluebird'); const { Pipeline } = require('ioredis'); -const MetaUpdate = require('./redis/update-metadata'); +const Metadata = require('./redis/metadata'); const Audience = require('./redis/audience'); const { USERS_METADATA, USERS_AUDIENCE } = require('../../constants'); @@ -19,7 +19,7 @@ class UserMetadata { this.redis = redis; this.userAudience = audience; this.userId = userId; - this.metadata = new MetaUpdate(this.redis, USERS_METADATA); + this.metadata = new Metadata(this.redis, USERS_METADATA); this.audience = new Audience(this.redis, USERS_AUDIENCE); } @@ -73,6 +73,26 @@ class UserMetadata { return result; } + /** + * Deletes user metadata for passed audience + * @param {String} Audience + */ + deleteMetadata(audience) { + const work = [ + this.audience.delete(this.userId, audience), + this.metadata.deleteKey(this.userId, audience), + ]; + return this.pipeline ? work : Promise.all(work); + } + + /** + * Gets all audiences assigned + * @returns {*} + */ + getAudience() { + return this.audience.get(this.userId); + } + async syncAudience() { const metaKeyTemplate = this.metadata.getMetadataKey('{{ID}}', '{{AUDIENCE}}'); return this.audience.resyncSet(this.userId, metaKeyTemplate); diff --git a/src/utils/organization/register-organization-members.js b/src/utils/organization/register-organization-members.js index 5522f9ecc..a35ce76fd 100644 --- a/src/utils/organization/register-organization-members.js +++ b/src/utils/organization/register-organization-members.js @@ -34,7 +34,7 @@ async function registerOrganizationMember(member) { const userDataKey = redisKey(userId, USERS_DATA); pipeline.hmset(userDataKey, basicInfo); pipeline.hset(USERS_USERNAME_TO_ID, email, userId); - await pipeline.exec().then(handlePipeline); + handlePipeline(await pipeline.exec()); await UserMetadata .using(userId, audience, redis) From 10d173054bf8c92359c79cd5d14d669d0572ee68 Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 8 Nov 2019 16:40:34 +0600 Subject: [PATCH 07/12] feat: update user and organization metadata * lua opt --- src/utils/metadata/redis/audience.js | 3 +-- src/utils/metadata/user.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/utils/metadata/redis/audience.js b/src/utils/metadata/redis/audience.js index 2e47b8d82..cf442a930 100644 --- a/src/utils/metadata/redis/audience.js +++ b/src/utils/metadata/redis/audience.js @@ -78,8 +78,7 @@ class Audience { const luaScript = ` local audiences = redis.call("SMEMBERS", KEYS[1]) for _, audience in pairs(audiences) do - local metaKey = string.gsub(KEYS[2], '{{ID}}', '${id}') - metaKey = string.gsub(metaKey, '{{AUDIENCE}}', audience) + local metaKey = string.gsub(KEYS[2], '{{AUDIENCE}}', audience) local keyLen = redis.call("HLEN", metaKey) if (keyLen < 1) then redis.call('SREM', KEYS[1], audience) diff --git a/src/utils/metadata/user.js b/src/utils/metadata/user.js index 37b524607..b7da73115 100644 --- a/src/utils/metadata/user.js +++ b/src/utils/metadata/user.js @@ -94,7 +94,7 @@ class UserMetadata { } async syncAudience() { - const metaKeyTemplate = this.metadata.getMetadataKey('{{ID}}', '{{AUDIENCE}}'); + const metaKeyTemplate = this.metadata.getMetadataKey(this.userId, '{{AUDIENCE}}'); return this.audience.resyncSet(this.userId, metaKeyTemplate); } From 9fdf6a18c82269d2271ffe1ae884aa11dfc26610 Mon Sep 17 00:00:00 2001 From: AVVS Date: Tue, 10 Dec 2019 14:05:43 -0800 Subject: [PATCH 08/12] chore: meta fix for activation --- src/actions/activate.js | 22 +++++++--------------- src/utils/metadata/redis/metadata.js | 2 +- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/actions/activate.js b/src/actions/activate.js index 26faa80a0..f74475cce 100644 --- a/src/actions/activate.js +++ b/src/actions/activate.js @@ -6,7 +6,8 @@ const jwt = require('../utils/jwt.js'); const { getInternalData } = require('../utils/userData'); const getMetadata = require('../utils/get-metadata'); const handlePipeline = require('../utils/pipeline-error'); -const setMetadata = require('../utils/update-metadata'); +const UserMetadata = require('../utils/metadata/user'); + const { USERS_INDEX, USERS_DATA, @@ -19,7 +20,7 @@ const { USERS_USERNAME_FIELD, USERS_ACTION_ACTIVATE, USERS_ACTIVATED_FIELD, -} = require('../constants.js'); +} = require('../constants'); // cache error const Forbidden = new HttpStatusError(403, 'invalid token'); @@ -121,19 +122,6 @@ async function activateAccount(data, metadata) { const userKey = redisKey(userId, USERS_DATA); const { defaultAudience, service } = this; const { redis } = service; - - // if this goes through, but other async calls fail its ok to repeat that - // adds activation field - await setMetadata.call(service, { - userId, - audience: defaultAudience, - metadata: { - $set: { - [USERS_ACTIVATED_FIELD]: Date.now(), - }, - }, - }); - // WARNING: `persist` is very important, otherwise we will lose user's information in 30 days // set to active & persist const pipeline = redis @@ -143,6 +131,10 @@ async function activateAccount(data, metadata) { .persist(userKey) .sadd(USERS_INDEX, userId); + UserMetadata + .using(userId, defaultAudience, pipeline) + .update(USERS_ACTIVATED_FIELD, Date.now()); + if (alias) { pipeline.sadd(USERS_PUBLIC_INDEX, userId); } diff --git a/src/utils/metadata/redis/metadata.js b/src/utils/metadata/redis/metadata.js index 591bb4b1c..fca50d026 100644 --- a/src/utils/metadata/redis/metadata.js +++ b/src/utils/metadata/redis/metadata.js @@ -119,7 +119,7 @@ class Metadata { const metaOps = Array.isArray(metadata) ? metadata : [metadata]; if (metaOps.length !== audiences.length) { - return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); + throw new HttpStatusError(400, 'audiences must match metadata entries'); } const operations = metaOps.map((meta, idx) => Metadata.handleAudience(pipe, keys[idx], meta)); From 58ca35f5236958a3ce08d427a4829bc9213bf8ed Mon Sep 17 00:00:00 2001 From: Oleg Tokar Date: Tue, 1 Jun 2021 10:26:06 +0300 Subject: [PATCH 09/12] feat: marge --- src/actions/remove.js | 1 + src/utils/asserts/integer.js | 5 ----- src/utils/asserts/string-not-empty.js | 6 ------ src/utils/organization/add-organization-members.js | 4 +--- src/utils/set-organization-metadata.js | 2 +- 5 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/actions/remove.js b/src/actions/remove.js index f2b0f51df..71d44f20e 100644 --- a/src/actions/remove.js +++ b/src/actions/remove.js @@ -16,6 +16,7 @@ const { USERS_USERNAME_TO_ID, USERS_USERNAME_FIELD, USERS_DATA, + USERS_METADATA, USERS_TOKENS, USERS_ID_FIELD, USERS_ALIAS_FIELD, diff --git a/src/utils/asserts/integer.js b/src/utils/asserts/integer.js index ace815139..d11cb6676 100644 --- a/src/utils/asserts/integer.js +++ b/src/utils/asserts/integer.js @@ -1,10 +1,5 @@ const { strictEqual } = require('assert'); -// TODO isInteger -// module.exports = function isInteger(value) { -// return Number.isInteger(value); -// }; - module.exports = function assertInteger(value, error) { strictEqual(Number.isInteger(value), true, error); }; diff --git a/src/utils/asserts/string-not-empty.js b/src/utils/asserts/string-not-empty.js index f43c75b68..b9697a158 100644 --- a/src/utils/asserts/string-not-empty.js +++ b/src/utils/asserts/string-not-empty.js @@ -1,11 +1,5 @@ const { strictEqual } = require('assert'); -// TODO -// module.exports = function stringNotEmpty(value) { -// return typeof value === 'string' && value.length !== 0; -// }; - - module.exports = function assertStringNotEmpty(value, error) { strictEqual(typeof value === 'string', true, error); strictEqual(value.length !== 0, true, error); diff --git a/src/utils/organization/add-organization-members.js b/src/utils/organization/add-organization-members.js index 45bc59f95..d38140886 100644 --- a/src/utils/organization/add-organization-members.js +++ b/src/utils/organization/add-organization-members.js @@ -33,7 +33,6 @@ async function addMember({ password, ...member }) { const { organizationId, audience, pipe, membersKey } = this; const memberKey = redisKey(organizationId, ORGANIZATIONS_MEMBERS, member.id); - const memberOrganizations = redisKey(member.id, USERS_METADATA, audience); member.username = member.email; member.invited = Date.now(); @@ -43,8 +42,6 @@ async function addMember({ password, ...member }) { const stringifyMember = mapValues(member, JSONStringify); pipe.hmset(memberKey, stringifyMember); - // pipe.hset(memberOrganizations, organizationId, stringifyMember.permissions); - // TODO!!!! UserMetadata .using(member.id, audience, pipe) .update(organizationId, stringifyMember.permissions); @@ -79,6 +76,7 @@ async function sendInvite(member) { /** * Updates metadata on a organization object * @param {Object} opts + * @param {Boolean} sendInviteFlag * @return {Promise} */ async function addOrganizationMembers({ organizationId, members, audience }, sendInviteFlag = false) { diff --git a/src/utils/set-organization-metadata.js b/src/utils/set-organization-metadata.js index 6480c9417..1ebca7b9f 100644 --- a/src/utils/set-organization-metadata.js +++ b/src/utils/set-organization-metadata.js @@ -1,5 +1,5 @@ /* eslint-disable no-mixed-operators */ -const OrganizationMetadata = require('../utils/metadata/organization'); +const OrganizationMetadata = require('./metadata/organization'); /** * Updates metadata on a organization object From fd6841958b29ac04b920a8913964f5f60b04def5 Mon Sep 17 00:00:00 2001 From: Oleg Tokar Date: Tue, 8 Jun 2021 10:35:20 +0300 Subject: [PATCH 10/12] feat: marge --- src/utils/asserts/string-or-array.js | 5 +++++ src/utils/metadata/redis/audience.js | 9 +++++---- src/utils/metadata/redis/metadata.js | 9 +++++---- 3 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 src/utils/asserts/string-or-array.js diff --git a/src/utils/asserts/string-or-array.js b/src/utils/asserts/string-or-array.js new file mode 100644 index 000000000..4fbc38ccb --- /dev/null +++ b/src/utils/asserts/string-or-array.js @@ -0,0 +1,5 @@ +const { strictEqual } = require('assert'); + +module.exports = function notEmptyStringOrArray(value, error) { + strictEqual((typeof value === 'string' && value.length !== 0) || Array.isArray(value), true, error); +}; diff --git a/src/utils/metadata/redis/audience.js b/src/utils/metadata/redis/audience.js index cf442a930..bb1d9eab5 100644 --- a/src/utils/metadata/redis/audience.js +++ b/src/utils/metadata/redis/audience.js @@ -1,6 +1,7 @@ const assert = require('assert'); const { isRedis } = require('../../asserts/redis'); const isNotEmptyString = require('../../asserts/string-not-empty'); +const notEmptyStringOrArray = require('../../asserts/string-or-array'); const isValidId = require('../../asserts/id'); /** @@ -13,7 +14,7 @@ class Audience { */ constructor(redis, audienceKeyBase) { assert(isRedis(redis), 'must be ioredis instance'); - assert(isNotEmptyString(audienceKeyBase), 'must be not empty string'); + isNotEmptyString(audienceKeyBase, 'must be not empty string'); this.redis = redis; this.audienceKeyBase = audienceKeyBase; } @@ -38,7 +39,7 @@ class Audience { */ add(id, audience, redis = this.redis) { assert(isRedis(redis), 'must be ioredis instance'); - assert(isNotEmptyString(audience) || Array.isArray(audience), 'must be not empty string or Array'); + notEmptyStringOrArray(audience, 'must be not empty string or Array'); return redis.sadd(this.getAudienceKey(id), audience); } @@ -51,7 +52,7 @@ class Audience { */ delete(id, audience, redis = this.redis) { assert(isRedis(redis), 'must be ioredis instance'); - assert(isNotEmptyString(audience) || Array.isArray(audience), 'must be not empty string or Array'); + notEmptyStringOrArray(audience, 'must be not empty string or Array'); return redis.srem(this.getAudienceKey(id), audience); } @@ -74,7 +75,7 @@ class Audience { */ resyncSet(id, metadataKeyTemplate, redis = this.redis) { assert(isRedis(this.redis), 'must be ioredis instance'); - assert(isNotEmptyString(metadataKeyTemplate), 'must be not empty string'); + isNotEmptyString(metadataKeyTemplate, 'must be not empty string'); const luaScript = ` local audiences = redis.call("SMEMBERS", KEYS[1]) for _, audience in pairs(audiences) do diff --git a/src/utils/metadata/redis/metadata.js b/src/utils/metadata/redis/metadata.js index fca50d026..f7bc4d077 100644 --- a/src/utils/metadata/redis/metadata.js +++ b/src/utils/metadata/redis/metadata.js @@ -6,6 +6,7 @@ const handlePipeline = require('../../pipeline-error'); const sha256 = require('../../sha256'); const { isRedis, isRedisPipeline } = require('../../asserts/redis'); const isNotEmptyString = require('../../asserts/string-not-empty'); +const notEmptyStringOrArray = require('../../asserts/string-or-array'); const isValidId = require('../../asserts/id'); const JSONStringify = (data) => JSON.stringify(data); @@ -20,7 +21,7 @@ class Metadata { */ constructor(redis, metadataKeyBase) { assert(isRedis(redis), 'must be ioredis instance'); - assert(isNotEmptyString(metadataKeyBase), 'must be not empty string'); + isNotEmptyString(metadataKeyBase, 'must be not empty string'); this.redis = redis; this.metadataKeyBase = metadataKeyBase; } @@ -34,7 +35,7 @@ class Metadata { */ getMetadataKey(id, audience) { assert(isValidId(id), 'must be valid Id'); - assert(isNotEmptyString(audience), 'must be not empty string'); + isNotEmptyString(audience, 'must be not empty string'); return `${id}!${this.metadataKeyBase}!${audience}`; } @@ -48,7 +49,7 @@ class Metadata { * @returns {Promise|Pipeline} */ update(id, audience, key, value, redis = this.redis) { - assert(isNotEmptyString(key), 'must be not empty string'); + isNotEmptyString(key, 'must be not empty string'); assert(isRedis(redis), 'must be ioredis instance'); assert(value, 'must not be empty'); @@ -77,7 +78,7 @@ class Metadata { * @returns {Promise|Pipeline} */ delete(id, audience, key, redis = this.redis) { - assert(isNotEmptyString(key) || Array.isArray(key), 'must be not empty string or Array'); + notEmptyStringOrArray(key, 'must be not empty string or Array'); assert(isRedis(redis), 'must be ioredis instance'); return redis.hdel(this.getMetadataKey(id, audience), key); } From 7087c89a36ccedf83feac4d52df380ec62ecad80 Mon Sep 17 00:00:00 2001 From: Oleg Tokar Date: Thu, 10 Jun 2021 12:31:13 +0300 Subject: [PATCH 11/12] feat: migration --- src/migrations/09-audience/index.js | 40 +++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/migrations/09-audience/index.js diff --git a/src/migrations/09-audience/index.js b/src/migrations/09-audience/index.js new file mode 100644 index 000000000..c95cc5978 --- /dev/null +++ b/src/migrations/09-audience/index.js @@ -0,0 +1,40 @@ +const { + USERS_INDEX, + ORGANIZATIONS_INDEX, + USERS_METADATA, + ORGANIZATIONS_METADATA, + USERS_AUDIENCE, + ORGANIZATIONS_AUDIENCE, +} = require('../../constants'); +const Audience = require('../../utils/metadata/redis/audience'); + +const script = async ({ config, redis, log }) => { + const { keyPrefix } = config.redis.options; + const usersAudience = new Audience(redis, USERS_AUDIENCE); + const organizationAudience = new Audience(redis, ORGANIZATIONS_AUDIENCE); + + const process = (audience, metadata) => (ids) => { + ids.forEach((id) => { + const prefix = `${keyPrefix}${id}!${metadata}!`; + redis.keys(`*${prefix}*`) + .map((key) => key.replace(prefix, '')) + .map((audienceString) => { + return audience.add(id, audienceString); + }); + }); + }; + + await redis.smembers(USERS_INDEX) + .tap(({ length }) => log.info('Users to be processed:', length)) + .map(process(usersAudience, USERS_METADATA)); + + await redis.smembers(ORGANIZATIONS_INDEX) + .tap(({ length }) => log.info('Organizations to be processed:', length)) + .map(process(organizationAudience, ORGANIZATIONS_METADATA)); +}; + +module.exports = { + script, + min: 8, + final: 9, +}; From 10ad38cf9640bf3a46bbe6fefb1ba40d0830e0bc Mon Sep 17 00:00:00 2001 From: Oleg Tokar Date: Fri, 11 Jun 2021 13:43:31 +0300 Subject: [PATCH 12/12] feat: migration LUA --- .../05-referrals-users-ids/index.js | 19 +------- src/migrations/09-audience/index.js | 45 ++++++++++--------- src/migrations/09-audience/migrate.lua | 17 +++++++ src/migrations/utils/get-redis-master-node.js | 19 ++++++++ 4 files changed, 60 insertions(+), 40 deletions(-) create mode 100644 src/migrations/09-audience/migrate.lua create mode 100644 src/migrations/utils/get-redis-master-node.js diff --git a/src/migrations/05-referrals-users-ids/index.js b/src/migrations/05-referrals-users-ids/index.js index 5a46461ce..12af467b7 100644 --- a/src/migrations/05-referrals-users-ids/index.js +++ b/src/migrations/05-referrals-users-ids/index.js @@ -1,24 +1,7 @@ const Promise = require('bluebird'); -const calcSlot = require('cluster-key-slot'); const { USERS_REFERRAL_INDEX } = require('../../constants.js'); const { getUserId } = require('../../utils/userData'); - -/** - * Return master node in case of redisCluster to be able to use - * specific commands like `keys`. We can use usual redis instance in - * other cases. - */ -function getRedisMasterNode(redis, config) { - if (!config.plugins.includes('redisCluster')) { - return redis; - } - const { keyPrefix } = config.redis.options; - const slot = calcSlot(keyPrefix); - const nodeKeys = redis.slots[slot]; - const masters = redis.connectionPool.nodes.master; - - return nodeKeys.reduce((node, key) => node || masters[key], null); -} +const getRedisMasterNode = require('../utils/get-redis-master-node'); /** * diff --git a/src/migrations/09-audience/index.js b/src/migrations/09-audience/index.js index c95cc5978..5e1b3f54f 100644 --- a/src/migrations/09-audience/index.js +++ b/src/migrations/09-audience/index.js @@ -1,3 +1,4 @@ +const fs = require('fs'); const { USERS_INDEX, ORGANIZATIONS_INDEX, @@ -6,31 +7,31 @@ const { USERS_AUDIENCE, ORGANIZATIONS_AUDIENCE, } = require('../../constants'); -const Audience = require('../../utils/metadata/redis/audience'); +const getRedisMasterNode = require('../utils/get-redis-master-node'); -const script = async ({ config, redis, log }) => { - const { keyPrefix } = config.redis.options; - const usersAudience = new Audience(redis, USERS_AUDIENCE); - const organizationAudience = new Audience(redis, ORGANIZATIONS_AUDIENCE); - - const process = (audience, metadata) => (ids) => { - ids.forEach((id) => { - const prefix = `${keyPrefix}${id}!${metadata}!`; - redis.keys(`*${prefix}*`) - .map((key) => key.replace(prefix, '')) - .map((audienceString) => { - return audience.add(id, audienceString); - }); - }); - }; +const SCRIPT = fs.readFileSync(`${__dirname}/migrate.lua`, 'utf8'); +const USERS_KEYS = [ + USERS_INDEX, + `id!${USERS_METADATA}!`, + `id!${USERS_AUDIENCE}`, +]; +const USERS_ARGS = [ + `.*id!${USERS_METADATA}!`, +]; +const ORGANIZATIONS_KEYS = [ + ORGANIZATIONS_INDEX, + `id!${ORGANIZATIONS_METADATA}!`, + `id!${ORGANIZATIONS_AUDIENCE}`, +]; +const ORGANIZATIONS_ARGS = [ + `.*id!${USERS_METADATA}!`, +]; - await redis.smembers(USERS_INDEX) - .tap(({ length }) => log.info('Users to be processed:', length)) - .map(process(usersAudience, USERS_METADATA)); +const script = async ({ config, redis }) => { + const masterNode = getRedisMasterNode(redis, config); - await redis.smembers(ORGANIZATIONS_INDEX) - .tap(({ length }) => log.info('Organizations to be processed:', length)) - .map(process(organizationAudience, ORGANIZATIONS_METADATA)); + await masterNode.eval(SCRIPT, USERS_KEYS.length, USERS_KEYS, USERS_ARGS); + await masterNode.eval(SCRIPT, ORGANIZATIONS_KEYS.length, ORGANIZATIONS_KEYS, ORGANIZATIONS_ARGS); }; module.exports = { diff --git a/src/migrations/09-audience/migrate.lua b/src/migrations/09-audience/migrate.lua new file mode 100644 index 000000000..6f4c1e7d8 --- /dev/null +++ b/src/migrations/09-audience/migrate.lua @@ -0,0 +1,17 @@ +local index = KEYS[1] +local metaData = KEYS[2] +local audience = KEYS[3] + +local patternTemplate = ARGV[1] + +local ids = redis.call('smembers', index) +for _, id in ipairs(ids) do + local metaDataKey = metaData:gsub('(id)', id, 1) + local audienceKey = audience:gsub('(id)', id, 1) + local pattern = patternTemplate:gsub('(id)', id, 1) + local keys = redis.call('keys', '*' .. metaDataKey .. '*') + for _, key in ipairs(keys) do + local newAudience = key:gsub(pattern, '', 1) + redis.call('sadd', audienceKey, newAudience) + end +end diff --git a/src/migrations/utils/get-redis-master-node.js b/src/migrations/utils/get-redis-master-node.js new file mode 100644 index 000000000..ddd1fe4b1 --- /dev/null +++ b/src/migrations/utils/get-redis-master-node.js @@ -0,0 +1,19 @@ +const calcSlot = require('cluster-key-slot'); + +/** + * Return master node in case of redisCluster to be able to use + * specific commands like `keys`. We can use usual redis instance in + * other cases. + */ + +module.exports = function getRedisMasterNode(redis, config) { + if (!config.plugins.includes('redisCluster')) { + return redis; + } + const { keyPrefix } = config.redis.options; + const slot = calcSlot(keyPrefix); + const nodeKeys = redis.slots[slot]; + const masters = redis.connectionPool.nodes.master; + + return nodeKeys.reduce((node, key) => node || masters[key], null); +};