Attr property info (#662)

Bart Koelman · maurei · commit 2b64e03b6e7f · 2020-01-09T14:47:40.000+01:00
* Removed AttrAttribute.InternalAttributeName

* Removed unused IRelationshipField
diff --git a/benchmarks/Query/QueryParser_Benchmarks.cs b/benchmarks/Query/QueryParser_Benchmarks.cs
@@ -25,7 +25,7 @@ public QueryParser_Benchmarks() {
             var requestMock = new Mock<IRequestContext>();
             requestMock.Setup(m => m.GetRequestResource()).Returns(new ResourceContext {
                 Attributes = new List<AttrAttribute> {
-                    new AttrAttribute(ATTRIBUTE, ATTRIBUTE)
+                    new AttrAttribute(ATTRIBUTE)
                 }
             });
             var options = new JsonApiOptions();
diff --git a/src/JsonApiDotNetCore/Builders/ResourceGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ResourceGraphBuilder.cs
@@ -99,8 +99,7 @@ protected virtual List<AttrAttribute> GetAttributes(Type entityType)
                     var idAttr = new AttrAttribute()
                     {
                         PublicAttributeName = _formatter.FormatPropertyName(prop),
-                        PropertyInfo = prop,
-                        InternalAttributeName = prop.Name
+                        PropertyInfo = prop
                     };
                     attributes.Add(idAttr);
                     continue;
@@ -111,7 +110,6 @@ protected virtual List<AttrAttribute> GetAttributes(Type entityType)
                     continue;
 
                 attribute.PublicAttributeName = attribute.PublicAttributeName ?? _formatter.FormatPropertyName(prop);
-                attribute.InternalAttributeName = prop.Name;
                 attribute.PropertyInfo = prop;
 
                 attributes.Add(attribute);
diff --git a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs
@@ -65,7 +65,7 @@ public static IQueryable<TSource> Filter<TSource>(this IQueryable<TSource> sourc
         }
 
         public static IQueryable<TSource> Select<TSource>(this IQueryable<TSource> source, IEnumerable<AttrAttribute> columns)
-            => CallGenericSelectMethod(source, columns.Select(attr => attr.InternalAttributeName).ToList());
+            => CallGenericSelectMethod(source, columns.Select(attr => attr.PropertyInfo.Name).ToList());
 
         public static IOrderedQueryable<TSource> Sort<TSource>(this IQueryable<TSource> source, SortQueryContext sortQuery)
         {
@@ -191,7 +191,7 @@ private static Expression GetFilterExpressionLambda(Expression left, Expression
         private static IQueryable<TSource> CallGenericWhereContainsMethod<TSource>(IQueryable<TSource> source, FilterQueryContext filter)
         {
             var concreteType = typeof(TSource);
-            var property = concreteType.GetProperty(filter.Attribute.InternalAttributeName);
+            var property = concreteType.GetProperty(filter.Attribute.PropertyInfo.Name);
 
             try
             {
@@ -201,10 +201,10 @@ private static IQueryable<TSource> CallGenericWhereContainsMethod<TSource>(IQuer
                 if (filter.IsAttributeOfRelationship)
                 {
                     var relation = Expression.PropertyOrField(entity, filter.Relationship.InternalRelationshipName);
-                    member = Expression.Property(relation, filter.Attribute.InternalAttributeName);
+                    member = Expression.Property(relation, filter.Attribute.PropertyInfo.Name);
                 }
                 else
-                    member = Expression.Property(entity, filter.Attribute.InternalAttributeName);
+                    member = Expression.Property(entity, filter.Attribute.PropertyInfo.Name);
 
                 var method = ContainsMethod.MakeGenericMethod(member.Type);
                 var obj = TypeHelper.ConvertListType(propertyValues, member.Type);
@@ -259,9 +259,9 @@ private static IQueryable<TSource> CallGenericWhereMethod<TSource>(IQueryable<TS
                     throw new ArgumentException($"'{filter.Relationship.InternalRelationshipName}' is not a valid relationship of '{concreteType}'");
 
                 var relatedType = filter.Relationship.RightType;
-                property = relatedType.GetProperty(filter.Attribute.InternalAttributeName);
+                property = relatedType.GetProperty(filter.Attribute.PropertyInfo.Name);
                 if (property == null)
-                    throw new ArgumentException($"'{filter.Attribute.InternalAttributeName}' is not a valid attribute of '{filter.Relationship.InternalRelationshipName}'");
+                    throw new ArgumentException($"'{filter.Attribute.PropertyInfo.Name}' is not a valid attribute of '{filter.Relationship.InternalRelationshipName}'");
 
                 var leftRelationship = Expression.PropertyOrField(parameter, filter.Relationship.InternalRelationshipName);
                 // {model.Relationship}
@@ -270,9 +270,9 @@ private static IQueryable<TSource> CallGenericWhereMethod<TSource>(IQueryable<TS
             // Is standalone attribute
             else
             {
-                property = concreteType.GetProperty(filter.Attribute.InternalAttributeName);
+                property = concreteType.GetProperty(filter.Attribute.PropertyInfo.Name);
                 if (property == null)
-                    throw new ArgumentException($"'{filter.Attribute.InternalAttributeName}' is not a valid property of '{concreteType}'");
+                    throw new ArgumentException($"'{filter.Attribute.PropertyInfo.Name}' is not a valid property of '{concreteType}'");
 
                 // {model.Id}
                 left = Expression.PropertyOrField(parameter, property.Name);
diff --git a/src/JsonApiDotNetCore/Internal/Query/BaseQueryContext.cs b/src/JsonApiDotNetCore/Internal/Query/BaseQueryContext.cs
@@ -23,9 +23,9 @@ protected BaseQueryContext(TQuery query)
         public string GetPropertyPath()
         {
             if (IsAttributeOfRelationship)
-                return string.Format("{0}.{1}", Relationship.InternalRelationshipName, Attribute.InternalAttributeName);
+                return string.Format("{0}.{1}", Relationship.InternalRelationshipName, Attribute.PropertyInfo.Name);
 
-            return Attribute.InternalAttributeName;
+            return Attribute.PropertyInfo.Name;
         }
     }
 }
diff --git a/src/JsonApiDotNetCore/Models/Annotation/AttrAttribute.cs b/src/JsonApiDotNetCore/Models/Annotation/AttrAttribute.cs
@@ -34,33 +34,12 @@ public AttrAttribute(string publicName = null, bool isImmutable = false, bool is
             IsSortable = isSortable;
         }
 
-        public string ExposedInternalMemberName => InternalAttributeName;
-
-
-        /// <summary>
-        /// Do not use this overload in your applications.
-        /// Provides a method for instantiating instances of `AttrAttribute` and specifying
-        /// the internal property name.
-        /// The primary intent for this was to enable certain types of unit tests to be possible.
-        /// This overload will be deprecated and removed in future releases and an alternative
-        /// for unit tests will be provided.
-        /// </summary>
-        public AttrAttribute(string publicName, string internalName, bool isImmutable = false)
-        {
-            PublicAttributeName = publicName;
-            InternalAttributeName = internalName;
-            IsImmutable = isImmutable;
-        }
+        public string ExposedInternalMemberName => PropertyInfo.Name;
 
         /// <summary>
         /// How this attribute is exposed through the API
         /// </summary>
-        public string PublicAttributeName { get; internal set;}
-
-        /// <summary>
-        /// The internal property name this attribute belongs to.
-        /// </summary>
-        public string InternalAttributeName { get; internal set; }
+        public string PublicAttributeName { get; internal set; }
 
         /// <summary>
         /// Prevents PATCH requests from updating the value.
@@ -84,7 +63,7 @@ public AttrAttribute(string publicName, string internalName, bool isImmutable =
         /// <summary>
         /// The member property info
         /// </summary>
-        internal PropertyInfo PropertyInfo { get; set; }
+        public PropertyInfo PropertyInfo { get; set; }
 
         /// <summary>
         /// Get the value of the attribute for the given object.
@@ -119,15 +98,15 @@ public void SetValue(object entity, object newValue)
         private PropertyInfo GetResourceProperty(object resource)
         {
             // There are some scenarios, especially ones where users are using a different
-            // data model than view model, where they may use a repository implmentation
+            // data model than view model, where they may use a repository implementation
             // that does not match the deserialized type. For now, we will continue to support
             // this use case.
             var targetType = resource.GetType();
             if (targetType != PropertyInfo.DeclaringType)
             {
                 var propertyInfo = resource
                     .GetType()
-                    .GetProperty(InternalAttributeName);
+                    .GetProperty(PropertyInfo.Name);
 
                 return propertyInfo;
 
diff --git a/src/JsonApiDotNetCore/Models/Annotation/IRelationshipField.cs b/src/JsonApiDotNetCore/Models/Annotation/IRelationshipField.cs
diff --git a/src/JsonApiDotNetCore/Models/Annotation/RelationshipAttribute.cs b/src/JsonApiDotNetCore/Models/Annotation/RelationshipAttribute.cs
@@ -5,7 +5,7 @@
 
 namespace JsonApiDotNetCore.Models
 {
-    public abstract class RelationshipAttribute : Attribute, IResourceField, IRelationshipField
+    public abstract class RelationshipAttribute : Attribute, IResourceField
     {
         protected RelationshipAttribute(string publicName, Link relationshipLinks, bool canInclude, string mappedBy)
         {
diff --git a/src/JsonApiDotNetCore/Serialization/Common/ResourceObjectBuilder.cs b/src/JsonApiDotNetCore/Serialization/Common/ResourceObjectBuilder.cs
@@ -1,4 +1,4 @@
-﻿using System;
+using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
@@ -31,7 +31,7 @@ public ResourceObject Build(IIdentifiable entity, IEnumerable<AttrAttribute> att
             var ro = new ResourceObject { Type = resourceContext.ResourceName, Id = entity.StringId.NullIfEmpty() };
 
             // populating the top-level "attribute" member of a resource object. never include "id" as an attribute
-            if (attributes != null && (attributes = attributes.Where(attr => attr.InternalAttributeName != _identifiablePropertyName)).Any())
+            if (attributes != null && (attributes = attributes.Where(attr => attr.PropertyInfo.Name != _identifiablePropertyName)).Any())
                 ProcessAttributes(entity, attributes, ro);
 
             // populating the top-level "relationship" member of a resource object.
@@ -145,4 +145,4 @@ private void ProcessAttributes(IIdentifiable entity, IEnumerable<AttrAttribute>
             }
         }
     }
-}
+}
diff --git a/test/IntegrationTests/Data/EntityRepositoryTests.cs b/test/IntegrationTests/Data/EntityRepositoryTests.cs
@@ -33,7 +33,7 @@ public async Task UpdateAsync_AttributesUpdated_ShouldHaveSpecificallyThoseAttri
                 arrangeContext.Add(todoItemUpdates);
                 arrangeContext.SaveChanges();
 
-                var descAttr = new AttrAttribute("description", "Description")
+                var descAttr = new AttrAttribute("description")
                 {
                     PropertyInfo = typeof(TodoItem).GetProperty(nameof(TodoItem.Description))
                 };
diff --git a/test/UnitTests/Models/ResourceDefinitionTests.cs b/test/UnitTests/Models/ResourceDefinitionTests.cs
@@ -29,7 +29,7 @@ public void Request_Filter_Uses_Member_Expression()
             var attrs = resource.GetAllowedAttributes();
 
             // Assert
-            Assert.DoesNotContain(attrs, a => a.InternalAttributeName == nameof(Model.AlwaysExcluded));
+            Assert.DoesNotContain(attrs, a => a.PropertyInfo.Name == nameof(Model.AlwaysExcluded));
         }
 
         [Fact]
@@ -42,8 +42,8 @@ public void Request_Filter_Uses_NewExpression()
             var attrs = resource.GetAllowedAttributes();
 
             // Assert
-            Assert.DoesNotContain(attrs, a => a.InternalAttributeName == nameof(Model.AlwaysExcluded));
-            Assert.DoesNotContain(attrs, a => a.InternalAttributeName == nameof(Model.Password));
+            Assert.DoesNotContain(attrs, a => a.PropertyInfo.Name == nameof(Model.AlwaysExcluded));
+            Assert.DoesNotContain(attrs, a => a.PropertyInfo.Name == nameof(Model.Password));
         }
     }
 
diff --git a/test/UnitTests/QueryParameters/SparseFieldsServiceTests.cs b/test/UnitTests/QueryParameters/SparseFieldsServiceTests.cs
@@ -34,11 +34,10 @@ public void Parse_ValidSelection_CanParse()
             // Arrange
             const string type = "articles";
             const string attrName = "someField";
-            const string internalAttrName = "SomeField";
-            var attribute = new AttrAttribute(attrName) { InternalAttributeName = internalAttrName };
-            var idAttribute = new AttrAttribute("id") { InternalAttributeName = "Id" };
+            var attribute = new AttrAttribute(attrName);
+            var idAttribute = new AttrAttribute("id");
 
-            var query = new KeyValuePair<string, StringValues>($"fields", new StringValues(attrName));
+            var query = new KeyValuePair<string, StringValues>("fields", new StringValues(attrName));
 
             var resourceContext = new ResourceContext
             {
@@ -64,9 +63,8 @@ public void Parse_TypeNameAsNavigation_Throws400ErrorWithRelationshipsOnlyMessag
             // Arrange
             const string type = "articles";
             const string attrName = "someField";
-            const string internalAttrName = "SomeField";
-            var attribute = new AttrAttribute(attrName) { InternalAttributeName = internalAttrName };
-            var idAttribute = new AttrAttribute("id") { InternalAttributeName = "Id" };
+            var attribute = new AttrAttribute(attrName);
+            var idAttribute = new AttrAttribute("id");
 
             var query = new KeyValuePair<string, StringValues>($"fields[{type}]", new StringValues(attrName));
 
@@ -90,9 +88,8 @@ public void Parse_DeeplyNestedSelection_Throws400ErrorWithDeeplyNestedMessage()
             const string type = "articles";
             const string relationship = "author.employer";
             const string attrName = "someField";
-            const string internalAttrName = "SomeField";
-            var attribute = new AttrAttribute(attrName) { InternalAttributeName = internalAttrName };
-            var idAttribute = new AttrAttribute("id") { InternalAttributeName = "Id" };
+            var attribute = new AttrAttribute(attrName);
+            var idAttribute = new AttrAttribute("id");
 
             var query = new KeyValuePair<string, StringValues>($"fields[{relationship}]", new StringValues(attrName));
 

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ public QueryParser_Benchmarks() {`
`25`	`25`	`var requestMock = new Mock<IRequestContext>();`
`26`	`26`	`requestMock.Setup(m => m.GetRequestResource()).Returns(new ResourceContext {`
`27`	`27`	`Attributes = new List<AttrAttribute> {`
`28`		`- new AttrAttribute(ATTRIBUTE, ATTRIBUTE)`
	`28`	`+ new AttrAttribute(ATTRIBUTE)`
`29`	`29`	`}`
`30`	`30`	`});`
`31`	`31`	`var options = new JsonApiOptions();`
Original file line number	Diff line number	Diff line change
`@@ -23,9 +23,9 @@ protected BaseQueryContext(TQuery query)`
`23`	`23`	`public string GetPropertyPath()`
`24`	`24`	`{`
`25`	`25`	`if (IsAttributeOfRelationship)`
`26`		`- return string.Format("{0}.{1}", Relationship.InternalRelationshipName, Attribute.InternalAttributeName);`
	`26`	`+ return string.Format("{0}.{1}", Relationship.InternalRelationshipName, Attribute.PropertyInfo.Name);`
`27`	`27`
`28`		`- return Attribute.InternalAttributeName;`
	`28`	`+ return Attribute.PropertyInfo.Name;`
`29`	`29`	`}`
`30`	`30`	`}`
`31`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@`
`5`	`5`
`6`	`6`	`namespace JsonApiDotNetCore.Models`
`7`	`7`	`{`
`8`		`- public abstract class RelationshipAttribute : Attribute, IResourceField, IRelationshipField`
	`8`	`+ public abstract class RelationshipAttribute : Attribute, IResourceField`
`9`	`9`	`{`
`10`	`10`	`protected RelationshipAttribute(string publicName, Link relationshipLinks, bool canInclude, string mappedBy)`
`11`	`11`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-using System;`
	`1`	`+using System;`
`2`	`2`	`using System.Collections;`
`3`	`3`	`using System.Collections.Generic;`
`4`	`4`	`using System.Linq;`
`@@ -31,7 +31,7 @@ public ResourceObject Build(IIdentifiable entity, IEnumerable<AttrAttribute> att`
`31`	`31`	`var ro = new ResourceObject { Type = resourceContext.ResourceName, Id = entity.StringId.NullIfEmpty() };`
`32`	`32`
`33`	`33`	`// populating the top-level "attribute" member of a resource object. never include "id" as an attribute`
`34`		`- if (attributes != null && (attributes = attributes.Where(attr => attr.InternalAttributeName != _identifiablePropertyName)).Any())`
	`34`	`+ if (attributes != null && (attributes = attributes.Where(attr => attr.PropertyInfo.Name != _identifiablePropertyName)).Any())`
`35`	`35`	`ProcessAttributes(entity, attributes, ro);`
`36`	`36`
`37`	`37`	`// populating the top-level "relationship" member of a resource object.`
`@@ -145,4 +145,4 @@ private void ProcessAttributes(IIdentifiable entity, IEnumerable<AttrAttribute>`
`145`	`145`	`}`
`146`	`146`	`}`
`147`	`147`	`}`
`148`		`-}`
	`148`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ public async Task UpdateAsync_AttributesUpdated_ShouldHaveSpecificallyThoseAttri`
`33`	`33`	`arrangeContext.Add(todoItemUpdates);`
`34`	`34`	`arrangeContext.SaveChanges();`
`35`	`35`
`36`		`- var descAttr = new AttrAttribute("description", "Description")`
	`36`	`+ var descAttr = new AttrAttribute("description")`
`37`	`37`	`{`
`38`	`38`	`PropertyInfo = typeof(TodoItem).GetProperty(nameof(TodoItem.Description))`
`39`	`39`	`};`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public void Request_Filter_Uses_Member_Expression()`
`29`	`29`	`var attrs = resource.GetAllowedAttributes();`
`30`	`30`
`31`	`31`	`// Assert`
`32`		`- Assert.DoesNotContain(attrs, a => a.InternalAttributeName == nameof(Model.AlwaysExcluded));`
	`32`	`+ Assert.DoesNotContain(attrs, a => a.PropertyInfo.Name == nameof(Model.AlwaysExcluded));`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`[Fact]`
`@@ -42,8 +42,8 @@ public void Request_Filter_Uses_NewExpression()`
`42`	`42`	`var attrs = resource.GetAllowedAttributes();`
`43`	`43`
`44`	`44`	`// Assert`
`45`		`- Assert.DoesNotContain(attrs, a => a.InternalAttributeName == nameof(Model.AlwaysExcluded));`
`46`		`- Assert.DoesNotContain(attrs, a => a.InternalAttributeName == nameof(Model.Password));`
	`45`	`+ Assert.DoesNotContain(attrs, a => a.PropertyInfo.Name == nameof(Model.AlwaysExcluded));`
	`46`	`+ Assert.DoesNotContain(attrs, a => a.PropertyInfo.Name == nameof(Model.Password));`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`