Fix: bug in custom cachekeyfactory

thanks to @wros for reporting the bug and suggesting the fix

closes #109

docs/EFCore.md

@@ -1,11 +1,13 @@
-### Introduction
+# Data Isolation with Entity Framework Core
+
+## Introduction
 Data isolation is one of the most important considerations in a multitenant app. Whether each tenant has its own database, a shared database, or a hybrid approach can make a significant different in app design. Finbuckle.MultiTenant supports each of these models by associating a connection string with each tenant. Tenant's using the same connection string will share a database and accordingly those with a unique connection string will have separate databases.
 
-In shared database scenarios it is important to make sure that queries and commands for a tenant do not affect the data belonging to other tenant's. Finbuckle.MultiTenant handles this automatically and removes the need to sprinkle "where" clauses all over an app. Applying the `MultiTenant` data attribute to an entity and using the `MultiTenantDbContext` as a base for class for an app's own database context tells Finbuckle.MultiTenant to ensure isolation of both queries and create/update/delete commands.
+In shared database scenarios it is important to make sure that queries and commands for a tenant do not affect the data belonging to other tenant's. Finbuckle.MultiTenant handles this automatically and removes the need to sprinkle "where" clauses all over an app. Applying the `[MultiTenant]` attribute to an entity and using the `MultiTenantDbContext` as a base for class for an app's own database context tells Finbuckle.MultiTenant to ensure isolation of both queries and create/update/delete commands.
 
 Internally Finbuckle.MultiTenant uses the `HasQueryFilter` function to set a filter on `TenantId` for the current tenant for all queries. For create/update/delete commands the framework checks entities during `SaveChanges` or `SaveChangesAsync` to ensure matches. This behavior can be modified as documented below.
 
-### Configuration
+## Configuration
 Add the `Finbuckle.MultiTenant.EntityFrameworkCore` package to the project:
 ```{.bash}
 dotnet add package Finbuckle.MultiTenant.EntityFrameworkCore
@@ -44,7 +46,7 @@ public class BloggingDbContext : MultiTenantDbContext
 
 >{.small} If the derived database context overrides OnModelCreating is it critical that the base class OnModelCreating is called.
 
-Finally, add the `[MultiTenant]` data annotation to entity classes which should be isolated per tenant. If an entity is common to all tenants, then do not apply the attribute:
+Finally, add the `[MultiTenant]` attribute to entity classes which should be isolated per tenant. If an entity is common to all tenants, then do not apply the attribute:
 
 ```
 [MultiTenant]
@@ -62,7 +64,7 @@ public class Post
 
 When the context is initialized, a shadow property named `TenantId` is added to the data model for these classes. This property is used internally to filter all requests and commands. If there already is a defined string property named "TenantId" then Finbuckle.Multitenant will use the existing property.
 
-### Configuring with ASP.NET Core
+## Configuring with ASP.NET Core
 
 If using ASP.NET Core [configure Finbuckle.MultiTenant](GettingStarted) as desired.
 
@@ -86,9 +88,9 @@ public class Startup
 }
 ```
 
->{.small} Do not use any of the configuration methods that sets a database provider or connection string if using the `AddDbContext` delegate overload—the delegate will not have access to the current `TenantContext` or its connection string.
+Do not use any of the configuration methods that sets a database provider or connection string if using the `AddDbContext` delegate overload—the delegate will not have access to the current `TenantContext` or its connection string.
 
-### Adding Data
+## Adding Data
 Added entities are automatically associated with the current `TenantContext`. If an entity is associated with a different `TenantContext` then a `MultiTenantException` is thrown in `SaveChanges` or `SaveChangesAsync`:
 
 ```
@@ -105,7 +107,7 @@ db.Blogs.Add(myBlog);
 await db.SaveChangesAsync(); // Throws MultiTenantException.
 ```
 
-### Querying Data
+## Querying Data
 Queries only return results associated to the `TenantContext`:
 
 ```
@@ -126,9 +128,9 @@ var db = new BloggingDbContext(myTenantContext, null);
 var tenantBlogs = db.Blogs.IgnoreQueryFilters().ToList(); 
 ```
 
-The query filter is applied only at the root level of a query. Any entity classes loaded via `Include` or `ThenInclude` are not filtered, but if all entity classes involved in a query have the `MultiTenant` data annotation then all results are associated to the same `TenantContext`.
+The query filter is applied only at the root level of a query. Any entity classes loaded via `Include` or `ThenInclude` are not filtered, but if all entity classes involved in a query have the `[MultiTenant]` attribute then all results are associated to the same tenant.
 
-### Updating and Deleting Data
+## Updating and Deleting Data
 Updated or deleted entities are checked to make sure they are associated with the `TenantContext`. If an entity is associated with a different `TenantContext` then a `MultiTenantException` is thrown in `SaveChanges` or `SaveChangesAsync`:
 
 ```
@@ -148,7 +150,7 @@ db.Blogs.Remove(myBlog);
 await db.SaveChangesAsync(); // Throws MultiTenantException.
 ```
 
-### Tenant Mismatch Mode
+## Tenant Mismatch Mode
 
 Normally Finbuckle.MultiTenant will automatically coordinate the `TenantId` property of each entity. However in certain situations the `TenantId` can be manually set.
 
@@ -158,7 +160,7 @@ By default attempting to add or update an entity with a different `TenantId` pro
 * TenantMismatchMode.Ignore - The entity is added or updated without modifying its `TenantId`.
 * TenantMismatchMode.Overwrite - The entity's `TenantId` is overwritten to match the database context's current `TenantContext`.
 
-### Tenant Not Set Mode
+## Tenant Not Set Mode
 
 If the `TenantId` on an entity is manually set to null the default behavior is to overwrite the `TenantId` for adde entities or to throw a `MultiTenantException` for updated entities. This occurs during a call to `SaveChanges` or `SaveChangesAsync`. This behavior can be changed by setting the `TenantNotSetMode' property on the database context:
 

docs/Identity.md

@@ -1,10 +1,12 @@
-### Introduction
+# Data Isolation with ASP.NET Core Identity
 
-Finbuckle.MultiTenant has limited support for data isolation with ASP.Net Core Identity. It works similarly to [normal Finbuckle.Multitenant Entity Framework Core data isolation](EFCore) except the database context derives from `MultiTenantIdentityDbContext<TUser>` instead of `MultiTenantDbContext`.
+## Introduction
+
+Finbuckle.MultiTenant has limited support for data isolation with ASP.NET Core Identity when Entity Framework Core is used as the backing store. It works similarly to [normal Finbuckle.Multitenant Entity Framework Core data isolation](EFCore) except the database context derives from `MultiTenantIdentityDbContext<TUser>` instead of `MultiTenantDbContext`.
 
 See the[IdentityDataIsolationSample](https://github.com/Finbuckle/Finbuckle.MultiTenant/tree/master/samples/IdentityDataIsolationSample) project for a comprehensive example on how to use Finbuckle.MultiTenant with ASP.NET Core Identity. This sample illustrates how to isolate the tenant Identity data and integrate the Identity UI to work with a route multitenant strategy.
 
-### Configuration
+## Configuration
 Add the `Finbuckle.MultiTenant.EntityFrameworkCore` and package to the project:
 ```{.bash}
 dotnet add package Finbuckle.MultiTenant.EntityFrameworkCore
@@ -24,7 +26,7 @@ public class MyIdentityDbContext : MultiTenantIdentityDbContext<appUser>
 
 >{.small} `TUser` must derive from `IdentityUser` which uses a string for its primary key.
 
-Add the `[MultiTenant]` data annotation to the User entity classes:
+Add the `[MultiTenant]` attribute to the User entity classes:
 
 ```
 [MultiTenant]
@@ -36,12 +38,43 @@ public class appUser : IdentityUser
 
 ASP.NET Core Identity class methods on `UserManager<TUser>` or `UserStore<TUser>` that search for a specific user will be isolated to users of the current tenant, with the exception of `FindByIdAsync` which will search users of all tenants.
 
-### Identity Options
+## Identity Options
 
 Many identity options will be limited to the current tenant. For example, the option to require a unique email address per user will only require that an email be unique within the users for the current tenant. The exception is any option that internally relies on `UserManager<TUser>.FindByIdAsync`.
 
-### Authentication
-
+## Authentication
 Internally, ASP.NET Core Identity uses regular ASP.NET Core authentication. It uses a [slightly different method for configuring cookies](https://docs.microsoft.com/en-us/aspnet/core/security/authentication/identity-configuration), but under the hood the end result is the same in that `CookieAuthenticationOptions` are being configured and consumed.
 
 Finbuckle.Multitenant can customize these options per tenant so that user sessions are unique per tenant. See [per-tenant cookie authentication options](Authentication#cookie-authentication-options) for information on how to customize authentication options per tenant.
+
+## Support for Identity Model Types
+The [ASP.NET Core Identity data model](https://docs.microsoft.com/en-us/aspnet/core/security/authentication/customize-identity-model?view=aspnetcore-2.2#the-identity-model) relies on several types which are passed to the database context as generic parameters: 
+- `TUser`
+- `TRole`
+- `TKey`
+- `TUserClaim`
+- `TUserToken`
+- `TUserLogin`
+- `TRoleClaim`
+- `TUserRole`
+
+Default classes exist such as the `IdentityUser`, `IdentityRole`, and `IdentityUserClaim`, which are commonly used as the generic parameters. The default for `TKey` is `string`. Apps can provide their own classes for any of these by using alternative forms of the database context which take varying number of generic type parameters. Simple use-cases derive from `IdentityDbContext` classes which require only a few generic parameters and plug in the default classes for the rest.
+
+Finbuckle.MultiTenant supports this approach by providing classes derived from each default model class with the `[MultiTenant]` attribute applied to them. These classes are:
+- `MultiTenantIdentityUser`
+- `MultiTenantIdentityRole`
+- `MultiTenantIdentityUserClaim`
+- `MultiTenantIdentityUserToken`
+- `MultiTenantIdentityUserLogin`
+- `MultiTenantIdentityRoleClaim`
+- `MultiTenantIDentityUserRole`
+
+Deriving an Identity database context from `MultiTenantIdentityDbContext` will use all of the default classes and `string` for `TKey`.
+
+Deriving from `MultiTenantIdentityDbContext<TUser>` will use the provided parameter for `TUser` and the defaults for the rest.
+
+Deriving from `MultiTenantIdentityDbContext<TUser, TRole, TKey>` will use the provided parameters for `<TUser>`, `TRole`, and `TKey` and the defaults for the rest.
+
+Deriving from `MultiTenantIdentityDbContext<TUser, TRole, TKey, TUserClaim, TUserRole, TUserLogin, TRoleClaim, TUserToken>` will only use provided parameters.
+
+When providing non-default parameters it is recommended that provided the classes have the `[MultiTenant]` attribute or derive from a type with the attribute.

src/Finbuckle.MultiTenant.EntityFrameworkCore/Internal/MultiTenantModelCacheKeyFactory.cs

@@ -39,7 +39,9 @@ private bool ContextDerivesFromMultiTenantIdentityDbContext(DbContext context)
             while (context != null && currentType != typeof(object))
             {
                 if (currentType.IsGenericType &&
-                    currentType.GetGenericTypeDefinition() == typeof(MultiTenantIdentityDbContext<>))
+                    (currentType.GetGenericTypeDefinition() == typeof(MultiTenantIdentityDbContext<>) ||
+                    currentType.GetGenericTypeDefinition() == typeof(MultiTenantIdentityDbContext<,,>) ||
+                    currentType.GetGenericTypeDefinition() == typeof(MultiTenantIdentityDbContext<,,,,,,,>)))
                 {
                     return true;
                 }

src/Finbuckle.MultiTenant.EntityFrameworkCore/MultiTenantIdentityDbContext.cs

@@ -40,7 +40,7 @@ protected MultiTenantIdentityDbContext(TenantInfo tenantInfo, DbContextOptions o
     }
 
     /// <summary>
-    /// A database context compatiable with Identity that enforces tenant integrity on entity types
+    /// A database context compatible with Identity that enforces tenant integrity on entity types
     /// marked with the MultiTenant attribute.
     /// </summary>
     public abstract class MultiTenantIdentityDbContext<TUser> : MultiTenantIdentityDbContext<TUser, MultiTenantIdentityRole, string>

test/Finbuckle.MultiTenant.EntityFrameworkCore.Test/MultiTenantModelCacheKeyFactoryShould.cs

@@ -30,13 +30,6 @@ public TestMultiTenantDbContext(TenantInfo tenantInfo, DbContextOptions options)
         }
     }
 
-    public class TestMultiTenantIdentityDbContext : MultiTenantIdentityDbContext
-    {
-        public TestMultiTenantIdentityDbContext(TenantInfo tenantInfo, DbContextOptions options) : base(tenantInfo, options)
-        {
-        }
-    }
-
     [Fact]
     public void ReturnTypeForNonMultiTenantDbContext()
     {
@@ -67,14 +60,60 @@ public void ReturnTypePlusTenantIdForMultiTenantDbContext()
     public void ReturnTypePlusTenantIdForMultiTenantIdentityDbContext()
     {
         var factory = new MultiTenantModelCacheKeyFactory();
-        var dbContext = new TestMultiTenantIdentityDbContext(
+        var dbContext = new TestIdentityDbContext(
+            new TenantInfo("test", null, null, null, null),
+            new DbContextOptions<TestIdentityDbContext>());
+
+        dynamic key = factory.Create(dbContext);
+
+        Assert.IsType<(Type, string)>(key);
+        Assert.Equal(typeof(TestIdentityDbContext), key.Item1);
+        Assert.Equal("test", key.Item2);
+    }
+
+    [Fact]
+    public void ReturnTypePlusTenantIdForMultiTenantIdentityDbContext_TUser()
+    {
+        // Test that it works for the MultiTenantIdentityDbContext<TUser>
+        var factory = new MultiTenantModelCacheKeyFactory();
+        var dbContext = new TestIdentityDbContext_TUser(
+            new TenantInfo("test", null, null, null, null),
+            new DbContextOptions<TestIdentityDbContext_TUser>());
+
+        dynamic key = factory.Create(dbContext);
+
+        Assert.IsType<(Type, string)>(key);
+        Assert.Equal(typeof(TestIdentityDbContext_TUser), key.Item1);
+        Assert.Equal("test", key.Item2);
+    }
+
+    [Fact]
+    public void ReturnTypePlusTenantIdForMultiTenantIdentityDbContext_TUser_TRole_String()
+    {
+        var factory = new MultiTenantModelCacheKeyFactory();
+        var dbContext = new TestIdentityDbContext_TUser_TRole_String(
+            new TenantInfo("test", null, null, null, null),
+            new DbContextOptions<TestIdentityDbContext_TUser_TRole_String>());
+
+        dynamic key = factory.Create(dbContext);
+
+        Assert.IsType<(Type, string)>(key);
+        Assert.Equal(typeof(TestIdentityDbContext_TUser_TRole_String), key.Item1);
+        Assert.Equal("test", key.Item2);
+    }
+
+    [Fact]
+    public void ReturnTypePlusTenantIdForMultiTenantIdentityDbContext_AllGenericParams()
+    {
+        var factory = new MultiTenantModelCacheKeyFactory();
+        var dbContext = new TestIdentityDbContext_AllGenericParams(
             new TenantInfo("test", null, null, null, null),
-            new DbContextOptions<TestMultiTenantIdentityDbContext>());
+            new DbContextOptions<TestIdentityDbContext_AllGenericParams>());
 
         dynamic key = factory.Create(dbContext);
 
         Assert.IsType<(Type, string)>(key);
-        Assert.Equal(typeof(TestMultiTenantIdentityDbContext), key.Item1);
+        Assert.Equal(typeof(TestIdentityDbContext_AllGenericParams), key.Item1);
         Assert.Equal("test", key.Item2);
     }
 }

test/Finbuckle.MultiTenant.EntityFrameworkCore.Test/TestEntitities.cs

@@ -128,14 +128,53 @@ public class ThingWithHigherTenantIdMaxLength
 }
 
 public class TestIdentityDbContext : MultiTenantIdentityDbContext
+{
+    public TestIdentityDbContext(TenantInfo tenantContext)
+        : base(tenantContext)
+    {
+    }
+
+    public TestIdentityDbContext(TenantInfo tenantContext, DbContextOptions options)
+        : base(tenantContext, options)
     {
-        public TestIdentityDbContext(TenantInfo tenantContext)
-            : base(tenantContext)
-        {
-        }
-
-        public TestIdentityDbContext(TenantInfo tenantContext, DbContextOptions options)
-            : base(tenantContext, options)
-        {
-        }
-    }
+    }
+}
+
+public class TestIdentityDbContext_TUser : MultiTenantIdentityDbContext<MultiTenantIdentityUser>
+{
+    public TestIdentityDbContext_TUser(TenantInfo tenantContext)
+        : base(tenantContext)
+    {
+    }
+
+    public TestIdentityDbContext_TUser(TenantInfo tenantContext, DbContextOptions options)
+        : base(tenantContext, options)
+    {
+    }
+}
+
+public class TestIdentityDbContext_TUser_TRole_String : MultiTenantIdentityDbContext<MultiTenantIdentityUser, MultiTenantIdentityRole, string>
+{
+    public TestIdentityDbContext_TUser_TRole_String(TenantInfo tenantContext)
+        : base(tenantContext)
+    {
+    }
+
+    public TestIdentityDbContext_TUser_TRole_String(TenantInfo tenantContext, DbContextOptions options)
+        : base(tenantContext, options)
+    {
+    }
+}
+
+public class TestIdentityDbContext_AllGenericParams : MultiTenantIdentityDbContext<MultiTenantIdentityUser, MultiTenantIdentityRole, string, MultiTenantIdentityUserClaim<string>, MultiTenantIdentityUserRole<string>, MultiTenantIdentityUserLogin<string>, MultiTenantIdentityRoleClaim<string>, MultiTenantIdentityUserToken<string>>
+{
+    public TestIdentityDbContext_AllGenericParams(TenantInfo tenantContext)
+        : base(tenantContext)
+    {
+    }
+
+    public TestIdentityDbContext_AllGenericParams(TenantInfo tenantContext, DbContextOptions options)
+        : base(tenantContext, options)
+    {
+    }
+}