Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/Umbraco.Core/Cache/DistributedCacheExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,30 @@ public static void RefreshMemberCache(this DistributedCache dc, params IMember[]
=> dc.RefreshMemberCache(members.AsEnumerable());

public static void RefreshMemberCache(this DistributedCache dc, IEnumerable<IMember> members)
=> dc.RefreshByPayload(MemberCacheRefresher.UniqueId, members.DistinctBy(x => (x.Id, x.Username)).Select(x => new MemberCacheRefresher.JsonPayload(x.Id, x.Username, false)));

=> dc.RefreshByPayload(
MemberCacheRefresher.UniqueId,
GetPayloads(members, false));

[Obsolete("Use the overload accepting IEnumerable instead to avoid allocating arrays. This overload will be removed in Umbraco 13.")]
public static void RemoveMemberCache(this DistributedCache dc, params IMember[] members)
=> dc.RemoveMemberCache(members.AsEnumerable());

public static void RemoveMemberCache(this DistributedCache dc, IEnumerable<IMember> members)
=> dc.RefreshByPayload(MemberCacheRefresher.UniqueId, members.DistinctBy(x => (x.Id, x.Username)).Select(x => new MemberCacheRefresher.JsonPayload(x.Id, x.Username, true)));
=> dc.RefreshByPayload(
MemberCacheRefresher.UniqueId,
GetPayloads(members, true));

// Internal for unit test.
internal static IEnumerable<MemberCacheRefresher.JsonPayload> GetPayloads(IEnumerable<IMember> members, bool removed)
=> members
.DistinctBy(x => (x.Id, x.Username))
.Select(x => new MemberCacheRefresher.JsonPayload(x.Id, x.Username, removed)
{
PreviousUsername = x.HasAdditionalData &&
x.AdditionalData!.TryGetValue(Cms.Core.Constants.Entities.AdditionalDataKeys.MemberPreviousUserName, out var previousUsername)
? previousUsername?.ToString()
: null,
});

#endregion

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ public JsonPayload(int id, string? username, bool removed)

public string? Username { get; }

public string? PreviousUsername { get; set; }

public bool Removed { get; }
}

Expand Down Expand Up @@ -121,6 +123,13 @@ private void ClearCache(params JsonPayload[] payloads)
// https://github.com/umbraco/Umbraco-CMS/pull/17350
// https://github.com/umbraco/Umbraco-CMS/pull/17815
memberCache.Result?.Clear(RepositoryCacheKeys.GetKey<IMember, string>(CacheKeys.MemberUserNameCachePrefix + p.Username));

// If provided, clear the cache by the previous user name too.
if (string.IsNullOrEmpty(p.PreviousUsername) is false)
{
memberCache.Result?.Clear(RepositoryCacheKeys.GetKey<IMember, string>(p.PreviousUsername));
Comment thread
AndyButland marked this conversation as resolved.
memberCache.Result?.Clear(RepositoryCacheKeys.GetKey<IMember, string>(CacheKeys.MemberUserNameCachePrefix + p.PreviousUsername));
}
}
}
}
14 changes: 14 additions & 0 deletions src/Umbraco.Core/Constants-Entities.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace Umbraco.Cms.Core;
Comment thread
AndyButland marked this conversation as resolved.

public static partial class Constants
{
public static class Entities
{
public static class AdditionalDataKeys
{
public const string MemberPreviousUserName = "previousUsername";

public const string MemberGroupPreviousName = "previousName";
}
}
}
11 changes: 6 additions & 5 deletions src/Umbraco.Core/Handlers/PublicAccessHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,17 @@ public PublicAccessHandler(IPublicAccessService publicAccessService) =>

