aspnet / Identity

[Archived] ASP.NET Core Identity is the membership system for building ASP.NET Core web applications, including membership, login, and user data. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.96k stars 869 forks source link

Shouldn't we have a Microsoft.AspNetCore.Identity.UserManager<TUser, TKey> below the UserManager<TUser>? #1934

Closed BenjaminCharlton closed 6 years ago

BenjaminCharlton commented 6 years ago

In Microsoft.AspNetCore.Identity there are two imprtant classes: UserManager<TUser> and RoleManager<TRole>

However, the go-to User and Role for a lot of people are Microsoft.AspNetCore.Identity.IdentityUser<TKey> and Microsoft.AspNetCore.Identity.IdentityRole<TKey>.

If in their implementation TKey is int (or anything other than string) then various functions such as FindByIdAsync(string userId) won't build.

I think that Microsoft.AspNetCore.Identity.UserManager<TUser> should inherit from a new generic base class Microsoft.AspNetCore.Identity.UserManager<TUser, TKey> and Microsoft.AspNetCore.Identity.RoleManager<TRole> should inherit from a new generic base class Microsoft.AspNetCore.Identity.RoleManager<TRole, TKey>

This would not be a breaking change for anyone using the existing implementation with a string Id but it would help those of us using other primary key types.

What do you think? If you like the idea I think I could contribute the necessary changes.

HaoK commented 6 years ago

We intentionally decided to move away from the concept of TKey at the manager layer in the initial version of AspNet.Core Identity. Its up to the stores to provide a mechanism to convert TKey to a string (usually its just calling .ToString()). The ids are always treated as strings at the core manager identity layer. That said Stores are free to persist the id string in whatever TKey format (and why the store implementations have a TKey)

For your specific example of ints, just call to string before passing into FindById

BenjaminCharlton commented 6 years ago

Thank you, HaoK. I think it'd be neater to have generic implementations to make things type safe and avoid unnecessary casts but your solution will certainly work in their absence. I appreciate your response :-)