Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Research and possible start of a fix for issue 1337 #1350

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions EFCore.BulkExtensions.Tests/IncludeGraph/Issue1337.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;
using Xunit;

namespace EFCore.BulkExtensions.Tests.IncludeGraph;

public class Issue1337
{
[Fact]
public async Task Issue1337Test()
{
using var context = new MyDbContext(ContextUtil.GetOptions<MyDbContext>(databaseName: $"{nameof(EFCoreBulkTest)}_Issue1337"));

context.Database.EnsureDeleted();
context.Database.EnsureCreated();

var bulkConfig = new BulkConfig { IncludeGraph = true, CalculateStats = true };

Cust[] entities = [
new()
{
CustId = "test1",
ContactMethods = new()
{
HomePhone = "homephone1",
EmailAdresses = [
new Email() { Address = "email1", Type = "emailtype1" },
new Email() { Address = "email2", Type = "emailtype2" },
]
},
}
];

// Without any changes, this crashes with System.InvalidOperationException : Column 'ContactMethodsCustomerId' does not allow DBNull.Value.

// With the changes in this PR until now, the merge of the emails works, but now I get an error when trying to read the output of that operation:
// System.InvalidOperationException : An exception was thrown while attempting to evaluate a LINQ query parameter expression. See the inner exception for more information.
// ----System.InvalidOperationException : Cannot create a DbSet for 'Email' because it is configured as an owned entity type and must be accessed through its owning entity type 'ContactMethods'.See https://aka.ms/efcore-docs-owned for more information.

await context.BulkInsertOrUpdateAsync(entities, bulkConfig);
}
}

public class Cust
{
public string CustId { get; set; } = default!;
public ContactMethods ContactMethods { get; set; } = default!;
}

public class ContactMethods
{
public string HomePhone { get; set; } = default!;
public ICollection<Email> EmailAdresses { get; set; } = default!;
}

public class Email
{
public string Address { get; set; } = default!;
public string Type { get; set; } = default!;
}

public class MyDbContext(DbContextOptions opts) : DbContext(opts)
{
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Cust>(c =>
c.OwnsOne(c => c.ContactMethods, cm => cm.OwnsMany(y => y.EmailAdresses)));
}
}
22 changes: 22 additions & 0 deletions EFCore.BulkExtensions/DbContextBulkTransactionGraphUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,28 @@ private static async Task ExecuteWithGraphAsync(DbContext context, IEnumerable<o

UpdateStats(stats, bulkConfig);
}

// We also have to set the foreign keys of potential dependents of owned types that are in the same table as this one
foreach (var ng in graphNodesGroupedByType.Where(ng =>
context.Model.FindEntityType(ng.Key) is { } entityType
&& OwnedTypeUtil.IsOwnedInSameTableAsOwner(entityType)))
{
foreach(var dependentOfSameType in SetForeignKeysForDependentsAndYieldSameTypeDependents(context, ng.Key, ng))
{
var dependentTableInfo = TableInfo.CreateInstance(context, entityClrType, dependentsOfSameType, operationType, bulkConfig);

if (isAsync)
{
await SqlBulkOperation.MergeAsync(context, entityClrType, dependentsOfSameType, dependentTableInfo, operationType, progress, cancellationToken).ConfigureAwait(false);
}
else
{
SqlBulkOperation.Merge(context, entityClrType, dependentsOfSameType, dependentTableInfo, operationType, progress);
}

UpdateStats(stats, bulkConfig);
}
}
}

if (bulkConfig.CalculateStats)
Expand Down
6 changes: 5 additions & 1 deletion EFCore.BulkExtensions/Util/GraphUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,11 @@ private static void SetDependencies(DbContext dbContext, GraphDependency graphDe
graphDependency.DependsOn.Add((navigationValue, navigation));
nestedDependency.Dependents.Add((graphEntity, navigation.Inverse ?? navigation));
}
else

// Not sure this is the right thing to do here, but this way the "owned types in same table as owner"
// also get added to dependents, which is necessary to be able to set foreign keys of potential dependents of this owned type
// (See the change in DbContextBulkTransactionGraphUtil ExecuteWithGraphAsync)
if (!navigation.IsOnDependent)
{
graphDependency.Dependents.Add((navigationValue, navigation));
nestedDependency.DependsOn.Add((graphEntity, navigation.Inverse ?? navigation));
Expand Down