Merge pull request #250 from json-api-dotnet/fix/#249

fix(ContextGraphBuilder): don't throw if DbContext contains non json:api resource

Author: jaredcnance

src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs

@@ -37,5 +37,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
 
         public DbSet<Article> Articles { get; set; }
         public DbSet<Author> Authors { get; set; }
+
+        public DbSet<NonJsonApiResource> NonJsonApiResources { get; set; }
     }
 }

src/Examples/JsonApiDotNetCoreExample/Models/NonJsonApiResource.cs

@@ -0,0 +1,7 @@
+namespace JsonApiDotNetCoreExample.Models
+{
+    public class NonJsonApiResource
+    {
+        public int Id { get; set; }
+    }
+}

src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs

@@ -6,12 +6,50 @@
 using JsonApiDotNetCore.Internal;
 using JsonApiDotNetCore.Models;
 using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging;
 
 namespace JsonApiDotNetCore.Builders
 {
+    public interface IContextGraphBuilder
+    {
+        /// <summary>
+        /// Construct the <see cref="ContextGraph"/>
+        /// </summary>
+        IContextGraph Build();
+
+        /// <summary>
+        /// Add a json:api resource
+        /// </summary>
+        /// <typeparam name="TResource">The resource model type</typeparam>
+        /// <param name="pluralizedTypeName">The pluralized name that should be exposed by the API</param>
+        IContextGraphBuilder AddResource<TResource>(string pluralizedTypeName) where TResource : class, IIdentifiable<int>;
+
+        /// <summary>
+        /// Add a json:api resource
+        /// </summary>
+        /// <typeparam name="TResource">The resource model type</typeparam>
+        /// <typeparam name="TId">The resource model identifier type</typeparam>
+        /// <param name="pluralizedTypeName">The pluralized name that should be exposed by the API</param>
+        IContextGraphBuilder AddResource<TResource, TId>(string pluralizedTypeName) where TResource : class, IIdentifiable<TId>;
+
+        /// <summary>
+        /// Add all the models that are part of the provided <see cref="DbContext" /> 
+        /// that also implement <see cref="IIdentifiable"/>
+        /// </summary>
+        /// <typeparam name="T">The <see cref="DbContext"/> implementation type.</typeparam>
+        IContextGraphBuilder AddDbContext<T>() where T : DbContext;
+
+        /// <summary>
+        /// Which links to include. Defaults to <see cref="Link.All"/>.
+        /// </summary>
+        Link DocumentLinks { get; set; }
+    }
+
     public class ContextGraphBuilder : IContextGraphBuilder
     {
         private List<ContextEntity> _entities = new List<ContextEntity>();
+        private List<ValidationResult> _validationResults = new List<ValidationResult>();
+
         private bool _usesDbContext;
         public Link DocumentLinks { get; set; } = Link.All;
 
@@ -20,7 +58,7 @@ public IContextGraph Build()
             // this must be done at build so that call order doesn't matter
             _entities.ForEach(e => e.Links = GetLinkFlags(e.EntityType));
 
-            var graph = new ContextGraph(_entities, _usesDbContext);
+            var graph = new ContextGraph(_entities, _usesDbContext, _validationResults);
 
             return graph;
         }
@@ -117,7 +155,10 @@ public IContextGraphBuilder AddDbContext<T>() where T : DbContext
 
                     AssertEntityIsNotAlreadyDefined(entityType);
 
-                    _entities.Add(GetEntity(GetResourceName(property), entityType, GetIdType(entityType)));
+                    var (isJsonApiResource, idType) = GetIdType(entityType);
+
+                    if (isJsonApiResource)
+                        _entities.Add(GetEntity(GetResourceName(property), entityType, idType));
                 }
             }
 
@@ -133,16 +174,18 @@ private string GetResourceName(PropertyInfo property)
             return ((ResourceAttribute)resourceAttribute).ResourceName;
         }
 
-        private Type GetIdType(Type resourceType)
+        private (bool isJsonApiResource, Type idType) GetIdType(Type resourceType)
         {
             var interfaces = resourceType.GetInterfaces();
             foreach (var type in interfaces)
             {
                 if (type.GetTypeInfo().IsGenericType && type.GetGenericTypeDefinition() == typeof(IIdentifiable<>))
-                    return type.GetGenericArguments()[0];
+                    return (true, type.GetGenericArguments()[0]);
             }
 
-            throw new ArgumentException("Type does not implement 'IIdentifiable<TId>'", nameof(resourceType));
+            _validationResults.Add(new ValidationResult(LogLevel.Warning, $"{resourceType} does not implement 'IIdentifiable<>'. "));
+
+            return (false, null);
         }
 
         private void AssertEntityIsNotAlreadyDefined(Type entityType)

src/JsonApiDotNetCore/Builders/IContextGraphBuilder.cs

@@ -1,29 +1,55 @@
+using JsonApiDotNetCore.Builders;
 using JsonApiDotNetCore.Configuration;
+using JsonApiDotNetCore.Internal;
 using JsonApiDotNetCore.Middleware;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
+using Microsoft.Extensions.Logging;
 
 namespace JsonApiDotNetCore.Extensions
 {
     // ReSharper disable once InconsistentNaming
     public static class IApplicationBuilderExtensions
     {
         public static IApplicationBuilder UseJsonApi(this IApplicationBuilder app, bool useMvc = true)
+        {
+            DisableDetailedErrorsIfProduction(app);
+            LogContextGraphValidations(app);
+
+            app.UseMiddleware<RequestMiddleware>();
+
+            if (useMvc)
+                app.UseMvc();
+
+            return app;
+        }
+
+        private static void DisableDetailedErrorsIfProduction(IApplicationBuilder app)
         {
             var environment = (IHostingEnvironment)app.ApplicationServices.GetService(typeof(IHostingEnvironment));
 
-            if(environment.IsProduction())
+            if (environment.IsProduction())
             {
                 JsonApiOptions.DisableErrorStackTraces = true;
                 JsonApiOptions.DisableErrorSource = true;
             }
+        }
 
-            app.UseMiddleware<RequestMiddleware>();
-
-            if (useMvc)
-                app.UseMvc();
+        private static void LogContextGraphValidations(IApplicationBuilder app)
+        {
+            var logger = app.ApplicationServices.GetService(typeof(ILogger<ContextGraphBuilder>)) as ILogger;
+            var contextGraph = app.ApplicationServices.GetService(typeof(IContextGraph)) as ContextGraph;
 
-            return app;
+            if (logger != null && contextGraph != null)
+            {
+                contextGraph.ValidationResults.ForEach((v) =>
+                    logger.Log(
+                        v.LogLevel,
+                        new EventId(),
+                        v.Message,
+                        exception: null,
+                        formatter: (m, e) => m));
+            }
         }
     }
 }

src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs

@@ -4,25 +4,47 @@
 
 namespace JsonApiDotNetCore.Internal
 {
+    public interface IContextGraph
+    {
+        object GetRelationship<TParent>(TParent entity, string relationshipName);
+        string GetRelationshipName<TParent>(string relationshipName);
+        ContextEntity GetContextEntity(string dbSetName);
+        ContextEntity GetContextEntity(Type entityType);
+        bool UsesDbContext { get; }
+    }
+
     public class ContextGraph : IContextGraph
     {
-        private List<ContextEntity> _entities;
+        internal List<ContextEntity> Entities { get; }
+        internal List<ValidationResult> ValidationResults { get; }
 
         public ContextGraph() { }
 
         public ContextGraph(List<ContextEntity> entities, bool usesDbContext)
         {
-            _entities = entities;
+            Entities = entities;
+            UsesDbContext = usesDbContext;
+            ValidationResults = new List<ValidationResult>();
+        }
+
+        // eventually, this is the planned public constructor
+        // to avoid breaking changes, we will be leaving the original constructor in place
+        // until the context graph validation process is completed
+        // you can track progress on this issue here: https://github.com/json-api-dotnet/JsonApiDotNetCore/issues/170
+        internal ContextGraph(List<ContextEntity> entities, bool usesDbContext, List<ValidationResult> validationResults)
+        {
+            Entities = entities;
             UsesDbContext = usesDbContext;
+            ValidationResults = validationResults;
         }
 
         public bool UsesDbContext { get; }
 
         public ContextEntity GetContextEntity(string entityName)
-            => _entities.SingleOrDefault(e => string.Equals(e.EntityName, entityName, StringComparison.OrdinalIgnoreCase));
+            => Entities.SingleOrDefault(e => string.Equals(e.EntityName, entityName, StringComparison.OrdinalIgnoreCase));
 
         public ContextEntity GetContextEntity(Type entityType)
-            => _entities.SingleOrDefault(e => e.EntityType == entityType);
+            => Entities.SingleOrDefault(e => e.EntityType == entityType);
 
         public object GetRelationship<TParent>(TParent entity, string relationshipName)
         {
@@ -41,7 +63,7 @@ public object GetRelationship<TParent>(TParent entity, string relationshipName)
         public string GetRelationshipName<TParent>(string relationshipName)
         {
             var entityType = typeof(TParent);
-            return _entities
+            return Entities
                 .SingleOrDefault(e => e.EntityType == entityType)
                 ?.Relationships
                 .SingleOrDefault(r => r.Is(relationshipName))

src/JsonApiDotNetCore/Internal/ContextGraph.cs

@@ -0,0 +1,16 @@
+using Microsoft.Extensions.Logging;
+
+namespace JsonApiDotNetCore.Internal
+{
+    internal class ValidationResult
+    {
+        public ValidationResult(LogLevel logLevel, string message)
+        {
+            LogLevel = logLevel;
+            Message = message;
+        }
+
+        public LogLevel LogLevel { get; set; }
+        public string Message { get; set; }
+    }
+}

src/JsonApiDotNetCore/Internal/IContextGraph.cs

@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <VersionPrefix>2.2.0</VersionPrefix>
+    <VersionPrefix>2.2.1</VersionPrefix>
     <TargetFrameworks>$(NetStandardVersion)</TargetFrameworks>
     <AssemblyName>JsonApiDotNetCore</AssemblyName>
     <PackageId>JsonApiDotNetCore</PackageId>

src/JsonApiDotNetCore/Internal/ValidationResults.cs

@@ -0,0 +1,48 @@
+using JsonApiDotNetCore.Builders;
+using JsonApiDotNetCore.Internal;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging;
+using Xunit;
+
+namespace UnitTests.Internal
+{
+    public class ContextGraphBuilder_Tests
+    {
+        [Fact]
+        public void AddDbContext_Does_Not_Throw_If_Context_Contains_Members_That_DoNot_Implement_IIdentifiable()
+        {
+            // arrange
+            var contextGraphBuilder = new ContextGraphBuilder();
+
+            // act
+            contextGraphBuilder.AddDbContext<TestContext>();
+            var contextGraph = contextGraphBuilder.Build() as ContextGraph;
+
+            // assert
+            Assert.Empty(contextGraph.Entities);
+        }
+
+        [Fact]
+        public void Adding_DbContext_Members_That_DoNot_Implement_IIdentifiable_Creates_Warning()
+        {
+            // arrange
+            var contextGraphBuilder = new ContextGraphBuilder();
+
+            // act
+            contextGraphBuilder.AddDbContext<TestContext>();
+            var contextGraph = contextGraphBuilder.Build() as ContextGraph;
+
+            // assert
+            Assert.Equal(1, contextGraph.ValidationResults.Count);
+            Assert.Contains(contextGraph.ValidationResults, v => v.LogLevel == LogLevel.Warning);
+        }
+
+        private class Foo { }
+
+        private class TestContext : DbContext
+        {
+            public DbSet<Foo> Foos { get; set; }
+        }
+    }
+
+}