From affb66f268a133916c17c7797138cd16dc67e728 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Thu, 24 Aug 2023 19:09:12 -0600 Subject: [PATCH] feat: AdjustIndex preserves existing filter (#711) * perf: reduce enumeration in MultiTenantDbContext logic * feat: AdjustIndex preserves existing filter --- .../IMultiTenantDbContextExtensions.cs | 28 ++-- .../MultiTenantEntityTypeBuilder.cs | 5 + .../MultiTenantEntityTypeBuilderShould.cs | 142 ++++++++++-------- .../TenantInfoShould.cs | 8 - 4 files changed, 96 insertions(+), 87 deletions(-) diff --git a/src/Finbuckle.MultiTenant.EntityFrameworkCore/Extensions/IMultiTenantDbContextExtensions.cs b/src/Finbuckle.MultiTenant.EntityFrameworkCore/Extensions/IMultiTenantDbContextExtensions.cs index b0b8ab2c..bafd067c 100644 --- a/src/Finbuckle.MultiTenant.EntityFrameworkCore/Extensions/IMultiTenantDbContextExtensions.cs +++ b/src/Finbuckle.MultiTenant.EntityFrameworkCore/Extensions/IMultiTenantDbContextExtensions.cs @@ -21,7 +21,7 @@ public static class IMultiTenantDbContextExtensions var changedMultiTenantEntities = changeTracker.Entries(). Where(e => e.State == EntityState.Added || e.State == EntityState.Modified || e.State == EntityState.Deleted). - Where(e => e.Metadata.IsMultiTenant()); + Where(e => e.Metadata.IsMultiTenant()).ToList(); // ensure tenant context is valid if (changedMultiTenantEntities.Any()) @@ -32,19 +32,19 @@ public static class IMultiTenantDbContextExtensions // get list of all added entities with MultiTenant annotation var addedMultiTenantEntities = changedMultiTenantEntities. - Where(e => e.State == EntityState.Added); + Where(e => e.State == EntityState.Added).ToList(); // handle Tenant Id mismatches for added entities var mismatchedAdded = addedMultiTenantEntities. Where(e => (string?)e.Property("TenantId").CurrentValue != null && - (string?)e.Property("TenantId").CurrentValue != tenantInfo.Id); + (string?)e.Property("TenantId").CurrentValue != tenantInfo.Id).ToList(); if (mismatchedAdded.Any()) { switch (tenantMismatchMode) { case TenantMismatchMode.Throw: - throw new MultiTenantException($"{mismatchedAdded.Count()} added entities with Tenant Id mismatch."); ; + throw new MultiTenantException($"{mismatchedAdded.Count} added entities with Tenant Id mismatch."); case TenantMismatchMode.Ignore: // no action needed @@ -70,19 +70,19 @@ public static class IMultiTenantDbContextExtensions // get list of all modified entities with MultiTenant annotation var modifiedMultiTenantEntities = changedMultiTenantEntities. - Where(e => e.State == EntityState.Modified); + Where(e => e.State == EntityState.Modified).ToList(); // handle Tenant Id mismatches for modified entities var mismatchedModified = modifiedMultiTenantEntities. Where(e => (string?)e.Property("TenantId").CurrentValue != null && - (string?)e.Property("TenantId").CurrentValue != tenantInfo.Id); + (string?)e.Property("TenantId").CurrentValue != tenantInfo.Id).ToList(); if (mismatchedModified.Any()) { switch (tenantMismatchMode) { case TenantMismatchMode.Throw: - throw new MultiTenantException($"{mismatchedModified.Count()} modified entities with Tenant Id mismatch."); ; + throw new MultiTenantException($"{mismatchedModified.Count} modified entities with Tenant Id mismatch."); case TenantMismatchMode.Ignore: // no action needed @@ -99,14 +99,14 @@ public static class IMultiTenantDbContextExtensions // handle Tenant Id not set for modified entities var notSetModified = modifiedMultiTenantEntities. - Where(e => (string?)e.Property("TenantId").CurrentValue == null); + Where(e => (string?)e.Property("TenantId").CurrentValue == null).ToList(); if (notSetModified.Any()) { switch (tenantNotSetMode) { case TenantNotSetMode.Throw: - throw new MultiTenantException($"{notSetModified.Count()} modified entities with Tenant Id not set."); ; + throw new MultiTenantException($"{notSetModified.Count} modified entities with Tenant Id not set."); case TenantNotSetMode.Overwrite: foreach (var e in notSetModified) @@ -119,19 +119,19 @@ public static class IMultiTenantDbContextExtensions // get list of all deleted entities with MultiTenant annotation var deletedMultiTenantEntities = changedMultiTenantEntities. - Where(e => e.State == EntityState.Deleted); + Where(e => e.State == EntityState.Deleted).ToList(); // handle Tenant Id mismatches for deleted entities var mismatchedDeleted = deletedMultiTenantEntities. Where(e => (string?)e.Property("TenantId").CurrentValue != null && - (string?)e.Property("TenantId").CurrentValue != tenantInfo.Id); + (string?)e.Property("TenantId").CurrentValue != tenantInfo.Id).ToList(); if (mismatchedDeleted.Any()) { switch (tenantMismatchMode) { case TenantMismatchMode.Throw: - throw new MultiTenantException($"{mismatchedDeleted.Count()} deleted entities with Tenant Id mismatch."); ; + throw new MultiTenantException($"{mismatchedDeleted.Count} deleted entities with Tenant Id mismatch."); case TenantMismatchMode.Ignore: // no action needed @@ -145,14 +145,14 @@ public static class IMultiTenantDbContextExtensions // handle Tenant Id not set for deleted entities var notSetDeleted = deletedMultiTenantEntities. - Where(e => (string?)e.Property("TenantId").CurrentValue == null); + Where(e => (string?)e.Property("TenantId").CurrentValue == null).ToList(); if (notSetDeleted.Any()) { switch (tenantNotSetMode) { case TenantNotSetMode.Throw: - throw new MultiTenantException($"{notSetDeleted.Count()} deleted entities with Tenant Id not set."); ; + throw new MultiTenantException($"{notSetDeleted.Count} deleted entities with Tenant Id not set."); case TenantNotSetMode.Overwrite: // no action needed diff --git a/src/Finbuckle.MultiTenant.EntityFrameworkCore/MultiTenantEntityTypeBuilder.cs b/src/Finbuckle.MultiTenant.EntityFrameworkCore/MultiTenantEntityTypeBuilder.cs index 00b70070..12d720b8 100644 --- a/src/Finbuckle.MultiTenant.EntityFrameworkCore/MultiTenantEntityTypeBuilder.cs +++ b/src/Finbuckle.MultiTenant.EntityFrameworkCore/MultiTenantEntityTypeBuilder.cs @@ -38,6 +38,11 @@ public MultiTenantEntityTypeBuilder AdjustIndex(IMutableIndex index) if (index.IsUnique) indexBuilder.IsUnique(); + if (index.GetFilter() is string filter) + { + indexBuilder.HasFilter(filter); + } + return this; } diff --git a/test/Finbuckle.MultiTenant.EntityFrameworkCore.Test/MultiTenantEntityTypeBuilder/MultiTenantEntityTypeBuilderShould.cs b/test/Finbuckle.MultiTenant.EntityFrameworkCore.Test/MultiTenantEntityTypeBuilder/MultiTenantEntityTypeBuilderShould.cs index 790f0317..0cb91a06 100644 --- a/test/Finbuckle.MultiTenant.EntityFrameworkCore.Test/MultiTenantEntityTypeBuilder/MultiTenantEntityTypeBuilderShould.cs +++ b/test/Finbuckle.MultiTenant.EntityFrameworkCore.Test/MultiTenantEntityTypeBuilder/MultiTenantEntityTypeBuilderShould.cs @@ -2,6 +2,7 @@ // Refer to the solution LICENSE file for more information. using System; +using System.Collections; using System.Linq; using Finbuckle.MultiTenant.EntityFrameworkCore; using Finbuckle.MultiTenant.EntityFrameworkCore.Test.MultiTenantEntityTypeBuilder; @@ -34,18 +35,16 @@ public void AdjustIndexOnAdjustIndex() { IMutableIndex? origIndex = null; - using (var db = GetDbContext(builder => - { - builder.Entity().HasIndex(e => e.BlogId); - - origIndex = builder.Entity().Metadata.GetIndexes().First(); - builder.Entity().IsMultiTenant().AdjustIndex(origIndex); - })) + using var db = GetDbContext(builder => { - var index = db.Model.FindEntityType(typeof(Blog))?.GetIndexes().First(); - Assert.Contains("BlogId", index!.Properties.Select(p => p.Name)); - Assert.Contains("TenantId", index.Properties.Select(p => p.Name)); - } + builder.Entity().HasIndex(e => e.BlogId); + + origIndex = builder.Entity().Metadata.GetIndexes().First(); + builder.Entity().IsMultiTenant().AdjustIndex(origIndex); + }); + var index = db.Model.FindEntityType(typeof(Blog))?.GetIndexes().First(); + Assert.Contains("BlogId", index!.Properties.Select(p => p.Name)); + Assert.Contains("TenantId", index.Properties.Select(p => p.Name)); } [Fact] @@ -53,57 +52,70 @@ public void PreserveIndexNameOnAdjustIndex() { IMutableIndex? origIndex = null; - using (var db = GetDbContext(builder => - { - builder.Entity() - .HasIndex(e => e.BlogId, "CustomIndexName") - .HasDatabaseName("CustomIndexDbName"); - - origIndex = builder.Entity().Metadata.GetIndexes().First(); - builder.Entity().IsMultiTenant().AdjustIndex(origIndex); - })) + using var db = GetDbContext(builder => { - var index = db.Model.FindEntityType(typeof(Blog))?.GetIndexes().First(); - Assert.Equal("CustomIndexName", index!.Name); - Assert.Equal("CustomIndexDbName", index.GetDatabaseName()); - } + builder.Entity() + .HasIndex(e => e.BlogId, "CustomIndexName") + .HasDatabaseName("CustomIndexDbName"); + + origIndex = builder.Entity().Metadata.GetIndexes().First(); + builder.Entity().IsMultiTenant().AdjustIndex(origIndex); + }); + var index = db.Model.FindEntityType(typeof(Blog))?.GetIndexes().First(); + Assert.Equal("CustomIndexName", index!.Name); + Assert.Equal("CustomIndexDbName", index.GetDatabaseName()); } [Fact] public void PreserveIndexUniquenessOnAdjustIndex() { - using (var db = GetDbContext(builder => - { - builder.Entity().HasIndex(e => e.BlogId).IsUnique(); - builder.Entity().HasIndex(e => e.Url); - - foreach (var index in builder.Entity().Metadata.GetIndexes().ToList()) - builder.Entity().IsMultiTenant().AdjustIndex(index); - })) + using var db = GetDbContext(builder => + { + builder.Entity().HasIndex(e => e.BlogId).IsUnique(); + builder.Entity().HasIndex(e => e.Url); + + foreach (var index in builder.Entity().Metadata.GetIndexes().ToList()) + builder.Entity().IsMultiTenant().AdjustIndex(index); + }); { var index = db.Model.FindEntityType(typeof(Blog))? - .GetIndexes() - .Single(i => i.Properties.Select(p => p.Name).Contains("BlogId")); + .GetIndexes() + .Single(i => i.Properties.Select(p => p.Name).Contains("BlogId")); Assert.True(index!.IsUnique); index = db.Model.FindEntityType(typeof(Blog))? - .GetIndexes() - .Single(i => i.Properties.Select(p => p.Name).Contains("Url")); + .GetIndexes() + .Single(i => i.Properties.Select(p => p.Name).Contains("Url")); Assert.False(index!.IsUnique); } } + + [Fact] + public void PreserveIndexFilterOnAdjustIndex() + { + using var db = GetDbContext(builder => + { + var index = builder.Entity().HasIndex(e => e.BlogId).IsUnique().HasFilter("some filter").Metadata; + builder.Entity().IsMultiTenant().AdjustIndex(index); + }); + + var index = db.GetService().Model.FindEntityType(typeof(Blog))? + .GetIndexes() + .Single(i => i.Properties.Select(p => p.Name).Contains("BlogId")); + Assert.Equal("some filter", index!.GetFilter()); + } [Fact] public void AdjustPrimaryKeyOnAdjustKey() { - using (var db = GetDbContext(builder => - { - var key = builder.Entity().Metadata.GetKeys().First(); - builder.Entity().IsMultiTenant().AdjustKey(key, builder); - })) + using var db = GetDbContext(builder => + { + var key = builder.Entity().Metadata.GetKeys().First(); + builder.Entity().IsMultiTenant().AdjustKey(key, builder); + }); { var key = db.Model.FindEntityType(typeof(Post))?.GetKeys().ToList(); - Assert.Single(key!); + Assert.Single((IEnumerable)key!); Assert.Equal(2, key![0].Properties.Count); Assert.Contains("PostId", key[0].Properties.Select(p => p.Name)); Assert.Contains("TenantId", key[0].Properties.Select(p => p.Name)); @@ -113,16 +125,16 @@ public void AdjustPrimaryKeyOnAdjustKey() [Fact] public void AdjustDependentForeignKeyOnAdjustPrimaryKey() { - using (var db = GetDbContext(builder => - { - var key = builder.Entity().Metadata.GetKeys().First(); + using var db = GetDbContext(builder => + { + var key = builder.Entity().Metadata.GetKeys().First(); - builder.Entity().IsMultiTenant().AdjustKey(key, builder); - })) + builder.Entity().IsMultiTenant().AdjustKey(key, builder); + }); { var key = db.Model.FindEntityType(typeof(Post))?.GetForeignKeys().ToList(); - Assert.Single(key!); + Assert.Single((IEnumerable)key!); Assert.Equal(2, key![0].Properties.Count); Assert.Contains("BlogId", key[0].Properties.Select(p => p.Name)); Assert.Contains("TenantId", key[0].Properties.Select(p => p.Name)); @@ -132,15 +144,15 @@ public void AdjustDependentForeignKeyOnAdjustPrimaryKey() [Fact] public void AdjustAlternateKeyOnAdjustKey() { - using (var db = GetDbContext(builder => - { - var key = builder.Entity().HasAlternateKey(b => b.Url).Metadata; - builder.Entity().IsMultiTenant().AdjustKey(key, builder); - })) + using var db = GetDbContext(builder => + { + var key = builder.Entity().HasAlternateKey(b => b.Url).Metadata; + builder.Entity().IsMultiTenant().AdjustKey(key, builder); + }); { var key = db.Model.FindEntityType(typeof(Blog))?.GetKeys().Where(k => !k.IsPrimaryKey()).ToList(); - Assert.Single(key!); + Assert.Single((IEnumerable)key!); Assert.Equal(2, key![0].Properties.Count); Assert.Contains("Url", key[0].Properties.Select(p => p.Name)); Assert.Contains("TenantId", key[0].Properties.Select(p => p.Name)); @@ -150,20 +162,20 @@ public void AdjustAlternateKeyOnAdjustKey() [Fact] public void AdjustDependentForeignKeyOnAdjustAlternateKey() { - using (var db = GetDbContext(builder => - { - var key = builder.Entity().HasAlternateKey(b => b.Url).Metadata; - builder.Entity() - .HasOne(p => p.Blog!) - .WithMany(b => b.Posts!) - .HasForeignKey(p => p.Title) // Since Title is a string lets use it as key to Blog.Url - .HasPrincipalKey(b => b.Url); - builder.Entity().IsMultiTenant().AdjustKey(key, builder); - })) + using var db = GetDbContext(builder => + { + var key = builder.Entity().HasAlternateKey(b => b.Url).Metadata; + builder.Entity() + .HasOne(p => p.Blog!) + .WithMany(b => b.Posts!) + .HasForeignKey(p => p.Title) // Since Title is a string lets use it as key to Blog.Url + .HasPrincipalKey(b => b.Url); + builder.Entity().IsMultiTenant().AdjustKey(key, builder); + }); { var key = db.Model.FindEntityType(typeof(Post))?.GetForeignKeys().ToList(); - Assert.Single(key!); + Assert.Single((IEnumerable)key!); Assert.Equal(2, key![0].Properties.Count); Assert.Contains("Title", key[0].Properties.Select(p => p.Name)); Assert.Contains("TenantId", key[0].Properties.Select(p => p.Name)); diff --git a/test/Finbuckle.MultiTenant.Test/TenantInfoShould.cs b/test/Finbuckle.MultiTenant.Test/TenantInfoShould.cs index 6524004b..b61c973c 100644 --- a/test/Finbuckle.MultiTenant.Test/TenantInfoShould.cs +++ b/test/Finbuckle.MultiTenant.Test/TenantInfoShould.cs @@ -8,20 +8,12 @@ namespace Finbuckle.MultiTenant.Test { public class TenantInfoShould { - [Fact] - public void AlwaysFail() - { - Assert.True(false); - } - [Fact] public void ThrowIfIdSetWithLengthAboveTenantIdMaxLength() { - // OK // ReSharper disable once ObjectCreationAsStatement new TenantInfo { Id = "".PadRight(1, 'a') }; - // OK // ReSharper disable once ObjectCreationAsStatement new TenantInfo { Id = "".PadRight(Constants.TenantIdMaxLength, 'a') };