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

@@ -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
+// or, if the limit is set to 0, has fetched all guilds the user has joined.
-// range.
 //
 // 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
+// passed limit, or, if the limit is set to 0, has fetched all guilds with an
-// the passed range.
+// 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
+	fetch := uint(maxGuildFetchLimit)
-	const hardLimit int = 100
 
 	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
+// passed limit, or, if the limit is set to 0, has fetched all guilds with an
-// the passed range.
+// 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
+	fetch := uint(maxGuildFetchLimit)
-	const hardLimit int = 100
 
 	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
 }
 

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
 }
 

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.
+// Messages returns a slice filled with the most recent messages sent in the
-// This method automatically paginates until it reaches the passed limit, or,
+// channel with the passed ID. The method automatically paginates until it
-// if the limit is set to 0, has fetched all guilds within the passed ange.
+// 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
+// As the underlying endpoint is capped at a maximum of 100 messages per
-// maximum a total of limit/100 rounded up requests will be made, although they
+// request, at maximum a total of limit/100 rounded up requests will be made,
-// may be less, if no more messages are available.
+// 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
+// MessagesBefore returns a slice filled with the messages sent in the channel
-// ID. This method automatically paginates until it reaches the passed limit,
+// with the passed id. The method automatically paginates until it reaches the
-// or, if the limit is set to 0, has fetched all guilds within the passed
+// passed limit, or, if the limit is set to 0, has fetched all messages in the
-// range.
+// 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
+// MessagesAfter returns a slice filled with the messages sent in the channel
-// ID. This method automatically paginates until it reaches the passed limit,
+// with the passed ID. The method automatically paginates until it reaches the
-// or, if the limit is set to 0, has fetched all guilds within the passed
+// passed limit, or, if the limit is set to 0, has fetched all messages in the
-// range.
+// 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
 }
 

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
 }