From 65798247e2e179b0b51b9861c49e88831ea6cd6e Mon Sep 17 00:00:00 2001 From: Mark Warpool Date: Mon, 19 Aug 2019 12:39:01 -0500 Subject: [PATCH] Fixed locations where an assumption is made that a ForeignKey.ReferenceDocumentType will not be null * Fixed several locations where an assumption is made that a ForeignKeyDefinition.ReferenceDocumentType will not be null * Fixed a typo * Fixes #1338 --- ...reignKeyDefinition_ReferenceDocumenType.cs | 105 ++++++++++++++++++ src/Marten/Services/UnitOfWork.cs | 2 +- src/Marten/Storage/StorageFeatures.cs | 3 +- .../BenchAgainst/StorageFeatures.cs | 3 +- .../BenchAgainst/UnitOfWorkBaseline.cs | 2 +- 5 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 src/Marten.Testing/Bugs/bug_1338_Validate_Null_ForeignKeyDefinition_ReferenceDocumenType.cs diff --git a/src/Marten.Testing/Bugs/bug_1338_Validate_Null_ForeignKeyDefinition_ReferenceDocumenType.cs b/src/Marten.Testing/Bugs/bug_1338_Validate_Null_ForeignKeyDefinition_ReferenceDocumenType.cs new file mode 100644 index 0000000000..afd22b5649 --- /dev/null +++ b/src/Marten.Testing/Bugs/bug_1338_Validate_Null_ForeignKeyDefinition_ReferenceDocumenType.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Marten.Schema; +using Marten.Storage; +using Npgsql; +using Shouldly; +using Xunit; + +namespace Marten.Testing.Bugs +{ + public class Bug_1338_Validate_Null_ForeignKeyDefinition_ReferenceDocumenType : IntegratedFixture + { + [Fact] + public void StorageFeatures_AllActiveFeatures_Should_Not_Throw_With_ExternalForeignKeyDefinitions() + { + CreateExternalTableForTesting(); + + StoreOptions(_ => + { + _.Schema.For().ForeignKey(x => x.ForeignId, _.DatabaseSchemaName, "external_table", "id"); + }); + + theStore.Storage.AllActiveFeatures(theStore.Tenancy.Default).All(x => x != null).ShouldBeTrue(); + } + + [Fact] + public void UnitOfWork_GetTypeDependencies_Should_Not_Throw_With_ExternalForeignKeyDefinitions() + { + CreateExternalTableForTesting(); + + StoreOptions(_ => + { + _.Schema.For().ForeignKey(x => x.ForeignId, _.DatabaseSchemaName, "external_table", "id"); + }); + + // Inserting a new document will force a call to: + // UnitOfWork.ApplyChanges() + // UnitOfWork.buildChangeSet() + // UnitOfWork.determineChanges() + // UnitOfWork.shouldSort() + // and finally, the function that we want to regression test" + // UnitOfWork.GetTypeDependencies(ClassWithExternalForeignKey) + using (var session = theStore.LightweightSession()) + { + session.Insert(new ClassWithExternalForeignKey{Id = 1, ForeignId = 1}); + session.SaveChanges(); + } + } + + private static void CreateExternalTableForTesting() + { + const string dropSql = "DROP TABLE IF EXISTS public.external_table CASCADE;"; + const string createSql = +@"CREATE TABLE public.external_table ( + id integer, + CONSTRAINT ""external_table_pkey"" PRIMARY KEY (id) +);"; + const string insertSql = "INSERT INTO public.external_table VALUES (1);"; + + using (var dbConn = new NpgsqlConnection(ConnectionSource.ConnectionString)) + { + dbConn.Open(); + + NpgsqlCommand cmd; + using (cmd = new NpgsqlCommand(dropSql, dbConn)) + { + cmd.ExecuteNonQuery(); + } + + using (cmd = new NpgsqlCommand(createSql, dbConn)) + { + cmd.ExecuteNonQuery(); + } + + using (cmd = new NpgsqlCommand(insertSql, dbConn)) + { + cmd.ExecuteNonQuery(); + } + } + } + + } + + public class FakeExternalTable: FeatureSchemaBase + { + public FakeExternalTable(StoreOptions options) : base("fake_external_table", options) + { + } + + protected override IEnumerable schemaObjects() + { + var table = new Table(new DbObjectName(Options.DatabaseSchemaName, "external_table")); + table.AddColumn("id", "integer"); + + yield return table; + } + } + + public class ClassWithExternalForeignKey + { + public int Id { get; set; } + public int ForeignId { get; set; } + } +} diff --git a/src/Marten/Services/UnitOfWork.cs b/src/Marten/Services/UnitOfWork.cs index 0307261bdf..c0c22d305f 100644 --- a/src/Marten/Services/UnitOfWork.cs +++ b/src/Marten/Services/UnitOfWork.cs @@ -278,7 +278,7 @@ private IEnumerable GetTypeDependencies(Type type) if (documentMapping == null) return Enumerable.Empty(); - return documentMapping.ForeignKeys.Where(x => x.ReferenceDocumentType != type) + return documentMapping.ForeignKeys.Where(x => x.ReferenceDocumentType != type && x.ReferenceDocumentType != null) .SelectMany(keyDefinition => { var results = new List(); diff --git a/src/Marten/Storage/StorageFeatures.cs b/src/Marten/Storage/StorageFeatures.cs index 2fe472e16e..e972f444d5 100644 --- a/src/Marten/Storage/StorageFeatures.cs +++ b/src/Marten/Storage/StorageFeatures.cs @@ -204,9 +204,8 @@ public IEnumerable AllActiveFeatures(ITenant tenant) .TopologicalSort(m => { return m.ForeignKeys - .Where(x => x.ReferenceDocumentType != m.DocumentType) + .Where(x => x.ReferenceDocumentType != m.DocumentType && x.ReferenceDocumentType != null) .Select(keyDefinition => keyDefinition.ReferenceDocumentType) - .Where(keyDefinition => keyDefinition != null) .Select(MappingFor); }); diff --git a/src/MartenBenchmarks/BenchAgainst/StorageFeatures.cs b/src/MartenBenchmarks/BenchAgainst/StorageFeatures.cs index 84cbde11ec..fdc477009b 100644 --- a/src/MartenBenchmarks/BenchAgainst/StorageFeatures.cs +++ b/src/MartenBenchmarks/BenchAgainst/StorageFeatures.cs @@ -194,6 +194,7 @@ public IEnumerable AllActiveFeatures(ITenant tenant) return m.ForeignKeys .Where(x => x.ReferenceDocumentType != m.DocumentType) .Select(keyDefinition => keyDefinition.ReferenceDocumentType) + .Where(referenceDocumentType => referenceDocumentType != null) .Select(MappingFor); }); @@ -231,4 +232,4 @@ internal bool SequenceIsRequired() return _documentMappings.Values.Any(x => x.IdStrategy.RequiresSequences); } } -} \ No newline at end of file +} diff --git a/src/MartenBenchmarks/BenchAgainst/UnitOfWorkBaseline.cs b/src/MartenBenchmarks/BenchAgainst/UnitOfWorkBaseline.cs index 9b8dc2e206..039ce93595 100644 --- a/src/MartenBenchmarks/BenchAgainst/UnitOfWorkBaseline.cs +++ b/src/MartenBenchmarks/BenchAgainst/UnitOfWorkBaseline.cs @@ -240,7 +240,7 @@ private IEnumerable GetTypeDependencies(Type type) if (documentMapping == null) return Enumerable.Empty(); - return documentMapping.ForeignKeys.Where(x => x.ReferenceDocumentType != type) + return documentMapping.ForeignKeys.Where(x => x.ReferenceDocumentType != type && x.ReferenceDocumentType != null) .SelectMany(keyDefinition => { var results = new List();