search

Youssef1313 / AskQ

3 stars 3 forks source link

IdentityUser exposure #7

Open fiseni opened 4 years ago

fiseni commented 4 years ago

You might start refactoring the usage of IdentityUser in the models:

Ps. I'll add other hints tomorrow, kinda late here :)

Youssef1313 commented 4 years ago

Thanks for the tips :) I really appreciate the offer to help.

I didn't get the "Services" and "Specifications" idea well and how the string guid will map correctly to the correct user.

It's late too here 😄 See you tomorrow, and thanks again for helping.

fiseni commented 4 years ago

I think you should forget the specifications for now. They offer keeping the queries in your domain model, and then materialize by some outer infrastructure. The other benefit is that if you decide to change the ORM, you would have much less work to do. You would just rewrite/adjust the abstract repository and the spec evaluator. Trust me, you won't face these issues for a long time, so just drop it for now. I think was talking on the subject in few other threads. https://github.com/dotnet-architecture/eShopOnWeb/issues/203 https://github.com/dotnet-architecture/eShopOnWeb/issues/165

I would advise to drop the repository pattern in your projects as well for now. DbSets are already form of a repository, and DbContext is already Unit of Work. By creating repositories, you add abstraction on top of these abstractions. You should be sure if you really need that additional abstraction before using it. They would offer switching the ORM easily, which also you won't do it often in your projects (as for different DBs, Efcore already providing providers for most of them).

Youssef1313 commented 4 years ago

@fiseni Thanks again :)

fiseni commented 4 years ago

Yes, you might say that. But the size of the project is also not always a factor. Once you get familiar with all the concepts, it won't be a burden to do it even for small projects. What I wanna say is just get one step at a time. Initially try to grasp some more basic concepts (like why you shouldn't have IdentityUser property, etc). You also would be surprised that even for very large projects, there are people not using repository pattern. There are a lot of approaches, they would go for vertical modularity, feature based structures.

Yes, exactly. Whenever you need more information for the user, you have the ID, and you just query the DB and get the whole user. This can be done within the controller, within some separate service etc. Don't be afraid to make some additional DB queries. There is always a trade off, in the name of better encapsulation, you will find yourself doing additional queries, and that's ok. Btw, use UserManager and SignInManager services of identity to work with the user, instead of retrieving directly from the context.

Youssef1313 commented 4 years ago

Thanks. I'm really glad with your help here.

One small question is I often see an ApplicationUser class that inherits from IdentityUser but not adding any additional properties to it. Is the reason for that just to be future-proof in case additional properties are needed? or there is another benefit?

fiseni commented 4 years ago

Yes, it just sets your own user entity, for future upgrades. In Startup, while registering the identity services you define your inherited user class if you're using one.

            services.AddDefaultIdentity<IdentityUser>(options => options.SignIn.RequireConfirmedAccount = true)
                .AddEntityFrameworkStores<ApplicationDbContext>();
Youssef1313 commented 4 years ago

@fiseni Thanks :) I've worked on separating the models to a separate project as a first step (the next step in mind is to separate EF-Related things to infrastructure project). I hope you can review the steps I've taken in #6 if you've the time to.