Attr property info (#662)

* Removed AttrAttribute.InternalAttributeName

* Removed unused IRelationshipField

Author: Bart Koelman

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();

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);

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);

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;
         }
     }
 }

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;
 

src/JsonApiDotNetCore/Models/Annotation/IRelationshipField.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)
         {

src/JsonApiDotNetCore/Models/Annotation/RelationshipAttribute.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>
             }
         }
     }
-}
+}

src/JsonApiDotNetCore/Serialization/Common/ResourceObjectBuilder.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))
                 };

test/IntegrationTests/Data/EntityRepositoryTests.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));
         }
     }
 

test/UnitTests/Models/ResourceDefinitionTests.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));