Replacing one-to-one relationship either breaks foreign key constraint or leaves orphan entities

[skyflyer]

While developing a rather complex interaction between entities, I stumbled upon a failing testcase, where child entity (one-to-one) is replaced.

Explanation of the setup:
We have two entities, InternalOrder and OrderTime, where InternalOrder has defined properties:

    public Guid? OrderTimeID { get; private set; }
    public OrderTime OrderTime { get; private set; }

We have an existing InternalOrder and when we remove OrderTime from it, EF detects this correctly and: updates the InternalOrder and then removes OrderTime.

If we're replacing the InternalOrder.OrderTime reference, then there are two different scenarios where this fails:

1. if we're using tracked entities, the previous OrderTime is left in the database.
2. if we're using disconnected entities and we're explicitly attaching the order, marking it as modified, marking the previous OrderTime as EntityState.Deleted and the new one as EntityState.Added, then we're getting foreign key violation from database (tested with SQLServer and SQLite).

What happens: EF is trying to delete an OrderTime from a DB, but the InternalOrder's OrderTimeID still holds the key of the OrderTime to be removed, so naturally, SQLite barfs with foreign key constraint violation.

Trivial examples did not reproduce this issue, so the reproduction is a bit contrived... I prepared a complete repro project (console app) here: https://github.com/skyflyer/efcore-one-to-one-orphans

InternalOrderRepository has the code for setting up EF tracking information for disconnected entities (this problem was detected when dealing with disconnected entities, while receiving data from remote clients).

It may very well be, that we need to configure some special handling (but the db snapshot created by migrations looks OK to me -- this project does not include snapshot for easier iterations).

The generated snapshot configures the relationship like this:

            modelBuilder.Entity("InternalOrder", b =>
                {
                    b.HasOne("OrderTime", "OrderTime")
                        .WithOne()
                        .HasForeignKey("InternalOrder", "OrderTimeID");
                });

What is really puzzling to me is this: if you check out method RemoveOrderTimeFromInternalOrder it does the right thing (e.g.: update InternalOrder first and then remove OrderTime). When using tracked entity (see RemoveOrderTimeFromInternalOrderWithTracking) OrderTime is left in the DB (orphaned).

If we're removing order time and then adding a new one, it breaks, because it would have to (IMO): insert new order time, update internal order, remove orphaned order time.

The logging produced indicates that correct tracking info is present:

Details

