Public Access: Honour custom IMemberGroupService in backoffice dialog (closes #22580)

src/Umbraco.Cms.Api.Management/Controllers/MemberGroup/Item/ItemMemberGroupItemController.cs

@@ -1,11 +1,11 @@
 using Asp.Versioning;
-using J2N.Collections.Generic.Extensions;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.DependencyInjection;
 using Umbraco.Cms.Api.Management.ViewModels.MemberGroup.Item;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Mapping;
 using Umbraco.Cms.Core.Models;
-using Umbraco.Cms.Core.Models.Entities;
 using Umbraco.Cms.Core.Services;
 
 namespace Umbraco.Cms.Api.Management.Controllers.MemberGroup.Item;
@@ -16,36 +16,50 @@ namespace Umbraco.Cms.Api.Management.Controllers.MemberGroup.Item;
 [ApiVersion("1.0")]
 public class ItemMemberGroupItemController : MemberGroupItemControllerBase
 {
-    private readonly IEntityService _entityService;
     private readonly IUmbracoMapper _mapper;
+    private readonly IMemberGroupService _memberGroupService;
+
+    // TODO (V19): When the obsolete constructor is removed, also remove the unused dependency on IEntityService.
 
     /// <summary>
-    /// Initializes a new instance of the <see cref="Umbraco.Cms.Api.Management.Controllers.MemberGroup.Item.ItemMemberGroupItemController"/> class, providing services for managing member group items.
+    /// Initializes a new instance of the <see cref="ItemMemberGroupItemController"/> class.
     /// </summary>
-    /// <param name="entityService">The service used to interact with entities in the Umbraco CMS.</param>
     /// <param name="mapper">The mapper used for mapping Umbraco objects.</param>
-    public ItemMemberGroupItemController(IEntityService entityService, IUmbracoMapper mapper)
+    /// <param name="memberGroupService">The service used to look up member groups.</param>
+    [ActivatorUtilitiesConstructor]
+    public ItemMemberGroupItemController(IEntityService entityService, IUmbracoMapper mapper, IMemberGroupService memberGroupService)
     {
-        _entityService = entityService;
         _mapper = mapper;
+        _memberGroupService = memberGroupService;
+    }
+
+    [Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 19.")]
+    public ItemMemberGroupItemController(IEntityService entityService, IUmbracoMapper mapper)
+        : this(
+            entityService,
+            mapper,
+            StaticServiceProvider.Instance.GetRequiredService<IMemberGroupService>())
+    {
     }
 
     [HttpGet]
     [MapToApiVersion("1.0")]
     [ProducesResponseType(typeof(IEnumerable<MemberGroupItemResponseModel>), StatusCodes.Status200OK)]
     [EndpointSummary("Gets a collection of member group items.")]
     [EndpointDescription("Gets a collection of member group items identified by the provided Ids.")]
-    public Task<IActionResult> Item(
+    public async Task<IActionResult> Item(
         CancellationToken cancellationToken,
         [FromQuery(Name = "id")] HashSet<Guid> ids)
     {
         if (ids.Count is 0)
         {
-            return Task.FromResult<IActionResult>(Ok(Enumerable.Empty<MemberGroupItemResponseModel>()));
+            return Ok(Enumerable.Empty<MemberGroupItemResponseModel>());
         }
 
-        IEnumerable<IEntitySlim> memberGroups = _entityService.GetAll(UmbracoObjectTypes.MemberGroup, ids.ToArray());
-        List<MemberGroupItemResponseModel> responseModel = _mapper.MapEnumerable<IEntitySlim, MemberGroupItemResponseModel>(memberGroups);
-        return Task.FromResult<IActionResult>(Ok(responseModel));
+        // Resolve via IMemberGroupService so custom implementations are honoured, rather than
+        // going directly to the entity/repository layer.
+        IEnumerable<IMemberGroup> memberGroups = await _memberGroupService.GetAsync(ids);
+        List<MemberGroupItemResponseModel> responseModel = _mapper.MapEnumerable<IMemberGroup, MemberGroupItemResponseModel>(memberGroups);
+        return Ok(responseModel);
     }
 }

src/Umbraco.Cms.Api.Management/Factories/PublicAccessPresentationFactory.cs

@@ -1,12 +1,12 @@
+using Microsoft.Extensions.DependencyInjection;
 using Umbraco.Cms.Api.Management.ViewModels;
 using Umbraco.Cms.Api.Management.ViewModels.Member.Item;
 using Umbraco.Cms.Api.Management.ViewModels.MemberGroup.Item;
 using Umbraco.Cms.Api.Management.ViewModels.PublicAccess;
 using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Mapping;
 using Umbraco.Cms.Core.Models;
-using Umbraco.Cms.Core.Models.Entities;
-using Umbraco.Cms.Core.Security;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
 using Umbraco.Cms.Web.Common.Security;
@@ -23,8 +23,10 @@
     private readonly IEntityService _entityService;
     private readonly IMemberService _memberService;
     private readonly IUmbracoMapper _mapper;
-    private readonly IMemberRoleManager _memberRoleManager;
     private readonly IMemberPresentationFactory _memberPresentationFactory;
+    private readonly IMemberGroupService _memberGroupService;
+
+    // TODO (V19): When the obsolete constructor is removed, consider also remove the unused dependency on IMemberRoleManager.
 
     /// <summary>
     /// Initializes a new instance of the <see cref="PublicAccessPresentationFactory"/> class.
@@ -34,18 +36,37 @@
     /// <param name="mapper">The Umbraco mapper for mapping entities to response models.</param>
     /// <param name="memberRoleManager">The member role manager for resolving member groups.</param>
     /// <param name="memberPresentationFactory">The member presentation factory for creating member item response models.</param>
+    /// <param name="memberGroupService">The member group service for resolving member groups by name.</param>
     public PublicAccessPresentationFactory(
         IEntityService entityService,
         IMemberService memberService,
         IUmbracoMapper mapper,
         IMemberRoleManager memberRoleManager,
-        IMemberPresentationFactory memberPresentationFactory)
+        IMemberPresentationFactory memberPresentationFactory,
+        IMemberGroupService memberGroupService)
     {
         _entityService = entityService;
         _memberService = memberService;
         _mapper = mapper;
-        _memberRoleManager = memberRoleManager;
         _memberPresentationFactory = memberPresentationFactory;
+        _memberGroupService = memberGroupService;
+    }
+
+    [Obsolete("Please use the constructor with all parameters. Scheduled for removal in Umbraco 19.")]
+    public PublicAccessPresentationFactory(
+        IEntityService entityService,
+        IMemberService memberService,
+        IUmbracoMapper mapper,
+        IMemberRoleManager memberRoleManager,
+        IMemberPresentationFactory memberPresentationFactory)
+        : this(
+            entityService,
+            memberService,
+            mapper,
+            memberRoleManager,
+            memberPresentationFactory,
+            StaticServiceProvider.Instance.GetRequiredService<IMemberGroupService>())
+    {
     }
 
     /// <inheritdoc/>
@@ -107,21 +128,15 @@
             .Select(_memberPresentationFactory.CreateItemResponseModel)
             .ToArray();
 
-        var allGroups = _memberRoleManager.Roles.Where(x => x.Name != null).ToDictionary(x => x.Name!);
-        IEnumerable<UmbracoIdentityRole> identityRoles = entry.Rules
+        // Resolve groups via IMemberGroupService so custom implementations (e.g. backed by an external
+        // user store) are honoured here, rather than going directly to IMemberRoleManager/IEntityService.
+        MemberGroupItemResponseModel[] memberGroups = entry.Rules
             .Where(rule => rule.RuleType == Constants.Conventions.PublicAccess.MemberRoleRuleType)
-            .Select(rule =>
-                rule.RuleValue is not null && allGroups.TryGetValue(rule.RuleValue, out UmbracoIdentityRole? memberRole)
-                    ? memberRole
-                    : null)
+            .Select(rule => rule.RuleValue is null ? null : _memberGroupService.GetByName(rule.RuleValue))
             .WhereNotNull()
+            .Select(group => _mapper.Map<MemberGroupItemResponseModel>(group)!)
             .ToArray();
 
-        IEnumerable<IEntitySlim> groupsEntities = identityRoles.Any()
-            ? _entityService.GetAll(UmbracoObjectTypes.MemberGroup, identityRoles.Select(x => Convert.ToInt32(x.Id)).ToArray())
-            : Enumerable.Empty<IEntitySlim>();
-        MemberGroupItemResponseModel[] memberGroups = groupsEntities.Select(x => _mapper.Map<MemberGroupItemResponseModel>(x)!).ToArray();
-
         var responseModel = new PublicAccessResponseModel
         {
             Members = members,

src/Umbraco.Cms.Api.Management/Mapping/Item/ItemTypeMapDefinition.cs

@@ -41,6 +41,7 @@ public void DefineMaps(IUmbracoMapper mapper)
         mapper.Define<IMediaType, MediaTypeItemResponseModel>((_, _) => new MediaTypeItemResponseModel(), Map);
         mapper.Define<MediaTypeFileExtensionMatchResult, AllowedMediaTypeItemResponseModel>((_, _) => new AllowedMediaTypeItemResponseModel(), Map);
         mapper.Define<IEntitySlim, MemberGroupItemResponseModel>((_, _) => new MemberGroupItemResponseModel(), Map);
+        mapper.Define<IMemberGroup, MemberGroupItemResponseModel>((_, _) => new MemberGroupItemResponseModel(), Map);
         mapper.Define<ITemplate, TemplateItemResponseModel>((_, _) => new TemplateItemResponseModel { Alias = string.Empty }, Map);
         mapper.Define<IMemberType, MemberTypeItemResponseModel>((_, _) => new MemberTypeItemResponseModel(), Map);
         mapper.Define<IRelationType, RelationTypeItemResponseModel>((_, _) => new RelationTypeItemResponseModel(), Map);
@@ -105,6 +106,13 @@ private static void Map(IEntitySlim source, MemberGroupItemResponseModel target,
         target.Id = source.Key;
     }
 
+    // Umbraco.Code.MapAll -Flags
+    private static void Map(IMemberGroup source, MemberGroupItemResponseModel target, MapperContext context)
+    {
+        target.Name = source.Name ?? string.Empty;
+        target.Id = source.Key;
+    }
+
     // Umbraco.Code.MapAll -Flags
     private static void Map(ITemplate source, TemplateItemResponseModel target, MapperContext context)
     {

src/Umbraco.Core/Services/IMemberGroupService.cs

@@ -74,6 +74,28 @@ public interface IMemberGroupService : IService
     /// <returns>A <see cref="IMemberGroup" /> object.</returns>
     Task<IMemberGroup?> GetAsync(Guid key);
 
+    /// <summary>
+    ///     Gets member groups by their keys.
+    /// </summary>
+    /// <param name="keys">The keys of the member groups to get.</param>
+    /// <returns>An enumerable list of matching <see cref="IMemberGroup" /> objects.</returns>
+    /// <remarks>
+    ///     The default implementation fetches all groups and filters in-memory. Implementations
+    ///     backed by a query-capable store may provide a more efficient override.
+    /// </remarks>
+    // TODO (V19): Remove default implementation.
+    async Task<IEnumerable<IMemberGroup>> GetAsync(IEnumerable<Guid> keys)
+    {
+        ICollection<Guid> keySet = keys as ICollection<Guid> ?? keys.ToArray();
+        if (keySet.Count == 0)
+        {
+            return [];
+        }
+
+        IEnumerable<IMemberGroup> all = await GetAllAsync();
+        return all.Where(x => keySet.Contains(x.Key));
+    }
+
     /// <summary>
     ///     Gets all member groups
     /// </summary>

src/Umbraco.Core/Services/MemberGroupService.cs

@@ -2,6 +2,7 @@
 using Umbraco.Cms.Core.Events;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Notifications;
+using Umbraco.Cms.Core.Persistence.Querying;
 using Umbraco.Cms.Core.Persistence.Repositories;
 using Umbraco.Cms.Core.Scoping;
 using Umbraco.Cms.Core.Services.OperationStatus;
@@ -95,6 +96,20 @@ public void Save(IMemberGroup memberGroup)
         return Task.FromResult(_memberGroupRepository.Get(key));
     }
 
+    /// <inheritdoc/>
+    public Task<IEnumerable<IMemberGroup>> GetAsync(IEnumerable<Guid> keys)
+    {
+        List<Guid> keysAsList = [.. keys];
+        if (keysAsList.Count == 0)
+        {
+            return Task.FromResult<IEnumerable<IMemberGroup>>([]);
+        }
+
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        IQuery<IMemberGroup> query = Query<IMemberGroup>().Where(x => keysAsList.Contains(x.Key));
+        return Task.FromResult(_memberGroupRepository.Get(query));
+    }
+
     /// <inheritdoc/>
     public Task<IEnumerable<IMemberGroup>> GetAllAsync()
     {

tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MemberGroupServiceTests.cs

@@ -1,4 +1,4 @@
 using NUnit.Framework;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
@@ -128,6 +128,27 @@
         });
     }
 
+    [Test]
+    public async Task Can_Get_MemberGroups_By_Keys()
+    {
+        var groupOne = new MemberGroup { Name = "GroupOne" };
+        var groupTwo = new MemberGroup { Name = "GroupTwo" };
+        var groupThree = new MemberGroup { Name = "GroupThree" };
+        await MemberGroupService.CreateAsync(groupOne);
+        await MemberGroupService.CreateAsync(groupTwo);
+        await MemberGroupService.CreateAsync(groupThree);
+
+        IMemberGroup[] result = (await MemberGroupService.GetAsync([groupOne.Key, groupThree.Key])).ToArray();
+
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(2, result.Length);
+            Assert.IsTrue(result.Any(x => x.Key == groupOne.Key));
+            Assert.IsTrue(result.Any(x => x.Key == groupThree.Key));
+            Assert.IsFalse(result.Any(x => x.Key == groupTwo.Key));
+        });
+    }
+
     [Test]
     public async Task Cannot_Update_MemberGroup_With_Duplicate_Name()
     {

tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Factories/PublicAccessPresentationFactoryTests.cs

@@ -6,8 +6,6 @@
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Mapping;
 using Umbraco.Cms.Core.Models;
-using Umbraco.Cms.Core.Models.Entities;
-using Umbraco.Cms.Core.Security;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
 using Umbraco.Cms.Web.Common.Security;
@@ -22,6 +20,7 @@
     private Mock<IUmbracoMapper> _mapper = null!;
     private Mock<IMemberRoleManager> _memberRoleManager = null!;
     private Mock<IMemberPresentationFactory> _memberPresentationFactory = null!;
+    private Mock<IMemberGroupService> _memberGroupService = null!;
     private PublicAccessPresentationFactory _factory = null!;
 
     [SetUp]
@@ -32,16 +31,15 @@
         _mapper = new Mock<IUmbracoMapper>();
         _memberRoleManager = new Mock<IMemberRoleManager>();
         _memberPresentationFactory = new Mock<IMemberPresentationFactory>();
-
-        // Default: no roles
-        _memberRoleManager.Setup(x => x.Roles).Returns(Enumerable.Empty<UmbracoIdentityRole>());
+        _memberGroupService = new Mock<IMemberGroupService>();
 
         _factory = new PublicAccessPresentationFactory(
             _entityService.Object,
             _memberService.Object,
             _mapper.Object,
             _memberRoleManager.Object,
-            _memberPresentationFactory.Object);
+            _memberPresentationFactory.Object,
+            _memberGroupService.Object);
     }
 
     [Test]
