dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.2k stars 9.94k forks source link

ASP.NET Core Identity database operations are extremely inefficient. #34717

Open TanvirArjel opened 3 years ago

TanvirArjel commented 3 years ago

I am a big fan of ASP.NET Core but its only sickening part is its Identity. All the database operations related to ASP.NET Core Identity has been written in an extremely inefficient way, maybe the possible worst way which is very frustrating to see when it is come from ASP.NET Core team.

The code for ASP.NET Core Identity was written in ASP.NET Core 1.0 and EF Core 1.0 is still same as in ASP.NET Core 6.0 and EF Core 6.0. Both ASP.NET Core and EF Core have been evolved greatly and ASP.NET Core Identity still in legacy mode.

Here are a few examples:

  1. Look at the following code for creating a User:
ApplicationUser applicationUser = new ApplicationUser { UserName = email, Email = email, EmailConfirmed = true };
IdentityResult userCreationResult = await _userManager.CreateAsync(applicationUser);

The above code generating the following SQL queries and commands:

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (3ms) [Parameters=[@__normalizedUserName_0='?' (Size = 50)], CommandType='Text', CommandTimeout='30']
      SELECT TOP(1) [a].[Id], [a].[AccessFailedCount], [a].[ConcurrencyStamp], [a].[DialCode], [a].[Email], [a].[EmailConfirmed], [a].[FullName], [a].[IsDisabled], [a].[LanguageCulture], [a].[LastLoggedInAtUtc], [a].[LockoutEnabled], [a].[LockoutEnd], [a].[NormalizedEmail], [a].[NormalizedUserName], [a].[PasswordHash], [a].[PhoneNumber], [a].[PhoneNumberConfirmed], [a].[SecurityStamp], [a].[TwoFactorEnabled], [a].[UserName]
      FROM [AspNetUsers] AS [a]
      WHERE [a].[NormalizedUserName] = @__normalizedUserName_0
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (1ms) [Parameters=[@__normalizedEmail_0='?' (Size = 50)], CommandType='Text', CommandTimeout='30']
      SELECT TOP(2) [a].[Id], [a].[AccessFailedCount], [a].[ConcurrencyStamp], [a].[DialCode], [a].[Email], [a].[EmailConfirmed], [a].[FullName], [a].[IsDisabled], [a].[LanguageCulture], [a].[LastLoggedInAtUtc], [a].[LockoutEnabled], [a].[LockoutEnd], [a].[NormalizedEmail], [a].[NormalizedUserName], [a].[PasswordHash], [a].[PhoneNumber], [a].[PhoneNumberConfirmed], [a].[SecurityStamp], [a].[TwoFactorEnabled], [a].[UserName]
      FROM [AspNetUsers] AS [a]
      WHERE [a].[NormalizedEmail] = @__normalizedEmail_0
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (3ms) [Parameters=[@p0='?' (DbType = Guid), @p1='?' (DbType = Int32), @p2='?' (Size = 4000), @p3='?' (Size = 4), @p4='?' (Size = 50), @p5='?' (DbType = Boolean), @p6='?' (Size = 100), @p7='?' (DbType = Boolean), @p8='?' (Size = 4), @p9='?' (DbType = DateTime2), @p10='?' (DbType = Boolean), @p11='?' (DbType = DateTimeOffset), @p12='?' (Size = 50), @p13='?' (Size = 50), @p14='?' (Size = 4000), @p15='?' (Size = 15), @p16='?' (DbType = Boolean), @p17='?' (Size = 4000), @p18='?' (DbType = Boolean), @p19='?' (Size = 50)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      INSERT INTO [AspNetUsers] ([Id], [AccessFailedCount], [ConcurrencyStamp], [DialCode], [Email], [EmailConfirmed], [FullName], [IsDisabled], [LanguageCulture], [LastLoggedInAtUtc], [LockoutEnabled], [LockoutEnd], [NormalizedEmail], [NormalizedUserName], [PasswordHash], [PhoneNumber], [PhoneNumberConfirmed], [SecurityStamp], [TwoFactorEnabled], [UserName])
      VALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10, @p11, @p12, @p13, @p14, @p15, @p16, @p17, @p18, @p19);

Look at the above commands, it made two completely unnecessary queries and pulling the same big object twice just to check whether a User already exists or not by UserName which could have been done by a single simple query and getting 1 bit of data.

  1. Look at the following code:
 IdentityResult addExternalLoginResult = await _userManager.AddLoginAsync(applicationUser, externalLoginInfo);

The above code generating the following SQL queries and commands:

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (1ms) [Parameters=[@__normalizedUserName_0='?' (Size = 50)], CommandType='Text', CommandTimeout='30']
      SELECT TOP(1) [a].[Id], [a].[AccessFailedCount], [a].[ConcurrencyStamp], [a].[DialCode], [a].[Email], [a].[EmailConfirmed], [a].[FullName], [a].[IsDisabled], [a].[LanguageCulture], [a].[LastLoggedInAtUtc], [a].[LockoutEnabled], [a].[LockoutEnd], [a].[NormalizedEmail], [a].[NormalizedUserName], [a].[PasswordHash], [a].[PhoneNumber], [a].[PhoneNumberConfirmed], [a].[SecurityStamp], [a].[TwoFactorEnabled], [a].[UserName]
      FROM [AspNetUsers] AS [a]
      WHERE [a].[NormalizedUserName] = @__normalizedUserName_0
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (1ms) [Parameters=[@__normalizedEmail_0='?' (Size = 50)], CommandType='Text', CommandTimeout='30']
      SELECT TOP(2) [a].[Id], [a].[AccessFailedCount], [a].[ConcurrencyStamp], [a].[DialCode], [a].[Email], [a].[EmailConfirmed], [a].[FullName], [a].[IsDisabled], [a].[LanguageCulture], [a].[LastLoggedInAtUtc], [a].[LockoutEnabled], [a].[LockoutEnd], [a].[NormalizedEmail], [a].[NormalizedUserName], [a].[PasswordHash], [a].[PhoneNumber], [a].[PhoneNumberConfirmed], [a].[SecurityStamp], [a].[TwoFactorEnabled], [a].[UserName]
      FROM [AspNetUsers] AS [a]
      WHERE [a].[NormalizedEmail] = @__normalizedEmail_0
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[@p0='?' (Size = 450), @p1='?' (Size = 450), @p2='?' (Size = 4000), @p3='?' (DbType = Guid)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      INSERT INTO [AspNetUserLogins] ([LoginProvider], [ProviderKey], [ProviderDisplayName], [UserId])
      VALUES (@p0, @p1, @p2, @p3);
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (4ms) [Parameters=[@p19='?' (DbType = Guid), @p0='?' (DbType = Int32), @p1='?' (Size = 4000), @p20='?' (Size = 4000), @p2='?' (Size = 4), @p3='?' (Size = 50), @p4='?' (DbType = Boolean), @p5='?' (Size = 100), @p6='?' (DbType = Boolean), @p7='?' (Size = 4), @p8='?' (DbType = DateTime2), @p9='?' (DbType = Boolean), @p10='?' (DbType = DateTimeOffset), @p11='?' (Size = 50), @p12='?' (Size = 50), @p13='?' (Size = 4000), @p14='?' (Size = 15), @p15='?' (DbType = Boolean), @p16='?' (Size = 4000), @p17='?' (DbType = Boolean), @p18='?' (Size = 50)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      UPDATE [AspNetUsers] SET [AccessFailedCount] = @p0, [ConcurrencyStamp] = @p1, [DialCode] = @p2, [Email] = @p3, [EmailConfirmed] = @p4, [FullName] = @p5, [IsDisabled] = @p6, [LanguageCulture] = @p7, [LastLoggedInAtUtc] = @p8, [LockoutEnabled] = @p9, [LockoutEnd] = @p10, [NormalizedEmail] = @p11, [NormalizedUserName] = @p12, [PasswordHash] = @p13, [PhoneNumber] = @p14, [PhoneNumberConfirmed] = @p15, [SecurityStamp] = @p16, [TwoFactorEnabled] = @p17, [UserName] = @p18
      WHERE [Id] = @p19 AND [ConcurrencyStamp] = @p20;
      SELECT @@ROWCOUNT;

This is even more disastrous! Just for single insert and related checking, it's making a complete massacre. This is in no way acceptable. Even the following command from the above code is completely unnecessary:

UPDATE [AspNetUsers] SET [AccessFailedCount] = @p0, [ConcurrencyStamp] = @p1, [DialCode] = @p2, [Email] = @p3, [EmailConfirmed] = @p4, [FullName] = @p5, [IsDisabled] = @p6, [LanguageCulture] = @p7, [LastLoggedInAtUtc] = @p8, [LockoutEnabled] = @p9, [LockoutEnd] = @p10, [NormalizedEmail] = @p11, [NormalizedUserName] = @p12, [PasswordHash] = @p13, [PhoneNumber] = @p14, [PhoneNumberConfirmed] = @p15, [SecurityStamp] = @p16, [TwoFactorEnabled] = @p17, [UserName] = @p18
      WHERE [Id] = @p19 AND [ConcurrencyStamp] = @p20;
      SELECT @@ROWCOUNT;
  1. Another instance, I have already submitted here: https://github.com/dotnet/aspnetcore/issues/32933
  2. Every UserManager method is expecting the IdentityUser while the only UserId is sufficient.

Last but not the least, I am sorry but to say, the whole ASP.NET Core Identity database operation code has been written in a possible worst way. It could have been accepted if was written by any other third party but it's from Microsoft where we believe the world's best software engineers do work.

Thank you.

ar0311 commented 3 years ago

Wouldn't this be a "by-design" Entity Framework issue? Not sure how it has much to do with Identity

TanvirArjel commented 3 years ago

@ar0311 No! It's not EF Core issue! EF Core is quite efficient if you can write EF Core code properly. It’s Microsoft ASP.NET Core Identity team who have written the EF Core code inside the Identify framework in possible worst way.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

SamJ26 commented 3 weeks ago

Any update on this issue? 3 years later and nothing has changed.