From efe757848e0a851eb86b047c13b098dd2fe5a25f Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Fri, 21 Feb 2020 14:24:03 +0100 Subject: [PATCH 1/4] Changed TypeLocator to use ConcurrentDictionary for its cache Also made ResourceDescriptor immutable, to prevent changing the shared empty instance. --- .../Graph/ResourceDescriptor.cs | 6 +++--- src/JsonApiDotNetCore/Graph/TypeLocator.cs | 17 ++++++----------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/JsonApiDotNetCore/Graph/ResourceDescriptor.cs b/src/JsonApiDotNetCore/Graph/ResourceDescriptor.cs index 2cb1a8b812..aac9824c56 100644 --- a/src/JsonApiDotNetCore/Graph/ResourceDescriptor.cs +++ b/src/JsonApiDotNetCore/Graph/ResourceDescriptor.cs @@ -10,9 +10,9 @@ public ResourceDescriptor(Type resourceType, Type idType) IdType = idType; } - public Type ResourceType { get; set; } - public Type IdType { get; set; } + public Type ResourceType { get; } + public Type IdType { get; } - internal static ResourceDescriptor Empty => new ResourceDescriptor(null, null); + internal static ResourceDescriptor Empty { get; } = new ResourceDescriptor(null, null); } } diff --git a/src/JsonApiDotNetCore/Graph/TypeLocator.cs b/src/JsonApiDotNetCore/Graph/TypeLocator.cs index d2a5bf1840..2f876c3083 100644 --- a/src/JsonApiDotNetCore/Graph/TypeLocator.cs +++ b/src/JsonApiDotNetCore/Graph/TypeLocator.cs @@ -1,6 +1,7 @@ -using JsonApiDotNetCore.Extensions; +using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Models; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -12,9 +13,7 @@ namespace JsonApiDotNetCore.Graph /// static class TypeLocator { - private static Dictionary _typeCache = new Dictionary(); - private static Dictionary> _identifiableTypeCache = new Dictionary>(); - + private static readonly ConcurrentDictionary> _identifiableTypeCache = new ConcurrentDictionary>(); /// /// Determine whether or not this is a json:api resource by checking if it implements . @@ -30,20 +29,16 @@ public static Type GetIdType(Type resourceType) /// Get all implementations of in the assembly /// public static IEnumerable GetIdentifiableTypes(Assembly assembly) - => (_identifiableTypeCache.TryGetValue(assembly, out var descriptors) == false) - ? FindIdentifiableTypes(assembly) - : _identifiableTypeCache[assembly]; + { + return _identifiableTypeCache.GetOrAdd(assembly, asm => FindIdentifiableTypes(asm).ToList()); + } private static IEnumerable FindIdentifiableTypes(Assembly assembly) { - var descriptors = new List(); - _identifiableTypeCache[assembly] = descriptors; - foreach (var type in assembly.GetTypes()) { if (TryGetResourceDescriptor(type, out var descriptor)) { - descriptors.Add(descriptor); yield return descriptor; } } From 3a210339ad1e38f5a9707ba9c917aec3e278e539 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Tue, 10 Mar 2020 15:14:52 +0100 Subject: [PATCH 2/4] Replaced static cache instance with singleton per IoC service collection I just realized that the strange interdependencies when running unit tests sequentially (causing tests to fail in AppVeyor, but not on TravisCI or locally) may actually be caused by the global cache instance. --- Build.ps1 | 16 +------- .../Graph/IdentifiableTypeCache.cs | 39 +++++++++++++++++++ .../Graph/ServiceDiscoveryFacade.cs | 15 ++++--- src/JsonApiDotNetCore/Graph/TypeLocator.cs | 22 ----------- .../Graph/IdentifiableTypeCacheTests.cs | 39 +++++++++++++++++++ test/UnitTests/Graph/TypeLocator_Tests.cs | 27 ------------- 6 files changed, 86 insertions(+), 72 deletions(-) create mode 100644 src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs create mode 100644 test/UnitTests/Graph/IdentifiableTypeCacheTests.cs diff --git a/Build.ps1 b/Build.ps1 index 7cdc82e3d9..aa0773a3a1 100644 --- a/Build.ps1 +++ b/Build.ps1 @@ -27,21 +27,7 @@ CheckLastExitCode dotnet build -c Release CheckLastExitCode -# Workaround: running 'dotnet test -c Release' fails for yet unknown reasons on AppVeyor, so we run tests one by one. - -dotnet test ./test/JsonApiDotNetCoreExampleTests/JsonApiDotNetCoreExampleTests.csproj -c Release --no-build -CheckLastExitCode - -dotnet test ./test/DiscoveryTests/DiscoveryTests.csproj -c Release --no-build -CheckLastExitCode - -dotnet test ./test/IntegrationTests/IntegrationTests.csproj -c Release --no-build -CheckLastExitCode - -dotnet test ./test/UnitTests/UnitTests.csproj -c Release --no-build -CheckLastExitCode - -dotnet test ./test/NoEntityFrameworkTests/NoEntityFrameworkTests.csproj -c Release --no-build +dotnet test -c Release --no-build CheckLastExitCode Write-Output "APPVEYOR_REPO_TAG: $env:APPVEYOR_REPO_TAG" diff --git a/src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs b/src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs new file mode 100644 index 0000000000..8304b79654 --- /dev/null +++ b/src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs @@ -0,0 +1,39 @@ +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using JsonApiDotNetCore.Models; + +namespace JsonApiDotNetCore.Graph +{ + /// + /// Used to cache and locate types, to facilitate auto-resource discovery + /// + internal sealed class IdentifiableTypeCache + { + private readonly IDictionary> _typeCache = new Dictionary>(); + + /// + /// Get all implementations of in the assembly + /// + public IEnumerable GetIdentifiableTypes(Assembly assembly) + { + if (!_typeCache.ContainsKey(assembly)) + { + _typeCache[assembly] = FindIdentifiableTypes(assembly).ToList(); + } + + return _typeCache[assembly]; + } + + private static IEnumerable FindIdentifiableTypes(Assembly assembly) + { + foreach (var type in assembly.GetTypes()) + { + if (TypeLocator.TryGetResourceDescriptor(type, out var descriptor)) + { + yield return descriptor; + } + } + } + } +} diff --git a/src/JsonApiDotNetCore/Graph/ServiceDiscoveryFacade.cs b/src/JsonApiDotNetCore/Graph/ServiceDiscoveryFacade.cs index ae3fef30f6..94da423b00 100644 --- a/src/JsonApiDotNetCore/Graph/ServiceDiscoveryFacade.cs +++ b/src/JsonApiDotNetCore/Graph/ServiceDiscoveryFacade.cs @@ -47,7 +47,7 @@ public class ServiceDiscoveryFacade : IServiceDiscoveryFacade }; private readonly IServiceCollection _services; private readonly IResourceGraphBuilder _resourceGraphBuilder; - private readonly List _identifiables = new List(); + private readonly IdentifiableTypeCache _typeCache = new IdentifiableTypeCache(); public ServiceDiscoveryFacade(IServiceCollection services, IResourceGraphBuilder resourceGraphBuilder) { @@ -68,7 +68,7 @@ public ServiceDiscoveryFacade AddAssembly(Assembly assembly) { AddDbContextResolvers(assembly); - var resourceDescriptors = TypeLocator.GetIdentifiableTypes(assembly); + var resourceDescriptors = _typeCache.GetIdentifiableTypes(assembly); foreach (var resourceDescriptor in resourceDescriptors) { AddResource(assembly, resourceDescriptor); @@ -78,7 +78,6 @@ public ServiceDiscoveryFacade AddAssembly(Assembly assembly) return this; } - public IEnumerable FindDerivedTypes(Type baseType) { return baseType.Assembly.GetTypes().Where(t => @@ -109,9 +108,9 @@ private void AddDbContextResolvers(Assembly assembly) /// The assembly to search for resources in. public ServiceDiscoveryFacade AddResources(Assembly assembly) { - var identifiables = TypeLocator.GetIdentifiableTypes(assembly); - foreach (var identifiable in identifiables) - AddResource(assembly, identifiable); + var resourceDescriptors = _typeCache.GetIdentifiableTypes(assembly); + foreach (var resourceDescriptor in resourceDescriptors) + AddResource(assembly, resourceDescriptor); return this; } @@ -149,7 +148,7 @@ private void AddResourceToGraph(ResourceDescriptor identifiable) /// The assembly to search for resources in. public ServiceDiscoveryFacade AddServices(Assembly assembly) { - var resourceDescriptors = TypeLocator.GetIdentifiableTypes(assembly); + var resourceDescriptors = _typeCache.GetIdentifiableTypes(assembly); foreach (var resourceDescriptor in resourceDescriptors) { AddServices(assembly, resourceDescriptor); @@ -171,7 +170,7 @@ private void AddServices(Assembly assembly, ResourceDescriptor resourceDescripto /// The assembly to search for resources in. public ServiceDiscoveryFacade AddRepositories(Assembly assembly) { - var resourceDescriptors = TypeLocator.GetIdentifiableTypes(assembly); + var resourceDescriptors = _typeCache.GetIdentifiableTypes(assembly); foreach (var resourceDescriptor in resourceDescriptors) { AddRepositories(assembly, resourceDescriptor); diff --git a/src/JsonApiDotNetCore/Graph/TypeLocator.cs b/src/JsonApiDotNetCore/Graph/TypeLocator.cs index 2f876c3083..467fecf5de 100644 --- a/src/JsonApiDotNetCore/Graph/TypeLocator.cs +++ b/src/JsonApiDotNetCore/Graph/TypeLocator.cs @@ -1,7 +1,6 @@ using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Models; using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -13,8 +12,6 @@ namespace JsonApiDotNetCore.Graph /// static class TypeLocator { - private static readonly ConcurrentDictionary> _identifiableTypeCache = new ConcurrentDictionary>(); - /// /// Determine whether or not this is a json:api resource by checking if it implements . /// Returns the status and the resultant id type, either `(true, Type)` OR `(false, null)` @@ -25,25 +22,6 @@ public static Type GetIdType(Type resourceType) return identifiableInterface?.GetGenericArguments()[0]; } - /// - /// Get all implementations of in the assembly - /// - public static IEnumerable GetIdentifiableTypes(Assembly assembly) - { - return _identifiableTypeCache.GetOrAdd(assembly, asm => FindIdentifiableTypes(asm).ToList()); - } - - private static IEnumerable FindIdentifiableTypes(Assembly assembly) - { - foreach (var type in assembly.GetTypes()) - { - if (TryGetResourceDescriptor(type, out var descriptor)) - { - yield return descriptor; - } - } - } - /// /// Attempts to get a descriptor of the resource type. /// diff --git a/test/UnitTests/Graph/IdentifiableTypeCacheTests.cs b/test/UnitTests/Graph/IdentifiableTypeCacheTests.cs new file mode 100644 index 0000000000..a0b4220f75 --- /dev/null +++ b/test/UnitTests/Graph/IdentifiableTypeCacheTests.cs @@ -0,0 +1,39 @@ +using JsonApiDotNetCore.Graph; +using JsonApiDotNetCore.Models; +using UnitTests.Internal; +using Xunit; + +namespace UnitTests.Graph +{ + public class IdentifiableTypeCacheTests + { + [Fact] + public void GetIdentifiableTypes_Locates_Identifiable_Resource() + { + // Arrange + var resourceType = typeof(Model); + var typeCache = new IdentifiableTypeCache(); + + // Act + var results = typeCache.GetIdentifiableTypes(resourceType.Assembly); + + // Assert + Assert.Contains(results, r => r.ResourceType == resourceType); + } + + [Fact] + public void GetIdentifiableTypes_Only_Contains_IIdentifiable_Types() + { + // Arrange + var resourceType = typeof(Model); + var typeCache = new IdentifiableTypeCache(); + + // Act + var resourceDescriptors = typeCache.GetIdentifiableTypes(resourceType.Assembly); + + // Assert + foreach(var resourceDescriptor in resourceDescriptors) + Assert.True(typeof(IIdentifiable).IsAssignableFrom(resourceDescriptor.ResourceType)); + } + } +} diff --git a/test/UnitTests/Graph/TypeLocator_Tests.cs b/test/UnitTests/Graph/TypeLocator_Tests.cs index ca70b32581..4b76b403d1 100644 --- a/test/UnitTests/Graph/TypeLocator_Tests.cs +++ b/test/UnitTests/Graph/TypeLocator_Tests.cs @@ -81,33 +81,6 @@ public void GetIdType_Correctly_Identifies_NonJsonApiResource() Assert.Equal(expectedIdType, idType); } - [Fact] - public void GetIdentifiableTypes_Locates_Identifiable_Resource() - { - // Arrange - var resourceType = typeof(Model); - - // Act - var results = TypeLocator.GetIdentifiableTypes(resourceType.Assembly); - - // Assert - Assert.Contains(results, r => r.ResourceType == resourceType); - } - - [Fact] - public void GetIdentifiableTypes_Only_Contains_IIdentifiable_Types() - { - // Arrange - var resourceType = typeof(Model); - - // Act - var resourceDescriptors = TypeLocator.GetIdentifiableTypes(resourceType.Assembly); - - // Assert - foreach(var resourceDescriptor in resourceDescriptors) - Assert.True(typeof(IIdentifiable).IsAssignableFrom(resourceDescriptor.ResourceType)); - } - [Fact] public void TryGetResourceDescriptor_Returns_True_If_Type_Is_IIdentifiable() { From 441aaa6f728247935f0ffb9105bce55a77f778da Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Tue, 10 Mar 2020 15:26:58 +0100 Subject: [PATCH 3/4] Nope, that did not resolve the CI build issue. But it was worth a try, though. Restored build script and re-added concurrent cache for running tests in parallel. --- Build.ps1 | 18 ++++++++++++++++-- .../Graph/IdentifiableTypeCache.cs | 10 +++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Build.ps1 b/Build.ps1 index aa0773a3a1..33cc65f4d9 100644 --- a/Build.ps1 +++ b/Build.ps1 @@ -27,7 +27,21 @@ CheckLastExitCode dotnet build -c Release CheckLastExitCode -dotnet test -c Release --no-build +# Workaround: running 'dotnet test -c Release' fails for yet unknown reasons on AppVeyor, so we run tests one by one. + +dotnet test ./test/JsonApiDotNetCoreExampleTests/JsonApiDotNetCoreExampleTests.csproj -c Release --no-build +CheckLastExitCode + +dotnet test ./test/DiscoveryTests/DiscoveryTests.csproj -c Release --no-build +CheckLastExitCode + +dotnet test ./test/IntegrationTests/IntegrationTests.csproj -c Release --no-build +CheckLastExitCode + +dotnet test ./test/UnitTests/UnitTests.csproj -c Release --no-build +CheckLastExitCode + +dotnet test ./test/NoEntityFrameworkTests/NoEntityFrameworkTests.csproj -c Release --no-build CheckLastExitCode Write-Output "APPVEYOR_REPO_TAG: $env:APPVEYOR_REPO_TAG" @@ -52,4 +66,4 @@ Else { Write-Output "RUNNING dotnet pack .\src\JsonApiDotNetCore -c Release -o .\artifacts --version-suffix=alpha1-$revision" dotnet pack .\src\JsonApiDotNetCore -c Release -o .\artifacts --version-suffix=alpha1-$revision --include-symbols CheckLastExitCode -} +} \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs b/src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs index 8304b79654..cc45386e9e 100644 --- a/src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs +++ b/src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -10,19 +11,14 @@ namespace JsonApiDotNetCore.Graph /// internal sealed class IdentifiableTypeCache { - private readonly IDictionary> _typeCache = new Dictionary>(); + private readonly ConcurrentDictionary> _typeCache = new ConcurrentDictionary>(); /// /// Get all implementations of in the assembly /// public IEnumerable GetIdentifiableTypes(Assembly assembly) { - if (!_typeCache.ContainsKey(assembly)) - { - _typeCache[assembly] = FindIdentifiableTypes(assembly).ToList(); - } - - return _typeCache[assembly]; + return _typeCache.GetOrAdd(assembly, asm => FindIdentifiableTypes(asm).ToList()); } private static IEnumerable FindIdentifiableTypes(Assembly assembly) From 8dfc220021cc9d648595050cdb36bdd9a8b6f419 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Tue, 10 Mar 2020 15:48:55 +0100 Subject: [PATCH 4/4] Empty commit to restart TravisCI