Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Make AddEntityFrameworkStores smarter with generics #1001

Closed
lixiaoyuan opened this issue Oct 20, 2016 · 18 comments
Closed

Make AddEntityFrameworkStores smarter with generics #1001

lixiaoyuan opened this issue Oct 20, 2016 · 18 comments

Comments

@lixiaoyuan
Copy link

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ArgumentException: GenericArguments[0], 'BusinessDb.Cor.EntityModel.SystemUser`1[System.Guid]', on 'Microsoft.AspNetCore.Identity.EntityFrameworkCore.UserStore`4[TUser,TRole,TContext,TKey]' violates the constraint of type 'TUser'. ---> System.TypeLoadException: GenericArguments[0], 'BusinessDb.Cor.EntityModel.SystemUser`1[System.Guid]', on 'Microsoft.AspNetCore.Identity.EntityFrameworkCore.UserStore`8[TUser,TRole,TContext,TKey,TUserClaim,TUserRole,TUserLogin,TUserToken]' violates the constraint of type parameter 'TUser'.
   at System.RuntimeTypeHandle.Instantiate(RuntimeTypeHandle handle, IntPtr* pInst, Int32 numGenericArgs, ObjectHandleOnStack type)
   at System.RuntimeTypeHandle.Instantiate(Type[] inst)
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   --- End of inner exception stack trace ---
   at System.RuntimeType.ValidateGenericArguments(MemberInfo definition, RuntimeType[] genericArguments, Exception e)
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at Microsoft.Extensions.DependencyInjection.IdentityEntityFrameworkBuilderExtensions.GetDefaultServices(Type userType, Type roleType, Type contextType, Type keyType)
   at Microsoft.Extensions.DependencyInjection.IdentityEntityFrameworkBuilderExtensions.AddEntityFrameworkStores[TContext,TKey](IdentityBuilder builder)
   at ErpApplication.Web.Startup.ConfigureServices(IServiceCollection services) in D:\外接系统\ErpWebApplication\src\ErpApplication.Web\Startup.cs:line 40
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
   at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.AspNetCore.Hosting.Internal.ConfigureServicesBuilder.Invoke(Object instance, IServiceCollection exportServices)
   at Microsoft.AspNetCore.Hosting.Internal.WebHost.EnsureApplicationServices()
   at Microsoft.AspNetCore.Hosting.Internal.WebHost.BuildApplication()

Here is my code

ApplicationDbContext.cs

public class ApplicationDbContext : IdentityDbContext<
        SystemUser<Guid>,
        EntityModel.IdentityRole, Guid,IdentityUserClaim,IdentityUserRole,IdentityUserLogin,IdentityRoleClaim,IdentityUserToken>
    {
        public ApplicationDbContext(DbContextOptions options)
            : base(options)
        {
        }        
        protected ApplicationDbContext()
        {            
        }
   }

Identity.cs

public class SystemUser<TKey> : IdentityUser<TKey,IdentityUserClaim,IdentityUserRole,IdentityUserLogin>
        where TKey : IEquatable<TKey>
    {
    }
    public class IdentityUserClaim : IdentityUserClaim<Guid>
    {
    }
    public class IdentityUserLogin : IdentityUserLogin<Guid>
    {
    }
    public class IdentityUserRole : IdentityUserRole<Guid>
    {
    }
    public class IdentityUserToken : IdentityUserToken<Guid>
    {
    }
    public class IdentityRole : IdentityRole<Guid,IdentityUserRole,IdentityRoleClaim>
    {
    }
    public class IdentityRoleClaim : IdentityRoleClaim<Guid>
    {
    }

Startup.cs

public void ConfigureServices(IServiceCollection services)
        {
            services.AddDbContext<ApplicationDbContext>((builder =>
                {
                    builder.UseSqlServer(Configuration.GetConnectionString("DefaultConnection"));
                }));
//this  throw new Exception
            services.AddIdentity<SystemUser<Guid>, BusinessDb.Cor.EntityModel.IdentityRole>()
                .AddEntityFrameworkStores<ApplicationDbContext,Guid>()
                .AddDefaultTokenProviders();

            services.AddMvc();
        }
@divega
Copy link

divega commented Oct 20, 2016

@lixiaoyuan what package versions are you using?

@lixiaoyuan
Copy link
Author

@divega

project.json

