Fix #995, Move Category Implementation from IGuildChannel to INestedChannel (#1004)

* Fix #995 ICategoryChannel.CategoryID throws NotSupportedException

* Add tests

* change run mode of TestChannelCategories

* Add throw for GetCategoryAsync

* Add xml doc explaining why exception is thrown

* Add test coverage for text and voice channel categories

* initial implementation of INestedChannel

* more implementation of INestedChannel design

* Add case in RestChannel Create for Category type

* set the CategoryID for RestVoiceChannel

* rewrite channel category tests to work with existing pattern

* remove outdated todo

* Make IVoiceChannel implement INestedChannel

* remove redundant interface implementation

* Add c#7 feature from feedback

* Remove redundant GetCategoryAsync methods from socket entities

* Added configureawait to async methods

* change signature of interface GetCategoryAsync

* Add check for cachemode in rest channel GetCategory

* remove redundant IGuildChannel interface from ITextChannel and IVoiceChannel

src/Discord.Net.Core/Entities/Channels/ICategoryChannel.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;

src/Discord.Net.Core/Entities/Channels/IGuildChannel.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
 
@@ -9,10 +9,6 @@ namespace Discord
         /// <summary> Gets the position of this channel in the guild's channel list, relative to others of the same type. </summary>
         int Position { get; }
 
-        /// <summary> Gets the parentid (category) of this channel in the guild's channel list. </summary>
-        ulong? CategoryId { get; }
-        /// <summary> Gets the parent channel (category) of this channel. </summary>
-        Task<ICategoryChannel> GetCategoryAsync();
         /// <summary> Gets the guild this channel is a member of. </summary>
         IGuild Guild { get; }
         /// <summary> Gets the id of the guild this channel is a member of. </summary>

src/Discord.Net.Core/Entities/Channels/INestedChannel.cs

@@ -0,0 +1,16 @@
+using System.Threading.Tasks;
+
+namespace Discord
+{
+    /// <summary>
+    /// A type of guild channel that can be nested within a category.
+    /// Contains a CategoryId that is set to the parent category, if it is set.
+    /// </summary>
+    public interface INestedChannel : IGuildChannel
+    {
+        /// <summary> Gets the parentid (category) of this channel in the guild's channel list. </summary>
+        ulong? CategoryId { get; }
+        /// <summary> Gets the parent channel (category) of this channel, if it is set. If unset, returns null.</summary>
+        Task<ICategoryChannel> GetCategoryAsync(CacheMode mode = CacheMode.AllowDownload, RequestOptions options = null);
+    }
+}

src/Discord.Net.Core/Entities/Channels/ITextChannel.cs

