Skip to content

Tests that reveal issues with resources that rely on heritance #846

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

Closed
wants to merge 22 commits into from
Closed
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

This file was deleted.

Original file line number Diff line number Diff line change
@@ -187,7 +187,7 @@ private IReadOnlyCollection<RelationshipAttribute> GetRelationships(Type resourc
var throughProperties = throughType.GetProperties();

// ArticleTag.Article
hasManyThroughAttribute.LeftProperty = throughProperties.SingleOrDefault(x => x.PropertyType == resourceType)
hasManyThroughAttribute.LeftProperty = throughProperties.SingleOrDefault(x => x.PropertyType.IsAssignableFrom(resourceType))
?? throw new InvalidConfigurationException($"{throughType} does not contain a navigation property to type {resourceType}");

// ArticleTag.ArticleId
Original file line number Diff line number Diff line change
@@ -264,15 +264,20 @@ private IEnumerable GetTrackedManyRelationshipValue(IEnumerable<IIdentifiable> r
{
if (relationshipValues == null) return null;
bool newWasAlreadyAttached = false;

var trackedPointerCollection = TypeHelper.CopyToTypedCollection(relationshipValues.Select(pointer =>
{
// convert each element in the value list to relationshipAttr.DependentType.
var tracked = AttachOrGetTracked(pointer);
if (tracked != null) newWasAlreadyAttached = true;
return Convert.ChangeType(tracked ?? pointer, relationshipAttr.RightType);

var trackedPointer = tracked ?? pointer;

// We should recalculate the target type for every iteration because types may vary. This is possible with resource inheritance.
return Convert.ChangeType(trackedPointer, trackedPointer.GetType());
}), relationshipAttr.Property.PropertyType);

if (newWasAlreadyAttached) wasAlreadyAttached = true;

return trackedPointerCollection;
}

5 changes: 5 additions & 0 deletions src/JsonApiDotNetCore/Resources/ResourceFactory.cs
Original file line number Diff line number Diff line change
@@ -116,6 +116,11 @@ private static ConstructorInfo GetLongestConstructor(Type type)
{
ConstructorInfo[] constructors = type.GetConstructors().Where(c => !c.IsStatic).ToArray();

if (constructors.Length == 0)
{
throw new InvalidOperationException($"No public constructor was found for '{type.FullName}'.");
}

ConstructorInfo bestMatch = constructors[0];
int maxParameterLength = constructors[0].GetParameters().Length;

27 changes: 18 additions & 9 deletions src/JsonApiDotNetCore/Serialization/BaseDeserializer.cs
Original file line number Diff line number Diff line change
@@ -106,7 +106,8 @@ protected virtual IIdentifiable SetRelationships(IIdentifiable resource, IDictio
var resourceProperties = resource.GetType().GetProperties();
foreach (var attr in relationshipAttributes)
{
if (!relationshipValues.TryGetValue(attr.PublicName, out RelationshipEntry relationshipData) || !relationshipData.IsPopulated)
var relationshipIsProvided = relationshipValues.TryGetValue(attr.PublicName, out RelationshipEntry relationshipData);
if (!relationshipIsProvided || !relationshipData.IsPopulated)
continue;

if (attr is HasOneAttribute hasOneAttribute)
@@ -168,15 +169,19 @@ private void SetHasOneRelationship(IIdentifiable resource,
var rio = (ResourceIdentifierObject)relationshipData.Data;
var relatedId = rio?.Id;

var relationshipType = relationshipData.SingleData == null
? attr.RightType
: ResourceContextProvider.GetResourceContext(relationshipData.SingleData.Type).ResourceType;

// this does not make sense in the following case: if we're setting the dependent of a one-to-one relationship, IdentifiablePropertyName should be null.
var foreignKeyProperty = resourceProperties.FirstOrDefault(p => p.Name == attr.IdentifiablePropertyName);

if (foreignKeyProperty != null)
// there is a FK from the current resource pointing to the related object,
// i.e. we're populating the relationship from the dependent side.
SetForeignKey(resource, foreignKeyProperty, attr, relatedId);
SetForeignKey(resource, foreignKeyProperty, attr, relatedId, relationshipType);

SetNavigation(resource, attr, relatedId);
SetNavigation(resource, attr, relatedId, relationshipType);

// depending on if this base parser is used client-side or server-side,
// different additional processing per field needs to be executed.
@@ -185,9 +190,10 @@ private void SetHasOneRelationship(IIdentifiable resource,

/// <summary>
/// Sets the dependent side of a HasOne relationship, which means that a
/// foreign key also will to be populated.
/// foreign key also will be populated.
/// </summary>
private void SetForeignKey(IIdentifiable resource, PropertyInfo foreignKey, HasOneAttribute attr, string id)
private void SetForeignKey(IIdentifiable resource, PropertyInfo foreignKey, HasOneAttribute attr, string id,
Type relationshipType)
{
bool foreignKeyPropertyIsNullableType = Nullable.GetUnderlyingType(foreignKey.PropertyType) != null
|| foreignKey.PropertyType == typeof(string);
@@ -198,23 +204,24 @@ private void SetForeignKey(IIdentifiable resource, PropertyInfo foreignKey, HasO
throw new FormatException($"Cannot set required relationship identifier '{attr.IdentifiablePropertyName}' to null because it is a non-nullable type.");
}

var typedId = TypeHelper.ConvertStringIdToTypedId(attr.Property.PropertyType, id, ResourceFactory);
var typedId = TypeHelper.ConvertStringIdToTypedId(relationshipType, id, ResourceFactory);
foreignKey.SetValue(resource, typedId);
}

/// <summary>
/// Sets the principal side of a HasOne relationship, which means no
/// foreign key is involved.
/// </summary>
private void SetNavigation(IIdentifiable resource, HasOneAttribute attr, string relatedId)
private void SetNavigation(IIdentifiable resource, HasOneAttribute attr, string relatedId,
Type relationshipType)
{
if (relatedId == null)
{
attr.SetValue(resource, null, ResourceFactory);
}
else
{
var relatedInstance = (IIdentifiable)ResourceFactory.CreateInstance(attr.RightType);
var relatedInstance = (IIdentifiable)ResourceFactory.CreateInstance(relationshipType);
relatedInstance.StringId = relatedId;
attr.SetValue(resource, relatedInstance, ResourceFactory);
}
@@ -232,8 +239,10 @@ private void SetHasManyRelationship(
{ // if the relationship is set to null, no need to set the navigation property to null: this is the default value.
var relatedResources = relationshipData.ManyData.Select(rio =>
{
var relatedInstance = (IIdentifiable)ResourceFactory.CreateInstance(attr.RightType);
var relationshipType = ResourceContextProvider.GetResourceContext(rio.Type).ResourceType;
var relatedInstance = (IIdentifiable)ResourceFactory.CreateInstance(relationshipType);
relatedInstance.StringId = rio.Id;

return relatedInstance;
});

Original file line number Diff line number Diff line change
@@ -72,11 +72,11 @@ protected override void AfterProcessField(IIdentifiable resource, ResourceFieldA
{
// add attributes and relationships of a parsed HasOne relationship
var rio = data.SingleData;
hasOneAttr.SetValue(resource, rio == null ? null : ParseIncludedRelationship(hasOneAttr, rio), ResourceFactory);
hasOneAttr.SetValue(resource, rio == null ? null : ParseIncludedRelationship(rio), ResourceFactory);
}
else if (field is HasManyAttribute hasManyAttr)
{ // add attributes and relationships of a parsed HasMany relationship
var items = data.ManyData.Select(rio => ParseIncludedRelationship(hasManyAttr, rio));
var items = data.ManyData.Select(rio => ParseIncludedRelationship(rio));
var values = TypeHelper.CopyToTypedCollection(items, hasManyAttr.Property.PropertyType);
hasManyAttr.SetValue(resource, values, ResourceFactory);
}
@@ -85,21 +85,26 @@ protected override void AfterProcessField(IIdentifiable resource, ResourceFieldA
/// <summary>
/// Searches for and parses the included relationship.
/// </summary>
private IIdentifiable ParseIncludedRelationship(RelationshipAttribute relationshipAttr, ResourceIdentifierObject relatedResourceIdentifier)
private IIdentifiable ParseIncludedRelationship(ResourceIdentifierObject relatedResourceIdentifier)
{
var relatedInstance = (IIdentifiable)ResourceFactory.CreateInstance(relationshipAttr.RightType);
var relatedResourceContext = ResourceContextProvider.GetResourceContext(relatedResourceIdentifier.Type);

if (relatedResourceContext == null)
{
throw new InvalidOperationException($"Included type '{relatedResourceIdentifier.Type}' is not a registered json:api resource.");
}

var relatedInstance = (IIdentifiable)ResourceFactory.CreateInstance(relatedResourceContext.ResourceType);
relatedInstance.StringId = relatedResourceIdentifier.Id;

var includedResource = GetLinkedResource(relatedResourceIdentifier);
if (includedResource == null)
return relatedInstance;

var resourceContext = ResourceContextProvider.GetResourceContext(relatedResourceIdentifier.Type);
if (resourceContext == null)
throw new InvalidOperationException($"Included type '{relationshipAttr.RightType}' is not a registered json:api resource.");

SetAttributes(relatedInstance, includedResource.Attributes, resourceContext.Attributes);
SetRelationships(relatedInstance, includedResource.Relationships, resourceContext.Relationships);
if (includedResource != null)
{
SetAttributes(relatedInstance, includedResource.Attributes, relatedResourceContext.Attributes);
SetRelationships(relatedInstance, includedResource.Relationships, relatedResourceContext.Relationships);
}

return relatedInstance;
}

2 changes: 1 addition & 1 deletion src/JsonApiDotNetCore/Serialization/JsonApiReader.cs
Original file line number Diff line number Diff line change
@@ -89,7 +89,7 @@ private void ValidateIncomingResourceType(InputFormatterContext context, object
var bodyResourceTypes = GetBodyResourceTypes(model);
foreach (var bodyResourceType in bodyResourceTypes)
{
if (bodyResourceType != endpointResourceType)
if (!endpointResourceType.IsAssignableFrom(bodyResourceType))
{
var resourceFromEndpoint = _resourceContextProvider.GetResourceContext(endpointResourceType);
var resourceFromBody = _resourceContextProvider.GetResourceContext(bodyResourceType);
Original file line number Diff line number Diff line change
@@ -5,16 +5,19 @@
using Bogus;
using FluentAssertions;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Controllers;
using JsonApiDotNetCore.Resources;
using JsonApiDotNetCore.Serialization.Client.Internal;
using JsonApiDotNetCore.Serialization.Objects;
using JsonApiDotNetCore.Services;
using JsonApiDotNetCoreExample;
using JsonApiDotNetCoreExample.Data;
using JsonApiDotNetCoreExample.Models;
using Microsoft.AspNetCore.Mvc.ApplicationParts;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json.Linq;
using Newtonsoft.Json.Serialization;
using Xunit;
@@ -32,12 +35,6 @@ public KebabCaseFormatterTests(IntegrationTestContext<KebabCaseStartup, AppDbCon

_faker = new Faker<KebabCasedModel>()
.RuleFor(m => m.CompoundAttr, f => f.Lorem.Sentence());

testContext.ConfigureServicesAfterStartup(services =>
{
var part = new AssemblyPart(typeof(EmptyStartup).Assembly);
services.AddMvcCore().ConfigureApplicationPartManager(apm => apm.ApplicationParts.Add(part));
});
}

[Fact]
@@ -192,4 +189,14 @@ protected override void ConfigureJsonApiOptions(JsonApiOptions options)
((DefaultContractResolver)options.SerializerSettings.ContractResolver).NamingStrategy = new KebabCaseNamingStrategy();
}
}

public sealed class KebabCasedModelsController : JsonApiController<KebabCasedModel>
{
public KebabCasedModelsController(
IJsonApiOptions options,
ILoggerFactory loggerFactory,
IResourceService<KebabCasedModel> resourceService)
: base(options, loggerFactory, resourceService)
{ }
}
}
Original file line number Diff line number Diff line change
@@ -2,13 +2,16 @@
using System.Threading.Tasks;
using FluentAssertions;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Controllers;
using JsonApiDotNetCore.Serialization.Objects;
using JsonApiDotNetCore.Services;
using JsonApiDotNetCoreExample;
using JsonApiDotNetCoreExample.Data;
using JsonApiDotNetCoreExample.Models;
using Microsoft.AspNetCore.Mvc.ApplicationParts;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Xunit;

namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec.DocumentTests
@@ -20,12 +23,6 @@ public sealed class LinksWithoutNamespaceTests : IClassFixture<IntegrationTestCo
public LinksWithoutNamespaceTests(IntegrationTestContext<NoNamespaceStartup, AppDbContext> testContext)
{
_testContext = testContext;

testContext.ConfigureServicesAfterStartup(services =>
{
var part = new AssemblyPart(typeof(EmptyStartup).Assembly);
services.AddMvcCore().ConfigureApplicationPartManager(apm => apm.ApplicationParts.Add(part));
});
}

[Fact]
@@ -35,24 +32,24 @@ public async Task GET_RelativeLinks_True_Without_Namespace_Returns_RelativeLinks
var options = (JsonApiOptions) _testContext.Factory.Services.GetRequiredService<IJsonApiOptions>();
options.UseRelativeLinks = true;

var person = new Person();
var blog = new Blog();

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
dbContext.People.Add(person);
dbContext.Blogs.Add(blog);

await dbContext.SaveChangesAsync();
});

var route = "/people/" + person.StringId;
var route = "/blogs/" + blog.StringId;

// Act
var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);

// Assert
httpResponse.Should().HaveStatusCode(HttpStatusCode.OK);

responseDocument.Links.Self.Should().Be("/people/" + person.StringId);
responseDocument.Links.Self.Should().Be("/blogs/" + blog.StringId);
}

[Fact]
@@ -62,24 +59,24 @@ public async Task GET_RelativeLinks_False_Without_Namespace_Returns_AbsoluteLink
var options = (JsonApiOptions) _testContext.Factory.Services.GetRequiredService<IJsonApiOptions>();
options.UseRelativeLinks = false;

var person = new Person();
var blog = new Blog();

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
dbContext.People.Add(person);
dbContext.Blogs.Add(blog);

await dbContext.SaveChangesAsync();
});

var route = "/people/" + person.StringId;
var route = "/blogs/" + blog.StringId;

// Act
var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);

// Assert
httpResponse.Should().HaveStatusCode(HttpStatusCode.OK);

responseDocument.Links.Self.Should().Be("http://localhost/people/" + person.StringId);
responseDocument.Links.Self.Should().Be("http://localhost/blogs/" + blog.StringId);
}
}

@@ -96,4 +93,14 @@ protected override void ConfigureJsonApiOptions(JsonApiOptions options)
options.Namespace = null;
}
}

public sealed class BlogsController : JsonApiController<Blog>
{
public BlogsController(
IJsonApiOptions options,
ILoggerFactory loggerFactory,
IResourceService<Blog> resourceService)
: base(options, loggerFactory, resourceService)
{ }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Controllers;
using JsonApiDotNetCore.Services;
using Microsoft.Extensions.Logging;

namespace JsonApiDotNetCoreExampleTests.IntegrationTests.ResourceInheritance
{
public sealed class AnimalsController : JsonApiController<Animal>
{
public AnimalsController(IJsonApiOptions options, ILoggerFactory loggerFactory,
IResourceService<Animal> resourceService)
: base(options, loggerFactory, resourceService) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Controllers;
using JsonApiDotNetCore.Services;
using Microsoft.Extensions.Logging;

namespace JsonApiDotNetCoreExampleTests.IntegrationTests.ResourceInheritance
{
public sealed class CatsController : JsonApiController<Cat>
{
public CatsController(IJsonApiOptions options, ILoggerFactory loggerFactory,
IResourceService<Cat> resourceService)
: base(options, loggerFactory, resourceService)
{
}
}
}
Loading