From 5165e0a54c6b8af7b3ecd907b9cac2423c7bc8b0 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Tue, 5 Jul 2016 17:19:38 +0200 Subject: [PATCH] feat(user.set): support preferences --- src/api/user.js | 7 ++-- src/index.js | 2 +- src/models/user.js | 34 ++++++++++++++----- src/utils.js | 14 +++++--- src/xo-mixins/acls.js | 15 +++++---- src/xo-mixins/subjects.js | 71 +++++++++++++++++++++------------------ 6 files changed, 88 insertions(+), 55 deletions(-) diff --git a/src/api/user.js b/src/api/user.js index 59a918fde..000959b33 100644 --- a/src/api/user.js +++ b/src/api/user.js @@ -57,11 +57,11 @@ getAll.permission = 'admin' // ------------------------------------------------------------------- -export async function set ({id, email, password, permission}) { +export async function set ({id, email, password, permission, preferences}) { if (permission && id === this.session.get('user_id')) { throw new InvalidParameters('a user cannot change it\'s own permission') } - await this.updateUser(id, {email, password, permission}) + await this.updateUser(id, {email, password, permission, preferences}) } set.description = 'changes the properties of an existing user' @@ -72,7 +72,8 @@ set.params = { id: { type: 'string' }, email: { type: 'string', optional: true }, password: { type: 'string', optional: true }, - permission: { type: 'string', optional: true } + permission: { type: 'string', optional: true }, + preferences: { type: 'object', optional: true } } // ------------------------------------------------------------------- diff --git a/src/index.js b/src/index.js index 7ac247612..027469699 100644 --- a/src/index.js +++ b/src/index.js @@ -418,7 +418,7 @@ const apiHelpers = { // Handles both properties and wrapped models. const properties = user.properties || user - return pick(properties, 'id', 'email', 'groups', 'permission', 'provider') + return pick(properties, 'id', 'email', 'groups', 'permission', 'preferences', 'provider') }, throw (errorId, data) { diff --git a/src/models/user.js b/src/models/user.js index ce65e28b6..0c51b3d4c 100644 --- a/src/models/user.js +++ b/src/models/user.js @@ -1,3 +1,5 @@ +import isEmpty from 'lodash/isEmpty' + import Collection from '../collection/redis' import Model from '../model' import { forEach } from '../utils' @@ -12,6 +14,18 @@ User.prototype.default = { // ------------------------------------------------------------------- +const parseProp = (obj, name) => { + const value = obj[name] + if (value == null) { + return + } + try { + return JSON.parse(value) + } catch (error) { + console.warn('cannot parse user[%s] (%s):', name, value, error) + } +} + export class Users extends Collection { get Model () { return User @@ -35,7 +49,13 @@ export class Users extends Collection { async save (user) { // Serializes. - user.groups = JSON.stringify(user.groups) + let tmp + if (!isEmpty(tmp = user.groups)) { + user.groups = JSON.stringify(tmp) + } + if (!isEmpty(tmp = user.preferences)) { + user.preferences = JSON.stringify(tmp) + } return /* await */ this.update(user) } @@ -45,13 +65,11 @@ export class Users extends Collection { // Deserializes forEach(users, user => { - const {groups} = user - try { - user.groups = groups ? JSON.parse(groups) : [] - } catch (_) { - console.warn('cannot parse user.groups:', groups) - user.groups = [] - } + let tmp + user.groups = ((tmp = parseProp(user, 'groups')) && tmp.length) + ? tmp + : undefined + user.preferences = parseProp(user, 'preferences') }) return users diff --git a/src/utils.js b/src/utils.js index 2e8832678..bfc9aa079 100644 --- a/src/utils.js +++ b/src/utils.js @@ -8,6 +8,7 @@ import humanFormat from 'human-format' import invert from 'lodash/invert' import isArray from 'lodash/isArray' import isString from 'lodash/isString' +import keys from 'lodash/keys' import kindOf from 'kindof' import multiKeyHashInt from 'multikey-hash' import xml2js from 'xml2js' @@ -232,10 +233,12 @@ export const parseXml = (function () { // - methods are already bound and chainable export const lightSet = collection => { const data = createRawObject() - collection && forEach(collection, value => { - data[value] = true - }) - collection = null + if (collection) { + forEach(collection, value => { + data[value] = true + }) + collection = null + } const set = { add: value => { @@ -252,7 +255,8 @@ export const lightSet = collection => { delete data[value] return set }, - has: value => data[value] + has: value => data[value], + toArray: () => keys(data) } return set } diff --git a/src/xo-mixins/acls.js b/src/xo-mixins/acls.js index fa572283d..6436870e3 100644 --- a/src/xo-mixins/acls.js +++ b/src/xo-mixins/acls.js @@ -27,16 +27,19 @@ export default class { } async _getAclsForUser (userId) { - const subjects = (await this._xo.getUser(userId)).groups.concat(userId) + const user = await this._xo.getUser(userId) + const { groups } = user + + const subjects = groups + ? groups.concat(userId) + : [ userId ] const acls = [] - const pushAcls = (function (push) { - return function (entries) { - push.apply(acls, entries) - } + const pushAcls = (push => entries => { + push.apply(acls, entries) })(acls.push) - const {_acls: collection} = this + const collection = this._acls await Promise.all(mapToArray( subjects, subject => collection.get({subject}).then(pushAcls) diff --git a/src/xo-mixins/subjects.js b/src/xo-mixins/subjects.js index b50504e15..dc8a6adff 100644 --- a/src/xo-mixins/subjects.js +++ b/src/xo-mixins/subjects.js @@ -17,8 +17,9 @@ import { Users } from '../models/user' import { - createRawObject, forEach, + isEmpty, + lightSet, mapToArray, noop, pCatch @@ -38,6 +39,11 @@ class NoSuchUser extends NoSuchObject { } } +const addToArraySet = (set, value) => set && !includes(set, value) + ? set.concat(value) + : [ value ] +const removeFromArraySet = (set, value) => set && filter(set, current => current !== value) + // =================================================================== export default class { @@ -109,7 +115,8 @@ export default class { name = email, password, - permission + permission, + preferences }) { const user = await this.getUser(id) @@ -123,6 +130,18 @@ export default class { user.pw_hash = await hash(password) } + const newPreferences = { ...user.preferences } + forEach(preferences, (value, name) => { + if (value == null) { + delete newPreferences[name] + } else { + newPreferences[name] = value + } + }) + user.preferences = isEmpty(newPreferences) + ? undefined + : newPreferences + // TODO: remove user.email = user.name delete user.name @@ -264,15 +283,8 @@ export default class { this.getGroup(groupId) ]) - const {groups} = user - if (!includes(groups, groupId)) { - user.groups.push(groupId) - } - - const {users} = group - if (!includes(users, userId)) { - group.users.push(userId) - } + user.groups = addToArraySet(user.groups, groupId) + group.users = addToArraySet(group.users, userId) await Promise.all([ this._users.save(user), @@ -281,14 +293,12 @@ export default class { } async _removeUserFromGroup (userId, group) { - // TODO: maybe not iterating through the whole arrays? - group.users = filter(group.users, id => id !== userId) + group.users = removeFromArraySet(group.users, userId) return this._groups.save(group) } async _removeGroupFromUser (groupId, user) { - // TODO: maybe not iterating through the whole arrays? - user.groups = filter(user.groups, id => id !== groupId) + user.groups = removeFromArraySet(user.groups, groupId) return this._users.save(user) } @@ -307,39 +317,36 @@ export default class { async setGroupUsers (groupId, userIds) { const group = await this.getGroup(groupId) - const newUsersIds = createRawObject() - const oldUsersIds = createRawObject() - forEach(userIds, id => { - newUsersIds[id] = null - }) + let newUsersIds = lightSet(userIds) + const oldUsersIds = [] forEach(group.users, id => { - if (id in newUsersIds) { - delete newUsersIds[id] + if (newUsersIds.has(id)) { + newUsersIds.delete(id) } else { - oldUsersIds[id] = null + oldUsers.push(id) } }) + newUsersIds = newUsersIds.toArray() + const getUser = ::this.getUser const [newUsers, oldUsers] = await Promise.all([ - Promise.all(mapToArray(newUsersIds, (_, id) => this.getUser(id))), - Promise.all(mapToArray(oldUsersIds, (_, id) => this.getUser(id))) + Promise.all(newUsersIds.map(getUser)), + Promise.all(oldUsersIds.map(getUser)) ]) forEach(newUsers, user => { - const {groups} = user - if (!includes(groups, groupId)) { - user.groups.push(groupId) - } + user.groups = addToArraySet(user.groups, groupId) }) forEach(oldUsers, user => { - user.groups = filter(user.groups, id => id !== groupId) + user.groups = removeFromArraySet(user.groups, groupId) }) group.users = userIds + const saveUser = ::this._users.save await Promise.all([ - Promise.all(mapToArray(newUsers, ::this._users.save)), - Promise.all(mapToArray(oldUsers, ::this._users.save)), + Promise.all(mapToArray(newUsers, saveUser)), + Promise.all(mapToArray(oldUsers, saveUser)), this._groups.save(group) ]) }