refactor: role stores (#2478)

* refactor: reduced duplication in role stores

* fix docs

* fix: typo

* most of requested changes

* rest of changes

* ACTUAL rest of changes

* docs

* doooocs

* reason

src/stores/GuildEmojiRoleStore.js

@@ -1,16 +1,26 @@
-const DataStore = require('./DataStore');
 const Collection = require('../util/Collection');
+const Util = require('../util/Util');
 const { TypeError } = require('../errors');
 
 /**
  * Stores emoji roles
- * @extends {DataStore}
+ * @extends {Collection}
  */
-class GuildEmojiRoleStore extends DataStore {
+class GuildEmojiRoleStore extends Collection {
   constructor(emoji) {
-    super(emoji.client, null, require('../structures/GuildEmoji'));
+    super();
     this.emoji = emoji;
     this.guild = emoji.guild;
+    Object.defineProperty(this, 'client', { value: emoji.client });
+  }
+
+  /**
+   * The filtered collection of roles of the guild emoji
+   * @type {Collection<Snowflake, Role>}
+   * @private
+   */
+  get _filtered() {
+    return this.guild.roles.filter(role => this.emoji._roles.includes(role.id));
   }
 
   /**
@@ -21,14 +31,14 @@ class GuildEmojiRoleStore extends DataStore {
   add(roleOrRoles) {
     if (roleOrRoles instanceof Collection) return this.add(roleOrRoles.keyArray());
     if (!(roleOrRoles instanceof Array)) return this.add([roleOrRoles]);
-
     roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolve(r));
 
     if (roleOrRoles.includes(null)) {
       return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
         'Array or Collection of Roles or Snowflakes', true));
     }
-    const newRoles = [...new Set(roleOrRoles.concat(this.array()))];
+
+    const newRoles = [...new Set(roleOrRoles.concat(...this.values()))];
     return this.set(newRoles);
   }
 
@@ -40,13 +50,13 @@ class GuildEmojiRoleStore extends DataStore {
   remove(roleOrRoles) {
     if (roleOrRoles instanceof Collection) return this.remove(roleOrRoles.keyArray());
     if (!(roleOrRoles instanceof Array)) return this.remove([roleOrRoles]);
-
     roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolveID(r));
 
     if (roleOrRoles.includes(null)) {
       return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
         'Array or Collection of Roles or Snowflakes', true));
     }
+
     const newRoles = this.keyArray().filter(role => !roleOrRoles.includes(role));
     return this.set(newRoles);
   }
@@ -72,7 +82,7 @@ class GuildEmojiRoleStore extends DataStore {
 
   clone() {
     const clone = new this.constructor(this.emoji);
-    clone._patch(this.keyArray());
+    clone._patch(this.keyArray().slice());
     return clone;
   }
 
@@ -82,31 +92,18 @@ class GuildEmojiRoleStore extends DataStore {
    * @private
    */
   _patch(roles) {
-    this.clear();
+    this.emoji._roles = roles;
-
-    for (let role of roles) {
-      role = this.guild.roles.resolve(role);
-      if (role) super.set(role.id, role);
-    }
   }
 
-  /**
+  *[Symbol.iterator]() {
-   * Resolves a RoleResolvable to a Role object.
+    yield* this._filtered.entries();
-   * @method resolve
+  }
-   * @memberof GuildEmojiRoleStore
-   * @instance
-   * @param {RoleResolvable} role The role resolvable to resolve
-   * @returns {?Role}
-   */
 
-  /**
+  valueOf() {
-   * Resolves a RoleResolvable to a role ID string.
+    return this._filtered;
-   * @method resolveID
+  }
-   * @memberof GuildEmojiRoleStore
-   * @instance
-   * @param {RoleResolvable} role The role resolvable to resolve
-   * @returns {?Snowflake}
-   */
 }
 
+Util.mixin(GuildEmojiRoleStore, ['set']);
+
 module.exports = GuildEmojiRoleStore;

src/stores/GuildMemberRoleStore.js

@@ -1,17 +1,57 @@
-const DataStore = require('./DataStore');
-const Role = require('../structures/Role');
 const Collection = require('../util/Collection');
+const Util = require('../util/Util');
 const { TypeError } = require('../errors');
 
 /**
  * Stores member roles
- * @extends {DataStore}
+ * @extends {Collection}
  */
-class GuildMemberRoleStore extends DataStore {
+class GuildMemberRoleStore extends Collection {
   constructor(member) {
-    super(member.client, null, Role);
+    super();
     this.member = member;
     this.guild = member.guild;
+    Object.defineProperty(this, 'client', { value: member.client });
+  }
+
+  /**
+   * The filtered collection of roles of the member
+   * @type {Collection<Snowflake, Role>}
+   * @private
+   */
+  get _filtered() {
+    return this.guild.roles.filter(role => this.member._roles.includes(role.id));
+  }
+
+  /**
+   * The role of the member used to hoist them in a separate category in the users list
+   * @type {?Role}
+   * @readonly
+   */
+  get hoist() {
+    const hoistedRoles = this._filtered.filter(role => role.hoist);
+    if (!hoistedRoles.size) return null;
+    return hoistedRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
+  }
+
+  /**
+   * The role of the member used to set their color
+   * @type {?Role}
+   * @readonly
+   */
+  get color() {
+    const coloredRoles = this._filtered.filter(role => role.color);
+    if (!coloredRoles.size) return null;
+    return coloredRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
+  }
+
+  /**
+   * The role of the member with the highest position
+   * @type {Role}
+   * @readonly
+   */
+  get highest() {
+    return this._filtered.reduce((prev, role) => role.comparePositionTo(prev) > 0 ? role : prev, this.first());
   }
 
   /**
@@ -20,18 +60,60 @@ class GuildMemberRoleStore extends DataStore {
    * @param {string} [reason] Reason for adding the role(s)
    * @returns {Promise<GuildMember>}
    */
-  add(roleOrRoles, reason) {
+  async add(roleOrRoles, reason) {
-    if (roleOrRoles instanceof Collection) return this.add(roleOrRoles.keyArray(), reason);
+    if (roleOrRoles instanceof Collection || roleOrRoles instanceof Array) {
-    if (!(roleOrRoles instanceof Array)) return this.add([roleOrRoles], reason);
+      roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolve(r));
+      if (roleOrRoles.includes(null)) {
+        return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
+          'Array or Collection of Roles or Snowflakes', true));
+      }
 
-    roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolve(r));
+      const newRoles = [...new Set(roleOrRoles.concat(...this.values()))];
+      return this.set(newRoles, reason);
+    } else {
+      roleOrRoles = this.guild.roles.resolve(roleOrRoles);
+      if (roleOrRoles === null) {
+        return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
+          'Array or Collection of Roles or Snowflakes', true));
+      }
 
-    if (roleOrRoles.includes(null)) {
+      await this.client.api.guilds[this.guild.id].members[this.member.id].roles[roleOrRoles.id].put({ reason });
-      return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
+
-        'Array or Collection of Roles or Snowflakes', true));
+      const clone = this.member._clone();
+      clone._patch({ roles: [...this.keys(), roleOrRoles.id] });
+      return clone;
+    }
+  }
+
+  /**
+   * Removes a role (or multiple roles) from the member.
+   * @param {RoleResolvable|RoleResolvable[]|Collection<Snowflake, Role>} roleOrRoles The role or roles to remove
+   * @param {string} [reason] Reason for removing the role(s)
+   * @returns {Promise<GuildMember>}
+   */
+  async remove(roleOrRoles, reason) {
+    if (roleOrRoles instanceof Collection || roleOrRoles instanceof Array) {
+      roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolve(r));
+      if (roleOrRoles.includes(null)) {
+        return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
+          'Array or Collection of Roles or Snowflakes', true));
+      }
+
+      const newRoles = this.guild.roles.filter(role => !roleOrRoles.includes(role.id));
+      return this.set(newRoles, reason);
+    } else {
+      roleOrRoles = this.guild.roles.resolve(roleOrRoles);
+      if (roleOrRoles === null) {
+        return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
+          'Array or Collection of Roles or Snowflakes', true));
+      }
+
+      await this.client.api.guilds[this.guild.id].members[this.member.id].roles[roleOrRoles.id].remove({ reason });
+
+      const clone = this.member._clone();
+      clone._patch({ roles: [...this.keys(), roleOrRoles.id] });
+      return clone;
     }
