From 397d288927c55305dc0c953d70b59ca17d79d1b0 Mon Sep 17 00:00:00 2001 From: Maximilian von Lindern <48887425+mavolin@users.noreply.github.com> Date: Mon, 19 Oct 2020 16:47:43 +0200 Subject: [PATCH] API: fix errors in message pagination and streamline changes with other pagination methods (#150) * API: fix faulty pagination behavior This fix fixes a condition which lead to all messages getting fetched if the limit was a multiple of 100, instead of just the limit. * API: add NewestMessages * API: clarify MessageAfter docs * API: adapt paginating methods for guild, member and message reaction to match the style of message's pagination methods * API: return nil if no items were fetched * API: remove Messages and Rename NewestMessages to Messages --- api/guild.go | 45 ++++++++++++++++---------- api/member.go | 24 +++++++++----- api/message.go | 70 ++++++++++++++++++++++++++++++----------- api/message_reaction.go | 36 +++++++++++++++------ 4 files changed, 122 insertions(+), 53 deletions(-) diff --git a/api/guild.go b/api/guild.go index a5930bd..84595dc 100644 --- a/api/guild.go +++ b/api/guild.go @@ -9,6 +9,10 @@ import ( "github.com/diamondburned/arikawa/utils/json/option" ) +// maxGuildFetchLimit is the limit of max guilds per request, as imposed by +// Discord. +const maxGuildFetchLimit = 100 + var EndpointGuilds = Endpoint + "guilds/" // https://discordapp.com/developers/docs/resources/guild#create-guild-json-params @@ -105,8 +109,7 @@ func (c *Client) GuildWithCount(id discord.GuildID) (*discord.Guild, error) { // Guilds returns a list of partial guild objects the current user is a member // of. This method automatically paginates until it reaches the passed limit, -// or, if the limit is set to 0, has fetched all guilds within the passed -// range. +// or, if the limit is set to 0, has fetched all guilds the user has joined. // // As the underlying endpoint has a maximum of 100 guilds per request, at // maximum a total of limit/100 rounded up requests will be made, although they @@ -125,8 +128,8 @@ func (c *Client) Guilds(limit uint) ([]discord.Guild, error) { // GuildsBefore returns a list of partial guild objects the current user is a // member of. This method automatically paginates until it reaches the -// passed limit, or, if the limit is set to 0, has fetched all guilds within -// the passed range. +// passed limit, or, if the limit is set to 0, has fetched all guilds with an +// id smaller than before. // // As the underlying endpoint has a maximum of 100 guilds per request, at // maximum a total of limit/100 rounded up requests will be made, although they @@ -134,15 +137,16 @@ func (c *Client) Guilds(limit uint) ([]discord.Guild, error) { // // Requires the guilds OAuth2 scope. func (c *Client) GuildsBefore(before discord.GuildID, limit uint) ([]discord.Guild, error) { - var guilds []discord.Guild + guilds := make([]discord.Guild, 0, limit) - // this is the limit of max guilds per request,as imposed by Discord - const hardLimit int = 100 + fetch := uint(maxGuildFetchLimit) unlimited := limit == 0 - for fetch := uint(hardLimit); limit > 0 || unlimited; fetch = uint(hardLimit) { + for limit > 0 || unlimited { if limit > 0 { + // Only fetch as much as we need. Since limit gradually decreases, + // we only need to fetch min(fetch, limit). if fetch > limit { fetch = limit } @@ -155,20 +159,24 @@ func (c *Client) GuildsBefore(before discord.GuildID, limit uint) ([]discord.Gui } guilds = append(g, guilds...) - if len(g) < hardLimit { + if len(g) < maxGuildFetchLimit { break } before = g[0].ID } + if len(guilds) == 0 { + return nil, nil + } + return guilds, nil } // GuildsAfter returns a list of partial guild objects the current user is a // member of. This method automatically paginates until it reaches the -// passed limit, or, if the limit is set to 0, has fetched all guilds within -// the passed range. +// passed limit, or, if the limit is set to 0, has fetched all guilds with an +// id higher than after. // // As the underlying endpoint has a maximum of 100 guilds per request, at // maximum a total of limit/100 rounded up requests will be made, although they @@ -176,14 +184,15 @@ func (c *Client) GuildsBefore(before discord.GuildID, limit uint) ([]discord.Gui // // Requires the guilds OAuth2 scope. func (c *Client) GuildsAfter(after discord.GuildID, limit uint) ([]discord.Guild, error) { - var guilds []discord.Guild + guilds := make([]discord.Guild, 0, limit) - // this is the limit of max guilds per request, as imposed by Discord - const hardLimit int = 100 + fetch := uint(maxGuildFetchLimit) unlimited := limit == 0 - for fetch := uint(hardLimit); limit > 0 || unlimited; fetch = uint(hardLimit) { + for limit > 0 || unlimited { + // Only fetch as much as we need. Since limit gradually decreases, + // we only need to fetch min(fetch, limit). if limit > 0 { if fetch > limit { fetch = limit @@ -197,13 +206,17 @@ func (c *Client) GuildsAfter(after discord.GuildID, limit uint) ([]discord.Guild } guilds = append(guilds, g...) - if len(g) < hardLimit { + if len(g) < maxGuildFetchLimit { break } after = g[len(g)-1].ID } + if len(guilds) == 0 { + return nil, nil + } + return guilds, nil } diff --git a/api/member.go b/api/member.go index bc64159..a7fbde9 100644 --- a/api/member.go +++ b/api/member.go @@ -6,7 +6,9 @@ import ( "github.com/diamondburned/arikawa/utils/json/option" ) -// Member returns a guild member object for the specified user.. +const maxMemberFetchLimit = 1000 + +// Member returns a guild member object for the specified user. func (c *Client) Member(guildID discord.GuildID, userID discord.UserID) (*discord.Member, error) { var m *discord.Member return m, c.RequestJSON(&m, "GET", EndpointGuilds+guildID.String()+"/members/"+userID.String()) @@ -14,11 +16,11 @@ func (c *Client) Member(guildID discord.GuildID, userID discord.UserID) (*discor // Members returns a list of members of the guild with the passed id. This // method automatically paginates until it reaches the passed limit, or, if the -// limit is set to 0, has fetched all members within the passed range. +// limit is set to 0, has fetched all members in the guild. // // As the underlying endpoint has a maximum of 1000 members per request, at // maximum a total of limit/1000 rounded up requests will be made, although -// they may be less, if no more members are available. +// they may be less if no more members are available. // // When fetching the members, those with the smallest ID will be fetched first. func (c *Client) Members(guildID discord.GuildID, limit uint) ([]discord.Member, error) { @@ -27,7 +29,7 @@ func (c *Client) Members(guildID discord.GuildID, limit uint) ([]discord.Member, // MembersAfter returns a list of members of the guild with the passed id. This // method automatically paginates until it reaches the passed limit, or, if the -// limit is set to 0, has fetched all members within the passed range. +// limit is set to 0, has fetched all members with an id higher than after. // // As the underlying endpoint has a maximum of 1000 members per request, at // maximum a total of limit/1000 rounded up requests will be made, although @@ -35,13 +37,15 @@ func (c *Client) Members(guildID discord.GuildID, limit uint) ([]discord.Member, func (c *Client) MembersAfter( guildID discord.GuildID, after discord.UserID, limit uint) ([]discord.Member, error) { - var mems []discord.Member + mems := make([]discord.Member, 0, limit) - const hardLimit int = 1000 + fetch := uint(maxMemberFetchLimit) unlimited := limit == 0 - for fetch := uint(hardLimit); limit > 0 || unlimited; fetch = uint(hardLimit) { + for limit > 0 || unlimited { + // Only fetch as much as we need. Since limit gradually decreases, + // we only need to fetch min(fetch, limit). if limit > 0 { if fetch > limit { fetch = limit @@ -56,13 +60,17 @@ func (c *Client) MembersAfter( mems = append(mems, m...) // There aren't any to fetch, even if this is less than limit. - if len(m) < hardLimit { + if len(m) < maxMemberFetchLimit { break } after = mems[len(mems)-1].User.ID } + if len(mems) == 0 { + return nil, nil + } + return mems, nil } diff --git a/api/message.go b/api/message.go index d4122f6..9c6c027 100644 --- a/api/message.go +++ b/api/message.go @@ -11,18 +11,23 @@ import ( // the limit of max messages per request, as imposed by Discord const maxMessageFetchLimit = 100 -// Messages returns a list of messages sent in the channel with the passed ID. -// This method automatically paginates until it reaches the passed limit, or, -// if the limit is set to 0, has fetched all guilds within the passed ange. +// Messages returns a slice filled with the most recent messages sent in the +// channel with the passed ID. The method automatically paginates until it +// reaches the passed limit, or, if the limit is set to 0, has fetched all +// messages in the channel. // -// As the underlying endpoint has a maximum of 100 messages per request, at -// maximum a total of limit/100 rounded up requests will be made, although they -// may be less, if no more messages are available. +// As the underlying endpoint is capped at a maximum of 100 messages per +// request, at maximum a total of limit/100 rounded up requests will be made, +// although they may be less, if no more messages are available. // -// When fetching the messages, those with the smallest ID will be fetched +// When fetching the messages, those with the highest ID, will be fetched // first. +// The returned slice will be sorted from latest to oldest. func (c *Client) Messages(channelID discord.ChannelID, limit uint) ([]discord.Message, error) { - return c.MessagesAfter(channelID, 0, limit) + // Since before is 0 it will be omitted by the http lib, which in turn + // will lead discord to send us the most recent messages without having to + // specify a Snowflake. + return c.MessagesBefore(channelID, 0, limit) } // MessagesAround returns messages around the ID, with a limit of 100. @@ -32,22 +37,28 @@ func (c *Client) MessagesAround( return c.messagesRange(channelID, 0, 0, around, limit) } -// MessagesBefore returns a list messages sent in the channel with the passed -// ID. This method automatically paginates until it reaches the passed limit, -// or, if the limit is set to 0, has fetched all guilds within the passed -// range. +// MessagesBefore returns a slice filled with the messages sent in the channel +// with the passed id. The method automatically paginates until it reaches the +// passed limit, or, if the limit is set to 0, has fetched all messages in the +// channel with an id smaller than before. // // As the underlying endpoint has a maximum of 100 messages per request, at // maximum a total of limit/100 rounded up requests will be made, although they // may be less, if no more messages are available. +// +// The returned slice will be sorted from latest to oldest. func (c *Client) MessagesBefore( channelID discord.ChannelID, before discord.MessageID, limit uint) ([]discord.Message, error) { - var msgs []discord.Message + msgs := make([]discord.Message, 0, limit) fetch := uint(maxMessageFetchLimit) - for limit >= 0 { + // Check if we are truly fetching unlimited messages to avoid confusion + // later on, if the limit reaches 0. + unlimited := limit == 0 + + for limit > 0 || unlimited { if limit > 0 { // Only fetch as much as we need. Since limit gradually decreases, // we only need to fetch min(fetch, limit). @@ -71,25 +82,42 @@ func (c *Client) MessagesBefore( before = m[len(m)-1].ID } + if len(msgs) == 0 { + return nil, nil + } + return msgs, nil } -// MessagesAfter returns a list messages sent in the channel with the passed -// ID. This method automatically paginates until it reaches the passed limit, -// or, if the limit is set to 0, has fetched all guilds within the passed -// range. +// MessagesAfter returns a slice filled with the messages sent in the channel +// with the passed ID. The method automatically paginates until it reaches the +// passed limit, or, if the limit is set to 0, has fetched all messages in the +// channel with an id higher than after. // // As the underlying endpoint has a maximum of 100 messages per request, at // maximum a total of limit/100 rounded up requests will be made, although they // may be less, if no more messages are available. +// +// The returned slice will be sorted from latest to oldest. func (c *Client) MessagesAfter( channelID discord.ChannelID, after discord.MessageID, limit uint) ([]discord.Message, error) { + // 0 is uint's zero value and will lead to the after param getting omitted, + // which in turn will lead to the most recent messages being returned. + // Setting this to 1 will prevent that. + if after == 0 { + after = 1 + } + var msgs []discord.Message fetch := uint(maxMessageFetchLimit) - for limit >= 0 { + // Check if we are truly fetching unlimited messages to avoid confusion + // later on, if the limit reaches 0. + unlimited := limit == 0 + + for limit > 0 || unlimited { if limit > 0 { // Only fetch as much as we need. Since limit gradually decreases, // we only need to fetch min(fetch, limit). @@ -113,6 +141,10 @@ func (c *Client) MessagesAfter( after = m[0].ID } + if len(msgs) == 0 { + return nil, nil + } + return msgs, nil } diff --git a/api/message_reaction.go b/api/message_reaction.go index 44ea28b..fe02830 100644 --- a/api/message_reaction.go +++ b/api/message_reaction.go @@ -7,6 +7,8 @@ import ( "github.com/diamondburned/arikawa/utils/httputil" ) +const maxMessageReactionFetchLimit = 100 + // React creates a reaction for the message. // // This endpoint requires the READ_MESSAGE_HISTORY permission to be present on @@ -42,7 +44,8 @@ func (c *Client) Reactions( // ReactionsBefore returns a list of users that reacted with the passed Emoji. // This method automatically paginates until it reaches the passed limit, or, -// if the limit is set to 0, has fetched all users within the passed range. +// if the limit is set to 0, has fetched all users with an id smaller than +// before. // // As the underlying endpoint has a maximum of 100 users per request, at // maximum a total of limit/100 rounded up requests will be made, although they @@ -51,13 +54,15 @@ func (c *Client) ReactionsBefore( channelID discord.ChannelID, messageID discord.MessageID, before discord.UserID, emoji Emoji, limit uint) ([]discord.User, error) { - var users []discord.User + users := make([]discord.User, 0, limit) - const hardLimit int = 100 + fetch := uint(maxMessageReactionFetchLimit) unlimited := limit == 0 - for fetch := uint(hardLimit); limit > 0 || unlimited; fetch = uint(hardLimit) { + for limit > 0 || unlimited { + // Only fetch as much as we need. Since limit gradually decreases, + // we only need to fetch min(fetch, limit). if limit > 0 { if fetch > limit { fetch = limit @@ -71,19 +76,24 @@ func (c *Client) ReactionsBefore( } users = append(r, users...) - if len(r) < hardLimit { + if len(r) < maxMessageReactionFetchLimit { break } before = r[0].ID } + if len(users) == 0 { + return nil, nil + } + return users, nil } // ReactionsAfter returns a list of users that reacted with the passed Emoji. // This method automatically paginates until it reaches the passed limit, or, -// if the limit is set to 0, has fetched all users within the passed range. +// if the limit is set to 0, has fetched all users with an id higher than +// after. // // As the underlying endpoint has a maximum of 100 users per request, at // maximum a total of limit/100 rounded up requests will be made, although they @@ -92,13 +102,15 @@ func (c *Client) ReactionsAfter( channelID discord.ChannelID, messageID discord.MessageID, after discord.UserID, emoji Emoji, limit uint) ([]discord.User, error) { - var users []discord.User + users := make([]discord.User, 0, limit) - const hardLimit int = 100 + fetch := uint(maxMessageReactionFetchLimit) unlimited := limit == 0 - for fetch := uint(hardLimit); limit > 0 || unlimited; fetch = uint(hardLimit) { + for limit > 0 || unlimited { + // Only fetch as much as we need. Since limit gradually decreases, + // we only need to fetch min(fetch, limit). if limit > 0 { if fetch > limit { fetch = limit @@ -112,13 +124,17 @@ func (c *Client) ReactionsAfter( } users = append(users, r...) - if len(r) < hardLimit { + if len(r) < maxMessageReactionFetchLimit { break } after = r[len(r)-1].ID } + if len(users) == 0 { + return nil, nil + } + return users, nil }