{
"buildOptions": {
"emitEntryPoint": true,
"preserveCompilationContext": true,
"debugType": "portable"
},
"dependencies": {
"Microsoft.NETCore.App": {
"version": "1.0.1",
"type": "platform"
},
//项目引用
"BusinessDb.Cor": "1.0.1",
//web
"Microsoft.AspNetCore.Server.IISIntegration": "1.0.0",
"Microsoft.AspNetCore.Server.Kestrel": "1.0.1",
"Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0",
"Microsoft.Extensions.Configuration.Json": "1.0.0",
"Microsoft.Extensions.Configuration.FileExtensions": "1.1.0-alpha1-21569",
//Diagnostics
"Microsoft.AspNetCore.Diagnostics": "1.0.0",
"Microsoft.Extensions.Logging.Console": "1.0.0",
//mvc
"Microsoft.AspNetCore.Mvc": "1.1.0-alpha1-21569",
//staticfiles
"Microsoft.AspNetCore.StaticFiles": "1.1.0-alpha1-21569",
//Identity
"Microsoft.AspNetCore.Identity": "1.0.0",
"Microsoft.AspNetCore.Identity.EntityFrameworkCore": "1.0.0",
//entityFramework
"Microsoft.EntityFrameworkCore": "1.0.1",
"Microsoft.EntityFrameworkCore.SqlServer": "1.0.1",
"Microsoft.EntityFrameworkCore.Tools": "1.0.0-preview2-final"
},
"frameworks": {
"netcoreapp1.0":{
"imports": [
"dotnet5.6",
"portable-net45+win8"
]
}
},
"publishOptions": {
"include": [
"wwwroot",
"web.config"
]
},
"runtimeOptions": {
"configProperties": {
"System.GC.Server": true
}
},
"scripts": {
"postpublish": [ "dotnet publish-iis --publish-folder %publish:OutputPath% --framework %publish:FullTargetFramework%" ]
},
"tools": {
"Microsoft.AspNetCore.Server.IISIntegration.Tools": "1.0.0-preview2-final"
}
}

@divega
Copy link

divega commented Oct 20, 2016

Note for triage:

The call to

            services.AddIdentity<SystemUser<Guid>, BusinessDb.Cor.EntityModel.IdentityRole>()
                .AddEntityFrameworkStores<ApplicationDbContext,Guid>()
                .AddDefaultTokenProviders();

Will try to register type UserStore<SystemUser, BusinessDb.Cor.EntityModel.IdentityRole, ApplicationDbContext, TKey> as the implementation for service type IUserStore<TUser>

Note that this generic version of UserStore in question has a generic constraint on TUser : IdentityUser<TKey>.

But our customer is trying to use a different generic version of IdentityUser and IdentityRole which allow for more customization:

    public class SystemUser<TKey> : IdentityUser<TKey, IdentityUserClaim, IdentityUserRole, IdentityUserLogin>
        where TKey : IEquatable<TKey>
    {
    }
...
    public class IdentityRole : IdentityRole<Guid,IdentityUserRole,IdentityRoleClaim>
    {
    }

Using the simpler versions works correctly:

    public class SystemUser<TKey> : IdentityUser<TKey> 
        where TKey : IEquatable<TKey>
    {
    }
    public class IdentityRole : IdentityRole<Guid>     {
    }

At this point I cannot remember if this is by design.

cc @HaoK

@divega
Copy link

divega commented Oct 20, 2016

@lixiaoyuan from your code snippets it is not clear whether you could use the simpler generic versions of IdentityUser and IdentityRole as the base classes for your entities:

    public class SystemUser<TKey> : IdentityUser<TKey> 
        where TKey : IEquatable<TKey>
    {
    }
    public class IdentityRole : IdentityRole<Guid>     {
    }

Could you clarify if in your scenario you actually need to use the base types that allow for full customization for some reason?

@lixiaoyuan
Copy link
Author

@divega

  1. I want to modify the entity primary key type.
  2. Add fields to user entity .

You said I did not have to inherit the seven base class?

But I still do not know the wrong code above the problem

If you can not read what I'm saying, because I am using it translate.google.cn

@divega
Copy link

divega commented Oct 21, 2016

@lixiaoyuan yes I understand. You should be able to change the key type and add properties by inheriting from the base types that only have one generic argument.

The explanation of why this currently doesn't work is in my previous comment at #1001 (comment). But please don't close the issue as I would like to discuss an approach for solving this with the team.

@lixiaoyuan
Copy link
Author

lixiaoyuan commented Oct 21, 2016

