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:
SkuValue does not need an FK to Product, that is assumed from it's parent ProductSkuOptionValue also does not need an FK to Product as this is assumed from it's parent OptionSkuValue 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; }
}
