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

Setting foreign key to null by PropertyEntry.CurrentValue deletes entity #25360

Closed
majbo opened this issue Jul 29, 2021 · 1 comment · Fixed by #25957
Closed

Setting foreign key to null by PropertyEntry.CurrentValue deletes entity #25360

majbo opened this issue Jul 29, 2021 · 1 comment · Fixed by #25957
Labels
area-change-tracking closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@majbo
Copy link

majbo commented Jul 29, 2021

Given is an entity with a self-referencing relationship that is configured with DeleteBehavior.Cascade.

public class Tag
{
    public Guid Id { get; set; }
    public string Name { get; set; } = null!;
    public Tag? SynonymFor { get; set; }
    public Guid? SynonymForId { get; set; }
    public IEnumerable<Tag> Synonyms { get; set; } = null!;
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Tag>().HasMany(p => p.Synonyms)
        .WithOne(p => p.SynonymFor)
        .OnDelete(DeleteBehavior.Cascade);
}

When setting the foreign key to null using the PropertyEntry.CurrentValue, the EntityEntry.State changes to Deleted and thereby the call to SaveChanges deletes the entity.

var barTag = context.Tags.Single(p => p.Name == "Bar");
var barEntry = context.Entry(barTag);
barEntry.Property(nameof(Tag.SynonymForId)).CurrentValue = null;

I created a simple app here: (https://github.com/majbo/EFCascadeDeleteError)
Execute the unit tests. The first one succeeds, it sets the foreign key directly on the entity. The second one fails, it sets the foreign key using PropertyEntry.CurrentValue.

Include provider and version information

EF Core version: 5.0.8
Database provider: Microsoft.EntityFrameworkCore.Sqlite
Target framework: .NET 5.0
Operating system: Windows 10
IDE: Rider 2021.1.5

@ajcvickers
Copy link
Contributor

Note for triage: this appears to be a bug. Setting the FK to null should trigger delete orphans behavior when the relationship is set to cascade and delete orphans timing has not been changed. However, the relationship here is not required, which is causing this to be behave inconsistently.

@ajcvickers ajcvickers self-assigned this Aug 1, 2021
@ajcvickers ajcvickers added this to the 6.0.0 milestone Aug 3, 2021
ajcvickers added a commit that referenced this issue Sep 9, 2021
Fixes #25360

This is a case where an optional relationship is configured with delete orphans behavior. Since the relationship is optional it means that the FK value can be explicitly set to null. (This is not possible for required relationships, which is the usual case for delete orphans.) In this case the navigation fixer was not triggering behavior in the state manager to process deleting orphans.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 9, 2021
ajcvickers added a commit that referenced this issue Sep 10, 2021
Fixes #25360

This is a case where an optional relationship is configured with delete orphans behavior. Since the relationship is optional it means that the FK value can be explicitly set to null. (This is not possible for required relationships, which is the usual case for delete orphans.) In this case the navigation fixer was not triggering behavior in the state manager to process deleting orphans.
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-rc2 Sep 10, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-rc2, 6.0.0 Nov 8, 2021
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants