Changed TypeLocator to use ConcurrentDictionary for its cache (#700)

* Changed TypeLocator to use ConcurrentDictionary for its cache
Also made ResourceDescriptor immutable, to prevent changing the shared empty instance.

* 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.

* 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.

* Empty commit to restart TravisCI

Author: Bart Koelman

Build.ps1

@@ -66,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
-}
+}

src/JsonApiDotNetCore/Graph/IdentifiableTypeCache.cs

@@ -0,0 +1,35 @@
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+using JsonApiDotNetCore.Models;
+
+namespace JsonApiDotNetCore.Graph
+{
+    /// <summary>
+    /// Used to cache and locate types, to facilitate auto-resource discovery
+    /// </summary>
+    internal sealed class IdentifiableTypeCache
+    {
+        private readonly ConcurrentDictionary<Assembly, List<ResourceDescriptor>> _typeCache = new ConcurrentDictionary<Assembly, List<ResourceDescriptor>>();
+
+        /// <summary>
+        /// Get all implementations of <see cref="IIdentifiable"/> in the assembly
+        /// </summary>
+        public IEnumerable<ResourceDescriptor> GetIdentifiableTypes(Assembly assembly)
+        {
+            return _typeCache.GetOrAdd(assembly, asm => FindIdentifiableTypes(asm).ToList());
+        }
+
+        private static IEnumerable<ResourceDescriptor> FindIdentifiableTypes(Assembly assembly)
+        {
+            foreach (var type in assembly.GetTypes())
+            {
+                if (TypeLocator.TryGetResourceDescriptor(type, out var descriptor))
+                {
+                    yield return descriptor;
+                }
+            }
+        }
+    }
+}

src/JsonApiDotNetCore/Graph/ResourceDescriptor.cs

@@ -13,6 +13,6 @@ public ResourceDescriptor(Type resourceType, Type idType)
         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);
     }
 }

src/JsonApiDotNetCore/Graph/ServiceDiscoveryFacade.cs

@@ -47,6 +47,7 @@ public class ServiceDiscoveryFacade : IServiceDiscoveryFacade
         };
         private readonly IServiceCollection _services;
         private readonly IResourceGraphBuilder _resourceGraphBuilder;
+        private readonly IdentifiableTypeCache _typeCache = new IdentifiableTypeCache();
 
         public ServiceDiscoveryFacade(IServiceCollection services, IResourceGraphBuilder resourceGraphBuilder)
         {
@@ -67,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);
@@ -77,7 +78,6 @@ public ServiceDiscoveryFacade AddAssembly(Assembly assembly)
             return this;
         }
 
-
         public IEnumerable<Type> FindDerivedTypes(Type baseType)
         {
             return baseType.Assembly.GetTypes().Where(t =>
@@ -108,9 +108,9 @@ private void AddDbContextResolvers(Assembly assembly)
         /// <param name="assembly">The assembly to search for resources in.</param>
         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;
         }
@@ -148,7 +148,7 @@ private void AddResourceToGraph(ResourceDescriptor identifiable)
         /// <param name="assembly">The assembly to search for resources in.</param>
         public ServiceDiscoveryFacade AddServices(Assembly assembly)
         {
-            var resourceDescriptors = TypeLocator.GetIdentifiableTypes(assembly);
+            var resourceDescriptors = _typeCache.GetIdentifiableTypes(assembly);
             foreach (var resourceDescriptor in resourceDescriptors)
             {
                 AddServices(assembly, resourceDescriptor);
@@ -170,7 +170,7 @@ private void AddServices(Assembly assembly, ResourceDescriptor resourceDescripto
         /// <param name="assembly">The assembly to search for resources in.</param>
         public ServiceDiscoveryFacade AddRepositories(Assembly assembly)
         {
-            var resourceDescriptors = TypeLocator.GetIdentifiableTypes(assembly);
+            var resourceDescriptors = _typeCache.GetIdentifiableTypes(assembly);
             foreach (var resourceDescriptor in resourceDescriptors)
             {
                 AddRepositories(assembly, resourceDescriptor);
@@ -186,7 +186,7 @@ private void AddRepositories(Assembly assembly, ResourceDescriptor resourceDescr
                 RegisterServiceImplementations(assembly, serviceInterface, resourceDescriptor);
             }
         }
-        
+
         private void RegisterServiceImplementations(Assembly assembly, Type interfaceType, ResourceDescriptor resourceDescriptor)
         {
             if (resourceDescriptor.IdType == typeof(Guid) && interfaceType.GetTypeInfo().GenericTypeParameters.Length == 1)

src/JsonApiDotNetCore/Graph/TypeLocator.cs

@@ -1,4 +1,4 @@
-using JsonApiDotNetCore.Extensions;
+using JsonApiDotNetCore.Extensions;
 using JsonApiDotNetCore.Models;
 using System;
 using System.Collections.Generic;
@@ -12,8 +12,6 @@ namespace JsonApiDotNetCore.Graph
     /// </summary>
     internal static class TypeLocator
     {
-        private static readonly Dictionary<Assembly, List<ResourceDescriptor>> _identifiableTypeCache = new Dictionary<Assembly, List<ResourceDescriptor>>();
-
         /// <summary>
         /// Determine whether or not this is a json:api resource by checking if it implements <see cref="IIdentifiable"/>.
         /// Returns the status and the resultant id type, either `(true, Type)` OR `(false, null)`
@@ -24,29 +22,6 @@ public static Type GetIdType(Type resourceType)
             return identifiableInterface?.GetGenericArguments()[0];
         }
 
-        /// <summary>
-        /// Get all implementations of <see cref="IIdentifiable"/> in the assembly
-        /// </summary>
-        public static IEnumerable<ResourceDescriptor> GetIdentifiableTypes(Assembly assembly)
-            => (_identifiableTypeCache.TryGetValue(assembly, out _) == false)
-                    ? FindIdentifiableTypes(assembly)
-                    : _identifiableTypeCache[assembly];
-
-        private static IEnumerable<ResourceDescriptor> FindIdentifiableTypes(Assembly assembly)
-        {
-            var descriptors = new List<ResourceDescriptor>();
-            _identifiableTypeCache[assembly] = descriptors;
-
-            foreach (var type in assembly.GetTypes())
-            {
-                if (TryGetResourceDescriptor(type, out var descriptor))
-                {
-                    descriptors.Add(descriptor);
-                    yield return descriptor;
-                }
-            }
-        }
-
         /// <summary>
         /// Attempts to get a descriptor of the resource type.
         /// </summary>

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

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()
         {