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

Convention-based model builder support for Data Annotations #107

Closed
divega opened this Issue Apr 22, 2014 · 27 comments

Comments

Projects
None yet
@divega
Copy link
Member

divega commented Apr 22, 2014

We should try to support at least the very basic ones, e.g. KeyAttribute for alpha, provided that the DataAnnotations build is ready for K.
Attributes to be implemented:

  • Key
  • Required
  • MaxLength
  • NotMapped
  • ConcurrencyCheck
  • TimeStamp
  • Table
  • Column
  • DatabaseGenerated
  • StringLength

Relationship

  • ForeignKey
  • InverseProperty
  • Required on Navigations

Not supported yet

  • ComplexType

@divega divega added this to the 1.0.0-alpha milestone Apr 22, 2014

@rowanmiller rowanmiller removed this from the 1.0.0-alpha milestone Apr 30, 2014

@bricelam bricelam added this to the Backlog milestone May 3, 2014

@rowanmiller rowanmiller modified the milestones: 1.0.0, Backlog May 22, 2014

@bricelam bricelam added the enhancement label Jul 1, 2014

@AndriySvyryd AndriySvyryd assigned AndriySvyryd and unassigned bricelam Oct 3, 2014

@rowanmiller rowanmiller added the pri0 label Nov 26, 2014

@rowanmiller rowanmiller changed the title Convention-based model builder support for DataAnnotations Convention-based model builder support for Data Annotations Jan 19, 2015

@divega divega assigned smitpatel and unassigned AndriySvyryd Jan 27, 2015

@divega

This comment has been minimized.

Copy link
Member Author

divega commented Jan 27, 2015

@smitpatel please talk to @AndriySvyryd about the details.

@pauldalyii

This comment has been minimized.

Copy link

pauldalyii commented Apr 9, 2015

I ran into another scenario where this issue causes problems.

Because .MaxLengh() isn't being honored on string properties, nvarchar(MAX) fields are generated in the db. Unique constraints aren't supported on nvarchar(MAX) fields, so an error is thrown during db generation not for applying the .MaxLength() annotation, but when you apply a Unique constraint on a string property where you think you've limited the length.

@ghost

This comment has been minimized.

Copy link

ghost commented May 1, 2015

👍
Please also provide annotation for uniqueness constraint as a model convention, so we don't have to "drop down" to manual model builder configuration probably like so:

[Unique]
public int MySurrogateKey { get; set; }
// here MySurrogateKey can be null as a unique value
[UniqueNotNull]
public int MySurrogateKey { get; set; }
// here MySurrogateKey can not be null
[UniqueAllowNull]
public int MySurrogateKey { get; set; }
// here MySurrogateKey can be null multiple times, but non-null values should be unique

copy: @Tratcher, @davidfowl, @NTaylorMullen

@smitpatel

This comment has been minimized.

Copy link
Contributor

smitpatel commented Jun 30, 2015

filed #2504 & #2505
Attributes to be implemented has been added in first post. We need to decide which relationship attributes we want to support.

@divega

This comment has been minimized.

Copy link
Member Author

divega commented Jun 30, 2015

We should support StringLength which is a synonym of MaxLength only applicable to strings.

@rowanmiller

This comment has been minimized.

Copy link
Member

rowanmiller commented Jun 30, 2015

+1 for [StringLength]
We also need to discuss [ForeignKey] and [InverseProperty]. My gut feel is we should probably support them with the same rules we had in EF6.

@smitpatel

This comment has been minimized.

Copy link
Contributor

smitpatel commented Aug 12, 2015

All the attributes mentioned in first post are implemented. Anything else remaining here?

@AndriySvyryd

This comment has been minimized.

Copy link
Member

AndriySvyryd commented Aug 13, 2015

This should be sufficient as the initial implementation. We'll file separate issues for remaining work.

@smitpatel smitpatel added 2 - Done and removed 1 - Working labels Aug 15, 2015

