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

Migrations: ModelDiffer should find diff on navigations #5155

Closed
smitpatel opened this issue Apr 22, 2016 · 9 comments
Closed

Migrations: ModelDiffer should find diff on navigations #5155

smitpatel opened this issue Apr 22, 2016 · 9 comments
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement

Comments

@smitpatel
Copy link
Member

ModelSnapshot contains navigation information now. ModelDiffer should utilize this info in computing differences.

@bricelam
Copy link
Contributor

Thinking about how this information can be used, I've can only think of one case:

  • Detect renamed foreign key columns by looking for the same navigation (or inverse navigation) property names.

Are there any others? @ajcvickers @AndriySvyryd @anpete @divega

@divega
Copy link
Contributor

divega commented Aug 12, 2016

@bricelam that is the same one I was aware of.

@divega
Copy link
Contributor

divega commented Aug 12, 2016

FWIW, you mentioned it when you created the original bug #2140.

@bricelam
Copy link
Contributor

Oh good, my brain is deterministic over a duration of at least one year.

@bricelam
Copy link
Contributor

bricelam commented Aug 24, 2016

Here is the rule I've come up with.

If a would-be-added property is part of one or more foreign key constraints, and any of those foreign key constraints...

  1. Has the same principal entity type, and
  2. Has the same number of properties, and
  3. Has a would-be-removed property in the same position, and
  4. Either
    1. The navigation property name is the same, or
    2. The inverse navigation property name is the same

...then treat it as a renamed column.

Irrelevant information:

  • The property's facets
  • Whether the navigation property is required
  • The principal properties (including their name, position, and facets)
  • The inverse navigation's multiplicity

Sound good?

@bricelam
Copy link
Contributor

bricelam commented Aug 24, 2016

Here are some simple cases it covers.

 class Person {
     public int Id { get; set; }
     public ICollection<Book> Books { get; }
 }
 class Book {
-    public int PersonId { get; set; }
+    public int AuthorId { get; set; }
     public Person Author { get; set; }
 }
 class Person {
     public int Id { get; set; }
-    public ICollection<Book> Books { get; }
+    public ICollection<Book> AuthoredBooks { get; }
 }
 class Book {
-    public int PersonId { get; set; }
+    public int AuthorId { get; set; }
     public Person Author { get; set; }
 }
 class Person {
     public int Id { get; set; }
     public ICollection<Book> Books { get; }
 }
 class Book {
-    public int PersonId { get; set; }
-    public Person Person { get; set; }
+    public int AuthorId { get; set; }
+    public Person Author { get; set; }
 }

It may lead to unexpected renames and alters if you change the type to a property that doesn't convert.

 class Person {
-    public int Id { get; set; }
+    public Guid Guid { get; set; }
     public ICollection<Book> Books { get; }
 }
 class Book {
-    public int AuthorId { get; set; }
+    public Guid AuthorGuid { get; set; }
     public Person Author { get; set; }
 }

Before this change, you'd get DropColumn("AuthorId") and AddColumn("AuthorGuid").
After this change, you'll get RenameColumn and AlterColumn.
However, if it went from int to long, the new operations are preferred.

@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 24, 2016
@ajcvickers
Copy link
Member

Sounds good to me.

@divega
Copy link
Contributor

divega commented Aug 25, 2016

@bricelam if you try to remove an existing FK and add a new FK that points to a different alternate key in the same entity type as the principal side, what would happen? (I think it is a bit ambiguous in the rules you described, but I hope it isn't in the implementation 😄).

@bricelam bricelam added blocked and removed closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels Aug 29, 2016
@bricelam
Copy link
Contributor

Closing in favor of doing #6582 instead.

@bricelam bricelam added closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed blocked labels Sep 21, 2016
@bricelam bricelam removed this from the 1.1.0 milestone Sep 21, 2016
@bricelam bricelam removed their assignment Sep 21, 2016
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. type-enhancement
Projects
None yet
Development

No branches or pull requests

5 participants