@@ -1,11 +1,11 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Threading.Tasks;
 
 namespace Discord
 {
-    public interface ITextChannel : IMessageChannel, IMentionable, IGuildChannel
+    public interface ITextChannel : IMessageChannel, IMentionable, INestedChannel
     {
         /// <summary> Checks if the channel is NSFW. </summary>
         bool IsNsfw { get; }

src/Discord.Net.Core/Entities/Channels/IVoiceChannel.cs

@@ -1,9 +1,9 @@
-using System;
+using System;
 using System.Threading.Tasks;
 
 namespace Discord
 {
-    public interface IVoiceChannel : IGuildChannel, IAudioChannel
+    public interface IVoiceChannel : INestedChannel, IAudioChannel
     {
         /// <summary> Gets the bitrate, in bits per second, clients in this voice channel are requested to use. </summary>
         int Bitrate { get; }

src/Discord.Net.Rest/Entities/Channels/ChannelHelper.cs

@@ -316,6 +316,16 @@ namespace Discord.Rest
             return models.Select(x => RestWebhook.Create(client, channel, x))
                 .ToImmutableArray();
         }
+        // Categories
+        public static async Task<ICategoryChannel> GetCategoryAsync(INestedChannel channel, BaseDiscordClient client, RequestOptions options)
+        {
+            // if no category id specified, return null
+            if (!channel.CategoryId.HasValue)
+                return null;
+            // CategoryId will contain a value here
+            var model = await client.ApiClient.GetChannelAsync(channel.CategoryId.Value, options).ConfigureAwait(false);
+            return RestCategoryChannel.Create(client, model) as ICategoryChannel;
+        }
 
         //Helpers
         private static IUser GetAuthor(BaseDiscordClient client, IGuild guild, UserModel model, ulong? webhookId)

src/Discord.Net.Rest/Entities/Channels/RestCategoryChannel.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
@@ -25,10 +25,6 @@ namespace Discord.Rest
         private string DebuggerDisplay => $"{Name} ({Id}, Category)";
 
         // IGuildChannel
-        IAsyncEnumerable<IReadOnlyCollection<IGuildUser>> IGuildChannel.GetUsersAsync(CacheMode mode, RequestOptions options)
-            => throw new NotSupportedException();
-        Task<IGuildUser> IGuildChannel.GetUserAsync(ulong id, CacheMode mode, RequestOptions options)
-            => throw new NotSupportedException();
         Task<IInviteMetadata> IGuildChannel.CreateInviteAsync(int? maxAge, int? maxUses, bool isTemporary, bool isUnique, RequestOptions options)
             => throw new NotSupportedException();
         Task<IReadOnlyCollection<IInviteMetadata>> IGuildChannel.GetInvitesAsync(RequestOptions options)

src/Discord.Net.Rest/Entities/Channels/RestChannel.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
@@ -24,6 +24,8 @@ namespace Discord.Rest
                 case ChannelType.DM:
                 case ChannelType.Group:
                     return CreatePrivate(discord, model) as RestChannel;
+                case ChannelType.Category:
+                    return RestCategoryChannel.Create(discord, new RestGuild(discord, model.GuildId.Value), model);
                 default:
                     return new RestChannel(discord, model.Id);
             }

src/Discord.Net.Rest/Entities/Channels/RestGuildChannel.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
@@ -16,7 +16,6 @@ namespace Discord.Rest
         internal IGuild Guild { get; }
         public string Name { get; private set; }
         public int Position { get; private set; }
-        public ulong? CategoryId { get; private set; }
         public ulong GuildId => Guild.Id;
 
         internal RestGuildChannel(BaseDiscordClient discord, IGuild guild, ulong id)
@@ -35,7 +34,6 @@ namespace Discord.Rest
                 case ChannelType.Category:
                     return RestCategoryChannel.Create(discord, guild, model);
                 default:
-                    // TODO: Channel categories
                     return new RestGuildChannel(discord, guild, model.Id);
             }
         }
@@ -64,13 +62,6 @@ namespace Discord.Rest
         public Task DeleteAsync(RequestOptions options = null)
             => ChannelHelper.DeleteAsync(this, Discord, options);
 
-        public async Task<ICategoryChannel> GetCategoryAsync()
-        {
-            if (CategoryId.HasValue)
-                return (await Guild.GetChannelAsync(CategoryId.Value).ConfigureAwait(false)) as ICategoryChannel;
-            return null;
-        }
-
         public OverwritePermissions? GetPermissionOverwrite(IUser user)
         {
             for (int i = 0; i < _overwrites.Length; i++)

src/Discord.Net.Rest/Entities/Channels/RestTextChannel.cs

@@ -12,6 +12,7 @@ namespace Discord.Rest
     public class RestTextChannel : RestGuildChannel, IRestMessageChannel, ITextChannel
     {
         public string Topic { get; private set; }
+        public ulong? CategoryId { get; private set; }
 
         public string Mention => MentionUtils.MentionChannel(Id);
 
@@ -31,7 +32,7 @@ namespace Discord.Rest
         internal override void Update(Model model)
         {
             base.Update(model);
-
+            CategoryId = model.CategoryId;
             Topic = model.Topic.Value;
             _nsfw = model.Nsfw.GetValueOrDefault();
         }
@@ -89,15 +90,18 @@ namespace Discord.Rest
         public Task<IReadOnlyCollection<RestWebhook>> GetWebhooksAsync(RequestOptions options = null)
             => ChannelHelper.GetWebhooksAsync(this, Discord, options);
     
+        public Task<ICategoryChannel> GetCategoryAsync(RequestOptions options = null)
+            => ChannelHelper.GetCategoryAsync(this, Discord, options);
+
         private string DebuggerDisplay => $"{Name} ({Id}, Text)";
 
         //ITextChannel
         async Task<IWebhook> ITextChannel.CreateWebhookAsync(string name, Stream avatar, RequestOptions options)
-            => await CreateWebhookAsync(name, avatar, options);
+            => await CreateWebhookAsync(name, avatar, options).ConfigureAwait(false);
         async Task<IWebhook> ITextChannel.GetWebhookAsync(ulong id, RequestOptions options)
-            => await GetWebhookAsync(id, options);
+            => await GetWebhookAsync(id, options).ConfigureAwait(false);
         async Task<IReadOnlyCollection<IWebhook>> ITextChannel.GetWebhooksAsync(RequestOptions options)
-            => await GetWebhooksAsync(options);
+            => await GetWebhooksAsync(options).ConfigureAwait(false);
 
         //IMessageChannel
         async Task<IMessage> IMessageChannel.GetMessageAsync(ulong id, CacheMode mode, RequestOptions options)
@@ -114,6 +118,7 @@ namespace Discord.Rest
             else
                 return AsyncEnumerable.Empty<IReadOnlyCollection<IMessage>>();
         }
+        
         IAsyncEnumerable<IReadOnlyCollection<IMessage>> IMessageChannel.GetMessagesAsync(ulong fromMessageId, Direction dir, int limit, CacheMode mode, RequestOptions options)
         {
             if (mode == CacheMode.AllowDownload)
@@ -172,5 +177,13 @@ namespace Discord.Rest
             else
                 return AsyncEnumerable.Empty<IReadOnlyCollection<IGuildUser>>();
         }
+
+        // INestedChannel
+        async Task<ICategoryChannel> INestedChannel.GetCategoryAsync(CacheMode mode, RequestOptions options)
+        {
+            if (CategoryId.HasValue && mode == CacheMode.AllowDownload)
+                return (await Guild.GetChannelAsync(CategoryId.Value, mode, options).ConfigureAwait(false)) as ICategoryChannel;
+            return null;
+        }
     }
 }

src/Discord.Net.Rest/Entities/Channels/RestVoiceChannel.cs

@@ -1,4 +1,4 @@
-using Discord.Audio;
+using Discord.Audio;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -13,6 +13,7 @@ namespace Discord.Rest
     {
         public int Bitrate { get; private set; }
         public int? UserLimit { get; private set; }
+        public ulong? CategoryId { get; private set; }
 
         internal RestVoiceChannel(BaseDiscordClient discord, IGuild guild, ulong id)
             : base(discord, guild, id)
@@ -27,7 +28,7 @@ namespace Discord.Rest
         internal override void Update(Model model)
         {
             base.Update(model);
-
+            CategoryId = model.CategoryId;
             Bitrate = model.Bitrate.Value;
             UserLimit = model.UserLimit.Value != 0 ? model.UserLimit.Value : (int?)null;
         }
@@ -38,6 +39,9 @@ namespace Discord.Rest
             Update(model);
         }
 
+        public Task<ICategoryChannel> GetCategoryAsync(RequestOptions options = null)
+            => ChannelHelper.GetCategoryAsync(this, Discord, options);
+
         private string DebuggerDisplay => $"{Name} ({Id}, Voice)";
 
         //IAudioChannel
@@ -48,5 +52,13 @@ namespace Discord.Rest
             => Task.FromResult<IGuildUser>(null);
         IAsyncEnumerable<IReadOnlyCollection<IGuildUser>> IGuildChannel.GetUsersAsync(CacheMode mode, RequestOptions options)
             => AsyncEnumerable.Empty<IReadOnlyCollection<IGuildUser>>();
+
+        // INestedChannel
+        async Task<ICategoryChannel> INestedChannel.GetCategoryAsync(CacheMode mode, RequestOptions options)
+        {
+            if (CategoryId.HasValue && mode == CacheMode.AllowDownload)
+                return (await Guild.GetChannelAsync(CategoryId.Value, mode, options).ConfigureAwait(false)) as ICategoryChannel;
+            return null;
+        }
     }
 }

src/Discord.Net.WebSocket/Entities/Channels/SocketCategoryChannel.cs

@@ -20,7 +20,7 @@ namespace Discord.WebSocket
                ChannelPermission.ViewChannel)).ToImmutableArray();
 
         public IReadOnlyCollection<SocketGuildChannel> Channels
-            => Guild.Channels.Where(x => x.CategoryId == Id).ToImmutableArray();
+            => Guild.Channels.Where(x => x is INestedChannel nestedChannel && nestedChannel.CategoryId == Id).ToImmutableArray();
 
         internal SocketCategoryChannel(DiscordSocketClient discord, ulong id, SocketGuild guild)
             : base(discord, id, guild)

src/Discord.Net.WebSocket/Entities/Channels/SocketGuildChannel.cs

@@ -1,4 +1,4 @@
-using Discord.Rest;
+using Discord.Rest;
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
@@ -17,9 +17,6 @@ namespace Discord.WebSocket
         public SocketGuild Guild { get; }
         public string Name { get; private set; }
         public int Position { get; private set; }        
-        public ulong? CategoryId { get; private set; }
-        public ICategoryChannel Category 
-            => CategoryId.HasValue ? Guild.GetChannel(CategoryId.Value) as ICategoryChannel : null;
 
         public IReadOnlyCollection<Overwrite> PermissionOverwrites => _overwrites;
         public new virtual IReadOnlyCollection<SocketGuildUser> Users => ImmutableArray.Create<SocketGuildUser>();
