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

Multiple One-To-One Cascades #5871

Closed
moraleslos opened this issue Jun 27, 2016 · 17 comments
Closed

Multiple One-To-One Cascades #5871

moraleslos opened this issue Jun 27, 2016 · 17 comments
Milestone

Comments

@moraleslos
Copy link

Fairly new to EF and started to incorporate EF Core 1.0.0 RC2 into an ASP.NET core project. In a project I'm working on, I have an entity that has a couple of 1-to-1 references to another entity. The parent entity has a reference to the dependent entity in the POCO but not vice-versa. Looks something like this:

 public class Blog
    {
        public int BlogId { get; set; }
        public string Url { get; set; }

        public BlogImage HeaderImage { get; set; }
        public BlogImage FooterImage { get; set; }
    }

    public class BlogImage
    {
        public int BlogImageId { get; set; }
        public byte[] Image { get; set; }
        public string Caption { get; set; }
    }

Now no one else can use BlogImage except Blog and each BlogImage is unique-- No two Blogs can use the same BlogImage.

Therefore when a Blog gets deleted, I would like EF to delete cascade the two BlogImages as well. Also would like any updates to the Blog POCO get cascaded to the corresponding BlogImage(s) as well assuming there were updates to the BlogImage (say a caption was updated).

I looked at how 1-to-1 is generally implemented in EF and this requires the dependent to have a reference back to the parent AND the parent's ID. Then you can set the "HasForeignKey" in the model builder so EF can tell parent vs dependent and do cascade deletes. I'm "somewhat" ok with this if there was only one 1-to-1 reference in my domain model (wish we can eliminate the need for the parent ID entirely since this kinda mucks up the domain model-- I think Hibernate does this appropriately). So essentially I need to do something like the below:

public class Blog
    {
        public int BlogId { get; set; }
        public string Url { get; set; }

        public BlogImage HeaderImage { get; set; }
        public BlogImage FooterImage { get; set; }
    }

    public class BlogImage
    {
        public int BlogImageId { get; set; }
        public byte[] Image { get; set; }
        public string Caption { get; set; }

        public int HBlogId { get; set; }
        public Blog HBlog { get; set; }

        public int FBlogId { get; set; }
        public Blog FBlog { get; set; }

    }


class MyContext : DbContext
    {
        public DbSet<Blog> Blogs { get; set; }
        public DbSet<BlogImage> BlogImages { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Blog>()
                .HasOne(p => p.HeaderImage)
                .WithOne(i => i.HBlog)
                .HasForeignKey<BlogImage>(b => b.HBlogId)
                .OnDelete(DeleteBehavior.Delete);

            modelBuilder.Entity<Blog>()
                .HasOne(p => p.FooterImage)
                .WithOne(i => i.FBlog)
                .HasForeignKey<BlogImage>(b => b.FBlogId)
                .OnDelete(DeleteBehavior.Delete);
        }
    }

However cascade deletes still won't work due to how the schema is generated and the FK constraints placed. Not only that, the domain model now has two references and two IDs back to the parent (what happens if I needed to add another dependent like SideBarImage).

So I was wondering if there was a way, maybe a new feature, where I can just specify who is the parent in the relationship and also define the cascades appropriately. Something like this:


         modelBuilder.Entity<Blog>()
               .HasOne(p => p.HeaderImage)
               .IsParent()
               .OnCascade(Cascade.ALL)  // options are ALL, ADD, DELETE, UPDATE, NONE
@ajcvickers
Copy link
Contributor

@moraleslos There is a two part answer to this. First. the way you had the relationships configured seemed basically correct to me, except that you don't need to add any additional properties to your entity types in order to do this. So, using your original types:

public class Blog
{
    public int BlogId { get; set; }
    public string Url { get; set; }

    public BlogImage HeaderImage { get; set; }
    public BlogImage FooterImage { get; set; }
}

public class BlogImage
{
    public int BlogImageId { get; set; }
    public byte[] Image { get; set; }
    public string Caption { get; set; }
}

The mapping would look like this:

modelBuilder.Entity<Blog>()
    .HasOne(p => p.HeaderImage)
    .WithOne()
    .HasForeignKey(typeof(BlogImage), "HBlogId")
    .OnDelete(DeleteBehavior.Cascade);

modelBuilder.Entity<Blog>()
    .HasOne(p => p.FooterImage)
    .WithOne()
    .HasForeignKey(typeof(BlogImage), "FBlogId")
    .OnDelete(DeleteBehavior.Cascade);

This is the same as the mappings you posted except:

  • The inverse navigation properties back to Blog are not needed--EF can handle this "unidirectional" relationship.
  • The FK properties are created in "shadow state". This is how EF handles mapping to a relational schema without having to create properties in your CLR type.
  • The FK properties are nullable. This isn't obvious from the code, but by default EF creates nullable FKs when they are created in shadow state. The properties are nullable because there will be images that are header images but not footer images, and vice versa.

So that's the model that EF needs, but unfortunately some relational databases, notably SQL Server, do not support having multiple cascade relationships like this. So if you are using EF Migrations to create the database you may get an error something like this:

Introducing FOREIGN KEY constraint 'FK_BlogImages_Blogs_HBlogId' on table 'BlogImages' may cause cycles or multiple cascade paths. Specify ON DELETE NO ACTION or ON UPDATE NO ACTION, or modify other FOREIGN KEY constraints.

The reason EF tries to create the cascade actions in the database is so that dependent entities will be deleted automatically even if they are not currently loaded into the context. But if the entities are loaded into the context, then cascade behavior internal to EF will send deletes to the database. This means that you can edit the migration script to remove the cascade actions and you will still get automatic deletion as long as the entities are loaded. For example, this will work fine:

using (var context = new MyContext())
{
    var blog = context.Blogs
        .Include(e => e.FooterImage)
        .Include(e => e.HeaderImage)
        .Single();

    context.Remove(blog);

    context.SaveChanges();
}

The footer and header images will be automatically deleted when SaveChanges is called.

Hope this helps.

@moraleslos
Copy link
Author

@ajcvickers

Thanks! Didn't know I didn't need to specify the inverse navigation properties. This is what I was looking for (before this answer I ended up creating extension methods to mark the state of the dependents appropriately before calling SaveChanges).

And yes about EF Migrations-- that message is exactly what I encountered during an update-database so I had to remove the DeleteBehavior.Cascade option which led me to do this post. Now I understand the error.

So I'm good with the above. Not sure if this is in your territory: Can you have EF Migrations recognize databases so that it can generate the appropriate schema and therefore remove developer dependence on adjusting the generated code (or in this case, avoid creating one code for all DBs that can fail in scenarios like this)? I think that is how Hibernate does this-- there is a "dialect" you can specify for the target database. Migrations should already know the DB used from the "UseSqlServer" option. Just a suggestion.

@moraleslos
Copy link
Author

@ajcvickers

BTW I just tried the above and the schema generated is not what I was looking for. It is generating this for BlogImage:

BlogImage Table:

| BlogImageId | Image | Caption | HBlogId | FBlogId |

I can see why MSSQL doesn't like cascade deletes-- there could be values for both HBlogId and FBlogId (even though the application will not allow for that). Plus if I wanted to add another BlogImage reference or two in Blog, then the schema will add another two columns. The problem with this is that only one FK ID will have a value while the other(s) will always be null.

Wouldn't it be better to put the FKs to BlogImage in the Blog table itself, something like this:

Blog Table:

| BlogId | Url | HeaderBlogId | FooterBlogId |

BlogImage Table:

| BlogImageId | Image | Caption |

This is not only "cleaner", but cascade deletes/adds/updates should work without issues from the DB side. Is it possible to do the above with cascade deletes working in EF?

@ajcvickers
Copy link
Contributor

@moraleslos We only support cascade deletes from the principal to the dependent. That is, if the principal side of a relationship is deleted, then entities on the dependent side are also deleted. You could easily set up your schema like you described, but nothing would be cascade deleted. Have you tried creating this schema in a database and setting up cascade deletes? Does it work?

@moraleslos
Copy link
Author

I guess that was my original question. My Blog table will always be the principal and BlogImage will always be the dependent. BlogImage can not exist alone without being referenced by a Blog. Also no two Blogs can associate to the same BlogImage (otherwise this would be a many-to-many). Come to think of it, I can have another principal entity, say Forum, that can contain unique BlogImages. So really BlogImage would be a table containing unique blogimages that can be used by multiple principal entities.

Is there a way to, maybe via Fluent API, specify who is the parent so that cascade deletes can be done? Something like this:

modelBuilder.Entity<Blog>()
    .HasOne(p => p.HeaderImage)
    .WithOne(typeof(BlogImage))
    .IsParent()
    .OnDelete(DeleteBehavior.Cascade);

modelBuilder.Entity<Blog>()
    .HasOne(p => p.FooterImage)
    .WithOne(typeof(BlogImage))
    .IsParent()
    .OnDelete(DeleteBehavior.Cascade);

So this will make Blog the principal in the 1-to-1 relationship (and hence, by default, BlogImage is the dependent) and therefore you can specify cascade deletes afterwards. Again this is specific for a 1-to-1 relationship so you won't need IsParent() with a .WithMany(). When generating the schema using EF Migrations, you can enforce HeaderImageId and FooterImageId to be unique so the DB won't complain on cascades (I think).

Is the above possible now? If not, and if the suggestion seems reasonable, can the above be implemented in the next release?

@ajcvickers
Copy link
Contributor

@moraleslos If the FK of the relationship is in the Blog, then by definition the Blog is the dependent.

@ajcvickers
Copy link
Contributor

ajcvickers commented Jun 28, 2016

@moraleslos To express this a different way, if what you are after is a domain model with unidirectional relationships and no FKs where you don't want them, then it doesn't really matter how this is implemented in the database as long as it works. If you also care about the database schema, then we have to start looking at what the mapping to the database looks like, which for relationships basically means where the FKs are. And if you put the FKs in Blog, then I don't think you can have cascade deletes working at the database level.

Could we implement dependent to principal 1:1 cascade deletes in the state manager? Yes, but this is the first time I have heard a request for it. I will discuss with others whether we should do this, but I am doubtful that it will be high priority if we do decide to do it.

@moraleslos
Copy link
Author

@ajcvickers

Is this an EF thing where if FK is in the entity, then it is the dependent? Maybe I'm thinking in a "Hibernate" way but with Hibernate, this is not the case. You can have your principal contain the foreign key to the dependent. I think this is called a unidirectional 1-to-1 on a foreign key. See this:http://docs.jboss.org/hibernate/orm/3.3/reference/en/html/associations.html#assoc-unidirectional-121

@moraleslos
Copy link
Author

Wanted to add that with "unidirectional", it's always principal --> dependent, hence with unidirectional 1-to-1 on a foreign key, Blog --> BlogImage so Cascades should work assuming the DB constraints are done properly. If DB constraints can't be done, then yes to the "state manager" =)

@ajcvickers
Copy link
Contributor

@moraleslos I don't see anything in that doc that talks about principals or dependents. I'm saying that in a relational database the table with the FK is the dependent table. Which means that when the FK is mapped, the entity with the FK is the dependent entity. It's not really an EF thing.

Unidirectional is not always principal to dependent. Unidirectional just means that the relationship can be navigated one way but not the other.

Like I said, we could implement 1:1 cascade delete in the state manager where when an entity that contains an FK is deleted, then the entity that that FK points to is also deleted. I can see value in this. I'd still be interested to see the database schema that does the same thing.

@moraleslos
Copy link
Author

@ajcvickers

Well to me a unidirectional 1-to-1, say A --> B, is like a principal-dependent relationship where principal --> dependent.

Either way, my original question was really two-fold:

  1. POCOs contained inverse relationships
  2. Schema generated

As for #1, you already answered that the inverse side was not needed in the POCOs (and only defined using Fluent), which is great from a domain modeling perspective. For #2, I feel that having something like a unidirectional 1-to-1 on a FK will make the generated schema "cleaner" than what is currently being generated.

The reason why I'm suggesting the above is the use of BlogImage. The Blog / BlogImage example may not be a good example to use. Say that instead of BlogImage, I needed a DateTime, and that there was no DateTime type. Plus DateTime has multiple fields. DateTime can be referenced many times by many POCOs. DateTime by itself has no meaning other than by a referencing POCO. Not sure if you would call DateTime a "dependent" but to me it's just a type on the end of a unidirectional relationship. This is how I perceived BlogImage as well. So say I have the following:

class A
{
  DateTime Start { get; set; }
  DateTime Finish { get; set; }
  String Name  { get; set; }
}

class B
{
  DateTime Create { get; set; }
  DateTime Update { get; set; }
  String Value  { get; set; }
}

class DateTime
{
     int M { get; set; }
     int D { get; set; }
     int Y { get; set; }
     int Hr { get; set; }
     int Min { get; set; }
     int Sec { get; set; }
}

Now the way EF currently would generate the schema would look like:


| A_Id | Name |


| B_Id | Value |


| DateTime_ID | M | D | Y | Hr | Min | Sec | A_Id | B_Id |

With the above, it looks "awkward" that DateTime should "know" about A and B, even though it doesn't in the domain model. Once you add more persistent POCOs with DateTime fields, the DateTime table is not going to look good with all these FKs even from a DB perspective. Also for each record in DateTime, you'll gonna have a lot of null values for the FKs really for no reason.

Using the unidirectional 1-to-1 on FK approach makes more sense from a schema perspective-- it keeps the DateTime table clean of FKs and puts the FKs at the "parent", where any null values make sense since they're optional.


| A_Id | Name | Start | End |


| B_Id | Value | Create | Update |


| DateTime_ID | M | D | Y | Hr | Min | Sec |

Anyway this is how I would have liked for this to have worked. If this can't be done with EF, that's ok. I'll work with what is being done currently. As for a proper DB schema using unidirectional 1-to-1 on FK, I'll have to see if I can create something. I'm not sure how Hibernate does this but my gut feeling is that it just adds unique and check constraints for the "parent" tables where there are more than one dependent fields, something like:

ADD CONSTRAINT Start_NOT_EQUAL_TO_End_CHK
CHECK (Start <> End) ;

@ajcvickers
Copy link
Contributor

@moraleslos You can do all of that, including EF creating the unique constraint in the database, but it just won't cascade delete, either in EF or in the database.

@moraleslos
Copy link
Author

@ajcvickers

Correct, I know you can do the above by not specifying .HasOne().WithOne() in Fluent. I did this first and the schema was a-ok. However I needed the Cascade Delete so that is why I noticed you needed to add that HasOne().WithOne() so that I can add OnDelete() to it. Thus this was creating an awkward schema.

So back to my original question was-- can we just specify who the parent is in the relationship so that cascade deletes will work (if this is all you need)? It may not necessarily work in the DB (I think it can, but I'm no DB person), but at least from a state manager perspective. The reference to Hibernate was not to make it look like Hibernate, but that it can be done and Hibernate calls it unidirectional 1-to-1 on FK.

@ajcvickers
Copy link
Contributor

@moraleslos The way you specify the parent in EF is by using HasForeignKey or HasPrincipalKey. We debated extensively abstracting this but ultimately from a database mapping perspective everything comes down to where the FK is, and so we settled on the FK as the thing that you tell us about to tell us which is the parent. I recognize that you are using parent in a different way. You want something to be a child (dependent) in the relational mapping, but to be a parent (principal) in your domain model, and for EF to reason about that inverted parent/child relationship when doing cascade deletes. EF's support for cascade deletes mirrors that of a relational database. That is, when an entity with a given key is deleted, then entities that reference that key are also deleted. EF doesn't have support for deleting an entity that is referenced by an FK when the entity containing that FK is deleted. As far as I know, relational databases don't do this (without triggers. etc.) either. As I said, it is something that has merit, but I don't know what the priority would be.

@moraleslos
Copy link
Author

@ajcvickers

Yup I get where you're coming from. One of those ORM design issues!

The only thing I can think of at the moment, if you want to stick to the DB side of things, is to generate a FK column, a "generic" table name column and a "generic" field name column, for the dependent table if there are multiple one-to-one associations between the same entities. Something like a discriminator column when using inheritance. So using the DateTime example above, this is how I would approach it:


| A_Id | Name |

| 1 | Foo |


| B_Id | Value |

| 1 | Bar |


| DateTime_Id | M | D | Y | Hr | Min | Sec | Table_Name | Table_Id | Field_Name |
| 1 | 1 | 1| 16 | 2 | 0 | 0 | A | 1 | Start |
| 2 | 2 | 2 | 16 | 0 | 0 | 0 | A | 1 | Finish |

| 3 | 3 | 3 | 16 | 0 | 0 | 0 | B | 1 | Update |

Anyway I don't know how you would add the appropriate constraints in the DB for cascades but this cleans up the schema to me a bit while keeping the FK in the dependent. (Sorry about the formatting of the example tables)

@moraleslos
Copy link
Author

@ajcvickers

Or you can put those misc fields-- table id, table name and field name, maybe another pk-- into a separate 1-to-1 "linking" table specific for multiple 1-to-1 relationships between the same entities. Just a thought.

@ajcvickers
Copy link
Contributor

Filed #5910 to capture the possible inverted cascade delete feature.

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
@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
None yet
Projects
None yet
Development

No branches or pull requests

3 participants