-    const newRoles = [...new Set(roleOrRoles.concat(this.array()))];
-    return this.set(newRoles, reason);
   }
 
   /**
@@ -54,74 +136,8 @@ class GuildMemberRoleStore extends DataStore {
     return this.member.edit({ roles }, reason);
   }
 
-  /**
-   * Removes a role (or multiple roles) from the member.
-   * @param {RoleResolvable|RoleResolvable[]|Collection<Snowflake, Role>} roleOrRoles The role or roles to remove
-   * @param {string} [reason] Reason for removing the role(s)
-   * @returns {Promise<GuildMember>}
-   */
-  remove(roleOrRoles, reason) {
-    if (roleOrRoles instanceof Collection) return this.remove(roleOrRoles.keyArray(), reason);
-    if (!(roleOrRoles instanceof Array)) return this.remove([roleOrRoles], reason);
-
-    roleOrRoles = roleOrRoles.map(r => this.guild.roles.resolveID(r));
-
-    if (roleOrRoles.includes(null)) {
-      return Promise.reject(new TypeError('INVALID_TYPE', 'roles',
-        'Array or Collection of Roles or Snowflakes', true));
-    }
-    const newRoles = this.keyArray().filter(role => !roleOrRoles.includes(role));
-    return this.set(newRoles, reason);
-  }
-
-  /**
-   * The role of the member used to hoist them in a separate category in the users list
-   * @type {?Role}
-   * @readonly
-   */
-  get hoist() {
-    const hoistedRoles = this.filter(role => role.hoist);
-    if (!hoistedRoles.size) return null;
-    return hoistedRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
-  }
-
-  /**
-   * The role of the member used to set their color
-   * @type {?Role}
-   * @readonly
-   */
-  get color() {
-    const coloredRoles = this.filter(role => role.color);
-    if (!coloredRoles.size) return null;
-    return coloredRoles.reduce((prev, role) => !prev || role.comparePositionTo(prev) > 0 ? role : prev);
-  }
-
-  /**
-   * The role of the member with the highest position
-   * @type {Role}
-   * @readonly
-   */
-  get highest() {
-    return this.reduce((prev, role) => role.comparePositionTo(prev) > 0 ? role : prev, this.first());
-  }
-
-  /**
-   * Patches the roles for this store
-   * @param {Snowflake[]} roles The new roles
-   * @private
-   */
   _patch(roles) {
-    this.clear();
+    this.member._roles = roles;
-
-    const everyoneRole = this.guild.roles.get(this.guild.id);
-    if (everyoneRole) super.set(everyoneRole.id, everyoneRole);
-
-    if (roles) {
-      for (const roleID of roles) {
-        const role = this.guild.roles.resolve(roleID);
-        if (role) super.set(role.id, role);
-      }
-    }
   }
 
   clone() {
@@ -130,23 +146,15 @@ class GuildMemberRoleStore extends DataStore {
     return clone;
   }
 
-  /**
+  *[Symbol.iterator]() {
-   * Resolves a RoleResolvable to a Role object.
+    yield* this._filtered.entries();
-   * @method resolve
+  }
-   * @memberof GuildMemberRoleStore
-   * @instance
-   * @param {RoleResolvable} role The role resolvable to resolve
-   * @returns {?Role}
-   */
 
-  /**
+  valueOf() {
-   * Resolves a RoleResolvable to a role ID string.
+    return this._filtered;
-   * @method resolveID
+  }
-   * @memberof GuildMemberRoleStore
-   * @instance
-   * @param {RoleResolvable} role The role resolvable to resolve
-   * @returns {?Snowflake}
-   */
 }
 
+Util.mixin(GuildMemberRoleStore, ['set']);
+
 module.exports = GuildMemberRoleStore;

src/structures/GuildEmoji.js

@@ -16,12 +16,7 @@ class GuildEmoji extends Emoji {
      */
     this.guild = guild;
 
-    /**
+    this._roles = [];
-     * A collection of roles this emoji is active for (empty if all), mapped by role ID
-     * @type {GuildEmojiRoleStore<Snowflake, Role>}
-     */
-    this.roles = new GuildEmojiRoleStore(this);
-
     this._patch(data);
   }
 
@@ -49,6 +44,14 @@ class GuildEmoji extends Emoji {
     return clone;
   }
 
+  /**
+   * A collection of roles this emoji is active for (empty if all), mapped by role ID
+   * @type {GuildEmojiRoleStore<Snowflake, Role>}
+   */
+  get roles() {
+    return new GuildEmojiRoleStore(this);
+  }
+
   /**
    * The timestamp the emoji was created at
    * @type {number}

src/structures/GuildMember.js

@@ -27,15 +27,6 @@ class GuildMember extends Base {
      */
     this.user = {};
 
-    /**
-     * A list of roles that are applied to this GuildMember, mapped by the role ID
-     * @type {GuildMemberRoleStore<Snowflake, Role>}
-     */
-
-    this.roles = new GuildMemberRoleStore(this);
-
-    if (data) this._patch(data);
-
     /**
      * The ID of the last message sent by the member in their guild, if one was sent
      * @type {?Snowflake}
@@ -47,6 +38,9 @@ class GuildMember extends Base {
      * @type {?Snowflake}
      */
     this.lastMessageChannelID = null;