@@ -48,7 +45,6 @@ namespace Discord.WebSocket
         {
             Name = model.Name.Value;
             Position = model.Position.Value;
-            CategoryId = model.CategoryId;
             
             var overwrites = model.PermissionOverwrites.Value;
             var newOverwrites = ImmutableArray.CreateBuilder<Overwrite>(overwrites.Length);
@@ -135,9 +131,6 @@ namespace Discord.WebSocket
         IGuild IGuildChannel.Guild => Guild;
         ulong IGuildChannel.GuildId => Guild.Id;
 
-        Task<ICategoryChannel> IGuildChannel.GetCategoryAsync()
-            => Task.FromResult(Category);
-
         async Task<IReadOnlyCollection<IInviteMetadata>> IGuildChannel.GetInvitesAsync(RequestOptions options)
             => await GetInvitesAsync(options).ConfigureAwait(false);
         async Task<IInviteMetadata> IGuildChannel.CreateInviteAsync(int? maxAge, int? maxUses, bool isTemporary, bool isUnique, RequestOptions options)

src/Discord.Net.WebSocket/Entities/Channels/SocketTextChannel.cs

@@ -16,6 +16,9 @@ namespace Discord.WebSocket
         private readonly MessageCache _messages;
 
         public string Topic { get; private set; }
+        public ulong? CategoryId { get; private set; }
+        public ICategoryChannel Category
+            => CategoryId.HasValue ? Guild.GetChannel(CategoryId.Value) as ICategoryChannel : null;
 
         private bool _nsfw;
         public bool IsNsfw => _nsfw || ChannelHelper.IsNsfw(this);
@@ -42,7 +45,7 @@ namespace Discord.WebSocket
         internal override void Update(ClientState state, Model model)
         {
             base.Update(state, model);
-
+            CategoryId = model.CategoryId;
             Topic = model.Topic.Value;
             _nsfw = model.Nsfw.GetValueOrDefault();
         }
@@ -169,5 +172,9 @@ namespace Discord.WebSocket
             => await SendMessageAsync(text, isTTS, embed, options).ConfigureAwait(false);
         IDisposable IMessageChannel.EnterTypingState(RequestOptions options)
             => EnterTypingState(options);
+
+        // INestedChannel
+        Task<ICategoryChannel> INestedChannel.GetCategoryAsync(CacheMode mode, RequestOptions options)
+            => Task.FromResult(Category);
     }
 }

src/Discord.Net.WebSocket/Entities/Channels/SocketVoiceChannel.cs

@@ -1,4 +1,4 @@
-using Discord.Audio;
+using Discord.Audio;
 using Discord.Rest;
 using System;
 using System.Collections.Generic;
@@ -15,6 +15,9 @@ namespace Discord.WebSocket
     {
         public int Bitrate { get; private set; }
         public int? UserLimit { get; private set; }
+        public ulong? CategoryId { get; private set; }
+        public ICategoryChannel Category
+            => CategoryId.HasValue ? Guild.GetChannel(CategoryId.Value) as ICategoryChannel : null;
 
         public override IReadOnlyCollection<SocketGuildUser> Users
             => Guild.Users.Where(x => x.VoiceChannel?.Id == Id).ToImmutableArray();
@@ -32,7 +35,7 @@ namespace Discord.WebSocket
         internal override void Update(ClientState state, Model model)
         {
             base.Update(state, model);
-
+            CategoryId = model.CategoryId;
             Bitrate = model.Bitrate.Value;
             UserLimit = model.UserLimit.Value != 0 ? model.UserLimit.Value : (int?)null;
         }
@@ -61,5 +64,9 @@ namespace Discord.WebSocket
             => Task.FromResult<IGuildUser>(GetUser(id));
         IAsyncEnumerable<IReadOnlyCollection<IGuildUser>> IGuildChannel.GetUsersAsync(CacheMode mode, RequestOptions options)
             => ImmutableArray.Create<IReadOnlyCollection<IGuildUser>>(Users).ToAsyncEnumerable();
+
+        // INestedChannel
+        Task<ICategoryChannel> INestedChannel.GetCategoryAsync(CacheMode mode, RequestOptions options)
+            => Task.FromResult(Category);
     }
 }

test/Discord.Net.Tests/Tests.Channels.cs

@@ -1,4 +1,5 @@
 using Discord.Rest;
+using System;
 using System.Linq;
 using System.Threading.Tasks;
 using Xunit;
@@ -15,17 +16,28 @@ namespace Discord
             var text4 = await guild.CreateTextChannelAsync("text4");
             var text5 = await guild.CreateTextChannelAsync("text5");
 
+            // create a channel category
+            var cat1 = await guild.CreateCategoryChannelAsync("cat1");
+
+            if (text1 == null)
+            {
+                // the guild did not have a default channel, so make a new one
+                text1 = await guild.CreateTextChannelAsync("default");
+            }
+
             //Modify #general
             await text1.ModifyAsync(x =>
             {
                 x.Name = "text1";
                 x.Position = 1;
                 x.Topic = "Topic1";
+                x.CategoryId = cat1.Id;
             });
 
             await text2.ModifyAsync(x =>
             {
                 x.Position = 2;
+                x.CategoryId = cat1.Id;
             });
             await text3.ModifyAsync(x =>
             {
@@ -89,10 +101,13 @@ namespace Discord
             var voice2 = await guild.CreateVoiceChannelAsync("voice2");
             var voice3 = await guild.CreateVoiceChannelAsync("voice3");
 
+            var cat2 = await guild.CreateCategoryChannelAsync("cat2");
+
             await voice1.ModifyAsync(x =>
             {
                 x.Bitrate = 96000;
                 x.Position = 1;
+                x.CategoryId = cat2.Id;
             });
             await voice2.ModifyAsync(x =>
             {
@@ -103,6 +118,7 @@ namespace Discord
                 x.Bitrate = 8000;
                 x.Position = 1;
                 x.UserLimit = 16;
+                x.CategoryId = cat2.Id;
             });
 
             CheckVoiceChannels(voice1, voice2, voice3);
@@ -140,5 +156,63 @@ namespace Discord
             Assert.Equal(1, voice3.Position);
             Assert.Equal(16, voice3.UserLimit);
         }
+
+        [Fact]
+        public async Task TestChannelCategories()
+        {
+            // (await _guild.GetVoiceChannelsAsync()).ToArray()
+            var channels = await _guild.GetCategoryChannelsAsync();
+
+            await CheckChannelCategories(channels.ToArray(), (await _guild.GetChannelsAsync()).ToArray());
+        }
+
+        private async Task CheckChannelCategories(RestCategoryChannel[] categories, RestGuildChannel[] allChannels)
+        {
+            // 2 categories
+            Assert.Equal(categories.Length, 2);
+
+            var cat1 = categories.Where(x => x.Name == "cat1").FirstOrDefault();
+            var cat2 = categories.Where(x => x.Name == "cat2").FirstOrDefault();
+
+            Assert.NotNull(cat1);
+            Assert.NotNull(cat2);
+
+            // get text1, text2, ensure they have category id == cat1
+            var text1 = allChannels.Where(x => x.Name == "text1").FirstOrDefault() as RestTextChannel;
+            var text2 = allChannels.Where(x => x.Name == "text2").FirstOrDefault() as RestTextChannel;
+
+            Assert.NotNull(text1);
+            Assert.NotNull(text2);
+
+            // check that CategoryID and .GetCategoryAsync work correctly
+            // for both of the text channels
+            Assert.Equal(text1.CategoryId, cat1.Id);
+            var text1Cat = await text1.GetCategoryAsync();
+            Assert.Equal(text1Cat.Id, cat1.Id);
+            Assert.Equal(text1Cat.Name, cat1.Name);
+
+            Assert.Equal(text2.CategoryId, cat1.Id);
+            var text2Cat = await text2.GetCategoryAsync();
+            Assert.Equal(text2Cat.Id, cat1.Id);
+            Assert.Equal(text2Cat.Name, cat1.Name);
+
+            // do the same for the voice channels
+            var voice1 = allChannels.Where(x => x.Name == "voice1").FirstOrDefault() as RestVoiceChannel;
+            var voice3 = allChannels.Where(x => x.Name == "voice3").FirstOrDefault() as RestVoiceChannel;
+
+            Assert.NotNull(voice1);
+            Assert.NotNull(voice3);
+            
+            Assert.Equal(voice1.CategoryId, cat2.Id);
+            var voice1Cat = await voice1.GetCategoryAsync();
+            Assert.Equal(voice1Cat.Id, cat2.Id);
+            Assert.Equal(voice1Cat.Name, cat2.Name);
+
+            Assert.Equal(voice3.CategoryId, cat2.Id);
+            var voice3Cat = await voice3.GetCategoryAsync();
+            Assert.Equal(voice3Cat.Id, cat2.Id);
+            Assert.Equal(voice3Cat.Name, cat2.Name);
+
+        }
     }
 }