Skip to content

fixes issue 727 [JsonIgnore] attribute ignored when using $expand #728

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

Merged
xuzhg merged 4 commits into from
Dec 9, 2022
Merged

fixes issue 727 [JsonIgnore] attribute ignored when using $expand #728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85dce73
OData#277 change contract of IPropertyMapper.MapProperty, so it can n…
mattperdeck Oct 27, 2022
e481812
OData#277 update implementation of both default MapProperty methods (…
mattperdeck Oct 27, 2022
d0a0296
OData#277 update unit tests to reflect that MapProperty now returns n…
mattperdeck Oct 27, 2022
0d9356b
OData#277 add unit tests for Newtonsoft Json version of MapProperty
mattperdeck Oct 27, 2022
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
13 changes: 13 additions & 0 deletions src/Microsoft.AspNetCore.OData.NewtonsoftJson/JsonPropertyNameMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ public string MapProperty(string propertyName)

IEdmProperty property = _type.Properties().Single(s => s.Name == propertyName);
PropertyInfo info = GetPropertyInfo(property);

JsonIgnoreAttribute jsonIgnore = GetJsonIgnore(info);

@corranrogue9 corranrogue9 Nov 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this really the direction we want to go? Are we planning to support all System.Text.Json attributes?

@corranrogue9 corranrogue9 Nov 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@xuzhg @mikepizzo

@xuzhg xuzhg Dec 9, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is newtonsoft related package.
I think we should support most of the newtonsoft property related attributes.

if (jsonIgnore != null)
{
return null;
}

JsonPropertyAttribute jsonProperty = GetJsonProperty(info);
if (jsonProperty != null && !string.IsNullOrWhiteSpace(jsonProperty.PropertyName))
{
Expand Down Expand Up @@ -82,5 +89,11 @@ private static JsonPropertyAttribute GetJsonProperty(PropertyInfo property)
return property.GetCustomAttributes(typeof(JsonPropertyAttribute), inherit: false)
.OfType<JsonPropertyAttribute>().SingleOrDefault();
}

private static JsonIgnoreAttribute GetJsonIgnore(PropertyInfo property)
{
return property.GetCustomAttributes(typeof(JsonIgnoreAttribute), inherit: false)
.OfType<JsonIgnoreAttribute>().SingleOrDefault();
}
}
}
5 changes: 4 additions & 1 deletion 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 @@ -6822,7 +6822,7 @@
</member>
<member name="P:Microsoft.AspNetCore.OData.SRResources.InvalidPropertyMapping">
<summary>
Looks up a localized string similar to The key mapping for the property &apos;{0}&apos; can&apos;t be null or empty..
Looks up a localized string similar to The key mapping for the property &apos;{0}&apos; can&apos;t be empty..
</summary>
</member>
<member name="P:Microsoft.AspNetCore.OData.SRResources.InvalidSegmentInSelectExpandPath">
Expand Down Expand Up @@ -7792,6 +7792,9 @@
properties in the <see cref="T:Microsoft.OData.Edm.IEdmStructuredType"/> that will be used during the serialization of the $select
and $expand projection by a given formatter. For example, to support custom serialization attributes of a
particular formatter.

It also allows you to ignore a field (ensure that the returned <see cref="T:System.Collections.Generic.IDictionary`2"/>
does not have a key for that field), by mapping the property name to null.
</summary>
</member>
<member name="M:Microsoft.AspNetCore.OData.Query.Container.IPropertyMapper.MapProperty(System.String)">
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.OData/Properties/SRResources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.OData/Properties/SRResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@
<value>A binary operator with incompatible types was detected. Found operand types '{0}' and '{1}' for operator kind '{2}'.</value>
</data>
<data name="InvalidPropertyMapping" xml:space="preserve">
<value>The key mapping for the property '{0}' can't be null or empty.</value>
<value>The key mapping for the property '{0}' can't be empty.</value>
</data>
<data name="ODataFunctionNotSupported" xml:space="preserve">
<value>Unknown function '{0}'.</value>
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.AspNetCore.OData/Query/Container/IPropertyMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ namespace Microsoft.AspNetCore.OData.Query.Container
/// properties in the <see cref="IEdmStructuredType"/> that will be used during the serialization of the $select
/// and $expand projection by a given formatter. For example, to support custom serialization attributes of a
/// particular formatter.
///
/// It also allows you to ignore a field (ensure that the returned <see cref="IDictionary{TKey,TValue}"/>
/// does not have a key for that field), by mapping the property name to null.
/// </summary>
public interface IPropertyMapper
{
Expand Down
13 changes: 13 additions & 0 deletions src/Microsoft.AspNetCore.OData/Query/Container/JsonPropertyNameMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ public string MapProperty(string propertyName)
{
IEdmProperty property = _type.Properties().Single(s => s.Name == propertyName);
PropertyInfo info = GetPropertyInfo(property);

JsonIgnoreAttribute jsonIgnore = GetJsonIgnore(info);
if (jsonIgnore != null)
{
return null;
}

JsonPropertyNameAttribute jsonProperty = GetJsonProperty(info);
if (jsonProperty != null && !String.IsNullOrWhiteSpace(jsonProperty.Name))
{
Expand Down Expand Up @@ -63,5 +70,11 @@ private static JsonPropertyNameAttribute GetJsonProperty(PropertyInfo property)
return property.GetCustomAttributes(typeof(JsonPropertyNameAttribute), inherit: false)
.OfType<JsonPropertyNameAttribute>().SingleOrDefault();
}

private static JsonIgnoreAttribute GetJsonIgnore(PropertyInfo property)
{
return property.GetCustomAttributes(typeof(JsonIgnoreAttribute), inherit: false)
.OfType<JsonIgnoreAttribute>().SingleOrDefault();
}
}
}
11 changes: 7 additions & 4 deletions src/Microsoft.AspNetCore.OData/Query/Container/NamedPropertyOfT.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ public override void ToDictionaryCore(Dictionary<string, object> dictionary, IPr
if (Name != null && (includeAutoSelected || !AutoSelected))
{
string mappedName = propertyMapper.MapProperty(Name);
if (String.IsNullOrEmpty(mappedName))
if (mappedName != null)
{
throw Error.InvalidOperation(SRResources.InvalidPropertyMapping, Name);
}
if (String.IsNullOrEmpty(mappedName))
{
throw Error.InvalidOperation(SRResources.InvalidPropertyMapping, Name);
}

dictionary.Add(mappedName, GetValue());
dictionary.Add(mappedName, GetValue());
}
}
}

Expand Down
11 changes: 7 additions & 4 deletions src/Microsoft.AspNetCore.OData/Query/Wrapper/SelectExpandWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,15 @@ public IDictionary<string, object> ToDictionary(Func<IEdmModel, IEdmStructuredTy
if (TryGetPropertyValue(property.Name, out propertyValue))
{
string mappingName = mapper.MapProperty(property.Name);
if (String.IsNullOrWhiteSpace(mappingName))
if (mappingName != null)
{
throw Error.InvalidOperation(SRResources.InvalidPropertyMapping, property.Name);
}
if (String.IsNullOrWhiteSpace(mappingName))
{
throw Error.InvalidOperation(SRResources.InvalidPropertyMapping, property.Name);
}

dictionary[mappingName] = propertyValue;
dictionary[mappingName] = propertyValue;
}
}
}
}
Expand Down
66 changes: 66 additions & 0 deletions test/Microsoft.AspNetCore.OData.NewtonsoftJson.Tests/JsonPropertyNameMapperTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//-----------------------------------------------------------------------------
// <copyright file="JsonPropertyNameMapperTest.cs" company=".NET Foundation">
// Copyright (c) .NET Foundation and Contributors. All rights reserved.
// See License.txt in the project root for license information.
// </copyright>
//------------------------------------------------------------------------------

using Newtonsoft.Json;
using Microsoft.AspNetCore.OData.NewtonsoftJson;
using Microsoft.OData.Edm;
using Microsoft.OData.ModelBuilder;
using Xunit;

namespace Microsoft.AspNetCore.OData.Tests.Query.Container
{
public class JsonPropertyNameMapperTests
{
[Fact]
public void MapProperty_Maps_PropertyName()
{
// Arrange
(IEdmModel model, IEdmStructuredType address) = GetOData();
JsonPropertyNameMapper mapper = new JsonPropertyNameMapper(model, address);

// Act & Assert
Assert.Equal("Road", mapper.MapProperty("Street"));

// Act & Assert
Assert.Equal("City", mapper.MapProperty("City"));

// Act & Assert
Assert.Null(mapper.MapProperty("IgnoreThis"));
}

private static (IEdmModel, IEdmStructuredType) GetOData()
{
EdmModel model = new EdmModel();
EdmComplexType address = new EdmComplexType("NS", "Address");
address.AddStructuralProperty("City", EdmPrimitiveTypeKind.String);
address.AddStructuralProperty("Street", EdmPrimitiveTypeKind.String);
address.AddStructuralProperty("IgnoreThis", EdmPrimitiveTypeKind.String);
model.AddElement(address);

model.SetAnnotationValue(address, new ClrTypeAnnotation(typeof(JAddress)));

model.SetAnnotationValue(address.FindProperty("Street"),
new ClrPropertyInfoAnnotation(typeof(JAddress).GetProperty("Street")));

model.SetAnnotationValue(address.FindProperty("IgnoreThis"),
new ClrPropertyInfoAnnotation(typeof(JAddress).GetProperty("IgnoreThis")));

return (model, address);
}

private class JAddress
{
public string City { get; set; }

[JsonProperty("Road")]
public string Street { get; set; }

[JsonIgnore]
public string IgnoreThis { get; set; }
}
}
}
10 changes: 10 additions & 0 deletions test/Microsoft.AspNetCore.OData.Tests/Query/Container/JsonPropertyNameMapperTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public void MapProperty_Maps_PropertyName()

// Act & Assert
Assert.Equal("City", mapper.MapProperty("City"));

// Act & Assert
Assert.Null(mapper.MapProperty("IgnoreThis"));
}

private static (IEdmModel, IEdmStructuredType) GetOData()
Expand All @@ -35,13 +38,17 @@ private static (IEdmModel, IEdmStructuredType) GetOData()
EdmComplexType address = new EdmComplexType("NS", "Address");
address.AddStructuralProperty("City", EdmPrimitiveTypeKind.String);
address.AddStructuralProperty("Street", EdmPrimitiveTypeKind.String);
address.AddStructuralProperty("IgnoreThis", EdmPrimitiveTypeKind.String);
model.AddElement(address);

model.SetAnnotationValue(address, new ClrTypeAnnotation(typeof(JAddress)));

model.SetAnnotationValue(address.FindProperty("Street"),
new ClrPropertyInfoAnnotation(typeof(JAddress).GetProperty("Street")));

model.SetAnnotationValue(address.FindProperty("IgnoreThis"),
new ClrPropertyInfoAnnotation(typeof(JAddress).GetProperty("IgnoreThis")));

return (model, address);
}

Expand All @@ -51,6 +58,9 @@ private class JAddress

[JsonPropertyName("Road")]
public string Street { get; set; }

[JsonIgnore]
public string IgnoreThis { get; set; }
}
}
}
10 changes: 6 additions & 4 deletions test/Microsoft.AspNetCore.OData.Tests/Query/Container/PropertyContainerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,16 @@ public void ToDictionary_AppliesMappingToAllProperties()
IList<NamedPropertyExpression> properties = new NamedPropertyExpression[]
{
new NamedPropertyExpression(name: Expression.Constant("PropA"), value: Expression.Constant(3)),
new NamedPropertyExpression(name: Expression.Constant("PropB"), value: Expression.Constant(6))
new NamedPropertyExpression(name: Expression.Constant("PropB"), value: Expression.Constant(6)),
new NamedPropertyExpression(name: Expression.Constant("PropC"), value: Expression.Constant(9))
};
Expression containerExpression = PropertyContainer.CreatePropertyContainer(properties);
PropertyContainer container = ToContainer(containerExpression);

Mock<IPropertyMapper> mapperMock = new Mock<IPropertyMapper>();
mapperMock.Setup(m => m.MapProperty("PropA")).Returns("PropertyA");
mapperMock.Setup(m => m.MapProperty("PropB")).Returns("PropB");
mapperMock.Setup(m => m.MapProperty("PropC")).Returns((string)null);

//Act
IDictionary<string, object> result = container.ToDictionary(mapperMock.Object);
Expand All @@ -219,12 +221,12 @@ public void ToDictionary_AppliesMappingToAllProperties()
Assert.NotNull(result);
Assert.True(result.ContainsKey("PropertyA"));
Assert.True(result.ContainsKey("PropB"));
Assert.False(result.ContainsKey("PropC"));
}

[Theory]
[InlineData(null)]
[InlineData("")]
public void ToDictionary_Throws_IfMappingFunctionReturns_NullOrEmpty(string mappedName)
public void ToDictionary_Throws_IfMappingFunctionReturns_Empty(string mappedName)
{
// Arrange
IList<NamedPropertyExpression> properties = new NamedPropertyExpression[]
Expand All @@ -239,7 +241,7 @@ public void ToDictionary_Throws_IfMappingFunctionReturns_NullOrEmpty(string mapp

// Act & Assert
ExceptionAssert.Throws<InvalidOperationException>(() =>
container.ToDictionary(mapperMock.Object), "The key mapping for the property 'PropA' can't be null or empty.");
container.ToDictionary(mapperMock.Object), "The key mapping for the property 'PropA' can't be empty.");
}

private static PropertyContainer ToContainer(Expression containerCreationExpression)
Expand Down
58 changes: 42 additions & 16 deletions test/Microsoft.AspNetCore.OData.Tests/Query/Wrapper/SelectExpandWrapperTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,8 @@ public void ToDictionary_Throws_IfMapperProvider_ReturnsNullPropertyMapper()
}

[Theory]
[InlineData(null)]
[InlineData("")]
public void ToDictionary_Throws_IfMappingIsNullOrEmpty_ForAGivenProperty(string propertyMapping)
public void ToDictionary_Throws_IfMappingIsEmpty_ForAGivenProperty(string propertyMapping)
{
// Arrange
EdmEntityType entityType = new EdmEntityType("NS", "Name");
Expand All @@ -282,38 +281,65 @@ public void ToDictionary_Throws_IfMappingIsNullOrEmpty_ForAGivenProperty(string
// Act & Assert
ExceptionAssert.Throws<InvalidOperationException>(() =>
testWrapper.ToDictionary(mapperProvider),
"The key mapping for the property 'SampleProperty' can't be null or empty.");
"The key mapping for the property 'SampleProperty' can't be empty.");
}

[Fact]
public void ToDictionary_AppliesMappingToAllProperties_IfInstanceIsNotNull()
public void ToDictionary_AppliesMappingToAllProperties_IfInstanceIsNotNull_IfPropertyIsRenamed()
{
// Arrange
var testWrapper = GetSamplePropertyTestWrapper();
var mapperProvider = GetMapperProvider("SampleProperty", "Sample");

// Act
var result = testWrapper.ToDictionary(mapperProvider);

// Assert
Assert.Equal(42, result["Sample"]);
}

[Fact]
public void ToDictionary_AppliesMappingToAllProperties_IfInstanceIsNotNull_IfPropertyIsIgnored()
{
// Arrange
var testWrapper = GetSamplePropertyTestWrapper();
var mapperProvider = GetMapperProvider("SampleProperty", null);

// Act
var result = testWrapper.ToDictionary(mapperProvider);

// Assert
Assert.False(result.ContainsKey("SampleProperty"));
}

private Func<IEdmModel, IEdmStructuredType, IPropertyMapper> GetMapperProvider(string mapFrom, string mapTo)
{
Mock<IPropertyMapper> mapperMock = new Mock<IPropertyMapper>();
mapperMock.Setup(m => m.MapProperty(mapFrom)).Returns(mapTo);
Func<IEdmModel, IEdmStructuredType, IPropertyMapper> mapperProvider =
(IEdmModel m, IEdmStructuredType t) => mapperMock.Object;

return mapperProvider;
}

private SelectExpandWrapper<TestEntity> GetSamplePropertyTestWrapper()
{
EdmEntityType entityType = new EdmEntityType("NS", "Name");
entityType.AddStructuralProperty("SampleProperty", EdmPrimitiveTypeKind.Int32);

EdmModel model = new EdmModel();
model.AddElement(entityType);
model.SetAnnotationValue(entityType, new ClrTypeAnnotation(typeof(TestEntity)));
IEdmTypeReference edmType = new EdmEntityTypeReference(entityType, isNullable: false);

SelectExpandWrapper<TestEntity> testWrapper = new SelectExpandWrapper<TestEntity>
{
Instance = new TestEntity { SampleProperty = 42 },
Model = model,
UseInstanceForProperties = true,
};

Mock<IPropertyMapper> mapperMock = new Mock<IPropertyMapper>();
mapperMock.Setup(m => m.MapProperty("SampleProperty")).Returns("Sample");
Func<IEdmModel, IEdmStructuredType, IPropertyMapper> mapperProvider =
(IEdmModel m, IEdmStructuredType t) => mapperMock.Object;

// Act
var result = testWrapper.ToDictionary(mapperProvider);

// Assert
Assert.Equal(42, result["Sample"]);
return testWrapper;
}

private class TestEntity
Expand Down