IAmNotARobot
IAmNotARobot

Reputation: 149

Entity Framework: Introducing FOREIGN KEY constraint '' on table '' may cause cycles or multiple cascade paths

While trying to implement the schema found on this answer with Entity Framework I get an error:

Introducing FOREIGN KEY constraint 'FK_OptionValues_Products_ProductId' on table 'OptionValues' may cause cycles or multiple cascade paths

+---------------+     +---------------+
| PRODUCTS      |-----< PRODUCT_SKUS  |
+---------------+     +---------------+
| #product_id   |     | #product_id   |
|  product_name |     | #sku_id       |
+---------------+     |  sku          |
        |             |  price        |
        |             +---------------+
        |                     |
+-------^-------+      +------^------+
| OPTIONS       |------< SKU_VALUES  |
+---------------+      +-------------+
| #product_id   |      | #product_id |
| #option_id    |      | #sku_id     |
|  option_name  |      | #option_id  |
+---------------+      |  value_id   |
        |              +------v------+
+-------^-------+             |
| OPTION_VALUES |-------------+
+---------------+
| #product_id   |
| #option_id    |
| #value_id     |
|  value_name   |
+---------------+

The model classes are currently like so

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

    [ForeignKey("ProductId")]
    public int ProductId { get; set; }

    public string OptionName { get; set; }
    public Product Product { get; set; }
}

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

    [ForeignKey("ProductId")]
    public int ProductId { get; set; }

    [ForeignKey("OptionId")]
    public int OptionId { get; set; }

    public string OptionValueName { get; set; }
    public Product Product { get; set; }
    public Option Option { get; set; }
}

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

    [ForeignKey("ProductId")]
    public int ProductId { get; set; }

    public string Sku { get; set; }
    public decimal Price { get; set; }
    public Product Product { get; set; }
}

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

    [ForeignKey("ProductId")]
    public int ProductId { get; set; }

    [ForeignKey("ProductSkuId")]
    public int ProductSkuId { get; set; }

    [ForeignKey("OptionId")]
    public int OptionId { get; set; }

    [ForeignKey("OptionValueId")]
    public int OptionValueId { get; set; }

    public Product Product { get; set; }
    public ProductSku ProductSku { get; set; }
    public Option Option { get; set; }
    public OptionValue OptionValue { get; set; }
}

What am I doing wrong here? How could I fix that?

Upvotes: 3

Views: 1173

Answers (4)

Chris Schaller
Chris Schaller

Reputation: 16554

Theres a few competing things here, your model looks like it trying to use multiple FKs to ensure integrity, this doesn't match the general expectations of the default EF implementation, which just means you will have to do a lot of manual updates to individual fields as you insert and update them, and more importantly EF wont be able to identify the relationships correctly on its own, you will likely need to manage more of this in Fluent Configuration to get it right.

The following statements show how I have interpreted your model:

  • a Product has many "Skus" - ProductSku
  • a Product has many "Options" - Option
  • an Option has many named "values" - OptionValue
  • each OptionValue can be linked to many "SKUs" - SkuValue

If that is the case, then your diagram is not quite right and you class definitions should be changed.

In general I would recommend the following:

  1. SkuValue does not need an FK to Product, that is assumed from it's parent ProductSku
  2. OptionValue also does not need an FK to Product as this is assumed from it's parent Option
  3. SkuValue does not need an FK to Option as this is assumed from it's parent OptionValue

By leaving the FK to Product in each of these tables, EF is confused because it can see that by allowing these extra navigation links in the child tables, it is possible for your code to assign a different product say to an OptionValue than the Product that is defined in it's linked Option.

So while we often think that by leaving the fields there it will assist maintaining the integrity of the data, they make it easier for your code to violate it by creating links that should not normally be viable

It is possible to leave these fields in, but when the model has inconsistencies like this we need to add more configuration to get it across the line.

even with these changes, you will still have a bi-directional link between all these tables, as explained in this response: https://stackoverflow.com/a/65514280/1690217 you will need to remove one (or all) of the default cascade delete behaviours.


As a rule, I find it is safer to remove the Cascade Delete behaviour and manage it explicitly through fluent configurations, especially when your child records have deep repeated links.

The default conventions in EF work reasonably well as long as you stick to simple models, only use single keys in tables, don't duplicate key references to child items that are defined in the parent records, and don't form any child relationships with records that this table is already a child of.

If you want to remove the default cascade delete behaviour, I recommend this line in your OnModelCreating method:

modelBuilder.Conventions.Remove<OneToManyCascadeDeleteConvention>();

If you haven't removed this convention, then you will need to use Fluent Notation to configure the ForeignKey, even though you have already attempted to use the Attribute Notation

In Fluent notation, you can specify the cascade behaviour and either enable or disable it on individual keys, Fluent configuration of foreign keys will override defined attributes for the same key fields.
See Cascade Delete in the EF Core docs

