Optimize the member save as part of the member login process, by-passing locking and audit steps and handling only the expected update properties

[AndyButland]

Prerequisites

• I have added steps to test this contribution in the description below

Attempts to resolve: #13768

Description

This PR is looking to handle reports of database locking when logging members in under load. I haven't been able to replicate it myself, but those in the linked issue have (along with a few forum posts).

It's clear though there's a lot going on post login for a member, so I thought to go straight to trying to optimize.

The main issue is that we save the member after login - specifically to update:

• The last login date.
• The security stamp (if concurrent logins are disabled, which is the default).
• The update date on the member.

We do this though via a full save operation (the same as if saved via the backoffice) where we have to consider quite a few things that aren't going to be amended in this scenario: all other system properties, all the "content" properties, tags, audit, and, in particular, locking of the whole member tree.

It doesn't seem we need any of these in this situation. In particular - though I'd like reviewers to consider and verify this - we don't need the locking of the member tree here. If we were updating content properties, that could have relations between each other - we should following what we do for documents and lock. But here we are just updating these system fields, and it's fine if they work in a "last one wins" fashion without locking.

So when merged this PR will:

• Add a check in the MemberUserStore.UpdateAsync method, and if we are only updating the last login date and/or the security stamp, no longer call Save on IMemberService but a new optimized, asynchronous method UpdateLoginPropertiesAsync.
• This will call up to the repository and save the member, but with:
  • No locking.
  • Only handling these two properties and the update date.
  • Not handling tags.
  • Not writing an audit record for content updates (though keeping the writes to the log table).
  • Also keeping the publishing of notifications)

Considerations for Review

Maybe it would be better to omit the notifications here? As we're only doing a partial save. It could be that someone made amends to the member object in the "Saving" notification that wouldn't actually get saved.

Other the other hand, if you are looking at when the member is "Saved", you might want to have the notification here.

Testing

To test you'll need to create a method of logging in as a member - either using a controller as described in the linked issue or setting up the necessary partials and view components as per the handy gist here or the docs.

Check the member login works as before.

Verify updates to the specific fields in the database via:

select lastLoginDate, securityStampToken from cmsMember where nodeId = <member id>
select versionDate from umbracoContentVersion where nodeid = <member id>

And via the front-end (i.e. to check there's no old values being cached) via something like:

@inject IMemberManager MemberManager;
@inject IMemberService MemberService;
@if (MemberManager.IsLoggedIn())
{
    var currentMemberIdentity = await MemberManager.GetCurrentMemberAsync();
    var currentMember = MemberService.GetByKey(currentMemberIdentity.Key);

    <div>
      Logged in as: @currentMemberIdentity.Name
      (last login: @currentMemberIdentity.LastLoginDateUtc,
      last update: @currentMember.UpdateDate)
    </div>
  }

[Copilot]

Pull Request Overview

This PR optimizes the member login process by bypassing the full save logic (including locking, tag updates, and audit logging) when only the login-related properties are updated. Key changes include introducing a new asynchronous method UpdateLoginPropertiesAsync in the member service and repository, modifying MemberUserStore.UpdateAsync to selectively call the optimized update method, and adding unit tests for the new login property update scenario.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs	Added new test validating login properties update and verifying that the full save is not called.
src/Umbraco.Infrastructure/Security/MemberUserStore.cs	Refactored UpdateAsync to call UpdateLoginPropertiesAsync when only login properties are updated.
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs	Introduced UpdateLoginPropertiesAsync with an optimized update query for login properties.
src/Umbraco.Core/Services/MemberService.cs	Added UpdateLoginPropertiesAsync implementation that bypasses audit recording.
src/Umbraco.Core/Services/IMemberService.cs	Updated the interface to include the new optimized login update method.
src/Umbraco.Core/Persistence/Repositories/IMemberRepository.cs	Updated the interface to include the new optimized method for updating login properties.

[Migaroez]

Regarding the consideration, I feel like we have to keep both notifications as else we would be introducing a functional change. In the future I think it would be wise to split this into its own set of notifications, but for now adding the following bit of code might help implementers react to the events in a more controlled manner

savingNotification.State.Add("LoginPropertiesOnly", true);

I have tried my best to break the fix but it seems solid 🚀

[AndyButland]

Thanks @Migaroez - and good idea on the notification state, I've added that. I'll merge this and include for 13.9 since that's the version where people are running into concerns, and from there merge up for 16.1.