DetectChanges starting for 'EfTestDbContext'.
Navigation property 'InternalOrder.OrderTime' for entity with key '{ID: 0d7f8ba3-cf21-4ddc-8166-40946639fb4f}' detected as changed.
'EfTestDbContext' generated value '5f1a746a-8b68-4cbc-b2a1-368536ecebf3' for the 'ID' property of new 'OrderTime' entity.
The 'InternalOrder' entity with key '{ID: 0d7f8ba3-cf21-4ddc-8166-40946639fb4f}' tracked by 'EfTestDbContext' changed from 'Unchanged' to 'Modified'.
Foreign key property 'InternalOrder.OrderTimeID' detected as changed from 'f77f7e0d-f1a9-4ae4-8761-7f90b58e7a6f' to '5f1a746a-8b68-4cbc-b2a1-368536ecebf3' for entity with key '{ID: 0d7f8ba3-cf21-4ddc-8166-40946639fb4f}'.
Context 'OrderTime' started tracking '{ID: 5f1a746a-8b68-4cbc-b2a1-368536ecebf3}' entity with key 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
The 'InternalOrder' entity with key '{ID: 0d7f8ba3-cf21-4ddc-8166-40946639fb4f}' tracked by 'EfTestDbContext' changed from 'Modified' to 'Detached'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
Opening connection to database 'main' on server 'test.db'.
Opened connection to database 'main' on server '/Users/miha/work/dev/eftest/test.db'.
Executing DbCommand [Parameters=[], CommandType='Text', CommandTimeout='30']
PRAGMA foreign_keys=ON;
Executed DbCommand (0ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
PRAGMA foreign_keys=ON;
Executing DbCommand [Parameters=[@__order_ID_0='0d7f8ba3-cf21-4ddc-8166-40946639fb4f' (DbType = String)], CommandType='Text', CommandTimeout='30']
SELECT "x"."ID", "x"."OrderNumber", "x"."OrderTimeID", "x"."Timestamp", "x.OrderTime"."ID", "x.OrderTime"."Display", "x.OrderTime"."ServiceOrderID", "x.OrderTime"."Timestamp"
FROM "InternalOrders" AS "x"
LEFT JOIN "OrderTimes" AS "x.OrderTime" ON "x"."OrderTimeID" = "x.OrderTime"."ID"
WHERE "x"."ID" = @__order_ID_0
LIMIT 2
Executed DbCommand (1ms) [Parameters=[@__order_ID_0='0d7f8ba3-cf21-4ddc-8166-40946639fb4f' (DbType = String)], CommandType='Text', CommandTimeout='30']
SELECT "x"."ID", "x"."OrderNumber", "x"."OrderTimeID", "x"."Timestamp", "x.OrderTime"."ID", "x.OrderTime"."Display", "x.OrderTime"."ServiceOrderID", "x.OrderTime"."Timestamp"
FROM "InternalOrders" AS "x"
LEFT JOIN "OrderTimes" AS "x.OrderTime" ON "x"."OrderTimeID" = "x.OrderTime"."ID"
WHERE "x"."ID" = @__order_ID_0
LIMIT 2
A data reader was disposed.
Closing connection to database 'main' on server '/Users/miha/work/dev/eftest/test.db'.
Closed connection to database 'main' on server 'test.db'.
Context 'InternalOrder' started tracking '{ID: 0d7f8ba3-cf21-4ddc-8166-40946639fb4f}' entity with key 'EfTestDbContext'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
The 'InternalOrder' entity with key '{ID: 0d7f8ba3-cf21-4ddc-8166-40946639fb4f}' tracked by 'EfTestDbContext' changed from 'Unchanged' to 'Modified'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
The 'OrderTime' entity with key '{ID: f77f7e0d-f1a9-4ae4-8761-7f90b58e7a6f}' tracked by 'EfTestDbContext' changed from 'Unchanged' to 'Deleted'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
-- Before save changes --
SaveChanges starting for 'EfTestDbContext'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
Opening connection to database 'main' on server 'test.db'.
Opened connection to database 'main' on server '/Users/miha/work/dev/eftest/test.db'.
Executing DbCommand [Parameters=[], CommandType='Text', CommandTimeout='30']
PRAGMA foreign_keys=ON;
Executed DbCommand (0ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
PRAGMA foreign_keys=ON;
Beginning transaction with isolation level 'Serializable'.
Executing DbCommand [Parameters=[@p0='f77f7e0d-f1a9-4ae4-8761-7f90b58e7a6f' (DbType = String)], CommandType='Text', CommandTimeout='30']
DELETE FROM "OrderTimes"
WHERE "ID" = @p0;
SELECT changes();
Failed executing DbCommand (1ms) [Parameters=[@p0='f77f7e0d-f1a9-4ae4-8761-7f90b58e7a6f' (DbType = String)], CommandType='Text', CommandTimeout='30']
DELETE FROM "OrderTimes"
WHERE "ID" = @p0;
SELECT changes();
Disposing transaction.
Closing connection to database 'main' on server '/Users/miha/work/dev/eftest/test.db'.
Closed connection to database 'main' on server 'test.db'.
An exception occurred in the database while saving changes for context type 'eftest.EfTestDbContext'.
Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> Microsoft.Data.Sqlite.SqliteException: SQLite Error 19: 'FOREIGN KEY constraint failed'.

And here is the log where we're just removing the OrderTime and EF updates InternalOrder before deleting OrderTime:

Details

Context 'InternalOrder' started tracking '{ID: 2e9cd61a-3edc-4ccc-9a92-ae89e81dc67d}' entity with key 'EfTestDbContext'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
The 'InternalOrder' entity with key '{ID: 2e9cd61a-3edc-4ccc-9a92-ae89e81dc67d}' tracked by 'EfTestDbContext' changed from 'Unchanged' to 'Modified'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
The 'OrderTime' entity with key '{ID: 8a1862bd-e3a8-423d-aaa3-c4a15c3a37bf}' tracked by 'EfTestDbContext' changed from 'Unchanged' to 'Deleted'.
SaveChanges starting for 'EfTestDbContext'.
DetectChanges starting for 'EfTestDbContext'.
DetectChanges completed for 'EfTestDbContext'.
Opening connection to database 'main' on server 'test.db'.
Opened connection to database 'main' on server '/Users/miha/work/dev/eftest/test.db'.
Executing DbCommand [Parameters=[], CommandType='Text', CommandTimeout='30']
PRAGMA foreign_keys=ON;
Executed DbCommand (0ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
PRAGMA foreign_keys=ON;
Beginning transaction with isolation level 'Serializable'.
Executing DbCommand [Parameters=[@p3='2e9cd61a-3edc-4ccc-9a92-ae89e81dc67d' (DbType = String), @p0='W-{seq}' (Size = 7), @p1='', @p2=''], CommandType='Text', CommandTimeout='30']
UPDATE "InternalOrders" SET "OrderNumber" = @p0, "OrderTimeID" = @p1, "Timestamp" = @p2
WHERE "ID" = @p3;
SELECT changes();
Executed DbCommand (1ms) [Parameters=[@p3='2e9cd61a-3edc-4ccc-9a92-ae89e81dc67d' (DbType = String), @p0='W-{seq}' (Size = 7), @p1='', @p2=''], CommandType='Text', CommandTimeout='30']
UPDATE "InternalOrders" SET "OrderNumber" = @p0, "OrderTimeID" = @p1, "Timestamp" = @p2
WHERE "ID" = @p3;
SELECT changes();
A data reader was disposed.
Executing DbCommand [Parameters=[@p0='8a1862bd-e3a8-423d-aaa3-c4a15c3a37bf' (DbType = String)], CommandType='Text', CommandTimeout='30']
DELETE FROM "OrderTimes"
WHERE "ID" = @p0;
SELECT changes();
Executed DbCommand (0ms) [Parameters=[@p0='8a1862bd-e3a8-423d-aaa3-c4a15c3a37bf' (DbType = String)], CommandType='Text', CommandTimeout='30']
DELETE FROM "OrderTimes"
WHERE "ID" = @p0;
SELECT changes();
A data reader was disposed.

@ajcvickers et al, do you have any ideas what we might be doing wrong? Or if it is too complex for EF to handle this?

Further technical details

EF Core version: 2.1.2
Database Provider: Microsoft.EntityFrameworkCore.SqlServer, Microsoft.EntityFrameworkCore.SQLite
Operating system: Mac OS X
IDE: VS Code

[skyflyer]

Correction: I've regenerated snapshot using dotnet ef migrations add Initial and I see now that the relation is different from what it was before:

            modelBuilder.Entity("InternalOrder", b =>
                {
                    b.HasOne("OrderTime", "OrderTime")
                        .WithMany()
                        .HasForeignKey("OrderTimeID");
                });

This one seems wrong. And if I change WithMany to WithOne, EF complains:

Unhandled Exception: System.InvalidOperationException: You are configuring a relationship between 'InternalOrder' and 'OrderTime' but have specified a foreign key on 'OrderTimeID'. The foreign key must be defined on a type that is part of the relationship.

I tried several other options for defining this relationship, but no dice for me. Omitting the HasForeignKey runs, but hits the same "foreign key violation" as described in my first post.

[ajcvickers]

@skyflyer It's possible that there is an EF bug here, but before digging too much further I want to make a few comments about the repository code. To me, this code seems overly complex for what it is doing, at least in the case that fails in the repro. Part of this complexity seems to be because it is detaching and attaching where it doesn't seem to be necessary--as a general rule of thumb, detaching entities from a context should be avoided. (This is why there is no Detach method, while there are Add, Attach, Update, and Remove methods.) Usually detaching like this is an indication that the context is trying to do more than a single unit-of-work, but in this case that doesn't seem to be the case. Instead, it just seems to be attempting to perform a unit-of-work in what appears to be a convoluted way. For example, if I change the piece of code that fails to use the change tracking provided by the context, then all the Asserts pass for me:

Guid previousOrderTimeId;
using (var dbContext = new EfTestDbContext())
{
    // Get the existing order and time--they are now tracked
    var order = await dbContext.InternalOrders
        .Include(x => x.OrderTime)
        .SingleAsync(x => x.ID == internalOrder.ID);
    
    previousOrderTimeId = order.OrderTime.ID;

    // Mark the old OrderTime as deleted
    dbContext.Remove(order.OrderTime);

    // Set a new OrderTime--it will be Added, Order will be Modified because of FK change
    order.SetTime(new OrderTime("Test 1234"));

    // Save
    await dbContext.SaveChangesAsync();
}

Could you adopt this kind of approach instead? If not, it would be great to understand why the more complex approach is needed.

[skyflyer]

@ajcvickers, you are right about the "convoluted" comment. And I should've probably taken the time to explain why it is so convoluted.

Here are the most important reasons for the convoluted nature of this setup:

• the objects are domain objects, hence they have no public setters and collections are read only, which means I can't modify object (aggregate root) at will without other side effects.
• the object's references can be constructed in a different DbContext (less likely)
• the object graphs come through the wire (most of the time) so there is no tracking info available

So, the repository needs to handle tracked and untracked cases (when the orders are being manipulated by the running program or when the orders are being received over the wire, already populated). Untracked cases are, of course, the most problematic, because they have references to existing related entities and the "repository" does not know which are existing and which are new. For instance, serialized order contains serialized ordertimes, but does not contain other related entities like (customer, vehicle, ....) -- only their foreign keys.

Your solution with dbContext.Remove(order.OrderTime) works (and I've tested this before) but it only works for tracked entities and the reference has to be explicitly removed. As you can see in the repro code I'm actually calling explicit remove in cases where it is not already tracked (otherwise EF refuses the operation with an exception telling me that this entity is already being tracked).

The reason I am detaching entities is because I don't know better :( If I detach them, I can then excplicitly attach the ones I need modifying and I can handle tracked/untracked in (mostly) the same way. The code in the repository is really ugly and I am aware of that, but there are just so many branching cases that need to be accounted for. Let me try to explain using words what I'm trying to achieve:

• InternalOrder can have an OrderTime.
• Both objects can change
• OrderTime can be removed from an InternalOrder
• OrderTime can be added to an InternalOrder
• OrderTime can be replaced with a new one.

Entities could be tracked (when doing operations on a tracked entity) or totally untracked, in which case the complete object graph is received in a serialized form. In the untracked case, the repository needs to figure out if order time has been removed, added or replaced and act accordingly. The real solution deals with two-way synchronization of data between mobile devices and server while at the same time, ti is supporting the regular entity operations on the server.

I appreciate your taking the time to address this issue, especially since it is very convoluted, as you already ascertained.

[skyflyer]

@ajcvickers, wanted to let you know, that you can close this ticket, as far as I am concerned. I went with the recreation of a complete graph in the solution.

The repro is really convoluted and hard to follow, though it would be nice if change tracking could be improved.