modelBuilder
     .Entity<Product>()
     .HasMany(p => p.ProductsSkus)
     .WithOne(sku => sku.Product)
     .HasForeignKey(sku => sku.ProductId)
     .OnDelete(DeleteBehavior.Restrict);

Regarding correct use of ForeignKeyAttribute

Your use of the [ForeignKey] attribute is non compliant, so you must already be overriding this in fluent notation. When annotating a Navigation property, use ForeignKeyAttribute to describe the name of the FK property. You can also use it in the reverse manner, you can place the attribute on the FK property but not it is used to describe the Navigation property.

Notice the subtle difference:

public class Option
{
    public int Id { get; set; }
    public int ProductId { get; set; }
    public string OptionName { get; set; }
    [ForeignKey("ProductId")]
    public Product Product { get; set; }
}

or this way:

public class Option
{
    public int Id { get; set; }
    [ForeignKey("Product")]
    public int ProductId { get; set; }
    public string OptionName { get; set; }
    public Product Product { get; set; }
}

But take this the next step, get rid of magic strings and use a compile-time safe reference to the matching property:

public class Option
{
    public int Id { get; set; }
    [ForeignKey(nameof(Option.Product))]
    public int ProductId { get; set; }
    public string OptionName { get; set; }
    public Product Product { get; set; }
}

In this way you get more of an understanding of the importance that the value passed into the FK attribute is an actual reference to another field. Remember that when using attribute notation there is no support currently to specify the cascade behaviour of a FK, so make sure you enable or disable the convention that applies cascade delete automatically so that you do not have to define every FK using Fluent Notation.

Ultimately the following class definition (for the snippet you have shown) should work, just remove any fluent notation that might be overriding this:

Note that I have also defined the navigation links from the parent records to access the subordinate records, this allows your Linq queries to traverse in either direction through the navigation links without having to use joining syntax.

public class Product
{
    public int Id { get; set; }
    ...
    public virtual ICollection<Option> Options { get; set; } = new HashSet<Option>();
    public virtual ICollection<ProductSku> Skus { get; set; } = new HashSet<ProductSku>();
}

public class Option
{
    public int Id { get; set; }
    [ForeignKey(nameof(Option.Product))]
    public int ProductId { get; set; }

    public string OptionName { get; set; }
    public Product Product { get; set; }

    public virtual ICollection<OptionValue> Values { get; set; } = new HashSet<OptionValue>();
}

public class OptionValue
{
    public int Id { get; set; }
    [ForeignKey(nameof(OptionValue.Option))]
    public int OptionId { get; set; }

    public string OptionValueName { get; set; }
    public Option Option { get; set; }

    public virtual ICollection<SkuValue> Skus { get; set; } = new HashSet<SkuValue>();
}

public class ProductSku
{
    public int Id { get; set; }
    [ForeignKey(nameof(ProductSku.Product))]
    public int ProductId { get; set; }

    public string Sku { get; set; }
    public decimal Price { get; set; }
    public Product Product { get; set; }

    public virtual ICollection<SkuValue> Options { get; set; } = new HashSet<SkuValue>();
}

// many : many link between ProductSku and OptionValue
public class SkuValue
{
    public int Id { get; set; }
    [ForeignKey(nameof(SkuValue.ProductSku))]
    public int ProductSkuId { get; set; }
    [ForeignKey(nameof(SkuValue.OptionValue))]
    public int OptionValueId { get; set; }

    public ProductSku ProductSku { get; set; }
    public OptionValue OptionValue { get; set; }
}

Upvotes: 1

SammuelMiranda
SammuelMiranda

Reputation: 460

The problem here is how the foreign key is declared. On SQL you can se a Foreign Key with the "ON UPDATE" and "ON DELETE" arguments as "CASCADE" or "NO ACTION".

The "NO ACTION" would not cause that error, but the "CASCADE" will, since it will interact with the row, and all its references that would (or better yet, could) cause a ciclic reference, for example, delete on OptionsValue could cascade to a delete in Options, and so, cascade to a delete in Products, due to it cause a delete on ProductsSku that would also delete SkusValues (that refer to produts also, so cascade again ...).

If all is set (or at least the "parent" tables) to "NO ACTION", then deleting a row would only occur if there's no reference for it on the other tables, and would be safe to use. So on your declarations you need to set that cascade options to false (equivalent to the "NO ACTION" on the sql statement that is generated).

Upvotes: 0

Serge
Serge

Reputation: 43850

Try to remove from OptionValue and SkuValue class

 [ForeignKey("ProductId")]
  public int ProductId { get; set; }

since you have already ProductId in Option and ProductSku class

Upvotes: 0

bera
bera

Reputation: 89

This is because deleting a row from OptionValues will delete multiple rows from the other tables. In MySQL you should not be getting an error, as i saw this happens a lot in SQL Server. Try adding:

.WillCascadeOnDelete(false);

On your modelBuilder method.

This will not cascade delete all the other rows with the foreign key.

Upvotes: 2

Related Questions