Skip to content

Commit

Permalink
Deserializer should handle nullable StringEnum<T> (#1760)
Browse files Browse the repository at this point in the history 
* add deserializer tests for nullable enums, StringEnum and nullable StringEnum properties

* Fix deserializing nullable structs by using the underlying type when nullable

* StringEnum<T> should behave like an enum, returning default(T) when it is uninitialised/null/blank

* Don't allow null to be passed into StringEnum ctor - if it needs to be null then it should be declared as nullable

* fix expected json

* move logic to determine if property is a StringEnum<T> into helper function

* serializer needs to treat StringEnum<T> specially by serializing the enum value according to existing serializer strategy (parameter attributes and so on)

* remove fallback to default(T)

* add test to assert ctor throws exception when null passed in

* remove test for default(T) fallback behaviour

* Fix exception in serializer test - StringEnum property must be initialized otherwise an exception is thrown when attempting to serialize

* Dont allow empty strings either
  • Loading branch information
@ryangribble
ryangribble authored Feb 16, 2018
1 parent b35b60f commit 41b4059
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 41 deletions.
29 changes: 15 additions & 14 deletions Octokit.Tests/Models/StringEnumTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ public class StringEnumTests
{
public class TheCtor
{
[Fact]
public void ShouldThrowForNullOrEmptyStringValues()
{
Assert.Throws<ArgumentNullException>(() => new StringEnum<AccountType>(null));

Assert.Throws<ArgumentException>(() => new StringEnum<AccountType>(""));
}

[Fact]
public void ShouldSetValue()
{
Expand Down Expand Up @@ -42,16 +50,12 @@ public void ShouldThrowForInvalidEnumValue()
}
}

public class TheParsedValueProperty
public class TheValueProperty
{
[Theory]
[InlineData("")]
[InlineData(null)]
[InlineData("Cow")]
public void ShouldThrowForInvalidValue(string value)
[Fact]
public void ShouldThrowForInvalidValue()
{
var stringEnum = new StringEnum<AccountType>(value);

var stringEnum = new StringEnum<AccountType>("Cow");
Assert.Throws<ArgumentException>(() => stringEnum.Value);
}

Expand Down Expand Up @@ -96,13 +100,10 @@ public void ShouldReturnTrueForValidValue()
Assert.True(result);
}

[Theory]
[InlineData("")]
[InlineData(null)]
[InlineData("Cow")]
public void ShouldReturnFalseForInvalidValue(string value)
[Fact]
public void ShouldReturnFalseForInvalidValue()
{
var stringEnum = new StringEnum<AccountType>(value);
var stringEnum = new StringEnum<AccountType>("Cow");

AccountType type;
var result = stringEnum.TryParse(out type);
Expand Down
102 changes: 99 additions & 3 deletions Octokit.Tests/SimpleJsonSerializerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,30 @@ public void HandlesEnum()
var item = new ObjectWithEnumProperty
{
Name = "Ferris Bueller",
SomeEnum = SomeEnum.PlusOne
SomeEnum = SomeEnum.Unicode,
SomeEnumNullable = SomeEnum.Unicode,
StringEnum = SomeEnum.SomethingElse,
StringEnumNullable = SomeEnum.SomethingElse
};

var json = new SimpleJsonSerializer().Serialize(item);

Assert.Equal("{\"name\":\"Ferris Bueller\",\"some_enum\":\"+1\"}", json);
Assert.Equal("{\"name\":\"Ferris Bueller\",\"some_enum\":\"unicode\",\"some_enum_nullable\":\"unicode\",\"string_enum\":\"something else\",\"string_enum_nullable\":\"something else\"}", json);
}

[Fact]
public void HandlesEnumDefaults()
{
var item = new ObjectWithEnumProperty
{
Name = "Ferris Bueller",
SomeEnum = SomeEnum.PlusOne,
StringEnum = SomeEnum.PlusOne
};

var json = new SimpleJsonSerializer().Serialize(item);

Assert.Equal("{\"name\":\"Ferris Bueller\",\"some_enum\":\"+1\",\"string_enum\":\"+1\"}", json);
}
}

Expand Down Expand Up @@ -301,7 +319,7 @@ public void DefaultsMissingParameters()
}