@@ -232,17 +230,11 @@
         SetupNodeKeyResolution(200, loginNodeKey);
         SetupNodeKeyResolution(300, errorNodeKey);
 
-        _memberRoleManager.Setup(x => x.Roles).Returns(new[]
-        {
-            new UmbracoIdentityRole("Editors") { Id = "42" },
-        });
-
-        var groupEntity = Mock.Of<IEntitySlim>();
-        _entityService.Setup(x => x.GetAll(UmbracoObjectTypes.MemberGroup, It.Is<int[]>(ids => ids.Contains(42))))
-            .Returns(new[] { groupEntity });
+        var memberGroup = Mock.Of<IMemberGroup>(x => x.Name == "Editors" && x.Key == groupKey);
+        _memberGroupService.Setup(x => x.GetByName("Editors")).Returns(memberGroup);
 
         var groupModel = new MemberGroupItemResponseModel { Id = groupKey, Name = "Editors" };
-        _mapper.Setup(x => x.Map<MemberGroupItemResponseModel>(groupEntity)).Returns(groupModel);
+        _mapper.Setup(x => x.Map<MemberGroupItemResponseModel>(memberGroup)).Returns(groupModel);
 
         // Act
         var result = _factory.CreatePublicAccessResponseModel(entry, protectedNodeKey);