+
+    this._roles = [];
+    if (data) this._patch(data);
   }
 
   _patch(data) {
@@ -71,16 +65,25 @@ class GuildMember extends Base {
      */
     if (data.joined_at) this.joinedTimestamp = new Date(data.joined_at).getTime();
 
-    this.user = this.guild.client.users.add(data.user);
+    if (data.user) this.user = this.guild.client.users.add(data.user);
     if (data.roles) this.roles._patch(data.roles);
   }
 
   _clone() {
     const clone = super._clone();
-    clone.roles = this.roles.clone();
+    clone._roles = this._roles.slice();
     return clone;
   }
 
+  /**
+   * A collection of roles that are applied to this GuildMember, mapped by the role ID
+   * @type {GuildMemberRoleStore<Snowflake, Role>}
+   * @readonly
+   */
+  get roles() {
+    return new GuildMemberRoleStore(this);
+  }
+
   /**
    * The Message object of the last message sent by the member in their guild, if one was sent
    * @type {?Message}

src/util/Util.js

@@ -1,4 +1,5 @@
 const snekfetch = require('snekfetch');
+const Collection = require('./Collection');
 const { Colors, DefaultOptions, Endpoints } = require('./Constants');
 const { Error: DiscordError, RangeError, TypeError } = require('../errors');
 const has = (o, k) => Object.prototype.hasOwnProperty.call(o, k);
@@ -413,6 +414,33 @@ class Util {
       setTimeout(resolve, ms);
     });
   }
+
+  /**
+   * Adds methods from collections and maps onto the provided store
+   * @param {DataStore} store The store to mixin
+   * @param {string[]} ignored The properties to ignore
+   * @private
+   */
+  /* eslint-disable func-names */
+  static mixin(store, ignored) {
+    Object.getOwnPropertyNames(Collection.prototype)
+      .concat(Object.getOwnPropertyNames(Map.prototype)).forEach(prop => {
+        if (ignored.includes(prop)) return;
+        if (prop === 'size') {
+          Object.defineProperty(store.prototype, prop, {
+            get: function() {
+              return this._filtered[prop];
+            },
+          });
+          return;
+        }
+        const func = Collection.prototype[prop];
+        if (prop === 'constructor' || typeof func !== 'function') return;
+        store.prototype[prop] = function(...args) {
+          return func.apply(this._filtered, ...args);
+        };
+      });
+  }
 }
 
 module.exports = Util;