Skip to content

Commit

Permalink
bugfix: improve fallback when StringEnum encounters null value (#2156)
Browse files Browse the repository at this point in the history
  • Loading branch information
shiftkey authored Mar 17, 2020
1 parent f6a9a47 commit e9516bb
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 2 deletions.
50 changes: 50 additions & 0 deletions Octokit.Tests/SimpleJsonSerializerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,56 @@ public void ShouldDeserializeMultipleEnumValues()
// Test passes if no exception thrown
}
}

// this test is the root cause of https://github.com/octokit/octokit.net/issues/2052
// as the API is returning a null value where the enum is expecting
// something like a string
//
// this should be removed once we can confirm the GitHub API is no
// longer returning a null for the parent Team's permission value,
// making the test redundant
[Fact]
public void ShouldDeserializeParentTeamWithNullPermission()
{
var teamJson = @"{
""id"": 1,
""node_id"": ""MDQ6VGVhbTE="",
""url"": ""https://api.github.com/teams/1"",
""html_url"": ""https://api.github.com/teams/justice-league"",
""name"": ""Justice League"",
""slug"": ""justice-league"",
""description"": ""A great team."",
""privacy"": ""closed"",
""permission"": ""admin"",
""members_url"": ""https://api.github.com/teams/1/members{/member}"",
""repositories_url"": ""https://api.github.com/teams/1/repos"",
""parent"": {
""id"": 1,
""node_id"": ""MDQ6LFJSbTE="",
""url"": ""https://api.github.com/teams/2"",
""html_url"": ""https://api.github.com/teams/super-friends"",
""name"": ""Super Friends"",
""slug"": ""super-friends"",
""description"": ""Also a great team."",
""privacy"": ""closed"",
""permission"": null,
""members_url"": ""https://api.github.com/teams/2/members{/member}"",
""repositories_url"": ""https://api.github.com/teams/2/repos"",
}
}
}";

var result = new SimpleJsonSerializer().Deserialize<Team>(teamJson);

// original value works as expected
Assert.Equal(PermissionLevel.Admin, result.Permission.Value);
Assert.Equal("admin", result.Permission.StringValue);

// parent permission is marked as null and cannot be parsed
Assert.Equal("null", result.Parent.Permission.StringValue);
PermissionLevel value;
Assert.False(result.Parent.Permission.TryParse(out value));
}
}

public class Sample
Expand Down
12 changes: 12 additions & 0 deletions Octokit/Http/SimpleJsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,18 @@ public override object DeserializeObject(object value, Type type)
return base.DeserializeObject(value, payloadType);
}

if (ReflectionUtils.IsStringEnumWrapper(type))
{
// this check is a workaround for https://github.com/octokit/octokit.net/issues/2052
// as the API is returning a null value where the enum is
// expecting something like a string
//
// this should be removed once we can confirm the GitHub API
// is no longer returning a null for the parent Team's
// permission value
return Activator.CreateInstance(type, "null");
}

return base.DeserializeObject(value, type);
}

Expand Down
4 changes: 2 additions & 2 deletions Octokit/Models/Response/Team.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class Team
{
public Team() { }

public Team(string url, int id, string nodeId, string slug, string name, string description, TeamPrivacy privacy, Permission permission, int membersCount, int reposCount, Organization organization, Team parent, string ldapDistinguishedName)
public Team(string url, int id, string nodeId, string slug, string name, string description, TeamPrivacy privacy, PermissionLevel permission, int membersCount, int reposCount, Organization organization, Team parent, string ldapDistinguishedName)
{
Url = url;
Id = id;
Expand Down Expand Up @@ -67,7 +67,7 @@ public Team(string url, int id, string nodeId, string slug, string name, string
/// <summary>
/// permission attached to this team
/// </summary>
public StringEnum<Permission> Permission { get; protected set; }
public StringEnum<PermissionLevel> Permission { get; protected set; }

/// <summary>
/// how many members in this team
Expand Down

0 comments on commit e9516bb

Please sign in to comment.