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

Use KeyAtrribute for scaffolding to support OData #15677

Closed
cilerler opened this issue May 10, 2019 · 6 comments · Fixed by #16682
Closed

Use KeyAtrribute for scaffolding to support OData #15677

cilerler opened this issue May 10, 2019 · 6 comments · Fixed by #16682
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@cilerler
Copy link

OData requires [Key] attribute for entities where the primary key property name is not Id (it can't use .HasKey() info in FluentAPI)

Please allow scaffolding to generate Key Attributes if the column is name is not Id or the key is different than the Id property.

@ajcvickers
Copy link
Member

Notes from triage: the discussion in #11003 on how [Key] is used by OData indicates that it can be useful to have [Key] on properties even if doing so is not sufficient for EF Core mapping. Another example of this is data binding where properties attributed with [Key] may be hidden by default.

A couple of concrete things that come out of this:

  • When working on Attribute (Data Annotations) mapping for composite primary keys #11003 we should consider the benefits of using [Key] with something else that specifies ordering, as opposed to a new attribute that specifies both. However, the new attribute approach is likely to be cleaner, so this is not a decision that we should do one verses the other, just a note to consider both.
  • When scaffolding from a database, we should consider putting [Key] on the properties (in addition to scaffolding fluent API) either:

Note that we should also make sure that we don't log a warning the properties have [Key] but are correctly configured in the fluent API anyway.

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 23, 2019

This would be really nice to have, also for EF Core Power Tools - is this up for grabs, of should I overide the service as suggested here in the meantime:

#11003 (comment)

@divega
Copy link
Contributor

divega commented Jun 23, 2019

@ErikEJ I think it is ok if you want to go ahead with a PR. @ajcvickers, any concerns?

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 23, 2019

As I understand this, the goal is to always scaffold Key attribute for all involved properties when DataAnnotations are requested. Or always?

@divega
Copy link
Contributor

divega commented Jun 24, 2019

My understanding is that KeyAtrribute being used will still depend on whether you ask for DataAnnotations to be used, but no longer on whether the key is composite. And in the latter case we will still generate the HasKey method so that the key mapping works for EF Core (because we need the key member ordering information).

The relevant comment is #11003 (comment).

@ajcvickers
Copy link
Member

@ErikEJ Agree with Diego's last comment. Also, almost any issue that isn't closed is something that we would take a PR for--it doesn't have to have good-first-issue on it, which is what up-for-grabs has now become.

ErikEJ added a commit to ErikEJ/EntityFramework that referenced this issue Jul 20, 2019
when DataAnnotations have been requested.

fixes dotnet#15677
ajcvickers pushed a commit that referenced this issue Jul 22, 2019
Always scaffold [Key] attribute on all involved properties
when DataAnnotations have been requested.

fixes #15677
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 22, 2019
@ajcvickers ajcvickers modified the milestones: Backlog, 3.0.0 Jul 22, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview8 Jul 29, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview8, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants