-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Attribute (Data Annotations) mapping for composite primary keys #11003
Comments
@ajcvickers this issue becomes a big deal upon OData usage, is there any chance to bump the priority up. |
@cilerler Can you provide some more details as to why OData changes things here? |
@ajcvickers in my case database may have new fields, and time to time I need to add those to code. The easy workaround for me is generating the new DbContext through scaffolding and compare it with the previously generated one. Since scaffolding uses namespace MyProject.Models
{
[Table("Items")]
public partial class Item
{
[Column("ID")]
public long Id { get; set; }
public string Notes { get; set; }
}
[Table("ProcessorSettings")]
public partial class ProcessorSettings
{
[StringLength(50)]
public string Name { get; set; } // <= IF it would set the [Key] for this property, I wouldn't need to define it later
[StringLength(2)]
public string Code { get; set; }
[StringLength(500)]
public string Description { get; set; }
}
} EF// ...
public virtual DbSet<ProcessorSetting> ProcessorSettings { get; set; }
public virtual DbSet<Item> Items { get; set; }
// ...
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
// ...
modelBuilder.Entity<ProcessorSettings>(entity =>
{
entity.HasKey(e => new { e.Name, e.Code });
entity.Property(e => e.Name)
.IsUnicode(false)
.ValueGeneratedNever();
entity.Property(e => e.Code).IsUnicode(false);
entity.Property(e => e.Description).IsUnicode(false);
entity.Property(e => e.Value).IsUnicode(false);
});
modelBuilder.Entity<Item>(entity =>
{
entity.Property(e => e.Notes).IsUnicode(false);
});
//...
} ODatapublic static IEdmModel GetEdmModel()
{
var builder = new ODataConventionModelBuilder();
// ...
builder.EntitySet<Item>("Item"); // Automatically locates the `Id` column as `Key`
builder.EntitySet<ProcessorSetting>("ProcessorSettings").EntityType.HasKey(e => new { e.Name, e.Code });
// ...
return builder.GetEdmModel();
} |
@cilerler I don't see any composite keys in that example. |
@ajcvickers updated as |
@cilerler Can you show the attributes that would be needed by OData to configure this as a composite key, including how the order of the two parts of the composite key is defined? |
@ajcvickers Just having |
@cilerler That is something we can never support because the order of the properties in the composite key is critical for correctness. I'm actually very surprised that OData allows this given they they presumably have the same problem since the order is also critical to the OData model, but maybe it is constrained in some other way that isn't apparent here. |
Based on stackoverflow post, OData is actually not caring about order at all. It's all about which are the key properties which should be bound when calling Get method. Order of properties does not matter because ODataRoute attribute specify which parameter should be bound with which key property. @cilerler - You are free to add KeyAttribute to your model to make OData work. The EF model will not complain since Fluent API is overriding data annotation based configuration. The OData issue does not change anything in this issue. |
Thanks @smitpatel for clarifying that. |
Thanks, @smitpatel.
I was under the impression that I was asking if you may make the issue priority, did I sound different? I got into the details because of @ajcvickers comments. As I mentioned above, it already works the way it is, and I have to do a manual check about the keys each time I do scaffolding which is an error-prone step. Just wanted to nudge to get attention on this issue. 😁 |
@cilerler - I just wanted to clarify 2 different aspect. This issue particularly talks about support for KeyAttribute at runtime. i.e. not requiring Fluent API configuration. In your particular case, we can scaffold the code which would work with OData & it would still work with EF Core. It is just matter of scaffolding KeyAttribute which would be ignored by EF. But I was commenting that it would not be good idea to EF to write some code which can be misleading. If you want to got bit crazy (warning: internal code so it can break between EF core releases): All you need is to remove the check for 1 property. Then your scaffolding code would KeyAttribute for composite key so OData is happy. And it will still generate Fluent API so EF Core is also happy. |
Thank you for clarifying that and even further providing me a workaround. 🤗 I'm sorry for hijacking this issue; it wasn't my intention. |
See also the notes on #15677 (comment) |
I'm currently doing a migration from EF6 to EF Core 2.2 and where we use composite PK's extensively and do it all through decoration with the KeyAttribute and ColumnAttribute. I couldn't bear to do literally hundreds of statements in fluent, so I think I have a workaround, but please note THIS HAS NOT BEEN PROPERLY TESTED YET. Add this to OnModelCreating // get all composite keys (entity decorated by more than 1 [Key] attribute
foreach (var entity in modelBuilder.Model.GetEntityTypes()
.Where(t =>
t.ClrType.GetProperties()
.Count(p => p.CustomAttributes.Any(a => a.AttributeType == typeof(KeyAttribute))) > 1))
{
// get the keys in the appropriate order
var orderedKeys = entity.ClrType
.GetProperties()
.Where(p => p.CustomAttributes.Any(a => a.AttributeType == typeof(KeyAttribute)))
.OrderBy(p =>
p.CustomAttributes.Single(x => x.AttributeType == typeof(ColumnAttribute))?
.NamedArguments?.Single(y => y.MemberName == nameof(ColumnAttribute.Order))
.TypedValue.Value ?? 0)
.Select(x => x.Name)
.ToArray();
// apply the keys to the model builder
modelBuilder.Entity(entity.ClrType).HasKey(orderedKeys);
} Let me know if this helps anyone looking for a workaround, as the key attribute was a really useful feature of EF6 IMHO |
Another one I'd very much like to see implemented in EF Core. Let's hope this kind of thing gets done soon so EF Core has feature parity with EF classic. |
@cmorgan091 I am also doing a port from EF6 and found your workaround useful. I understand the reasons for the team not supporting KeyAttribute and ColumnAttribute but for those of us trying to get onto the current version, this makes life a bit eaiser! |
You are life saver @cmorgan091 |
Closing the summer of 2021 and still no support for DataAnnotations for this (3 years old feature enhancement item)? |
Fixes #11003 This PR introduces a new `[PrimaryKey]` which follows the same pattern as `[Index]` in that it is applied to the entity type class and takes an ordered list of property names. It takes precedence over any `[Key]` attributes on properties, since these may still be needed for OData or other technologies. `PrimaryKey` and `Keyless` cannot be used on the same type.
Fixes #11003 This PR introduces a new `[PrimaryKey]` which follows the same pattern as `[Index]` in that it is applied to the entity type class and takes an ordered list of property names. It takes precedence over any `[Key]` attributes on properties, since these may still be needed for OData or other technologies. `PrimaryKey` and `Keyless` cannot be used on the same type.
Fixes #11003 This PR introduces a new `[PrimaryKey]` which follows the same pattern as `[Index]` in that it is applied to the entity type class and takes an ordered list of property names. It takes precedence over any `[Key]` attributes on properties, since these may still be needed for OData or other technologies. `PrimaryKey` and `Keyless` cannot be used on the same type.
I'd like to declare a primary key that consists of multiple columns only with the
Key
attribute, just like for all other keys and as it was supported in EF6. This is the only situation when I have to go to theOnModelCreating
method in another file to find out about the basic structure of the entity.Currently I have to write this scattered code outside of where the entity is defined:
I'd prefer to keep it simple and in a single place, the entity class:
I could imagine it would be helpful to get rid of the slightly unintuitive
Column
attribute just for this and define the order where it belongs and is used, theKey
attribute, like in[Key(Order = 1)]
.I'm wondering why there isn't already an issue for this. I have found one or two issues that might be similar but I don't understand them well enough to consider them a match for this topic. Also I couldn't see this mentioned in the sparse information I could find about the upcoming EF Core 2.1 so I have to assume it's not included in the next version.
The text was updated successfully, but these errors were encountered: