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

* #277 change contract of IPropertyMapper.MapProperty, so it can now return null if the field should not be serialized at all (as in, ignored) * #277 update implementation of both default MapProperty methods (Newtonsoft.Json and System.Text), so they return null if the field has the JsonIgnore attribute * #277 update unit tests to reflect that MapProperty now returns null if the field has the JsonIgnore attribute * #277 add unit tests for Newtonsoft Json version of MapProperty
OData · Dec 9, 2022 · fbddff0 · fbddff0
1 parent 4ad8262
commit fbddff0
Show file tree

Hide file tree

Showing 12 changed files with 173 additions and 31 deletions.
diff --git a/src/Microsoft.AspNetCore.OData.NewtonsoftJson/JsonPropertyNameMapper.cs b/src/Microsoft.AspNetCore.OData.NewtonsoftJson/JsonPropertyNameMapper.cs
@@ -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);
+            if (jsonIgnore != null)
+            {
+                return null;
+            }
+
             JsonPropertyAttribute jsonProperty = GetJsonProperty(info);
             if (jsonProperty != null && !string.IsNullOrWhiteSpace(jsonProperty.PropertyName))
             {
@@ -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();
+        }
     }
 }
diff --git a/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml b/src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
@@ -6830,7 +6830,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">
@@ -7805,6 +7805,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)">

diff --git a/src/Microsoft.AspNetCore.OData/Properties/SRResources.Designer.cs b/src/Microsoft.AspNetCore.OData/Properties/SRResources.Designer.cs
diff --git a/src/Microsoft.AspNetCore.OData/Properties/SRResources.resx b/src/Microsoft.AspNetCore.OData/Properties/SRResources.resx
@@ -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>

diff --git a/src/Microsoft.AspNetCore.OData/Query/Container/IPropertyMapper.cs b/src/Microsoft.AspNetCore.OData/Query/Container/IPropertyMapper.cs
@@ -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
     {

diff --git a/src/Microsoft.AspNetCore.OData/Query/Container/JsonPropertyNameMapper.cs b/src/Microsoft.AspNetCore.OData/Query/Container/JsonPropertyNameMapper.cs
@@ -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))
             {
@@ -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();
+        }
     }
 }
diff --git a/src/Microsoft.AspNetCore.OData/Query/Container/NamedPropertyOfT.cs b/src/Microsoft.AspNetCore.OData/Query/Container/NamedPropertyOfT.cs
@@ -27,12 +27,15 @@ internal class NamedProperty<T> : PropertyContainer
             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());
+                }
             }
         }
 

diff --git a/src/Microsoft.AspNetCore.OData/Query/Wrapper/SelectExpandWrapper.cs b/src/Microsoft.AspNetCore.OData/Query/Wrapper/SelectExpandWrapper.cs
@@ -144,12 +144,15 @@ public bool TryGetPropertyValue(string propertyName, out object value)
                     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;
+                        }
                     }
                 }
             }

diff --git a/test/Microsoft.AspNetCore.OData.NewtonsoftJson.Tests/JsonPropertyNameMapperTest.cs b/test/Microsoft.AspNetCore.OData.NewtonsoftJson.Tests/JsonPropertyNameMapperTest.cs
@@ -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; }
+        }
+    }
+}
diff --git a/test/Microsoft.AspNetCore.OData.Tests/Query/Container/JsonPropertyNameMapperTest.cs b/test/Microsoft.AspNetCore.OData.Tests/Query/Container/JsonPropertyNameMapperTest.cs
@@ -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()
@@ -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);
         }
 
@@ -51,6 +58,9 @@ private class JAddress
 
             [JsonPropertyName("Road")]
             public string Street { get; set; }
+
+            [JsonIgnore]
+            public string IgnoreThis { get; set; }
         }
     }
 }
diff --git a/test/Microsoft.AspNetCore.OData.Tests/Query/Container/PropertyContainerTest.cs b/test/Microsoft.AspNetCore.OData.Tests/Query/Container/PropertyContainerTest.cs
@@ -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);
@@ -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[]
@@ -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)

diff --git a/test/Microsoft.AspNetCore.OData.Tests/Query/Wrapper/SelectExpandWrapperTest.cs b/test/Microsoft.AspNetCore.OData.Tests/Query/Wrapper/SelectExpandWrapperTest.cs
@@ -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");
@@ -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