Skip to content

Fix: crash when deserializing post with relationship to abstract base class #833

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 46 commits into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
2649203
feat: resource inheritance compatabiliy
maurei Sep 18, 2020
9104cb9
test: fix and add new tests
maurei Sep 18, 2020
a49a99e
feat: finished resource inheritance
maurei Sep 18, 2020
b8837da
fix: whitespace
maurei Sep 18, 2020
8a7392c
fix: tests
maurei Sep 18, 2020
1a8f41b
chore: review
maurei Sep 24, 2020
172126e
feat: improved inheritance support. Partially doneE
maurei Sep 24, 2020
0d20475
fix: tests
maurei Sep 25, 2020
b4649b3
chore: revert unnecessary changes
maurei Sep 25, 2020
198ffcf
test: reveal issue with filtering
maurei Sep 25, 2020
c0d3994
fix: better error handling inheritance bug
maurei Sep 28, 2020
a8d296f
chore: rename tests
maurei Sep 28, 2020
1ded442
chore: ignore tests for unsupported features
maurei Sep 28, 2020
03cfb07
fix: inheritance support in response deserializer
maurei Sep 28, 2020
0c7f327
fix: overlapping tests
maurei Sep 28, 2020
6c76785
fix: white space
maurei Sep 28, 2020
632fe53
Merge branch 'master' into fix/696
Sep 29, 2020
753a5bf
chore: review
maurei Sep 29, 2020
431d8aa
chore: remove suggestive tests
maurei Sep 29, 2020
d717614
merge
maurei Sep 29, 2020
1f393f6
fix: more intuitive test models
maurei Sep 29, 2020
5dab42d
fix: more intuitive test models
maurei Sep 29, 2020
27cc727
fix: renamed
maurei Sep 29, 2020
365cd32
fix: tests
maurei Sep 29, 2020
0193d40
Merge branch 'master' into fix/696
Oct 1, 2020
c1a5b71
review
maurei Oct 2, 2020
59ce978
fix: remove unused controllers
maurei Oct 2, 2020
3f4079d
Merge branch 'fix/696' of https://github.com/json-api-dotnet/JsonApiD…
maurei Oct 2, 2020
f20fe5f
fix: undo unwanted remove
maurei Oct 2, 2020
273ddf7
review
maurei Oct 2, 2020
a4c1d55
fix: disable warning
maurei Oct 2, 2020
44a607e
fix: rename models
maurei Oct 2, 2020
e2569ba
fix: review
maurei Oct 2, 2020
b5ac3ee
fix: review
maurei Oct 2, 2020
d127188
fix: examples
maurei Oct 2, 2020
88b1de6
fix
maurei Oct 2, 2020
3fef707
improved example
maurei Oct 2, 2020
b39a9b8
fix: unused ref
maurei Oct 2, 2020
c1f8c30
fix: improve examples
maurei Oct 2, 2020
bd9ad25
review
maurei Oct 2, 2020
6ccb4bd
fix tests
maurei Oct 2, 2020
e76b437
fix: tests
maurei Oct 2, 2020
2fe2b40
fix
maurei Oct 2, 2020
c657bb5
fix
maurei Oct 2, 2020
7f53304
fix: review
maurei Oct 3, 2020
8ce063c
fix: verified lambda parameter names for consistency
maurei Oct 3, 2020
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
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions src/JsonApiDotNetCore/Resources/ResourceFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
27 changes: 18 additions & 9 deletions src/JsonApiDotNetCore/Serialization/BaseDeserializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/JsonApiDotNetCore/Serialization/JsonApiReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@
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;
Expand All @@ -32,12 +34,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]
Expand Down Expand Up @@ -192,4 +188,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)
{ }
}
}
57 changes: 43 additions & 14 deletions test/JsonApiDotNetCoreExampleTests/AppDbContextExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,52 @@ public static class AppDbContextExtensions
{
public static async Task ClearTableAsync<TEntity>(this DbContext dbContext) where TEntity : class
{
var entityType = dbContext.Model.FindEntityType(typeof(TEntity));
if (entityType == null)
await ClearTablesAsync(dbContext,typeof(TEntity));
}

public static async Task ClearTablesAsync<TEntity1, TEntity2>(this DbContext dbContext) where TEntity1 : class
where TEntity2 : class
{
await ClearTablesAsync(dbContext,typeof(TEntity1), typeof(TEntity2));
}

public static async Task ClearTablesAsync<TEntity1, TEntity2, TEntity3>(this DbContext dbContext) where TEntity1 : class
where TEntity2 : class
where TEntity3 : class
{
await ClearTablesAsync(dbContext,typeof(TEntity1), typeof(TEntity2), typeof(TEntity3));
}

public static async Task ClearTablesAsync<TEntity1, TEntity2, TEntity3, TEntity4>(this DbContext dbContext) where TEntity1 : class
where TEntity2 : class
where TEntity3 : class
where TEntity4 : class
{
await ClearTablesAsync(dbContext, typeof(TEntity1), typeof(TEntity2), typeof(TEntity3), typeof(TEntity4));
}

private static async Task ClearTablesAsync(this DbContext dbContext, params Type[] models)
{
foreach (var model in models)
{
throw new InvalidOperationException($"Table for '{typeof(TEntity).Name}' not found.");
}
var entityType = dbContext.Model.FindEntityType(model);
if (entityType == null)
{
throw new InvalidOperationException($"Table for '{model.Name}' not found.");
}

string tableName = entityType.GetTableName();
string tableName = entityType.GetTableName();

// PERF: We first try to clear the table, which is fast and usually succeeds, unless foreign key constraints are violated.
// In that case, we recursively delete all related data, which is slow.
try
{
await dbContext.Database.ExecuteSqlRawAsync("delete from \"" + tableName + "\"");
}
catch (PostgresException)
{
await dbContext.Database.ExecuteSqlRawAsync("truncate table \"" + tableName + "\" cascade");
// PERF: We first try to clear the table, which is fast and usually succeeds, unless foreign key constraints are violated.
// In that case, we recursively delete all related data, which is slow.
try
{
await dbContext.Database.ExecuteSqlRawAsync("delete from \"" + tableName + "\"");
}
catch (PostgresException)
{
await dbContext.Database.ExecuteSqlRawAsync("truncate table \"" + tableName + "\" cascade");
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using JsonApiDotNetCoreExampleTests.IntegrationTests.ResourceInheritance.Models;
using Microsoft.EntityFrameworkCore;

namespace JsonApiDotNetCoreExampleTests.IntegrationTests.ResourceInheritance
{
public sealed class InheritanceDbContext : DbContext
{
public InheritanceDbContext(DbContextOptions<InheritanceDbContext> options) : base(options) { }

public DbSet<Human> Humans { get; set; }

public DbSet<Man> Men { get; set; }

public DbSet<CompanyHealthInsurance> CompanyHealthInsurances { get; set; }

public DbSet<ContentItem> ContentItems { get; set; }

public DbSet<HumanFavoriteContentItem> HumanFavoriteContentItems { get; set; }

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Human>()
.HasDiscriminator<int>("Type")
.HasValue<Man>(1)
.HasValue<Woman>(2);

modelBuilder.Entity<HealthInsurance>()
.HasDiscriminator<int>("Type")
.HasValue<CompanyHealthInsurance>(1)
.HasValue<FamilyHealthInsurance>(2);

modelBuilder.Entity<ContentItem>()
.HasDiscriminator<int>("Type")
.HasValue<Video>(1)
.HasValue<Book>(2);

modelBuilder.Entity<HumanFavoriteContentItem>()
.HasKey(hfci => new { ContentPersonId = hfci.ContentItemId, PersonId = hfci.HumanId });
}
}
}
Loading