@@ -305,11 +297,8 @@
         SetupNodeKeyResolution(200, loginNodeKey);
         SetupNodeKeyResolution(300, errorNodeKey);
 
-        // No matching role exists
-        _memberRoleManager.Setup(x => x.Roles).Returns(new[]
-        {
-            new UmbracoIdentityRole("Administrators") { Id = "1" },
-        });
+        // No matching group exists
+        _memberGroupService.Setup(x => x.GetByName("DeletedGroup")).Returns((IMemberGroup?)null);
 
         // Act
         var result = _factory.CreatePublicAccessResponseModel(entry, protectedNodeKey);
@@ -350,14 +339,9 @@
             .Returns(new MemberItemResponseModel { Id = memberKey });
 
         // Group setup
-        _memberRoleManager.Setup(x => x.Roles).Returns(new[]
-        {
-            new UmbracoIdentityRole("VIPs") { Id = "10" },
-        });
-        var groupEntity = Mock.Of<IEntitySlim>();
-        _entityService.Setup(x => x.GetAll(UmbracoObjectTypes.MemberGroup, It.Is<int[]>(ids => ids.Contains(10))))
-            .Returns(new[] { groupEntity });
-        _mapper.Setup(x => x.Map<MemberGroupItemResponseModel>(groupEntity))
+        var memberGroup = Mock.Of<IMemberGroup>(x => x.Name == "VIPs" && x.Key == groupKey);
+        _memberGroupService.Setup(x => x.GetByName("VIPs")).Returns(memberGroup);
+        _mapper.Setup(x => x.Map<MemberGroupItemResponseModel>(memberGroup))
             .Returns(new MemberGroupItemResponseModel { Id = groupKey, Name = "VIPs" });
 
         // Act