Unit of work implementation needed

[ghidello]

Hi, accordingly to this issue the identity managers, including the UserManager, are based on the assumption that the user store is based on an unit of work pattern so basically the only methods actually performing crud operations are the ones in the IUserStore interface. I didn't read all your code so I'd like to ask if you had taken that issue in account with your implementation.

[giorgos07]

Hello @ghidello Thank you for your comment. The issue you provided on your link has an interesting conversation. From what i know so far and as an outcome from the conversation in the link you provided, ASP.NET Identity is built with the following in mind: the CreateAsync, UpdateAsync and DeleteAsync methods of base Store interfaces are the ones responsible for persisting changes to the database. The rest of the methods are either retrieving data or manipulate ApplicationUser and ApplicationRole entities without committing to the database. So that is the philosophy in which i built this sample project. If you have some time please take a look at the implementation and report anything that you do not like or can be improved. And i am sorry for my late response :)

[ghidello]

Ciao @giorgos07!
I started writing you a long comment then I reread yours and I realized that I could write you a very short answer: my understanding of the linked comment is that noone can write to the database (except for CreateAsync, UpdateAsync and DeleteAsync) including all the methods referring to the other tables like UserLogins and UserClaims.

I guess that my previous explanation can be handy anyway so here you are (even only for clarifying if I understood the topic correctly):

The IUserStore methods are called by the IUserManager ones which in turn are called by the application logic for the identities management. Let's say that you want to add a user login:

public virtual async Task<IdentityResult> AddLoginAsync(TUser user, UserLoginInfo login)
{
	ThrowIfDisposed();
	var loginStore = GetLoginStore();
	if (login == null)
	{
		throw new ArgumentNullException(nameof(login));
	}
	if (user == null)
	{
		throw new ArgumentNullException(nameof(user));
	}

	var existingUser = await FindByLoginAsync(login.LoginProvider, login.ProviderKey);
	if (existingUser != null)
	{
		Logger.LogWarning(4, "AddLogin for user {userId} failed because it was already assocated with another user.", await GetUserIdAsync(user));
		return IdentityResult.Failed(ErrorDescriber.LoginAlreadyAssociated());
	}
	await loginStore.AddLoginAsync(user, login, CancellationToken);
	return await UpdateUserAsync(user);
}

The UserManager method calls the store's AddLoginAsync method first and then it calls the UpdateUserAsync method:

private async Task<IdentityResult> UpdateUserAsync(TUser user)
{
	var result = await ValidateUserInternal(user);
	if (!result.Succeeded)
	{
		return result;
	}
	await UpdateNormalizedUserNameAsync(user);
	await UpdateNormalizedEmailAsync(user);
	return await Store.UpdateAsync(user, CancellationToken);
}

As you can see the user is validated first and the eventually saved: in the EntityFramework implementation the user and the user login would be saved togheter at this point because their AddLoginAsync is simply adding it to the Context.

In your AddLoginAsync implementation there is a call to the method _usersLoginsTable.AddLoginAsync(user, login); which in my understanding inserts the login in the database without an explicit transaction:

public Task AddLoginAsync(ApplicationUser user, UserLoginInfo login)
{
	const string command = "INSERT INTO dbo.UsersLogins " +
						   "VALUES (@LoginProvider, @ProviderKey, @UserId, @ProviderDisplayName);";

	return _sqlConnection.ExecuteAsync(command, new
	{
		login.LoginProvider,
		login.ProviderKey,
		UserId = user.Id,
		login.ProviderDisplayName
	});
}

Actually, I'm not sure I followed your code and the Identity one correctly, and I don't know if I'm raising an issue that cannot happen but, theoretically, in this example if the user is not valid it's update operation will be stopped but only after having inserted the user login.

Does it make sense?

[giorgos07]

@ghidello thank you again for your extensive comment. I was not aware of the behavior you described and it seems that it can cause some problems in certain situations.

Entity Framework has a built-in mechanism to track changes in objects and then commit all changes at once. Dapper has a different logic and does not have something similar. So i will have to think of a way to mimic EF's behavior.

[giorgos07]

@ghidello It's been a long time since you opened this issue but i found some time to work on this project and i really made some great improvements. In short, i made the User and Role stores to work with the UOW pattern in mind as we discussed (a lot) earlier in this thread. Please feel free to take look
on my implementation so i can close this thread. Thank you again for your useful comment.

[ghidello]

Sorry for not answering sooner but I haven’t had a chance to go through your updated code. Next time I’ll need to use dapper and identity I’ll check your library for sure.