feat(user.set): support preferences

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 }
 }
 
 // -------------------------------------------------------------------

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) {

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
+      let tmp
-      try {
+      user.groups = ((tmp = parseProp(user, 'groups')) && tmp.length)
-        user.groups = groups ? JSON.parse(groups) : []
+        ? tmp
-      } catch (_) {
+        : undefined
-        console.warn('cannot parse user.groups:', groups)
+      user.preferences = parseProp(user, 'preferences')
-        user.groups = []
-      }
     })
 
     return users

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 => {
+  if (collection) {
-    data[value] = true
+    forEach(collection, value => {
-  })
+      data[value] = true
-  collection = null
+    })
+    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
 }

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) {
+    const pushAcls = (push => entries => {
-      return function (entries) {
+      push.apply(acls, 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)

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
+    user.groups = addToArraySet(user.groups, groupId)
-    if (!includes(groups, groupId)) {
+    group.users = addToArraySet(group.users, userId)
-      user.groups.push(groupId)
-    }
-
-    const {users} = group
-    if (!includes(users, userId)) {
-      group.users.push(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 = removeFromArraySet(group.users, userId)
-    group.users = filter(group.users, id => id !== userId)
     return this._groups.save(group)
   }
 
   async _removeGroupFromUser (groupId, user) {
-    // TODO: maybe not iterating through the whole arrays?
+    user.groups = removeFromArraySet(user.groups, groupId)
-    user.groups = filter(user.groups, id => id !== 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()
+    let newUsersIds = lightSet(userIds)
-    const oldUsersIds = createRawObject()
+    const oldUsersIds = []
-    forEach(userIds, id => {
-      newUsersIds[id] = null
-    })
     forEach(group.users, id => {
-      if (id in newUsersIds) {
+      if (newUsersIds.has(id)) {
-        delete newUsersIds[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(newUsersIds.map(getUser)),
-      Promise.all(mapToArray(oldUsersIds, (_, id) => this.getUser(id)))
+      Promise.all(oldUsersIds.map(getUser))
     ])
 
     forEach(newUsers, user => {
-      const {groups} = user
+      user.groups = addToArraySet(user.groups, groupId)
-      if (!includes(groups, groupId)) {
-        user.groups.push(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(newUsers, saveUser)),
-      Promise.all(mapToArray(oldUsers, ::this._users.save)),
+      Promise.all(mapToArray(oldUsers, saveUser)),
       this._groups.save(group)
     ])
   }