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

Using a navigation property to clear a foreign key fails in 2.0.0 #9466

Closed
svallis opened this issue Aug 18, 2017 · 10 comments
Closed

Using a navigation property to clear a foreign key fails in 2.0.0 #9466

svallis opened this issue Aug 18, 2017 · 10 comments
Labels
closed-no-further-action The issue is closed and no further action is planned.

Comments

@svallis
Copy link

svallis commented Aug 18, 2017

Given the following two (simplified) models, configured with the foreign key as DeleteBehavior.Restrict:

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

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

    public int? DownloadAssetId { get; set; }
    public Asset DownloadAsset { get; set; }
}

Trying to clear the asset with product.DownloadAsset = null; (after loading the data with .Include(x => x.DownloadAsset)) results in the following error:

The association between entity types 'Asset' and 'Product' has been severed but the foreign key for this relationship cannot be set to null. If the dependent entity should be deleted, then setup the relationship to use cascade deletes.

Clearing with product.DownloadAssetId = null; works as expected.

Obviously I don't want any entities to be deleted in this case; the only action I expect to occur is for the specified DownloadAssetId to be set to null.

It's kind of difficult for me to test it now, but I'm pretty sure this worked as I expected in 1.1.x.

Further technical details

EF Core version: 2.0.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: Visual Studio 2017 15.3

@ajcvickers
Copy link
Member

@svallis We made some changes to the way DeleteBehaviors work in 2.0. I can't find the announcement for this (@divega--do you know where it is?) but here is the draft I wrote for it in #8923:

New ClientSetNull delete behavior

In previous releases, DeleteBehavior.Restrict had a behavior for entities tracked by the context that more closed matched SetNull semantics. In EF Core 2.0, a new ClientSetNull behavior has been introduced as the default for optional relationships. This behavior has SetNull semantics for tracked entities and Restrict behavior for databases created using EF Core. In our experience, these are the most expected/useful behaviors for tracked entities and the database. DeleteBehavior.Restrict is now honored for tracked entities when set for optional relationships.

So it looks like you need to configure the relationship as DeleteBehavior.ClientSetNull, or just don't configure it at all since this is the default. Question: why were you explicitly configuring Restrict in your 1.1.x code? We were expecting this to be quite rare since it was the default for optional relationships anyway.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Aug 18, 2017
@divega
Copy link
Contributor

divega commented Aug 19, 2017

@ajcvickers I ended up not creating announcements for many of the changes. Instead this information was added to the documentation, both in the cascade delete topic and on https://docs.microsoft.com/en-us/ef/core/miscellaneous/1x-2x-upgrade.

I am also planning to update that topic with additional information for Visual Studio 2015 and .NET Framework users that I have been adding to the prerequisite section of the announcement blog post. That way we can have a single resource that we can keep up to date and we can point customers too.

Then an announcement pointing to that topic should help. Thoughts?

@svallis
Copy link
Author

svallis commented Aug 19, 2017

Thanks for the responses.

Question: why were you explicitly configuring Restrict in your 1.1.x code?

Bear in mind this code base has been around since pre 1.0.0. Some of the foreign keys were being created in my migrations with ReferentialAction.Cascade, which I didn't want. I added the following in OnModelCreating():

// make referential delete behaviour restrict instead of cascade for everything
foreach (var relationship in builder.Model.GetEntityTypes().SelectMany(x => x.GetForeignKeys()))
{
    relationship.DeleteBehavior = DeleteBehavior.Restrict;
}

Commenting that out in 2.0.0 and generating a new migration is still trying to create ReferentialAction.Cascade relationships in places that I don't want. Switching to the new DeleteBehavior.ClientSetNull is perfect though, many thanks!

@ajcvickers
Copy link
Member

@divega Sounds good.

@ajcvickers
Copy link
Member

@svallis Thanks for the information. Glad it is working for you now. (FYI: relationships get cascade delete by convention when the FK is not nullable.)

@ajcvickers
Copy link
Member

Hi @svallis. We are gathering information on the use of EF Core pre-release builds. You reported this issue shortly after the release of 2.0.0 RTM. It would be really helpful if you could let us know:

  • Did you consider testing your code against the pre-release builds?
  • Was there anything blocking you from using pre-release builds?
  • What do you think could make it easier for you to use pre-release builds in the future?

Thanks in advance for any feedback. Hopefully this will help us to increase the value of pre-release builds going forward.

@svallis
Copy link
Author

svallis commented Sep 12, 2017

Did you consider testing your code against the pre-release builds?
No. A day or so after 2.0.0 dropped I just switched. Seat of your pants stuff.

Was there anything blocking you from using pre-release builds?
Time. I know it's probably the excuse everybody gives, but as much as new releases get me excited to move code over, time spent on a pre-release ultimately doesn't feel like time well spent when I have dev work to be getting on with. I realise this isn't necessarily in the spirit of things, but no point me answering unless it's honest!

What do you think could make it easier for you to use pre-release builds in the future?
I don't think there is anything. I plan to continue being an early adopter for new releases, and I am comfortable with the fact that things may be a little hairy until x.x.1 or x.x.2, but getting in on pre-release builds is not on my agenda currently.

@ajcvickers
Copy link
Member

@svallis Thanks for the feedback; much appreciated! Curious, how long would you consider it acceptable to wait for the x.x.1 version?

@svallis
Copy link
Author

svallis commented Sep 13, 2017

@ajcvickers I don't think there's a fixed time-frame, it'd always scale with how stable the x..x.0 release was, and whether or not any issues directly impacted my projects. If while migrating to a new release I hit something that was causing serious problems I'd just shelve the upgrade until such a time as it makes sense to do so. Clients see the value (and will pay) to keep their web applications on the latest version of the framework, but if it doesn't happen for weeks (or even months) after the initial release there's no problem.

@ajcvickers
Copy link
Member

Thanks @svallis

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned.
Projects
None yet
Development

No branches or pull requests

3 participants