Skip to content

Commit

Permalink
Fixed locations where an assumption is made that a ForeignKey.Referen…
Browse files Browse the repository at this point in the history
…ceDocumentType 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
  • Loading branch information
Mark Warpool authored and mysticmind committed Aug 19, 2019
1 parent 0839ec6 commit 6579824
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -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<ClassWithExternalForeignKey>().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<ClassWithExternalForeignKey>().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<ISchemaObject> 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; }
}
}
2 changes: 1 addition & 1 deletion src/Marten/Services/UnitOfWork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ private IEnumerable<Type> GetTypeDependencies(Type type)
if (documentMapping == null)
return Enumerable.Empty<Type>();

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<Type>();
Expand Down
3 changes: 1 addition & 2 deletions src/Marten/Storage/StorageFeatures.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,8 @@ public IEnumerable<IFeatureSchema> 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);
});

Expand Down
3 changes: 2 additions & 1 deletion src/MartenBenchmarks/BenchAgainst/StorageFeatures.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ public IEnumerable<IFeatureSchema> AllActiveFeatures(ITenant tenant)
return m.ForeignKeys
.Where(x => x.ReferenceDocumentType != m.DocumentType)
.Select(keyDefinition => keyDefinition.ReferenceDocumentType)
.Where(referenceDocumentType => referenceDocumentType != null)
.Select(MappingFor);
});

Expand Down Expand Up @@ -231,4 +232,4 @@ internal bool SequenceIsRequired()
return _documentMappings.Values.Any(x => x.IdStrategy.RequiresSequences);
}
}
}
}
2 changes: 1 addition & 1 deletion src/MartenBenchmarks/BenchAgainst/UnitOfWorkBaseline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ private IEnumerable<Type> GetTypeDependencies(Type type)
if (documentMapping == null)
return Enumerable.Empty<Type>();

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<Type>();
Expand Down

0 comments on commit 6579824

Please sign in to comment.