Skip to content

Commit b1396c9

Browse files
bountyC0d3rbountyC0d3r
committed
incorporated the review comments
1 parent 0014d18 commit b1396c9

File tree

9 files changed

+4153
-3414
lines changed

9 files changed

+4153
-3414
lines changed

‎package-lock.json

Lines changed: 4060 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"express": "^4.17.1",
2323
"get-parameter-names": "^0.3.0",
2424
"http-status-codes": "^2.2.0",
25-
"joi": "^17.7.0",
25+
"joi": "^14.0.0",
2626
"jsonwebtoken": "^8.5.1",
2727
"lodash": "^4.17.19",
2828
"morgan": "^1.7.0",

‎src/common/helper.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const config = require('config')
88
const neo4j = require('neo4j-driver')
99
const nodeCache = require('node-cache')
1010
const querystring = require('querystring')
11-
const uuid = require('uuid/v4')
11+
const uuid = require('uuid')
1212
const validate = require('uuid-validate')
1313

1414
const errors = require('./errors')
@@ -335,7 +335,7 @@ async function createGroup(tx, data, currentUser) {
335335
const groupData = data
336336

337337
// generate next group id
338-
groupData.id = uuid()
338+
groupData.id = uuid.v4()
339339
groupData.createdAt = new Date().toISOString()
340340
groupData.createdBy = currentUser === 'M2M' ? '00000000' : currentUser.userId
341341
groupData.domain = groupData.domain ? groupData.domain : ''
@@ -390,7 +390,7 @@ async function deleteGroup(tx, group) {
390390
}
391391

392392
async function initiateCache() {
393-
cache = new NodeCache({
393+
cache = new nodeCache({
394394
stdTTL: 432000,
395395
checkperiod: 518400
396396
});

‎src/controllers/GroupController.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ async function getGroupByOldId(req, res) {
8787
* @param res the response
8888
*/
8989
async function flushCache(req, res) {
90-
const cachedData = helper.getCacheInstance()
90+
const cachedData = await helper.getCacheInstance()
9191
cachedData.flushAll()
9292

9393
res.send({

‎src/services/GroupMembershipService.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
const _ = require('lodash')
55
const config = require('config')
66
const Joi = require('joi')
7-
const uuid = require('uuid/v4')
7+
const uuid = require('uuid')
88
const helper = require('../common/helper')
99
const logger = require('../common/logger')
1010
const errors = require('../common/errors')
@@ -61,6 +61,13 @@ async function addGroupMember(currentUser, groupId, data) {
6161
if (group.privateGroup && !childGroup.privateGroup) {
6262
throw new errors.ConflictError('Parent group is private, the child group must be private too.')
6363
}
64+
65+
// update the cache
66+
const cache = await helper.getCacheInstance()
67+
const cachedGroup = cache.get(group.id)
68+
cachedGroup.subGroups.push(childGroup)
69+
cachedGroup.flattenGroupIdTree.push(childGroup.id)
70+
cache.set(group.id, cachedGroup)
6471
} else {
6572
logger.debug(`Check for memberId ${memberId} exist or not`)
6673
await helper.ensureExists(tx, 'User', memberId)
@@ -98,7 +105,7 @@ async function addGroupMember(currentUser, groupId, data) {
98105
}
99106

100107
// add membership
101-
const membershipId = uuid()
108+
const membershipId = uuid.v4()
102109
const createdAt = new Date().toISOString()
103110

104111
let query

‎src/services/GroupRoleService.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const constants = require('../../app-constants')
1515
* @param {Object} criteria the pagination properties
1616
* @returns {Object} an object contains an array of objects, where the object has two properties: the groupId and the role
1717
*/
18-
async function getGroupRole (userId, criteria) {
18+
async function getGroupRole(userId, criteria) {
1919
logger.debug(`Get Group Role - UserId - ${userId} , Criteria - ${JSON.stringify(criteria)}`)
2020
const session = helper.createDBSession()
2121
try {
@@ -24,8 +24,7 @@ async function getGroupRole (userId, criteria) {
2424
const total = totalRes.records[0].get(0).low || 0
2525

2626
const pageRes = await session.run(
27-
`${matchClause} RETURN g.id, role ORDER BY g.id, role SKIP ${(criteria.page - 1) * criteria.perPage} LIMIT ${
28-
criteria.perPage
27+
`${matchClause} RETURN g.id, role ORDER BY g.id, role SKIP ${(criteria.page - 1) * criteria.perPage} LIMIT ${criteria.perPage
2928
}`
3029
)
3130

@@ -56,7 +55,7 @@ getGroupRole.schema = {
5655
* @param {String} role the name of role to add
5756
* @returns {Object} the added role
5857
*/
59-
async function addGroupRole (currentUser, userId, groupId, role) {
58+
async function addGroupRole(currentUser, userId, groupId, role) {
6059
logger.debug(`Add Group Role - user - ${userId} , group - ${groupId}, role - ${role}`)
6160
const session = helper.createDBSession()
6261
const tx = session.beginTransaction()
@@ -120,7 +119,7 @@ addGroupRole.schema = {
120119
* @param {Boolean} isAdmin flag indicating whether the current user is an admin or not
121120
* @returns {Object} the deleted group role
122121
*/
123-
async function deleteGroupRole (userId, groupId, role, isAdmin) {
122+
async function deleteGroupRole(userId, groupId, role, isAdmin) {
124123
const session = helper.createDBSession()
125124
const tx = session.beginTransaction()
126125
try {

‎src/services/GroupService.js

Lines changed: 71 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ async function createGroup(currentUser, data) {
193193
await tx.commit()
194194

195195
// set the cache
196-
const cache = helper.getCacheInstance()
196+
const cache = await helper.getCacheInstance()
197197
cache.set(group.id, group)
198198
cache.set(`${group.id}-members`, [])
199199

@@ -276,7 +276,7 @@ async function updateGroup(currentUser, groupId, data) {
276276
await tx.commit()
277277

278278
// update the cache
279-
const cache = helper.getCacheInstance()
279+
const cache = await helper.getCacheInstance()
280280
cache.set(group.id, updateGroup)
281281

282282
return updatedGroup
@@ -350,7 +350,6 @@ async function getGroup(currentUser, groupId, criteria) {
350350
'oldId'
351351
]
352352

353-
354353
if (_.uniq(fieldNames).length !== fieldNames.length) {
355354
throw new errors.BadRequestError(`duplicate field names are not allowed`)
356355
}
@@ -367,110 +366,110 @@ async function getGroup(currentUser, groupId, criteria) {
367366
}
368367
}
369368

370-
let session = undefined;
369+
const session = helper.createDBSession()
370+
const cache = await helper.getCacheInstance()
371371

372-
const cache = helper.getCacheInstance()
373-
const flattenGroupIdTree = []
372+
let groupToReturn
374373

375374
try {
376-
let cachedGroup
377375
if (criteria.isCache) {
378376
// check for the availibility of the group in cache
379-
cachedGroup = cache.get(groupId)
377+
groupToReturn = cache.get(groupId)
380378
}
381379

382-
if (criteria.isCache && cachedGroup) {
383-
if (!isAdmin) delete cachedGroup.status
380+
if (criteria.isCache && groupToReturn) {
381+
if (!isAdmin) delete groupToReturn.status
384382

385383
// if the group is private, the user needs to be a member of the group, or an admin
386-
if (cachedGroup.privateGroup && currentUser !== 'M2M' && !helper.hasAdminRole(currentUser)) {
384+
if (groupToReturn.privateGroup && currentUser !== 'M2M' && !helper.hasAdminRole(currentUser)) {
387385
const cachedGroupMembers = cache.get(`${groupId}-members`)
388-
if (!cachedGroupMembers) {
389-
session = helper.createDBSession()
386+
387+
if (!_.includes(cachedGroupMembers, currentUser.userId)) {
390388
await helper.ensureGroupMember(session, group.id, currentUser.userId)
391389

392390
cachedGroupMembers.push(currentUser.userId)
391+
cache.set(`${groupId}-members`, cachedGroupMembers)
393392
}
394393
}
395-
if (fieldNames) {
396-
fieldNames.push('subGroups')
397-
fieldNames.push('parentGroups')
398-
fieldNames.push('flattenGroupIdTree')
399-
cachedGroup = _.pick(cachedGroup, fieldNames)
400-
}
401-
return cachedGroup
402394
} else {
403-
const session = helper.createDBSession()
404-
405-
let group = await helper.ensureExists(session, 'Group', groupId, isAdmin)
395+
groupToReturn = await helper.ensureExists(session, 'Group', groupId, isAdmin)
396+
cache.set(groupId, groupToReturn)
406397

407-
if (!isAdmin) delete group.status
398+
if (!isAdmin) delete groupToReturn.status
408399

409400
// if the group is private, the user needs to be a member of the group, or an admin
410-
if (group.privateGroup && currentUser !== 'M2M' && !helper.hasAdminRole(currentUser)) {
411-
await helper.ensureGroupMember(session, group.id, currentUser.userId)
401+
if (groupToReturn.privateGroup && currentUser !== 'M2M' && !helper.hasAdminRole(currentUser)) {
402+
await helper.ensureGroupMember(session, groupToReturn.id, currentUser.userId)
403+
404+
cache.set(`${groupId}-members`, [currentUser.userId])
412405
}
406+
}
413407

414-
// get parent or sub groups using breadth first search algorithm,
415-
// this is equivalent to recursive algorithm, but more efficient than latter,
416-
// see https://en.wikipedia.org/wiki/Breadth-first_search
417-
// handled group will be reused, won't be handled duplicately
418-
419-
// pending group to expand
420-
const pending = []
421-
const expanded = []
422-
if (criteria.includeSubGroups || criteria.includeParentGroup) {
423-
pending.push(group)
424-
while (pending.length > 0) {
425-
const groupToExpand = pending.shift()
426-
const found = _.find(expanded, (g) => g.id === groupToExpand.id)
427-
if (found) {
428-
// this group was already expanded, so re-use the fields
429-
groupToExpand.subGroups = found.subGroups
430-
groupToExpand.parentGroups = found.parentGroups
431-
continue
432-
}
433-
expanded.push(groupToExpand)
434-
if (criteria.includeSubGroups) {
435-
// find child groups
436-
groupToExpand.subGroups = await helper.getChildGroups(session, groupToExpand.id)
408+
// get parent or sub groups using breadth first search algorithm,
409+
// this is equivalent to recursive algorithm, but more efficient than latter,
410+
// see https://en.wikipedia.org/wiki/Breadth-first_search
411+
// handled group will be reused, won't be handled duplicately
412+
413+
// pending group to expand
414+
const pending = []
415+
const expanded = []
416+
if (criteria.includeSubGroups || criteria.includeParentGroup || criteria.flattenGroupIdTree) {
417+
pending.push(groupToReturn)
418+
while (pending.length > 0) {
419+
const groupToExpand = pending.shift()
420+
const found = _.find(expanded, (g) => g.id === groupToExpand.id)
421+
if (found) {
422+
// this group was already expanded, so re-use the fields
423+
groupToExpand.subGroups = found.subGroups
424+
groupToExpand.parentGroups = found.parentGroups
425+
continue
426+
}
427+
expanded.push(groupToExpand)
428+
if ((criteria.includeSubGroups && !groupToReturn.subGroups) || (criteria.flattenGroupIdTree && !groupToReturn.flattenGroupIdTree)) {
429+
const flattenGroupIdTree = []
430+
431+
// find child groups
432+
groupToExpand.subGroups = await helper.getChildGroups(session, groupToExpand.id)
433+
// add child groups to pending if needed
434+
if (!criteria.oneLevel) {
437435
_.forEach(groupToExpand.subGroups, (g) => {
438436
pending.push(g)
439437
flattenGroupIdTree.push(g.id)
440438
})
441-
// add child groups to pending if needed
442-
// if (!criteria.oneLevel) {
443-
// _.forEach(groupToExpand.subGroups, (g) => pending.push(g))
444-
// }
445-
} else {
446-
// find parent groups
447-
groupToExpand.parentGroups = await helper.getParentGroups(session, groupToExpand.id)
439+
440+
groupToReturn.flattenGroupIdTree = flattenGroupIdTree
441+
cache.set(groupId, groupToReturn)
442+
}
443+
} else if (criteria.includeParentGroup && !groupToReturn.parentGroups) {
444+
// find parent groups
445+
groupToExpand.parentGroups = await helper.getParentGroups(session, groupToExpand.id)
446+
// add parent groups to pending if needed
447+
if (!criteria.oneLevel) {
448448
_.forEach(groupToExpand.parentGroups, (g) => pending.push(g))
449-
// add parent groups to pending if needed
450-
// if (!criteria.oneLevel) {
451-
// _.forEach(groupToExpand.parentGroups, (g) => pending.push(g))
452-
// }
453449
}
454450
}
455451
}
456-
if (fieldNames) {
457-
fieldNames.push('subGroups')
458-
fieldNames.push('parentGroups')
459-
group = _.pick(group, fieldNames)
460-
}
452+
}
461453

462-
group.flattenGroupIdTree = flattenGroupIdTree
463454

464-
cache.set(groupId, group)
465-
return group
455+
if (fieldNames) {
456+
fieldNames.push('subGroups')
457+
fieldNames.push('parentGroups')
458+
459+
groupToReturn = _.pick(groupToReturn, fieldNames)
466460
}
461+
462+
if (!criteria.includeSubGroups) delete groupToReturn.subGroups
463+
if (!criteria.includeParentGroup) delete groupToReturn.parentGroups
464+
if (!criteria.flattenGroupIdTree) delete groupToReturn.flattenGroupIdTree
465+
466+
return groupToReturn
467467
} catch (error) {
468468
logger.error(error)
469469
throw error
470470
} finally {
471471
logger.debug('Session Close')
472-
if (session)
473-
await session.close()
472+
await session.close()
474473
}
475474
}
476475

@@ -480,6 +479,7 @@ getGroup.schema = {
480479
criteria: Joi.object().keys({
481480
includeSubGroups: Joi.boolean().default(false),
482481
includeParentGroup: Joi.boolean().default(false),
482+
flattenGroupIdTree: Joi.boolean().default(false),
483483
oneLevel: Joi.boolean(),
484484
fields: Joi.string()
485485
})
@@ -506,7 +506,7 @@ async function deleteGroup(groupId, isAdmin) {
506506
await tx.commit()
507507

508508
// delete the cache
509-
const cache = helper.getCacheInstance()
509+
const cache = await helper.getCacheInstance()
510510
cache.del(group.id)
511511
cache.del(`${group.id}-members`)
512512

‎src/services/SubGroupService.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44
const config = require('config')
55
const Joi = require('joi')
6-
const uuid = require('uuid/v4')
6+
const uuid = require('uuid')
77
const helper = require('../common/helper')
88
const logger = require('../common/logger')
99
const errors = require('../common/errors')
@@ -16,7 +16,7 @@ const constants = require('../../app-constants')
1616
* @param {Object} data the sub group data to create group
1717
* @returns {Object} the created group
1818
*/
19-
async function createSubGroup (currentUser, groupId, data) {
19+
async function createSubGroup(currentUser, groupId, data) {
2020
logger.debug(`Create Sub Group - user - ${currentUser} , groupId - ${groupId} , data - ${JSON.stringify(data)}`)
2121

2222
const session = helper.createDBSession()
@@ -41,7 +41,7 @@ async function createSubGroup (currentUser, groupId, data) {
4141

4242
logger.debug(`SubGroup = ${JSON.stringify(subGroup)}`)
4343

44-
const membershipId = uuid()
44+
const membershipId = uuid.v4()
4545

4646
await tx.run('MATCH (g:Group {id: {groupId}}) MATCH (o:Group {id: {subGroupId}}) CREATE (g)-[r:GroupContains {id: {membershipId}, type: {membershipType}, createdAt: {createdAt}, createdBy: {createdBy}}]->(o) RETURN r',
4747
{ groupId, subGroupId: subGroup.id, membershipId, membershipType: config.MEMBERSHIP_TYPES.Group, createdAt: new Date().toISOString(), createdBy: currentUser === 'M2M' ? '00000000' : currentUser.userId })
@@ -93,7 +93,7 @@ createSubGroup.schema = {
9393
* @param {String} subGroupId the sub group id
9494
* @returns {Object} the deleted group
9595
*/
96-
async function deleteSubGroup (currentUser, groupId, subGroupId) {
96+
async function deleteSubGroup(currentUser, groupId, subGroupId) {
9797
logger.debug(`Delete Sub Group - ${groupId}, Sub Group - ${subGroupId}`)
9898
const session = helper.createDBSession()
9999
const tx = session.beginTransaction()

0 commit comments

Comments
 (0)