Skip to content

Commit

Permalink
Fix deserializing of Emoji types (#2577)
Browse files Browse the repository at this point in the history
  • Loading branch information
JonruAlveus authored Sep 20, 2022
1 parent 3c05db4 commit 2701be5
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 11 deletions.
12 changes: 6 additions & 6 deletions Octokit.Tests/Clients/MiscellaneousClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ public class TheGetEmojisMethod
[Fact]
public async Task RequestsTheEmojiEndpoint()
{
IReadOnlyList<Emoji> response = new List<Emoji>
IDictionary<string, string> response = new Dictionary<string, string>
{
{ new Emoji("foo", "http://example.com/foo.gif") },
{ new Emoji("bar", "http://example.com/bar.gif") }
{ "foo", "http://example.com/foo.gif" },
{ "bar", "http://example.com/bar.gif" }
};

var apiConnection = Substitute.For<IApiConnection>();
apiConnection.GetAll<Emoji>(Args.Uri)
.Returns(Task.FromResult(response));
apiConnection.Get<IDictionary<string, string>>(Args.Uri)
.Returns(Task.FromResult(response));

var client = new MiscellaneousClient(apiConnection);

Expand All @@ -82,7 +82,7 @@ public async Task RequestsTheEmojiEndpoint()
Assert.Equal(2, emojis.Count);
Assert.Equal("foo", emojis[0].Name);
apiConnection.Received()
.GetAll<Emoji>(Arg.Is<Uri>(u => u.ToString() == "emojis"));
.Get<IDictionary<string, string>>(Arg.Is<Uri>(u => u.ToString() == "emojis"));
}
}

Expand Down
11 changes: 7 additions & 4 deletions Octokit/Clients/MiscellaneousClient.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading.Tasks;

namespace Octokit
Expand Down Expand Up @@ -39,12 +41,13 @@ public MiscellaneousClient(IApiConnection apiConnection)
/// Gets all the emojis available to use on GitHub.
/// </summary>
/// <exception cref="ApiException">Thrown when a general API error occurs.</exception>
/// <returns>An <see cref="IReadOnlyDictionary{TKey,TValue}"/> of emoji and their URI.</returns>
/// <returns>An <see cref="IReadOnlyList{Emoji}"/> of emoji and their URI.</returns>
[ManualRoute("GET", "/emojis")]
[Obsolete("This client is being deprecated and will be removed in the future. Use EmojisClient.GetAllEmojis instead.")]
public Task<IReadOnlyList<Emoji>> GetAllEmojis()
public async Task<IReadOnlyList<Emoji>> GetAllEmojis()
{
return _emojisClient.GetAllEmojis();
var result = await ApiConnection.Get<IDictionary<string, string>>(ApiUrls.Emojis());

return result.Select(x => new Emoji(x.Key, x.Value)).ToList();
}

/// <summary>
Expand Down
40 changes: 40 additions & 0 deletions Octokit/Http/AssignableExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using System;
using System.Linq;

namespace Octokit
{
public static class AssignableExtensions
{
/// <summary>
/// Determines whether the <paramref name="genericType"/> is assignable from
/// <paramref name="givenType"/> taking into account generic definitions
/// </summary>
public static bool IsAssignableToGenericType(this Type givenType, Type genericType)
{
if (givenType == null || genericType == null)
{
return false;
}

return givenType == genericType
|| givenType.MapsToGenericTypeDefinition(genericType)
|| givenType.HasInterfaceThatMapsToGenericTypeDefinition(genericType)
|| givenType.BaseType.IsAssignableToGenericType(genericType);
}

private static bool HasInterfaceThatMapsToGenericTypeDefinition(this Type givenType, Type genericType)
{
return givenType
.GetInterfaces()
.Where(it => it.IsGenericType)
.Any(it => it.GetGenericTypeDefinition() == genericType);
}

private static bool MapsToGenericTypeDefinition(this Type givenType, Type genericType)
{
return genericType.IsGenericTypeDefinition
&& givenType.IsGenericType
&& givenType.GetGenericTypeDefinition() == genericType;
}
}
}
2 changes: 1 addition & 1 deletion Octokit/Http/JsonHttpPipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public IApiResponse<T> DeserializeResponse<T>(IResponse response)
// simple json does not support the root node being empty. Will submit a pr but in the mean time....
if (!string.IsNullOrEmpty(body) && body != "{}")
{
var typeIsDictionary = typeof(IDictionary).IsAssignableFrom(typeof(T));
var typeIsDictionary = typeof(IDictionary).IsAssignableFrom(typeof(T)) || typeof(T).IsAssignableToGenericType(typeof(System.Collections.Generic.IDictionary<,>));
var typeIsEnumerable = typeof(IEnumerable).IsAssignableFrom(typeof(T));
var responseIsObject = body.StartsWith("{", StringComparison.Ordinal);

Expand Down

0 comments on commit 2701be5

Please sign in to comment.