@smitpatel smitpatel modified the milestones: 7.0.0-beta7, 7.0.0 Aug 15, 2015

@salarcode

This comment has been minimized.

Copy link

salarcode commented Aug 26, 2015

How about the ability to change column name?

[Column(Name = "")]
@smitpatel

This comment has been minimized.

Copy link
Contributor

smitpatel commented Aug 26, 2015

@salarcode - ColumnAttribute has been implemented already. You can change column name using the method you have mentioned. 😄

@justdmitry

This comment has been minimized.

Copy link

justdmitry commented Aug 26, 2015

What about Index attribute? Defining non-unique indexes very popular/important feature. But I can't find issue about it.

@smitpatel smitpatel referenced this issue Aug 28, 2015

Closed

Beta 7 todos #23

2 of 2 tasks complete
@divega

This comment has been minimized.

Copy link
Member Author

divega commented Aug 28, 2015

@justdmitry In EF7 we support defining indexes using the fluent API but not an attribute, at least no yet. The IndexAttribute you are possibly referring to is something we added to the EF 6.x package at some point but never really became a standard DataAnnotation.

We don't want to copy the original attribute from EF6 as is because there are a few things in it that we would like to change. Also, having it in DataAnnotations directly would likely make more sense than adding it to the EF7 package.

The priority is going to be driven by customer feedback.

@divega

This comment has been minimized.

Copy link
Member Author

divega commented Aug 28, 2015

The priority is going to be driven by customer feedback.

I should mention though that it is highly unlikely that we will add IndexAttribute in the EF7 RTM timeframe.

@salarcode

This comment has been minimized.

Copy link

salarcode commented Aug 29, 2015

+1 for IndexAttribute
I don't know what changes you think of, but the Index itself and the IsUnique property is essential in my opinion, other properties are not that much.

@farlop

This comment has been minimized.

Copy link

farlop commented Sep 11, 2015

+1 for IndexAttribute.
I prefer to use DataAnnotations over FluentAPI whenever its possible. Keeps the code easier to understand.

@staff0rd

This comment has been minimized.

Copy link

staff0rd commented Sep 12, 2015

I've always thought that something like

entity.Property(e => e.ShipDate).DefaultValueSql("getutcdate()");

should instead be (or at least support) something like this;

[DefaultValueSql("getutcdate()")]
public DateTime ShipDate { get; set; }

Would this be considered for inclusion, or is it something an extension library should offer?

@ghk

This comment has been minimized.

Copy link

ghk commented Sep 12, 2015

+1 for IndexAttribute

@Yorie

This comment has been minimized.

Copy link

Yorie commented Sep 28, 2015

+1 for IndexAttribute, that was nice feature. Of course in parallel with "fluent API" implementation.

@yukozh

This comment has been minimized.

Copy link
Contributor

yukozh commented Oct 2, 2015

+1 for IndexAttribute

3 similar comments
@YZahringer

This comment has been minimized.

Copy link

YZahringer commented Nov 8, 2015

+1 for IndexAttribute

@rposener

This comment has been minimized.

Copy link

rposener commented Nov 17, 2015

+1 for IndexAttribute

@PavelKalsin

This comment has been minimized.

Copy link

PavelKalsin commented Nov 29, 2015

+1 for IndexAttribute

@yukozh

This comment has been minimized.

Copy link
Contributor

yukozh commented Nov 29, 2015

@reekoheek

This comment has been minimized.

Copy link

reekoheek commented Dec 8, 2015

+1 for IndexAttribute

@19317362

This comment has been minimized.

Copy link

19317362 commented Oct 2, 2017

+1 for IndexAttribute.

@AndriySvyryd

This comment has been minimized.

Copy link
Member

AndriySvyryd commented Oct 2, 2017

This issue is closed and will not be considered for planning purposes. Please direct all IndexAttribute feedback to #4050

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.