From 961383564aed289de335af5ce7fe9ce77d283196 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 4 Apr 2018 15:50:21 -0500 Subject: [PATCH 1/2] fix(ContextGraphBuilder): don't throw if DbContext contains non json:api resource Also begins work for #170 --- .../Data/AppDbContext.cs | 2 + .../Models/NonJsonApiResource.cs | 7 +++ .../Builders/ContextGraphBuilder.cs | 53 +++++++++++++++++-- .../Builders/IContextGraphBuilder.cs | 15 ------ .../IApplicationBuilderExtensions.cs | 38 ++++++++++--- .../Internal/ContextGraph.cs | 32 +++++++++-- .../Internal/IContextGraph.cs | 13 ----- .../Internal/ValidationResults.cs | 16 ++++++ .../Internal/ContextGraphBuilder_Tests.cs | 48 +++++++++++++++++ 9 files changed, 180 insertions(+), 44 deletions(-) create mode 100644 src/Examples/JsonApiDotNetCoreExample/Models/NonJsonApiResource.cs delete mode 100644 src/JsonApiDotNetCore/Builders/IContextGraphBuilder.cs delete mode 100644 src/JsonApiDotNetCore/Internal/IContextGraph.cs create mode 100644 src/JsonApiDotNetCore/Internal/ValidationResults.cs create mode 100644 test/UnitTests/Internal/ContextGraphBuilder_Tests.cs diff --git a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs index 4266ca9741..4b9a40f7fd 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs @@ -37,5 +37,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) public DbSet
Articles { get; set; } public DbSet Authors { get; set; } + + public DbSet NonJsonApiResources { get; set; } } } diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/NonJsonApiResource.cs b/src/Examples/JsonApiDotNetCoreExample/Models/NonJsonApiResource.cs new file mode 100644 index 0000000000..7f979f4cfb --- /dev/null +++ b/src/Examples/JsonApiDotNetCoreExample/Models/NonJsonApiResource.cs @@ -0,0 +1,7 @@ +namespace JsonApiDotNetCoreExample.Models +{ + public class NonJsonApiResource + { + public int Id { get; set; } + } +} diff --git a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs index 643df13bc3..73e355b2de 100644 --- a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs +++ b/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 + { + /// + /// Construct the + /// + IContextGraph Build(); + + /// + /// Add a json:api resource + /// + /// The resource model type + /// The pluralized name that should be exposed by the API + IContextGraphBuilder AddResource(string pluralizedTypeName) where TResource : class, IIdentifiable; + + /// + /// Add a json:api resource + /// + /// The resource model type + /// The resource model identifier type + /// The pluralized name that should be exposed by the API + IContextGraphBuilder AddResource(string pluralizedTypeName) where TResource : class, IIdentifiable; + + /// + /// Add all the models that are part of the provided + /// that also implement + /// + /// The implementation type. + IContextGraphBuilder AddDbContext() where T : DbContext; + + /// + /// Which links to include. Defaults to . + /// + Link DocumentLinks { get; set; } + } + public class ContextGraphBuilder : IContextGraphBuilder { private List _entities = new List(); + private List _validationResults = new List(); + 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() 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'", nameof(resourceType)); + _validationResults.Add(new ValidationResult(LogLevel.Warning, $"{resourceType} does not implement 'IIdentifiable<>'. ")); + + return (false, null); } private void AssertEntityIsNotAlreadyDefined(Type entityType) diff --git a/src/JsonApiDotNetCore/Builders/IContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/IContextGraphBuilder.cs deleted file mode 100644 index 9f3c7bd1a8..0000000000 --- a/src/JsonApiDotNetCore/Builders/IContextGraphBuilder.cs +++ /dev/null @@ -1,15 +0,0 @@ -using JsonApiDotNetCore.Internal; -using JsonApiDotNetCore.Models; -using Microsoft.EntityFrameworkCore; - -namespace JsonApiDotNetCore.Builders -{ - public interface IContextGraphBuilder - { - Link DocumentLinks { get; set; } - IContextGraph Build(); - IContextGraphBuilder AddResource(string pluralizedTypeName) where TResource : class, IIdentifiable; - IContextGraphBuilder AddResource(string pluralizedTypeName) where TResource : class, IIdentifiable; - IContextGraphBuilder AddDbContext() where T : DbContext; - } -} diff --git a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs index 651fbb44aa..996d7eb9de 100644 --- a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs @@ -1,7 +1,10 @@ +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 { @@ -9,21 +12,44 @@ namespace JsonApiDotNetCore.Extensions public static class IApplicationBuilderExtensions { public static IApplicationBuilder UseJsonApi(this IApplicationBuilder app, bool useMvc = true) + { + DisableDetailedErrorsIfProduction(app); + LogContextGraphValidations(app); + + app.UseMiddleware(); + + 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(); - - if (useMvc) - app.UseMvc(); + private static void LogContextGraphValidations(IApplicationBuilder app) + { + var logger = app.ApplicationServices.GetService(typeof(ILogger)) 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)); + } } } } diff --git a/src/JsonApiDotNetCore/Internal/ContextGraph.cs b/src/JsonApiDotNetCore/Internal/ContextGraph.cs index d21231f067..c27a01b7d8 100644 --- a/src/JsonApiDotNetCore/Internal/ContextGraph.cs +++ b/src/JsonApiDotNetCore/Internal/ContextGraph.cs @@ -4,25 +4,47 @@ namespace JsonApiDotNetCore.Internal { + public interface IContextGraph + { + object GetRelationship(TParent entity, string relationshipName); + string GetRelationshipName(string relationshipName); + ContextEntity GetContextEntity(string dbSetName); + ContextEntity GetContextEntity(Type entityType); + bool UsesDbContext { get; } + } + public class ContextGraph : IContextGraph { - private List _entities; + internal List Entities { get; } + internal List ValidationResults { get; } public ContextGraph() { } public ContextGraph(List entities, bool usesDbContext) { - _entities = entities; + Entities = entities; + UsesDbContext = usesDbContext; + ValidationResults = new List(); + } + + // 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 entities, bool usesDbContext, List 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 entity, string relationshipName) { @@ -41,7 +63,7 @@ public object GetRelationship(TParent entity, string relationshipName) public string GetRelationshipName(string relationshipName) { var entityType = typeof(TParent); - return _entities + return Entities .SingleOrDefault(e => e.EntityType == entityType) ?.Relationships .SingleOrDefault(r => r.Is(relationshipName)) diff --git a/src/JsonApiDotNetCore/Internal/IContextGraph.cs b/src/JsonApiDotNetCore/Internal/IContextGraph.cs deleted file mode 100644 index 5aa05bdacd..0000000000 --- a/src/JsonApiDotNetCore/Internal/IContextGraph.cs +++ /dev/null @@ -1,13 +0,0 @@ -using System; - -namespace JsonApiDotNetCore.Internal -{ - public interface IContextGraph - { - object GetRelationship(TParent entity, string relationshipName); - string GetRelationshipName(string relationshipName); - ContextEntity GetContextEntity(string dbSetName); - ContextEntity GetContextEntity(Type entityType); - bool UsesDbContext { get; } - } -} diff --git a/src/JsonApiDotNetCore/Internal/ValidationResults.cs b/src/JsonApiDotNetCore/Internal/ValidationResults.cs new file mode 100644 index 0000000000..fbaa6eb462 --- /dev/null +++ b/src/JsonApiDotNetCore/Internal/ValidationResults.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; } + } +} diff --git a/test/UnitTests/Internal/ContextGraphBuilder_Tests.cs b/test/UnitTests/Internal/ContextGraphBuilder_Tests.cs new file mode 100644 index 0000000000..ce8316b89d --- /dev/null +++ b/test/UnitTests/Internal/ContextGraphBuilder_Tests.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(); + 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(); + 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 Foos { get; set; } + } + } + +} From 5e6ccf67d4bccebebb6858fcae2634c53ac36644 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 4 Apr 2018 15:51:18 -0500 Subject: [PATCH 2/2] chore(csproj): bump package version to 2.2.1 --- src/JsonApiDotNetCore/JsonApiDotNetCore.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj index 5aa6b6b897..c4b9a6d932 100755 --- a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj +++ b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj @@ -1,6 +1,6 @@  - 2.2.0 + 2.2.1 $(NetStandardVersion) JsonApiDotNetCore JsonApiDotNetCore