Skip to content

Commit

Permalink
Fix Member methods due to variant joined_at values
Browse files Browse the repository at this point in the history
Fix an issue where the library would check the Id of the guild that a
member is in by checking the Member's ID and joined_at value with those
of the members of guilds present in the cache.

Performing this check would have the issue of where a joined_at
for a member received over websocket would potentially have a varying
value than that of the same member retrieved over REST.

To fix this, attach the relevant guild's Id to the member on creation,
where the Id is available.

Fixes #68.
  • Loading branch information
Zeyla Hellyer committed Mar 21, 2017
1 parent 6b0b9b2 commit cd914f5
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 23 deletions.
4 changes: 4 additions & 0 deletions definitions/structs/member.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ fields:
- name: deaf
description: True if user isn't allowed to hear in voice channels.
type: bool
- name: guild_id
description: The Id of the guild that the member is a part of.
optional: true
type: GuildId
- name: joined_at
description: Timestamp representing the date when the member joined.
type: string
Expand Down
9 changes: 6 additions & 3 deletions src/client/rest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use std::sync::{Arc, Mutex};
use ::constants::{self, ErrorCode};
use ::internal::prelude::*;
use ::model::*;
use ::utils::decode_array;
use ::utils::{decode_array, into_array};

/// An method used for ratelimiting special routes.
///
Expand Down Expand Up @@ -1048,7 +1048,10 @@ pub fn get_guild_members(guild_id: u64, limit: Option<u64>, after: Option<u64>)
limit.unwrap_or(500),
after.unwrap_or(0));

decode_array(serde_json::from_reader(response)?, Member::decode)
into_array(serde_json::from_reader(response)?)
.and_then(|x| x.into_iter()
.map(|v| Member::decode_guild(GuildId(guild_id), v))
.collect())
}

/// Gets the amount of users that can be pruned.
Expand Down Expand Up @@ -1167,7 +1170,7 @@ pub fn get_member(guild_id: u64, user_id: u64) -> Result<Member> {
guild_id,
user_id);

Member::decode(serde_json::from_reader(response)?)
Member::decode_guild(GuildId(guild_id), serde_json::from_reader(response)?)
}

/// Gets a message by an Id, bots only.
Expand Down
1 change: 1 addition & 0 deletions src/ext/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,7 @@ impl Cache {
if !found {
guild.members.insert(event.user.id, Member {
deaf: false,
guild_id: Some(event.guild_id),
joined_at: String::default(),
mute: false,
nick: event.nick.clone(),
Expand Down
12 changes: 8 additions & 4 deletions src/model/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,11 @@ impl GuildMemberAddEvent {
#[doc(hidden)]
#[inline]
pub fn decode(mut map: Map) -> Result<Self> {
let guild_id = remove(&mut map, "guild_id").and_then(GuildId::decode)?;

Ok(GuildMemberAddEvent {
guild_id: remove(&mut map, "guild_id").and_then(GuildId::decode)?,
member: Member::decode(Value::Object(map))?,
guild_id: guild_id,
member: Member::decode_guild(guild_id, Value::Object(map))?,
})
}
}
Expand Down Expand Up @@ -367,9 +369,11 @@ impl GuildMembersChunkEvent {
#[doc(hidden)]
#[inline]
pub fn decode(mut map: Map) -> Result<Self> {
let guild_id = remove(&mut map, "guild_id").and_then(GuildId::decode)?;

Ok(GuildMembersChunkEvent {
guild_id: remove(&mut map, "guild_id").and_then(GuildId::decode)?,
members: remove(&mut map, "members").and_then(decode_members)?,
guild_id: guild_id,
members: remove(&mut map, "members").and_then(|x| decode_guild_members(guild_id, x))?,
})
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/model/guild.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1734,6 +1734,15 @@ impl Member {
roles.iter().find(|r| r.colour.0 != default.0).map(|r| r.colour)
}

#[doc(hidden)]
pub fn decode_guild(guild_id: GuildId, mut value: Value) -> Result<Member> {
if let Some(v) = value.as_object_mut() {
v.insert("guild_id".to_owned(), Value::U64(guild_id.0));
}

Self::decode(value)
}

/// Calculates the member's display name.
///
/// The nickname takes priority over the member's username if it exists.
Expand Down Expand Up @@ -1768,15 +1777,22 @@ impl Member {

/// Finds the Id of the [`Guild`] that the member is in.
///
/// If some value is present in [`guild_id`], then that value is returned.
///
/// # Errors
///
/// Returns a [`ClientError::GuildNotFound`] if the guild could not be
/// found.
///
/// [`ClientError::GuildNotFound`]: ../client/enum.ClientError.html#variant.GuildNotFound
/// [`Guild`]: struct.Guild.html
/// [`guild_id`]: #structfield.guild_id
#[cfg(feature="cache")]
pub fn find_guild(&self) -> Result<GuildId> {
if let Some(guild_id) = self.guild_id {
return Ok(guild_id);
}

for guild in CACHE.read().unwrap().guilds.values() {
let guild = guild.read().unwrap();

Expand Down
34 changes: 18 additions & 16 deletions src/model/utils.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,6 @@
use std::collections::{BTreeMap, HashMap};
use std::sync::{Arc, RwLock};
use super::{
Channel,
ChannelId,
Emoji,
EmojiId,
Member,
Message,
Presence,
ReadState,
Relationship,
Role,
RoleId,
User,
UserId,
VoiceState,
};
use super::*;
use ::internal::prelude::*;
use ::utils::{decode_array, into_array};

Expand Down Expand Up @@ -100,6 +85,23 @@ pub fn decode_members(value: Value) -> Result<HashMap<UserId, Member>> {
Ok(members)
}

pub fn decode_guild_members(guild_id: GuildId, value: Value) -> Result<HashMap<UserId, Member>> {
let mut members = HashMap::new();
let member_vec = into_array(value).map(|x| x
.into_iter()
.map(|v| Member::decode_guild(guild_id, v))
.filter_map(Result::ok)
.collect::<Vec<_>>())?;

for member in member_vec {
let user_id = member.user.read().unwrap().id;

members.insert(user_id, member);
}

Ok(members)
}

// Clippy's lint is incorrect here and will result in invalid code.
//
// Bit more detaul: `result_unwrap_or_default` is not yet stable as of rustc
Expand Down
21 changes: 21 additions & 0 deletions tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
### Running the Ignored Test Suite

Some tests are ignored by default, as they require authentication. These should
only be run when you need to actually test something that _requires_ hitting
Discord's REST API.

### Generic Tests

Provide the token by setting the environment variable `DISCORD_TOKEN`.

e.g.:

```sh
$ DISCORD_TOKEN=aaaaaaaaaaa TEST_USER=285951325479632862 cargo test -- --ignored
```

### Notes for Specific Tests

issues/69:

Provide a `TEST_USER`
16 changes: 16 additions & 0 deletions tests/issues/69.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
extern crate serenity;

use serenity::ext::cache::Cache;
use serenity::model::event::ChannelCreateEvent;
use serenity::model::GuildId;

#[ignore]
fn test_private_channel_create() {
let cache = Cache::default();

let event = ChannelCreateEvent {
channel: Channel {

}
}
}

0 comments on commit cd914f5

Please sign in to comment.