Efcore: Circular reference issues in 3.1

[svallis]

Currently migrating a project from 2.2 to 3.1 and hitting an issue with a circular reference which used to work correctly.

The simplified tables in question are:

```C#
public class Client
{
public int Id { get; set; }

[Required]
public string Name { get; set; }

public int PrimaryContactId { get; set; }
public Contact PrimaryContact { get; set; }

public List<Contact> Contacts { get; set; }

}

public class Contact
{
public int Id { get; set; }

[Required]
public string Name { get; set; }

public int? ClientId { get; set; }
public Client Client { get; set; }

}

The relationship is being defined as follows:

```C#
builder.Entity<Contact>().HasOne(x => x.Client).WithMany(x => x.Contacts);
builder.Entity<Client>().HasMany(x => x.Contacts).WithOne(x => x.Client);
builder.Entity<Client>().HasOne(x => x.PrimaryContact);

And the error being reported is:

Invalid column name 'PrimaryContactId1'.
Invalid column name 'PrimaryAddressId1'.

Looking at the generated SQL (again simplified below) shows duplicated/phantom columns are selected and used in a join:

SELECT ..., [c].[PrimaryContactId], [c].[PrimaryContactId1], [c0].[Name], ...
FROM [Client] AS [c]
LEFT JOIN [Contact] AS [c0] ON [c].[PrimaryContactId1] = [c0].[Id]

Looked through the breaking changes but didn't see anything obvious... are we missing something?

[ajcvickers]

@svallis A workaround for this is to add the missing call to WithMany as described below.

@AndriySvyryd The issue here appears to be the use of HasOne without a corresponding With method:
```C#
builder.Entity().HasOne(x => x.PrimaryContact);

With this configuration the model is:

Model:
EntityType: Client
Properties:
Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
Name (string) Required
PrimaryContactId (int) Required

PrimaryContactId1 (no field, Nullable) Shadow FK Index
Navigations:
Contacts (List) Collection ToDependent Contact Inverse: Client
PrimaryContact (Contact) ToPrincipal Contact
Keys:
Id PK
Foreign keys:
Client {'PrimaryContactId1'} -> Contact {'Id'} ToPrincipal: PrimaryContact
EntityType: Contact
Properties:
Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
ClientId (Nullable) FK Index
Name (string) Required
Navigations:
Client (Client) ToPrincipal Client Inverse: Contacts
Keys:
Id PK
Foreign keys:
Contact {'ClientId'} -> Client {'Id'} ToDependent: Contacts ToPrincipal: Client

If I change the configuration to:
```C#
builder.Entity<Client>().HasOne(x => x.PrimaryContact).WithMany();

then I get the expected model:

Model: 
  EntityType: Client
    Properties: 
      Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
      Name (string) Required
      PrimaryContactId (int) Required FK Index
    Navigations: 
      Contacts (List<Contact>) Collection ToDependent Contact Inverse: Client
      PrimaryContact (Contact) ToPrincipal Contact
    Keys: 
      Id PK
    Foreign keys: 
      Client {'PrimaryContactId'} -> Contact {'Id'} ToPrincipal: PrimaryContact
  EntityType: Contact
    Properties: 
      Id (int) Required PK AfterSave:Throw ValueGenerated.OnAdd
      ClientId (Nullable<int>) FK Index
      Name (string) Required
    Navigations: 
      Client (Client) ToPrincipal Client Inverse: Contacts
    Keys: 
      Id PK
    Foreign keys: 
      Contact {'ClientId'} -> Client {'Id'} ToDependent: Contacts ToPrincipal: Client

[svallis]

Thank you @ajcvickers, issue resolved. Is this workaround something we should flag for looking at again in the future on our end, or should it be safe to leave alone now? Many thanks.

[ajcvickers]

@svallis In some sense, the "correct" way to configure the relationship is using both HasOne and WithMany--it's what we document. So, yes, you should leave it like this. I plan to discuss with the team what should be expected behavior of your original code.

[svallis]

@ajcvickers Understood. Thank you for your quick responses, they are much appreciated!

[ajcvickers]

Note from team discussion: we should at least investigate this for 5.0, even if we ultimately don't fix it.

[suadev]

@ajcvickers this doesn't work for me. I am on ef core 3.1.7. I see you planned for ef core 6.0. Any other workaround?

public partial class Reservation
{     
    public List<ReservationGuest> Guests { get; set; } = new List<ReservationGuest>();

    public Guid OwnerId { get; set; }

    public ReservationGuest Owner { get; set; }
}

public class ReservationGuest 
{
    public Reservation Reservation { get; set; }
    public Guid? ReservationId { get; set; }
}

public override void Configure(EntityTypeBuilder<Reservation> builder)
{
    base.Configure(builder);
    builder.ToTable("reservations");

    builder.HasMany(x => x.Guests)
    .WithOne(x => x.Reservation);

    builder.HasOne(x => x.Owner)
    .WithMany();
}