private void Handle(IEnumerable<IMemberGroup> affectedEntities)
{
var keyName = Constants.Entities.AdditionalDataKeys.MemberGroupPreviousName;
foreach (IMemberGroup grp in affectedEntities)
{
// check if the name has changed
if ((grp.AdditionalData?.ContainsKey("previousName") ?? false)
&& grp.AdditionalData["previousName"] != null
&& grp.AdditionalData["previousName"]?.ToString().IsNullOrWhiteSpace() == false
&& grp.AdditionalData["previousName"]?.ToString() != grp.Name)
if ((grp.AdditionalData?.ContainsKey(keyName) ?? false)
&& grp.AdditionalData[keyName] != null
&& grp.AdditionalData[keyName]?.ToString().IsNullOrWhiteSpace() == false
&& grp.AdditionalData[keyName]?.ToString() != grp.Name)
{
_publicAccessService.RenameMemberGroupRoleRules(
grp.AdditionalData["previousName"]?.ToString(),
grp.AdditionalData[keyName]?.ToString(),
grp.Name);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Core/Models/MemberGroup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public string? Name
// if the name has changed, add the value to the additional data,
// this is required purely for event handlers to know the previous name of the group
// so we can keep the public access up to date.
AdditionalData["previousName"] = _name;
AdditionalData[Constants.Entities.AdditionalDataKeys.MemberGroupPreviousName] = _name;
}

SetPropertyValueAndDetectChanges(value, ref _name, nameof(Name));
Expand Down
17 changes: 14 additions & 3 deletions src/Umbraco.Core/Services/MemberService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -770,16 +770,27 @@
throw new ArgumentException("Cannot save member with empty name.");
}

var previousUsername = _memberRepository.Get(member.Id)?.Username;

scope.WriteLock(Constants.Locks.MemberTree);

_memberRepository.Save(member);

if (publishNotificationSaveOptions.HasFlag(PublishNotificationSaveOptions.Saved))
{
scope.Notifications.Publish(
savingNotification is null
// If the user name has changed, populate the previous user name in the additional data, so the cache refreshers
// have it available to clear the cache by the old name as well as the new.
if (string.IsNullOrWhiteSpace(previousUsername) is false &&
string.Equals(previousUsername, member.Username, StringComparison.OrdinalIgnoreCase) is false)
{
member.AdditionalData![Constants.Entities.AdditionalDataKeys.MemberPreviousUserName] = previousUsername;
}

MemberSavedNotification memberSavedNotification = savingNotification is null
? new MemberSavedNotification(member, evtMsgs)
: new MemberSavedNotification(member, evtMsgs).WithStateFrom(savingNotification));
: new MemberSavedNotification(member, evtMsgs).WithStateFrom(savingNotification);

scope.Notifications.Publish(memberSavedNotification);

Check warning on line 793 in src/Umbraco.Core/Services/MemberService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

❌ New issue: Bumpy Road Ahead

Save has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
}

Audit(AuditType.Save, 0, member.Id);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.

using NUnit.Framework;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Builders.Extensions;
using Umbraco.Extensions;

namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Cache;

[TestFixture]
public class DistributedCacheExtensionsTests
{
[Test]
public void Member_GetPayloads_CorrectlyCreatesPayloads()
{
var members = new List<IMember>()
{
CreateMember(1, "Fred", "fred", "fred@test.com"),
CreateMember(1, "Fred", "fred", "fred@test.com"),
CreateMember(2, "Sally", "sally", "sally@test.com"),
CreateMember(3, "Jane", "jane", "jane@test.com", "janeold"),
};

var payloads = DistributedCacheExtensions.GetPayloads(members, false);
Assert.AreEqual(3, payloads.Count());

var payloadForFred = payloads.First();
Assert.AreEqual("fred", payloadForFred.Username);
Assert.AreEqual(1, payloadForFred.Id);
Assert.IsNull(payloadForFred.PreviousUsername);

var payloadForSally = payloads.Skip(1).First();
Assert.AreEqual("sally", payloadForSally.Username);
Assert.AreEqual(2, payloadForSally.Id);
Assert.IsNull(payloadForSally.PreviousUsername);

var payloadForJane = payloads.Skip(2).First();
Assert.AreEqual("jane", payloadForJane.Username);
Assert.AreEqual(3, payloadForJane.Id);
Assert.AreEqual("janeold", payloadForJane.PreviousUsername);
}

private static IMember CreateMember(int id, string name, string username, string email, string? previousUserName = null)
{
var memberBuilder = new MemberBuilder()
.AddMemberType()
.Done()
.WithId(id)
.WithName(name)
.WithLogin(username, "password")
.WithEmail(email);

if (previousUserName != null)
{
memberBuilder.AddAdditionalData()
.WithKeyValue(global::Umbraco.Cms.Core.Constants.Entities.AdditionalDataKeys.MemberPreviousUserName, previousUserName)
.Done();
}

return memberBuilder.Build();
}

Check warning on line 63 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/DistributedCacheExtensionsTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

❌ New issue: Excess Number of Function Arguments

CreateMember has 5 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.
}
Loading