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

IdentityUser exposure #7

Open
fiseni opened this issue Jul 5, 2020 · 7 comments
Open

IdentityUser exposure #7

fiseni opened this issue Jul 5, 2020 · 7 comments

Comments

@fiseni
Copy link
Contributor

fiseni commented Jul 5, 2020

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

  • Models should not contain properties of type IdentityUser, just id of the user (in this case guid). Make the property type string. The responsibility of the services would be to reconcile the id with the appropriate user persisted in db (if required for some operation)
  • ViewModels should not contain IdentityUser properties either. Here just add properties that you would show on the view, name, email or whatever. Dont expose more information than needed.

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

@Youssef1313
Copy link
Owner

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

fiseni commented Jul 6, 2020

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.
dotnet-architecture/eShopOnWeb#203
dotnet-architecture/eShopOnWeb#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
Copy link
Owner

Youssef1313 commented Jul 6, 2020

@fiseni Thanks again :)

  • As far as I understood, the repository pattern and the specification pattern are not really needed for relatively small projects. So, there is no need to bother with them for small projects as they'll make complexity without a real benefit. and I should start using them only when I get to bigger projects. Is that correct?
  • Regarding the IdentityUser, I'm still not getting how to correctly have the guid in my models. For sure I'll replace IdentityUser with a guid string, but what after that? should I make a query to the database to get the IdentityUser whenever I need it and I've only the guid? Or there's something better than that?

@fiseni
Copy link
Contributor Author

fiseni commented Jul 6, 2020

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

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

fiseni commented Jul 6, 2020

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

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

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

2 participants