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

Co-authored-by: Bart Koelman <[email protected]>

Author: maurei

src/Examples/JsonApiDotNetCoreExample/Controllers/KebabCasedModelsController.cs

@@ -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

src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs

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

src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs

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

src/JsonApiDotNetCore/Resources/ResourceFactory.cs

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

src/JsonApiDotNetCore/Serialization/BaseDeserializer.cs

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

src/JsonApiDotNetCore/Serialization/Client/Internal/ResponseDeserializer.cs

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

src/JsonApiDotNetCore/Serialization/JsonApiReader.cs

@@ -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;
@@ -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]
@@ -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)
+        { }
+    }
 }

test/JsonApiDotNetCoreExampleTests/Acceptance/KebabCaseFormatterTests.cs

@@ -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");
+                }   
             }
         }
 

test/JsonApiDotNetCoreExampleTests/AppDbContextExtensions.cs

@@ -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 });
+        }
+    }
+}