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
13 tasks done
divega opened this issue Apr 22, 2014 · 27 comments
Closed
13 tasks done

Convention-based model builder support for Data Annotations #107

divega opened this issue Apr 22, 2014 · 27 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@divega
Copy link
Contributor

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
@AndriySvyryd AndriySvyryd assigned AndriySvyryd and unassigned bricelam Oct 3, 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
Copy link
Contributor Author

divega commented Jan 27, 2015

@smitpatel please talk to @AndriySvyryd about the details.

@pauldalyii
Copy link

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
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
Copy link
Member

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
Copy link
Contributor Author

divega commented Jun 30, 2015

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

@rowanmiller
Copy link
Contributor

+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.

@AndriySvyryd
Copy link
Member

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

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

How about the ability to change column name?

[Column(Name = "")]

@smitpatel
Copy link
Member

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

@justdmitry
Copy link

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

@divega
Copy link
Contributor 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
Copy link
Contributor 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
Copy link

+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
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
Copy link

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
Copy link

ghk commented Sep 12, 2015

+1 for IndexAttribute

@Yorie
Copy link

Yorie commented Sep 28, 2015

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

@yukozh
Copy link
Contributor

yukozh commented Oct 2, 2015

+1 for IndexAttribute

3 similar comments
@YZahringer
Copy link

+1 for IndexAttribute

@rposener
Copy link

+1 for IndexAttribute

@PavelKalsin
Copy link

+1 for IndexAttribute

@yukozh
Copy link
Contributor

yukozh commented Nov 29, 2015

@divega

@reekoheek
Copy link

+1 for IndexAttribute

@19317362
Copy link

19317362 commented Oct 2, 2017

+1 for IndexAttribute.

@AndriySvyryd
Copy link
Member

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

@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 Oct 15, 2022
@ajcvickers ajcvickers modified the milestones: 1.0.0-beta7, 1.0.0 Oct 15, 2022
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. type-enhancement
Projects
None yet
Development

No branches or pull requests