public override void Configure(EntityTypeBuilder<ReservationGuest> builder)
{
    base.Configure(builder);

    builder.ToTable("reservation_guests");

    builder.HasOne(x => x.Reservation).WithMany(x => x.Guests);
}

"Unable to save changes because a circular dependency was detected in the data to be saved: 'Reservation { 'Id': b100f795-873a-4351-a3ee-59304d76eafa } [Added] <- Guests Reservation { 'ReservationId': b100f795-873a-4351-a3ee-59304d76eafa } ReservationGuest { 'Id': f8487ddc-023d-408c-b6fd-300ebec31754 } [Added] <- Owner { 'OwnerId': f8487ddc-023d-408c-b6fd-300ebec31754 } Reservation { 'Id': b100f795-873a-4351-a3ee-59304d76eafa } [Added]'.",
" at Microsoft.EntityFrameworkCore.Internal.Multigraph2.ThrowCycle(List1 cycle, Func2 formatCycle)\r\n at Microsoft.EntityFrameworkCore.Internal.Multigraph2.BatchingTopologicalSort(Func2 formatCycle)\r\n at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.TopologicalSort(IEnumerable1 commands)\r\n at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IList1 entries, IUpdateAdapter updateAdapter)+MoveNext()\r\n at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)\r\n at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)\r\n at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList1 entriesToSave, CancellationToken cancellationToken)\r\n at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(DbContext _, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)\r\n at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementationAsyncTState,TResult\r\n at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementationAsyncTState,TResult\r\n at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)\r\n at Infrastructure.EfCore.ProtelDbContext1.SaveChangesAsync(CancellationToken cancellationToken) in C:\\source\\Pms.Backend\\src\\Infrastructure\\EfCore\\ProtelDbContext.cs:line 48\r\n at Infrastructure.EfCore.Repositories.EfBaseUnitOfWork.SaveChangesAsync() in C:\\source\\Pms.Backend\\src\\Infrastructure\\EfCore\\Repositories\\BaseUnitOfWork.cs:line 55\r\n at Pms.Core.Application.Reservation.Commands.CreateReservationCommandHandler.Handle(CreateReservationCommand command, CancellationToken cancellationToken) in C:\\source\\Pms.Backend\\src\\Pms.Core\\Application\\Reservation\\Commands\\CreateReservationCommandHandler.cs:line 33\r\n at MediatR.Pipeline.RequestExceptionProcessorBehavior2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate1 next)\r\n at MediatR.Pipeline.RequestExceptionProcessorBehavior2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate1 next)\r\n at MediatR.Pipeline.RequestExceptionActionProcessorBehavior2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate1 next)\r\n at MediatR.Pipeline.RequestExceptionActionProcessorBehavior2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate1 next)\r\n at MediatR.Pipeline.RequestPostProcessorBehavior2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate1 next)\r\n at MediatR.Pipeline.RequestPreProcessorBehavior2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate1 next)\r\n at Infrastructure.StartupConfiguration.MediatR.RequestLoggerBehavior2.Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate1 next) in C:\\source\\Pms.Backend\\src\\Infrastructure\\StartupConfiguration\\MediatR\\RequestLoggerBehavior.cs:line 24\r\n at Pms.Api.Controllers.ReservationsController.Create(CreateReservationCommand command) in C:\\source\\Pms.Backend\\src\\Pms.Api\\Controllers\\ReservationsController.cs:line 63\r\n at lambda_method(Closure , Object )\r\n at Microsoft.Extensions.Internal.ObjectMethodExecutorAwaitable.Awaiter.GetResult()\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.AwaitableObjectResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask1 actionResultValueTask)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|24_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|19_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\r\n at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Logged|17_1(ResourceInvoker invoker)\r\n at Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)\r\n at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)\r\n at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)\r\n at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)\r\n at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)\r\n at Infrastructure.Middleware.GlobalExceptionHandlerMiddleware.Invoke(HttpContext context, IResourceRepository resources) in C:\source\Pms.Backend\src\Infrastructure\Middleware\GlobalExceptionHandlerMiddleware.cs:line 34"

[suadev]

@ajcvickers Sorry, it looks this issue was created for about querying. I have trouble creating a new reservation. Any idea except making SaveChanges() twice?

[ajcvickers]

@suadev I think this is the issue you are looking for: #1699

[suadev]

@ajcvickers thank you, but I didn't get how to resolve this. You said optional relationships covered by 1699 but it is an open issue.

[ajcvickers]

@suadev Sorry, I should have been more explicit. This is currently not supported in EF Core--support is being tracked by #1699. For now, you will have to make two calls to SaveChanges--the first with null FK values, the second to set the FK values appropriately once there are entities saved to the database that can be referenced.