Skip to content

Attr property info #662

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.

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
merged 2 commits into from
Jan 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion benchmarks/Query/QueryParser_Benchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 1 addition & 3 deletions src/JsonApiDotNetCore/Builders/ResourceGraphBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
16 changes: 8 additions & 8 deletions src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
{
Expand All @@ -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);
Expand Down Expand Up @@ -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}
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/JsonApiDotNetCore/Internal/Query/BaseQueryContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
31 changes: 5 additions & 26 deletions src/JsonApiDotNetCore/Models/Annotation/AttrAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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;

Expand Down
6 changes: 0 additions & 6 deletions src/JsonApiDotNetCore/Models/Annotation/IRelationshipField.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -145,4 +145,4 @@ private void ProcessAttributes(IIdentifiable entity, IEnumerable<AttrAttribute>
}
}
}
}
}
2 changes: 1 addition & 1 deletion test/IntegrationTests/Data/EntityRepositoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
};
Expand Down
6 changes: 3 additions & 3 deletions test/UnitTests/Models/ResourceDefinitionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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));
}
}

Expand Down
17 changes: 7 additions & 10 deletions test/UnitTests/QueryParameters/SparseFieldsServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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));

Expand All @@ -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));

Expand Down