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

Add navigation property on model #28

Closed
nbarbettini opened this issue Nov 14, 2017 · 13 comments
Closed

Add navigation property on model #28

nbarbettini opened this issue Nov 14, 2017 · 13 comments
Milestone

Comments

@nbarbettini
Copy link
Owner

nbarbettini commented Nov 14, 2017

Is it necessary to add a navigation property on the TodoItem model pointing to the User model? I may have forgotten about this. (Reminder to investigate)

@sadqiang
Copy link

sadqiang commented Dec 4, 2017

Is it also necessary to add a navigation property of type IEnumerable(TodoItem) to a derived class ApplicationUser?

@nbarbettini
Copy link
Owner Author

Is it also necessary to add a navigation property of type IEnumerable(TodoItem) to a derived class ApplicationUser?

Hypothetically, how would you use that property?

@mattwelke
Copy link

mattwelke commented Dec 18, 2017

According to https://docs.microsoft.com/en-us/ef/core/modeling/relationships, you must use navigation properties on both sides of the one-to-many relationship. The one on the "one" side is a collection navigation property. Interestingly, while in traditional ASP.NET (at least according to what I learned in school), the navigation properties were IEnumerable, t seems with EF Core, the official documentation instructs you to use the more specific List.

The example in the official documentation is:

public class Blog
{
    public int BlogId { get; set; }
    public string Url { get; set; }

    public List<Post> Posts { get; set; }
}

public class Post
{
    public int PostId { get; set; }
    public string Title { get; set; }
    public string Content { get; set; }

    public int BlogId { get; set; }
    public Blog Blog { get; set; }
}

Concerning the idea of adding a navigation property for the User, according to the class I'm taking on ASP.NET in school right now, it's recommended to not touch the built in User classes. Instead, we create layers in between, that are capable of attaching information about the user to the controller so that you can decide what to do with authentication and authorization during the request, while leaving ASP.NET itself the only thing that directly manipulates the User classes.

So, @sadqiang , to implement what you described, where users have their TodoItems associated with them, you wouldn't mix the models together with EF but you would store information about the User on a new column on the TodoItems (perhaps called "owner") and this would be one of the values from the Identity Framework tables, probably the user's "name". Then, in your Controller or Service layer you can query for the TodoItems owned by a particular User. This extends to anything you do with Users, like create tokens with claims for them, etc. Now the token is linked to TodoItems, indirectly.

I'd have to dig more into the ASP.NET Core documentation to see if this style my class uses is inspired by official best practices. But my hunch right now is that it's a good approach. I'd be happy to post some examples later that we can consider adding to the book to demonstrate this layer in between.

@sadqiang
Copy link

@Welkie , Thank you very much for guiding me to the correct path. Your well-written elaboration should be documented in the book.

@mattwelke
Copy link

@sadqiang No problem. :)

To expand on what I said earlier a bit more, one advantage this approach has (not linking the User model to the other EF models) is that you've decoupled your identity management from the rest of your app. What if, one day, you choose to use Auth0 for your identity management instead of Identity Framework? Or, what if you choose to extract your identity management out into its own app (perhaps not even ASP.NET Core) so that multiple client apps can use it? Since it's decoupled, it won't matter that the model classes can't see the the User class, because your client apps can talk to the identity server app over the wire, exchanging the user's name (like I described above) as a string to identity who owns the resource.

This is the way I like to design my apps (aka spend most of my time thinking about the future, and hoping that doesn't prove to be a waste of time :P).

I guess we can consider this issue closed? When I have time I plan to submit a pull request with some content for the book demonstrating this approach.

@sadqiang
Copy link

@Welkie : Thank you again. You can close this issue.

@mattwelke
Copy link

@nbarbettini ^

@nbarbettini
Copy link
Owner Author

Going to leave this open to remind myself to add extra explanation when I revise the book. 👍

@nbarbettini nbarbettini added this to the 2.0 milestone Jan 15, 2018
@sadqiang
Copy link

@Welkie : Loose-coupling the users and TodoItem model by not providing navigation property ends is a good practice. However, if we really consider coupling issue, why don't we separate both tables into each own database? In the current implementation, user identity tables and TodoItem table are placed in the same database under the same ApplicationDbContext. Is it not 100% a decoupled approach, right?

@mattwelke
Copy link

mattwelke commented Jan 18, 2018

@sadqiang It depends on how much you want to decouple. In this case, we're talking about classes and how they depend on each other. What you're describing is a way to separate how the user data is stored from the business data, which I absolutely think is worth doing for a complex application. For example, maybe we want to move to MongoDB for storing the data after we realize our application fits that data model.

Funny enough, I'm looking into doing a rewrite of an app whose data fits the document model, where I'll be sticking to MongoDB, and I'm considering ASP.NET Core. I would definitely have to use a separate SQL database for the user data so that it works with Entity Framework.

But I think @nbarbettini wouldn't want to go that far for this book. I think that most web apps we create tend to use just one particular SQL database.

@sadqiang
Copy link

sadqiang commented Jan 19, 2018

@Welkie : OK. Thank you very much. By the way, a single Asp.net core app can have more than one DbContext, right?

@nbarbettini
Copy link
Owner Author

@sadqiang There shouldn't be any problem with multiple DbContexts (as long as they are different classes), although that's somewhat unusual in my experience. I've used multiple database types before (SQL/relational for main data plus Redis for caching, or Elastic for search) but that's more a matter of using specialized tools for a particular task.

@nbarbettini nbarbettini modified the milestones: 2.0, 1.1 Apr 6, 2018
@nbarbettini
Copy link
Owner Author

store information about the User on a new column on the TodoItems (perhaps called "owner") and this would be one of the values from the Identity Framework tables, probably the user's "name". Then, in your Controller or Service layer you can query for the TodoItems owned by a particular User.

Great suggestion - this is in fact what the application already does 👍 (The TodoItem.OwnerId property, which is enforced at the service layer)

@sadqiang To add on to what @Welkie said about loose coupling - most people would not argue that storing two tables in the same database is too much coupling. What tight coupling usually refers to is mixing concerns, classes that do "too much", or classes that depend directly on other classes (instead of interfaces that can be swapped out or mocked).

Loose coupling can be tricky in practice! It's one of the things that you need to spend a lot of time thinking about if you are architecting a new application or API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants