From bfaccec368997c3f23002efa0eb75c1fc7573b6b Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Fri, 13 Mar 2020 13:23:11 +0530 Subject: [PATCH 01/12] caching member api call --- .circleci/config.yml | 2 +- package.json | 3 ++- src/common/broadcastAPIHelper.js | 14 ++++++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 7bc897d..6bb70a8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -102,7 +102,7 @@ workflows: context : org-global filters: branches: - only: [dev, 'hotfix/V5-API-Standards', 'v5-upgrade', 'feature/bulk-notification'] + only: [dev, 'bug/rate-limit'] - "build-prod": context : org-global filters: diff --git a/package.json b/package.json index c7f4382..89da03b 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,8 @@ "tc-core-library-js": "appirio-tech/tc-core-library-js.git#v2.6.2", "topcoder-healthcheck-dropin": "^1.0.3", "urijs": "^1.19.1", - "winston": "^2.2.0" + "winston": "^2.2.0", + "node-cache": "^5.1.0" }, "engines": { "node": "6.x" diff --git a/src/common/broadcastAPIHelper.js b/src/common/broadcastAPIHelper.js index b196cf4..b6235ba 100644 --- a/src/common/broadcastAPIHelper.js +++ b/src/common/broadcastAPIHelper.js @@ -6,10 +6,14 @@ const _ = require('lodash') const config = require('config') const request = require('superagent') const logger = require('./logger') -const m2mAuth = require('tc-core-library-js').auth.m2m; -const m2m = m2mAuth(config); +const m2mAuth = require('tc-core-library-js').auth.m2m +const NodeCache = require('node-cache') + +const m2m = m2mAuth(config) +const cache = new NodeCache() const logPrefix = "BroadcastAPI: " +const cachedTimeInSeconds = 60 //60 seconds /** * Helper Function - get m2m token @@ -27,6 +31,11 @@ async function getMemberInfo(userId) { "/members/_search/?" + `query=userId%3A${userId}` + `&limit=1` + if (cachedMemberInfo = cache.get(url)) { + return new Promise( (resolve, reject) => { + resolve(cachedMemberInfo) + }) + } return new Promise(async function (resolve, reject) { let memberInfo = [] logger.info(`calling member api ${url} `) @@ -37,6 +46,7 @@ async function getMemberInfo(userId) { } memberInfo = _.get(res, 'body.result.content') logger.info(`BCA Memeber API: Feteched ${memberInfo.length} record(s) from member api`) + cache.set(url, memberInfo, cachedTimeInSeconds) resolve(memberInfo) } catch (err) { reject(new Error(`BCA Memeber API: Failed to get member ` + From faff5fecf70b74c7e3a723e1ea2e5584551adeb1 Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Fri, 13 Mar 2020 17:43:48 +0530 Subject: [PATCH 02/12] fixing multiple member and group calls and duplicate user notifications --- src/common/broadcastAPIHelper.js | 25 +++++++++++++++++-------- src/common/tcApiHelper.js | 2 +- src/hooks/hookBulkMessage.js | 18 ++++++++++++++---- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/common/broadcastAPIHelper.js b/src/common/broadcastAPIHelper.js index b6235ba..4d46151 100644 --- a/src/common/broadcastAPIHelper.js +++ b/src/common/broadcastAPIHelper.js @@ -13,7 +13,7 @@ const m2m = m2mAuth(config) const cache = new NodeCache() const logPrefix = "BroadcastAPI: " -const cachedTimeInSeconds = 60 //60 seconds +const cachedTimeInSeconds = 300 //300 seconds /** * Helper Function - get m2m token @@ -112,12 +112,16 @@ async function callApi(url, machineToken) { /** * Helper function - check Skills and Tracks condition + * + * @param {Integer} userId + * @param {Object} bulkMessage + * @param {Object} m memberInfo + * */ -async function checkUserSkillsAndTracks(userId, bulkMessage) { +async function checkUserSkillsAndTracks(userId, bulkMessage, m) { try { const skills = _.get(bulkMessage, 'recipients.skills') const tracks = _.get(bulkMessage, 'recipients.tracks') - const m = await getMemberInfo(userId) let skillMatch, trackMatch = false // default if (skills && skills.length > 0) { const ms = _.get(m[0], "skills") // get member skills @@ -169,11 +173,10 @@ async function checkUserSkillsAndTracks(userId, bulkMessage) { /** * Helper function - check group condition */ -async function checkUserGroup(userId, bulkMessage) { +async function checkUserGroup(userId, bulkMessage, userGroupInfo) { try { const groups = _.get(bulkMessage, 'recipients.groups') let flag = false // default - const userGroupInfo = await getUserGroup(userId) if (groups.length > 0) { _.map(userGroupInfo, (o) => { // particular group only condition @@ -199,12 +202,16 @@ async function checkUserGroup(userId, bulkMessage) { * * @param {Integer} userId * @param {Object} bulkMessage + * @param {Object} memberInfo + * @param {Object} userGroupInfo + * + * @return Promise */ -async function checkBroadcastMessageForUser(userId, bulkMessage) { +async function checkBroadcastMessageForUser(userId, bulkMessage, memberInfo, userGroupInfo) { return new Promise(function (resolve, reject) { Promise.all([ - checkUserSkillsAndTracks(userId, bulkMessage), - checkUserGroup(userId, bulkMessage), + checkUserSkillsAndTracks(userId, bulkMessage, memberInfo), + checkUserGroup(userId, bulkMessage, userGroupInfo), ]).then((results) => { let flag = true // TODO need to be sure about default value _.map(results, (r) => { @@ -224,4 +231,6 @@ async function checkBroadcastMessageForUser(userId, bulkMessage) { module.exports = { checkBroadcastMessageForUser, + getMemberInfo, + getUserGroup, } \ No newline at end of file diff --git a/src/common/tcApiHelper.js b/src/common/tcApiHelper.js index dcd7489..221125b 100644 --- a/src/common/tcApiHelper.js +++ b/src/common/tcApiHelper.js @@ -288,7 +288,7 @@ function filterChallengeUsers(usersInfo, filterOnRoles = [], filterOnUsers = []) } }); logger.info(`Total roles available in this challenge are: ${rolesAvailable.join(',')}`); - return users; + return _.uniqBy(users, 'userId'); } /** diff --git a/src/hooks/hookBulkMessage.js b/src/hooks/hookBulkMessage.js index cacbe44..c0eae73 100644 --- a/src/hooks/hookBulkMessage.js +++ b/src/hooks/hookBulkMessage.js @@ -66,8 +66,14 @@ async function syncBulkMessageForUser(userId) { " FROM bulk_message_user_refs AS bmur WHERE bmur.user_id=$1)" + " AS b ON a.id=b.bulk_message_id WHERE b.refid IS NULL" models.sequelize.query(q, { bind: [userId] }) - .then(function (res) { - Promise.all(res[0].map((r) => isBroadCastMessageForUser(userId, r))) + .then(async function (res) { + try { + const memberInfo = await api.getMemberInfo(userId) + const userGroupInfo = await getUserGroup(userId) + } catch(e) { + reject(`${logPrefix} Failed to get member/group info: ${e}`) + } + Promise.all(res[0].map((r) => isBroadCastMessageForUser(userId, r, memberInfo, userGroupInfo))) .then((results) => { Promise.all(results.map((o) => { if (o.result) { @@ -94,9 +100,13 @@ async function syncBulkMessageForUser(userId) { * Check if current user in broadcast recipent group * @param {Integer} userId * @param {Object} bulkMessage + * @param {Object} memberInfo + * @param {Object} userGroupInfo + * + * @retun promise */ -async function isBroadCastMessageForUser(userId, bulkMessage) { - return api.checkBroadcastMessageForUser(userId, bulkMessage) +async function isBroadCastMessageForUser(userId, bulkMessage, memberInfo, userGroupInfo) { + return api.checkBroadcastMessageForUser(userId, bulkMessage, memberInfo, userGroupInfo) } /** From d70c9ee3a067c5e77a9e07844404a138aa008b32 Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Fri, 13 Mar 2020 17:56:02 +0530 Subject: [PATCH 03/12] fixing typo --- src/hooks/hookBulkMessage.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/hooks/hookBulkMessage.js b/src/hooks/hookBulkMessage.js index c0eae73..6329bbe 100644 --- a/src/hooks/hookBulkMessage.js +++ b/src/hooks/hookBulkMessage.js @@ -65,11 +65,13 @@ async function syncBulkMessageForUser(userId) { " LEFT OUTER JOIN (SELECT id as refid, bulk_message_id " + " FROM bulk_message_user_refs AS bmur WHERE bmur.user_id=$1)" + " AS b ON a.id=b.bulk_message_id WHERE b.refid IS NULL" + let memberInfo = [] + let userGroupInfo = [] models.sequelize.query(q, { bind: [userId] }) .then(async function (res) { try { - const memberInfo = await api.getMemberInfo(userId) - const userGroupInfo = await getUserGroup(userId) + memberInfo = await api.getMemberInfo(userId) + userGroupInfo = await api.getUserGroup(userId) } catch(e) { reject(`${logPrefix} Failed to get member/group info: ${e}`) } From ae649d2920a18933ef4067427e7bfea04cc3ee2d Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Mon, 16 Mar 2020 13:34:59 +0530 Subject: [PATCH 04/12] as per request at git-issue https://github.com/topcoder-platform/community-app/issues/4108, disabling events 'A new submission is uploaded' --- config/default.js | 39 ++++++++++++++++---------------- src/common/broadcastAPIHelper.js | 4 ++-- src/hooks/hookBulkMessage.js | 2 +- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/config/default.js b/config/default.js index 904b068..062b729 100644 --- a/config/default.js +++ b/config/default.js @@ -81,24 +81,25 @@ module.exports = { }, }, ], - 'submission.notification.create': [ - { - handleSubmission: - { - resource: 'submission', - roles: ['Copilot', 'Reviewer'], - selfOnly: true /** Submitter only */, - notification: - { - id: 0, /** challengeid or projectid */ - name: '', /** challenge name */ - group: 'submission', - title: 'A new submission is uploaded.', - }, - }, - }, - ], - 'admin.notification.broadcast' : [{ + // /** 'submission.notification.create': [ + // { + // handleSubmission: + // { + // resource: 'submission', + // roles: ['Copilot', 'Reviewer'], + // selfOnly: true /** Submitter only */, + // notification: + // { + // id: 0, /** challengeid or projectid */ + // name: '', /** challenge name */ + // group: 'submission', + // title: 'A new submission is uploaded.', + // }, + // }, + // }, + // ], + // */ // issue - https://github.com/topcoder-platform/community-app/issues/4108 + 'admin.notification.broadcast': [{ handleBulkNotification: {} } ] @@ -112,5 +113,5 @@ module.exports = { ENABLE_DEV_MODE: process.env.ENABLE_DEV_MODE ? Boolean(process.env.ENABLE_DEV_MODE) : true, DEV_MODE_EMAIL: process.env.DEV_MODE_EMAIL, DEFAULT_REPLY_EMAIL: process.env.DEFAULT_REPLY_EMAIL, - ENABLE_HOOK_BULK_NOTIFICATION : process.env.ENABLE_HOOK_BULK_NOTIFICATION || false, + ENABLE_HOOK_BULK_NOTIFICATION: process.env.ENABLE_HOOK_BULK_NOTIFICATION || false, }; diff --git a/src/common/broadcastAPIHelper.js b/src/common/broadcastAPIHelper.js index 4d46151..05fa61a 100644 --- a/src/common/broadcastAPIHelper.js +++ b/src/common/broadcastAPIHelper.js @@ -32,10 +32,10 @@ async function getMemberInfo(userId) { `query=userId%3A${userId}` + `&limit=1` if (cachedMemberInfo = cache.get(url)) { - return new Promise( (resolve, reject) => { + return new Promise((resolve, reject) => { resolve(cachedMemberInfo) }) - } + } return new Promise(async function (resolve, reject) { let memberInfo = [] logger.info(`calling member api ${url} `) diff --git a/src/hooks/hookBulkMessage.js b/src/hooks/hookBulkMessage.js index 6329bbe..b4006f1 100644 --- a/src/hooks/hookBulkMessage.js +++ b/src/hooks/hookBulkMessage.js @@ -72,7 +72,7 @@ async function syncBulkMessageForUser(userId) { try { memberInfo = await api.getMemberInfo(userId) userGroupInfo = await api.getUserGroup(userId) - } catch(e) { + } catch (e) { reject(`${logPrefix} Failed to get member/group info: ${e}`) } Promise.all(res[0].map((r) => isBroadCastMessageForUser(userId, r, memberInfo, userGroupInfo))) From 8d42c73449fb558cf714e327a6c707fb3526a9ea Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Wed, 18 Mar 2020 11:01:28 +0530 Subject: [PATCH 05/12] Changes according to https://github.com/topcoder-platform/community-app/issues/4114 --- config/default.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/config/default.js b/config/default.js index 062b729..b78a66e 100644 --- a/config/default.js +++ b/config/default.js @@ -70,13 +70,30 @@ module.exports = { { phaseTypeName: 'Checkpoint Screening', state: 'START', - roles: ['Copilot', 'Reviewer'], + roles: ['Primary Screener'], notification: { id: 0, /** challengeid or projectid */ name: '', /** challenge name */ group: 'challenge', - title: 'Challenge checkpoint review.', + title: 'Checkpoint screening phase is now open. You may now begin screening checkpoint submissions.', + }, + }, + }, + ], + 'notifications.autopilot.events': [ + { + handleAutoPilot: + { + phaseTypeName: 'Checkpoint Screening', + state: 'START', + roles: ['Copilot'], + notification: + { + id: 0, /** challengeid or projectid */ + name: '', /** challenge name */ + group: 'challenge', + title: 'Checkpoint Review is now open. You may now begin reviewing checkpoint submissions.', }, }, }, From 75ff36e439c11a860bbaed4daffd35b85068f21a Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Wed, 18 Mar 2020 11:27:56 +0530 Subject: [PATCH 06/12] changes in public group logic so that only some groups should not get public messasge as per slack discussion - https://app.slack.com/client/T03R80JP7/CG2J9RH70/thread/CG2J9RH70-1583899598.019100 --- src/common/broadcastAPIHelper.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/common/broadcastAPIHelper.js b/src/common/broadcastAPIHelper.js index 05fa61a..a0a67b8 100644 --- a/src/common/broadcastAPIHelper.js +++ b/src/common/broadcastAPIHelper.js @@ -184,9 +184,17 @@ async function checkUserGroup(userId, bulkMessage, userGroupInfo) { }) } else { // no group condition means its for `public` no private group flag = true // default allow for all + let excludeGroups = [] + _.map(groups, (g) => { + if (_.startWith(g, '!')) { + excludeGroups.push(g) + } + }) _.map(userGroupInfo, (o) => { - // not allow if user is part of any private group - flag = (_.get(o, "privateGroup")) ? false : flag + // not allow if user is part of any private group i.e. excludeGroups + if (_.indexOf(excludeGroups, _.get(o, "name"))) { + flag = false + } }) logger.info(`public group condition for userId ${userId}` + ` and BC messageId ${bulkMessage.id}, the result is: ${flag}`) From 73671a8637d38b67aaf637aa73924f73bfd694ee Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Wed, 18 Mar 2020 15:10:40 +0530 Subject: [PATCH 07/12] fixing typo --- src/common/broadcastAPIHelper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/broadcastAPIHelper.js b/src/common/broadcastAPIHelper.js index a0a67b8..5aec2d9 100644 --- a/src/common/broadcastAPIHelper.js +++ b/src/common/broadcastAPIHelper.js @@ -192,7 +192,7 @@ async function checkUserGroup(userId, bulkMessage, userGroupInfo) { }) _.map(userGroupInfo, (o) => { // not allow if user is part of any private group i.e. excludeGroups - if (_.indexOf(excludeGroups, _.get(o, "name"))) { + if (_.indexOf(excludeGroups, _.get(o, "name")) >= 0) { flag = false } }) From c5f45e512f40b795dbd5d4b9384f160475973ed6 Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Wed, 18 Mar 2020 15:50:53 +0530 Subject: [PATCH 08/12] typo --- config/default.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/default.js b/config/default.js index b78a66e..66ea12c 100644 --- a/config/default.js +++ b/config/default.js @@ -85,7 +85,7 @@ module.exports = { { handleAutoPilot: { - phaseTypeName: 'Checkpoint Screening', + phaseTypeName: 'Checkpoint Review', state: 'START', roles: ['Copilot'], notification: From 4c5f80035c87bbcae835ce800e6b8da99c6d2c5c Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Wed, 18 Mar 2020 16:45:21 +0530 Subject: [PATCH 09/12] setting one(topic) to many processor(handlers) --- config/default.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/config/default.js b/config/default.js index 66ea12c..6b93c99 100644 --- a/config/default.js +++ b/config/default.js @@ -79,10 +79,7 @@ module.exports = { title: 'Checkpoint screening phase is now open. You may now begin screening checkpoint submissions.', }, }, - }, - ], - 'notifications.autopilot.events': [ - { + }, { handleAutoPilot: { phaseTypeName: 'Checkpoint Review', From 533397fca58df51da8ebdc7f5f6c7293e835d3ae Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Wed, 18 Mar 2020 17:02:09 +0530 Subject: [PATCH 10/12] correcting message --- config/default.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/default.js b/config/default.js index 6b93c99..9563efe 100644 --- a/config/default.js +++ b/config/default.js @@ -76,7 +76,7 @@ module.exports = { id: 0, /** challengeid or projectid */ name: '', /** challenge name */ group: 'challenge', - title: 'Checkpoint screening phase is now open. You may now begin screening checkpoint submissions.', + title: 'Checkpoint Screening phase is now open. You may now begin screening checkpoint submissions.', }, }, }, { From 1e8cd5a3d03c8a43533f348e9def6c3c4f0af1ec Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Thu, 19 Mar 2020 13:55:06 +0530 Subject: [PATCH 11/12] logical error in checking exclude group --- src/common/broadcastAPIHelper.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/common/broadcastAPIHelper.js b/src/common/broadcastAPIHelper.js index 5aec2d9..cb9f5de 100644 --- a/src/common/broadcastAPIHelper.js +++ b/src/common/broadcastAPIHelper.js @@ -175,8 +175,10 @@ async function checkUserSkillsAndTracks(userId, bulkMessage, m) { */ async function checkUserGroup(userId, bulkMessage, userGroupInfo) { try { + const excludeGroupSign = '!' const groups = _.get(bulkMessage, 'recipients.groups') let flag = false // default + if (groups.length > 0) { _.map(userGroupInfo, (o) => { // particular group only condition @@ -186,13 +188,13 @@ async function checkUserGroup(userId, bulkMessage, userGroupInfo) { flag = true // default allow for all let excludeGroups = [] _.map(groups, (g) => { - if (_.startWith(g, '!')) { + if (_.startWith(g, excludeGroupSign)) { excludeGroups.push(g) } }) _.map(userGroupInfo, (o) => { // not allow if user is part of any private group i.e. excludeGroups - if (_.indexOf(excludeGroups, _.get(o, "name")) >= 0) { + if (_.indexOf(excludeGroups, (excludeGroupSign + _.get(o, "name"))) >= 0) { flag = false } }) From 4ba2b876e7b44dc7e9e3d9e5b31d75947c86862c Mon Sep 17 00:00:00 2001 From: Sachin Maheshwari Date: Thu, 19 Mar 2020 15:38:37 +0530 Subject: [PATCH 12/12] correcting logical error in checking exclude group --- src/common/broadcastAPIHelper.js | 33 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/common/broadcastAPIHelper.js b/src/common/broadcastAPIHelper.js index cb9f5de..32c4988 100644 --- a/src/common/broadcastAPIHelper.js +++ b/src/common/broadcastAPIHelper.js @@ -177,30 +177,39 @@ async function checkUserGroup(userId, bulkMessage, userGroupInfo) { try { const excludeGroupSign = '!' const groups = _.get(bulkMessage, 'recipients.groups') + let flag = false // default + let excludeGroups = [] + let includeGroups = [] + + _.map(groups, (g) => { + if (_.startsWith(g, excludeGroupSign)) { + excludeGroups.push(g) + } else { + includeGroups.push(g) + } + }) - if (groups.length > 0) { + if (includeGroups.length > 0) { _.map(userGroupInfo, (o) => { // particular group only condition - flag = (_.indexOf(groups, _.get(o, "name")) >= 0) ? true : flag + flag = (_.indexOf(includeGroups, _.get(o, "name")) >= 0) ? true : flag }) - } else { // no group condition means its for `public` no private group + } + if (excludeGroups.length > 0) { flag = true // default allow for all - let excludeGroups = [] - _.map(groups, (g) => { - if (_.startWith(g, excludeGroupSign)) { - excludeGroups.push(g) - } - }) _.map(userGroupInfo, (o) => { // not allow if user is part of any private group i.e. excludeGroups - if (_.indexOf(excludeGroups, (excludeGroupSign + _.get(o, "name"))) >= 0) { - flag = false - } + flag = (_.indexOf(excludeGroups, (excludeGroupSign + _.get(o, "name"))) >= 0) ? false : flag }) logger.info(`public group condition for userId ${userId}` + ` and BC messageId ${bulkMessage.id}, the result is: ${flag}`) } + + if (groups.length === 0) { + flag = true // no restriction + } + return flag } catch (e) { throw new Error(`checkUserGroup(): ${e}`)