@divega Yes Mr,
Thank you very much for your answer.
I use only have one generic argument to solve.

and also ,Where on the Identity structure diagram?

@HaoK
Copy link
Member

HaoK commented Oct 25, 2016

This was by design we don't have any sugar for the stores that use the more complicated generic parameters.

@PaulRReynolds
Copy link

PaulRReynolds commented Oct 26, 2016

I'm also running into this issue since I need to customise the UserRole entity, but I don't see a way of doing this while using the simpler base type.

public class ApplicationRole : IdentityRole<string, ApplicationUserRole, IdentityRoleClaim<string>>
{
    public ApplicationRole() : base() { }
    public ApplicationRole(string name) : base(name) { }
}

public class ApplicationUserRole : IdentityUserRole<string>
{
    //Extra property required on User-Role entity
    public Guid LevelID { get; set; }
}
public class ApplicationDbContext : IdentityDbContext<ApplicationUser, ApplicationRole, string>
{
    public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options)
    : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder builder)
    {
            base.OnModelCreating(builder);            
    }
}

Is there any way to work around this restriction while using an extended IdentityUserRole?

@blowdart blowdart changed the title System.TypeLoadException GenericArguments[0], 'EntityModel.SystemUser1[System.Guid]', on 'Microsoft.AspNetCore.Identity.EntityFrameworkCore.UserStore8 Make AddEntityFrameworkStores smarter with generics Oct 26, 2016
@blowdart blowdart added this to the 1.2.0 milestone Oct 26, 2016
@HaoK
Copy link
Member

HaoK commented Nov 14, 2016

We think we should be able to make AddEntityFramework stores inspect the store to 'do the right thing' when using the base generic user store

@HaoK
Copy link
Member

HaoK commented Nov 16, 2016

@divega I was able to get to the point where using reflection we can infer all of the child entity types using TUser. Unfortunately the last step appears to be not infer able, we need a concrete class for UserStore that knows how to materialize the child entitys, today we have:

https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity.EntityFrameworkCore/UserStore.cs#L72

which is able to explicitly new up the default child entities. The problem when they plug in their own child entities, is that unless we ask them to specify the store type, we don't know how to implement these abstract types (we could switch directions and require the pocos to have a new() constraint instead)...

So the best we can do without adding new constraints I think is to add something like:
AddStores<TUserStore, TRoleStore>() which is just sugar for adding a user role/store. To use the base generics today, they HAVE to have a concrete store type which extends from our UserStore, we already added the simple overload that just takes TKey.

Thoughts?

@HaoK
Copy link
Member

HaoK commented Nov 16, 2016

Note the AddStores would be something we add on the builder in core since it has no actual dependencies on our EF implementation, its just a sugar method to add the store types correctly.

@HaoK
Copy link
Member

HaoK commented Nov 16, 2016

Nevermind, we already have AddUserStore<T>, AddRoleStore<T>, so I'm not sure how much better we can do than what we have today...

@HaoK
Copy link
Member

HaoK commented Nov 16, 2016

So the steps to make a custom generic base class EF store work is:

  1. Implement your EFStoreWithGenerics : UserStore<TUser, TKey, ...>
  2. Replace AddEntityFrameworkStores with: AddIdentity().AddUserStore<EFStoreWithGenerics>().AddRoleStore<EFRoleStoreWithGenerics>()

I don't think this is that unreasonable

@divega
Copy link

divega commented Nov 16, 2016

@HaoK From what I remember at the moment this should help. BTW, the worst part right is the exception you get if you deviate slightly from the only pattern that is supported. Can we improve that?

@HaoK
Copy link
Member

HaoK commented Nov 16, 2016

Sure, I'll take a look what we can do to improve the error message, that's probably the only thing we can do, (Point them to use AddUser/RoleStore with their store instead of AddEntityFrameworkStores)

@HaoK
Copy link
Member

HaoK commented Nov 16, 2016

How about these for the new exception messages:

"AddEntityFrameworkStores can only be called with a user that derives from IdentityUser<TKey>. If you are specifying more generic arguments, use IdentityBuilder.AddUserStore<TStore>() instead."

"AddEntityFrameworkStores can only be called with a role that derives from IdentityRole<TKey>. If you are specifying more generic arguments, use IdentityBuilder.AddRoleStore<TStore>() instead."

@HaoK
Copy link
Member

HaoK commented Nov 17, 2016

13ae7b2

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

No branches or pull requests

5 participants