Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Default ODataEnumSerializer and ODataEnumDeserializer Fails to Convert Multi-Value Flag Enum Values to Lower Camel Case #1367

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Fix Default ODataEnumSerializer and ODataEnumDeserializer Fails to Convert Multi-Value Flag Enum Values to Lower Camel Case #1367

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ee01349
Be able to write multi-value flag enum members in lower camel case
WanjohiSammy Nov 29, 2024
acfb6c0
Added functionality to read value of a flags enum
WanjohiSammy Dec 4, 2024
c82e73c
Refactor ODataEnumDeserializer and tests
WanjohiSammy Dec 4, 2024
1343e4d
Refactor to use ReadOnlySpan other than string[]
WanjohiSammy Dec 5, 2024
0f9fd31
Remove duplicate assertion
WanjohiSammy Dec 5, 2024
d4c84d7
Refactor
WanjohiSammy Dec 9, 2024
261d0fc
Refactor split logic
WanjohiSammy Dec 13, 2024
301208a
Added more tests
WanjohiSammy Dec 13, 2024
b038cb9
Refactor
WanjohiSammy Dec 14, 2024
9781cd0
Remove unused usings
WanjohiSammy Dec 14, 2024
5166202
Check if clrEnumMember is null before Convert.ToInt64
WanjohiSammy Dec 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 64 additions & 2 deletions src/Microsoft.AspNetCore.OData/Formatter/Deserialization/ODataEnumDeserializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
using System.Diagnostics.Contracts;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.OData.Common;
using Microsoft.AspNetCore.OData.Edm;
using Microsoft.AspNetCore.OData.Formatter.Value;
using Microsoft.OData;
using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;

namespace Microsoft.AspNetCore.OData.Formatter.Deserialization;

Expand Down Expand Up @@ -83,13 +85,15 @@ public override object ReadInline(object item, IEdmTypeReference edmType, ODataD

IEdmEnumType enumType = enumTypeReference.EnumDefinition();

Type clrType = readContext.Model.GetClrType(edmType);

// Enum member supports model alias case. So, try to use the Edm member name to retrieve the Enum value.
var memberMapAnnotation = readContext.Model.GetClrEnumMemberAnnotation(enumType);
if (memberMapAnnotation != null)
{
if (enumValue != null)
{
IEdmEnumMember enumMember = enumType.Members.FirstOrDefault(m => m.Name == enumValue.Value);
IEdmEnumMember enumMember = enumType.Members.FirstOrDefault(m => m.Name.Equals(enumValue.Value, StringComparison.InvariantCultureIgnoreCase));
if (enumMember != null)
{
var clrMember = memberMapAnnotation.GetClrEnumMember(enumMember);
Expand All @@ -98,10 +102,68 @@ public override object ReadInline(object item, IEdmTypeReference edmType, ODataD
return clrMember;
}
}
else if (enumType.IsFlags)
{
var result = ReadFlagsEnumValue(enumValue, enumType, clrType, memberMapAnnotation);
if (result != null)
{
return result;
}
}
}
}

Type clrType = readContext.Model.GetClrType(edmType);
return EnumDeserializationHelpers.ConvertEnumValue(item, clrType);
}

/// <summary>
/// Reads the value of a flags enum.
/// </summary>
/// <param name="enumValue">The OData enum value.</param>
/// <param name="enumType">The EDM enum type.</param>
/// <param name="clrType">The EDM enum CLR type.</param>
/// <param name="memberMapAnnotation">The annotation containing the mapping of CLR enum members to EDM enum members.</param>
/// <returns>The deserialized flags enum value.</returns>
private static object ReadFlagsEnumValue(ODataEnumValue enumValue, IEdmEnumType enumType, Type clrType, ClrEnumMemberAnnotation memberMapAnnotation)
{
long result = 0;
Type clrEnumType = TypeHelper.GetUnderlyingTypeOrSelf(clrType);

// use `stackalloc` to allocate a Span<Range> on the stack, which avoids heap allocations.
Span<Range> ranges = stackalloc Range[enumValue.Value.Count(c => c == ',') + 1];
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
ReadOnlySpan<char> source = enumValue.Value.AsSpan();

// For flags enum, we need to split the value and convert it to the enum value.
int count = source.Split(ranges, ',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
WanjohiSammy marked this conversation as resolved.
Show resolved Hide resolved
for(int i = 0; i < count; i++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could consider having a single loop that also handle the split of the strings, this also avoids the need to allocate the ranges array:

ReadOnlySpan<char> source = enumValue.AsSpan();
int currentRangeStart = 0;
for (int i = 0; i , source.Length; i++)
{
    if (source[i] != ',')
    {
       continue;
    }
    ReadOnlySpan<char> rawValue = source[currentRangeStart..i];
    ReadOnlySpan<char> value = rawValue.Trim();
    currentRangeStart = i;
   // handle value
}

// handle the last value
ReadOnlySpan<char> rawValue = source[currentRangeStart..];
ReadOnlySpan<char> value = rawValue.Trim();

PS: I didn't test it, verify for correctness. But the idea is we can get the ranges and the value in single scan. The Trim() is probably unnecessary if we check for spaces as well, but didn't want to complicate it too match especially since the Trim() is non-allocating.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer to: #1367 (comment)

{
ReadOnlySpan<char> value = source[ranges[i]];
IEdmEnumMember enumMember = null;
foreach (IEdmEnumMember member in enumType.Members)
{
if (value.Equals(member.Name, StringComparison.InvariantCultureIgnoreCase))
{
enumMember = member;
break;
}
}

if (enumMember != null)
{
Enum clrEnumMember = memberMapAnnotation.GetClrEnumMember(enumMember);
result |= Convert.ToInt64(clrEnumMember);
}
else if (Enum.TryParse(clrEnumType, value, true, out var enumMemberParsed))
{
result |= Convert.ToInt64((Enum)enumMemberParsed);
}
else
{
return null;
}
}

return result == 0 ? null : Enum.ToObject(clrEnumType, result);
}

}
44 changes: 43 additions & 1 deletion src/Microsoft.AspNetCore.OData/Formatter/Serialization/ODataEnumSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
//------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Diagnostics.Contracts;
using System.Threading.Tasks;
using Microsoft.AspNetCore.OData.Common;
using Microsoft.AspNetCore.OData.Edm;
using Microsoft.AspNetCore.OData.Formatter.Value;
using Microsoft.OData;
using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;

namespace Microsoft.AspNetCore.OData.Formatter.Serialization;

Expand Down Expand Up @@ -102,11 +104,18 @@ public virtual ODataEnumValue CreateODataEnumValue(object graph, IEdmEnumTypeRef
var memberMapAnnotation = writeContext?.Model.GetClrEnumMemberAnnotation(enumType.EnumDefinition());
if (memberMapAnnotation != null)
{
var edmEnumMember = memberMapAnnotation.GetEdmEnumMember((Enum)graph);
Enum graphEnum = (Enum)graph;

var edmEnumMember = memberMapAnnotation.GetEdmEnumMember(graphEnum);
if (edmEnumMember != null)
{
value = edmEnumMember.Name;
}
// If the enum is a flags enum, we need to handle the case where multiple flags are set
else if (enumType.EnumDefinition().IsFlags)
{
value = GetFlagsEnumValue(graphEnum, memberMapAnnotation);
}
}

ODataEnumValue enumValue = new ODataEnumValue(value, enumType.FullName());
Expand Down Expand Up @@ -171,4 +180,37 @@ private static bool ShouldSuppressTypeNameSerialization(ODataMetadataLevel metad
return false;
}
}

/// <summary>
/// Gets the combined names of the flags set in a Flags enum value.
/// </summary>
/// <param name="graphEnum">The enum value.</param>
/// <param name="memberMapAnnotation">The annotation containing the mapping of CLR enum members to EDM enum members.</param>
/// <returns>A comma-separated string of the names of the flags that are set.</returns>
private string GetFlagsEnumValue(Enum graphEnum, ClrEnumMemberAnnotation memberMapAnnotation)
{
List<string> flagsList = new List<string>();

// Convert the enum value to a long for bitwise operations
long graphValue = Convert.ToInt64(graphEnum);

// Iterate through all enum values
foreach (Enum flag in Enum.GetValues(graphEnum.GetType()))
{
// Convert the current flag to a long
long flagValue = Convert.ToInt64(flag);

// Using bitwise operations to check if a flag is set, which is more efficient than Enum.HasFlag
if ((graphValue & flagValue) != 0 && flagValue != 0)
{
IEdmEnumMember flagMember = memberMapAnnotation.GetEdmEnumMember(flag);
if (flagMember != null)
{
flagsList.Add(flagMember.Name);
}
}
}

return string.Join(", ", flagsList);
}
}
18 changes: 18 additions & 0 deletions src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3522,6 +3522,16 @@
<member name="M:Microsoft.AspNetCore.OData.Formatter.Deserialization.ODataEnumDeserializer.ReadInline(System.Object,Microsoft.OData.Edm.IEdmTypeReference,Microsoft.AspNetCore.OData.Formatter.Deserialization.ODataDeserializerContext)">
<inheritdoc />
</member>
<member name="M:Microsoft.AspNetCore.OData.Formatter.Deserialization.ODataEnumDeserializer.ReadFlagsEnumValue(Microsoft.OData.ODataEnumValue,Microsoft.OData.Edm.IEdmEnumType,System.Type,Microsoft.OData.ModelBuilder.ClrEnumMemberAnnotation)">
<summary>
Reads the value of a flags enum.
</summary>
<param name="enumValue">The OData enum value.</param>
<param name="enumType">The EDM enum type.</param>
<param name="clrType">The EDM enum CLR type.</param>
<param name="memberMapAnnotation">The annotation containing the mapping of CLR enum members to EDM enum members.</param>
<returns>The deserialized flags enum value.</returns>
</member>
<member name="T:Microsoft.AspNetCore.OData.Formatter.Deserialization.ODataPrimitiveDeserializer">
<summary>
Represents an <see cref="T:Microsoft.AspNetCore.OData.Formatter.Deserialization.ODataDeserializer"/> that can read OData primitive types.
Expand Down Expand Up @@ -4651,6 +4661,14 @@
<param name="writeContext">The serializer write context.</param>
<returns>The created <see cref="T:Microsoft.OData.ODataEnumValue"/>.</returns>
</member>
<member name="M:Microsoft.AspNetCore.OData.Formatter.Serialization.ODataEnumSerializer.GetFlagsEnumValue(System.Enum,Microsoft.OData.ModelBuilder.ClrEnumMemberAnnotation)">
<summary>
Gets the combined names of the flags set in a Flags enum value.
</summary>
<param name="graphEnum">The enum value.</param>
<param name="memberMapAnnotation">The annotation containing the mapping of CLR enum members to EDM enum members.</param>
<returns>A comma-separated string of the names of the flags that are set.</returns>
</member>
<member name="T:Microsoft.AspNetCore.OData.Formatter.Serialization.ODataErrorSerializer">
<summary>
Represents an <see cref="T:Microsoft.AspNetCore.OData.Formatter.Serialization.ODataSerializer"/> to serialize <see cref="T:Microsoft.OData.ODataError"/>s.
Expand Down
10 changes: 10 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Enums/EnumsController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ private void InitEmployees()
SkillSet=new List<Skill> { Skill.CSharp, Skill.Sql },
Gender=Gender.Female,
AccessLevel=AccessLevel.Execute,
EmployeeType = EmployeeType.FullTime | EmployeeType.PartTime,
FavoriteSports=new FavoriteSports()
{
LikeMost=Sport.Pingpong,
Expand All @@ -58,6 +59,7 @@ private void InitEmployees()
SkillSet=new List<Skill>(),
Gender=Gender.Female,
AccessLevel=AccessLevel.Read,
EmployeeType = EmployeeType.Contract,
FavoriteSports=new FavoriteSports()
{
LikeMost=Sport.Pingpong,
Expand All @@ -70,6 +72,7 @@ private void InitEmployees()
SkillSet=new List<Skill> { Skill.Web, Skill.Sql },
Gender=Gender.Female,
AccessLevel=AccessLevel.Read|AccessLevel.Write,
EmployeeType = EmployeeType.Intern | EmployeeType.FullTime | EmployeeType.PartTime,
FavoriteSports=new FavoriteSports()
{
LikeMost=Sport.Pingpong|Sport.Basketball,
Expand Down Expand Up @@ -121,6 +124,13 @@ public IActionResult GetFavoriteSportsFromEmployee(int key)
return Ok(employee.FavoriteSports);
}

[EnableQuery]
public IActionResult GetEmployeeTypeFromEmployee(int key)
{
var employee = Employees.SingleOrDefault(e => e.ID == key);
return Ok(employee.EmployeeType);
}

[HttpGet("Employees({key})/FavoriteSports/LikeMost")]
public IActionResult GetFavoriteSportLikeMost(int key)
{
Expand Down
19 changes: 19 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Enums/EnumsDataModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class Employee

public AccessLevel AccessLevel { get; set; }

public EmployeeType EmployeeType { get; set; }

public FavoriteSports FavoriteSports { get; set; }
}

Expand All @@ -36,6 +38,23 @@ public enum AccessLevel
Execute = 4
}

[Flags]
[DataContract(Name = "employeeType")]
public enum EmployeeType
{
[EnumMember(Value = "fulltime")]
FullTime = 1,

[EnumMember(Value = "parttime")]
PartTime = 2,

[EnumMember(Value = "contract")]
Contract = 4,

[EnumMember(Value = "intern")]
Intern = 8
}

public enum Gender
{
Male = 1,
Expand Down
7 changes: 7 additions & 0 deletions test/Microsoft.AspNetCore.OData.E2E.Tests/Enums/EnumsEdmModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public static IEdmModel GetExplicitModel()
employee.CollectionProperty<Skill>(c => c.SkillSet);
employee.EnumProperty<Gender>(c => c.Gender);
employee.EnumProperty<AccessLevel>(c => c.AccessLevel);
employee.EnumProperty<EmployeeType>(c => c.EmployeeType);
employee.ComplexProperty<FavoriteSports>(c => c.FavoriteSports);

var skill = builder.EnumType<Skill>();
Expand All @@ -37,6 +38,12 @@ public static IEdmModel GetExplicitModel()
accessLevel.Member(AccessLevel.Read);
accessLevel.Member(AccessLevel.Write);

var employeeType = builder.EnumType<EmployeeType>();
employeeType.Member(EmployeeType.FullTime);
employeeType.Member(EmployeeType.PartTime);
employeeType.Member(EmployeeType.Contract);
employeeType.Member(EmployeeType.Intern);

var favoriteSports = builder.ComplexType<FavoriteSports>();
favoriteSports.EnumProperty<Sport>(f => f.LikeMost);
favoriteSports.CollectionProperty<Sport>(f => f.Like);
Expand Down
Loading