[Fact]
public void DeserializesEnumWithParameterAttribute()
public void DeserializesEnums()
{
const string json1 = @"{""some_enum"":""+1""}";
const string json2 = @"{""some_enum"":""utf-8""}";
Expand All @@ -322,6 +340,78 @@ public void DeserializesEnumWithParameterAttribute()
Assert.Equal(SomeEnum.Unicode, sample5.SomeEnum);
}

[Fact]
public void DeserializesNullableEnums()
{
const string json1 = @"{""some_enum_nullable"":""+1""}";
const string json2 = @"{""some_enum_nullable"":""utf-8""}";
const string json3 = @"{""some_enum_nullable"":""something else""}";
const string json4 = @"{""some_enum_nullable"":""another_example""}";
const string json5 = @"{""some_enum_nullable"":""unicode""}";
const string json6 = @"{""some_enum_nullable"":null}";

var sample1 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json1);
var sample2 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json2);
var sample3 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json3);
var sample4 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json4);
var sample5 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json5);
var sample6 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json6);

Assert.Equal(SomeEnum.PlusOne, sample1.SomeEnumNullable);
Assert.Equal(SomeEnum.Utf8, sample2.SomeEnumNullable);
Assert.Equal(SomeEnum.SomethingElse, sample3.SomeEnumNullable);
Assert.Equal(SomeEnum.AnotherExample, sample4.SomeEnumNullable);
Assert.Equal(SomeEnum.Unicode, sample5.SomeEnumNullable);
Assert.False(sample6.SomeEnumNullable.HasValue);
}

[Fact]
public void DeserializesStringEnums()
{
const string json1 = @"{""string_enum"":""+1""}";
const string json2 = @"{""string_enum"":""utf-8""}";
const string json3 = @"{""string_enum"":""something else""}";
const string json4 = @"{""string_enum"":""another_example""}";
const string json5 = @"{""string_enum"":""unicode""}";

var sample1 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json1);
var sample2 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json2);
var sample3 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json3);
var sample4 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json4);
var sample5 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json5);

Assert.Equal(SomeEnum.PlusOne, sample1.StringEnum);
Assert.Equal(SomeEnum.Utf8, sample2.StringEnum);
Assert.Equal(SomeEnum.SomethingElse, sample3.StringEnum);
Assert.Equal(SomeEnum.AnotherExample, sample4.StringEnum);
Assert.Equal(SomeEnum.Unicode, sample5.StringEnum);
}

[Fact]
public void DeserializesNullableStringEnums()
{
const string json1 = @"{""string_enum_nullable"":""+1""}";
const string json2 = @"{""string_enum_nullable"":""utf-8""}";
const string json3 = @"{""string_enum_nullable"":""something else""}";
const string json4 = @"{""string_enum_nullable"":""another_example""}";
const string json5 = @"{""string_enum_nullable"":""unicode""}";
const string json6 = @"{""string_enum_nullable"":null}";

var sample1 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json1);
var sample2 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json2);
var sample3 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json3);
var sample4 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json4);
var sample5 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json5);
var sample6 = new SimpleJsonSerializer().Deserialize<ObjectWithEnumProperty>(json6);

Assert.Equal(SomeEnum.PlusOne, sample1.StringEnumNullable);
Assert.Equal(SomeEnum.Utf8, sample2.StringEnumNullable);
Assert.Equal(SomeEnum.SomethingElse, sample3.StringEnumNullable);
Assert.Equal(SomeEnum.AnotherExample, sample4.StringEnumNullable);
Assert.Equal(SomeEnum.Unicode, sample5.StringEnumNullable);
Assert.False(sample6.StringEnumNullable.HasValue);
}

[Fact]
public void ShouldDeserializeMultipleEnumValues()
{
Expand Down Expand Up @@ -369,6 +459,12 @@ public class ObjectWithEnumProperty
public string Name { get; set; }

public SomeEnum SomeEnum { get; set; }

public SomeEnum? SomeEnumNullable { get; set; }

public StringEnum<SomeEnum> StringEnum { get; set; }

public StringEnum<SomeEnum>? StringEnumNullable { get; set; }
}

public enum SomeEnum
Expand Down
39 changes: 22 additions & 17 deletions Octokit/Http/SimpleJsonSerializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,21 @@ protected override bool TrySerializeUnknownTypes(object input, out object output
Ensure.ArgumentNotNull(input, "input");

var type = input.GetType();
var jsonObject = new JsonObject();
var getters = GetCache[type];

if (ReflectionUtils.IsStringEnumWrapper(type))
{
// Handle StringEnum<T> by getting the underlying enum value, then using the enum serializer
// Note this will throw if the StringEnum<T> was initialised using a string that is not a valid enum member
var inputEnum = (getters["value"](input) as Enum);
if (inputEnum != null)
{
output = SerializeEnum(inputEnum);
return true;
}
}

var jsonObject = new JsonObject();
foreach (var getter in getters)
{
if (getter.Value != null)
Expand Down Expand Up @@ -134,22 +147,19 @@ public override object DeserializeObject(object value, Type type)

if (stringValue != null)
{
// If it's a nullable type, use the underlying type
if (ReflectionUtils.IsNullableType(type))
{
type = Nullable.GetUnderlyingType(type);
}

var typeInfo = ReflectionUtils.GetTypeInfo(type);

if (typeInfo.IsEnum)
{
return DeserializeEnumHelper(stringValue, type);
}

if (ReflectionUtils.IsNullableType(type))
{
var underlyingType = Nullable.GetUnderlyingType(type);
if (ReflectionUtils.GetTypeInfo(underlyingType).IsEnum)
{
return DeserializeEnumHelper(stringValue, underlyingType);
}
}

if (ReflectionUtils.IsTypeGenericeCollectionInterface(type))
{
// OAuth tokens might be a string of comma-separated values
Expand All @@ -161,14 +171,9 @@ public override object DeserializeObject(object value, Type type)
}
}

if (typeInfo.IsGenericType)
if (ReflectionUtils.IsStringEnumWrapper(type))
{
var typeDefinition = typeInfo.GetGenericTypeDefinition();

if (typeof(StringEnum<>).IsAssignableFrom(typeDefinition))
{
return Activator.CreateInstance(type, stringValue);
}
return Activator.CreateInstance(type, stringValue);
}
}
else if (jsonValue != null)
Expand Down
10 changes: 3 additions & 7 deletions Octokit/Models/Response/StringEnum.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ public StringEnum(TEnum parsedValue)

public StringEnum(string stringValue)
{
_stringValue = stringValue ?? string.Empty;
Ensure.ArgumentNotNullOrEmptyString(stringValue, nameof(stringValue));

_stringValue = stringValue;
_parsedValue = null;
}

Expand Down Expand Up @@ -66,12 +68,6 @@ public bool TryParse(out TEnum value)
return true;
}

if (string.IsNullOrEmpty(StringValue))
{
value = default(TEnum);
return false;
}

try
{
// Use the SimpleJsonSerializer to parse the string to Enum according to the GitHub Api strategy
Expand Down
12 changes: 12 additions & 0 deletions Octokit/SimpleJson.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1772,6 +1772,18 @@ public static bool IsAssignableFrom(Type type1, Type type2)
return GetTypeInfo(type1).IsAssignableFrom(GetTypeInfo(type2));
}

public static bool IsStringEnumWrapper(Type type)
{
var typeInfo = ReflectionUtils.GetTypeInfo(type);
if (typeInfo.IsGenericType)
{
var typeDefinition = typeInfo.GetGenericTypeDefinition();

return typeof(StringEnum<>).IsAssignableFrom(typeDefinition);
}
return false;
}

public static IEnumerable<Type> GetInterfaces(Type type)
{
#if SIMPLE_JSON_TYPEINFO
Expand Down

0 comments on commit 41b4059

Please sign in to comment.