From 118cb5b5f73a5a8cf439a3032a14a75b8b09f050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Halil=20=C4=B0brahim=20Kalkan?= Date: Wed, 8 Apr 2020 15:06:28 +0300 Subject: [PATCH] Refactor IdentityUserManager & fix unit tests. --- .../Volo/Abp/Identity/IdentityErrorCodes.cs | 7 +- .../Volo/Abp/Identity/Localization/en.json | 3 +- .../Volo/Abp/Identity/Localization/tr.json | 3 +- .../Volo/Abp/Identity/IdentityUserManager.cs | 74 +++++++++---------- .../IOrganizationUnitRepository.cs | 1 - .../Identity/OrganizationUnitManager_Tests.cs | 23 ++++-- 6 files changed, 56 insertions(+), 55 deletions(-) rename modules/identity/src/{Volo.Abp.Identity.Application.Contracts => Volo.Abp.Identity.Domain.Shared}/Volo/Abp/Identity/IdentityErrorCodes.cs (53%) diff --git a/modules/identity/src/Volo.Abp.Identity.Application.Contracts/Volo/Abp/Identity/IdentityErrorCodes.cs b/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/IdentityErrorCodes.cs similarity index 53% rename from modules/identity/src/Volo.Abp.Identity.Application.Contracts/Volo/Abp/Identity/IdentityErrorCodes.cs rename to modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/IdentityErrorCodes.cs index 4bdfc5d8e9..d77ee9f5a9 100644 --- a/modules/identity/src/Volo.Abp.Identity.Application.Contracts/Volo/Abp/Identity/IdentityErrorCodes.cs +++ b/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/IdentityErrorCodes.cs @@ -1,11 +1,8 @@ -using System; -using System.Collections.Generic; -using System.Text; - -namespace Volo.Abp.Identity +namespace Volo.Abp.Identity { public static class IdentityErrorCodes { public const string UserSelfDeletion = "Volo.Abp.Identity:010001"; + public const string MaxAllowedOuMembership = "Volo.Abp.Identity:010002"; } } \ No newline at end of file diff --git a/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/en.json b/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/en.json index 4615f97fd2..8305ca9589 100644 --- a/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/en.json +++ b/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/en.json @@ -101,6 +101,7 @@ "Description:Abp.Identity.SignIn.EnablePhoneNumberConfirmation": "Whether the phoneNumber can be confirmed by the user.", "Description:Abp.Identity.SignIn.RequireConfirmedPhoneNumber": "Whether a confirmed telephone number is required to sign in.", "Description:Abp.Identity.User.IsUserNameUpdateEnabled": "Whether the username can be updated by the user.", - "Description:Abp.Identity.User.IsEmailUpdateEnabled": "Whether the email can be updated by the user." + "Description:Abp.Identity.User.IsEmailUpdateEnabled": "Whether the email can be updated by the user.", + "Volo.Abp.Identity:010002": "Can not set more than {MaxUserMembershipCount} organization unit for a user!" } } diff --git a/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/tr.json b/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/tr.json index cfc58d4dad..8d006d41c4 100644 --- a/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/tr.json +++ b/modules/identity/src/Volo.Abp.Identity.Domain.Shared/Volo/Abp/Identity/Localization/tr.json @@ -72,6 +72,7 @@ "Permission:Delete": "Silme", "Permission:ChangePermissions": "İzinleri değiştirme", "Permission:UserManagement": "Kullanıcı yönetimi", - "Permission:UserLookup": "Kullanıcı sorgulama" + "Permission:UserLookup": "Kullanıcı sorgulama", + "Volo.Abp.Identity:010002": "Bir kullanıcı en fazla {MaxUserMembershipCount} organizasyon birimine üye olabilir!" } } diff --git a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs index 6e97c1ad76..b7228b05a1 100644 --- a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs +++ b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/IdentityUserManager.cs @@ -100,47 +100,42 @@ namespace Volo.Abp.Identity public virtual async Task IsInOrganizationUnitAsync(Guid userId, Guid ouId) { - return await IsInOrganizationUnitAsync( - await GetByIdAsync(userId), - await OrganizationUnitRepository.GetAsync(ouId) - ); + var user = await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken); + return user.IsInOrganizationUnit(ouId); } - public virtual Task IsInOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou) + public virtual async Task IsInOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou) { - return Task.FromResult(user.IsInOrganizationUnit(ou.Id)); + await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token); + return user.IsInOrganizationUnit(ou.Id); } public virtual async Task AddToOrganizationUnitAsync(Guid userId, Guid ouId) { await AddToOrganizationUnitAsync( - await IdentityUserRepository.GetAsync(userId, true), - await OrganizationUnitRepository.GetAsync(ouId) + await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken), + await OrganizationUnitRepository.GetAsync(ouId, cancellationToken: CancellationToken) ); } public virtual async Task AddToOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou) { await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token); - - var currentOus = user.OrganizationUnits; - if (currentOus.Any(cou => cou.OrganizationUnitId == ou.Id && cou.UserId == user.Id)) + if (user.OrganizationUnits.Any(cou => cou.OrganizationUnitId == ou.Id)) { return; } - await CheckMaxUserOrganizationUnitMembershipCountAsync(user.TenantId, currentOus.Count + 1); + await CheckMaxUserOrganizationUnitMembershipCountAsync(user.OrganizationUnits.Count + 1); user.AddOrganizationUnit(ou.Id); } public virtual async Task RemoveFromOrganizationUnitAsync(Guid userId, Guid ouId) { - await RemoveFromOrganizationUnitAsync( - await IdentityUserRepository.GetAsync(userId, true), - await OrganizationUnitRepository.GetAsync(ouId) - ); + var user = await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken); + user.RemoveOrganizationUnit(ouId); } public virtual async Task RemoveFromOrganizationUnitAsync(IdentityUser user, OrganizationUnit ou) @@ -153,9 +148,9 @@ namespace Volo.Abp.Identity public virtual async Task SetOrganizationUnitsAsync(Guid userId, params Guid[] organizationUnitIds) { await SetOrganizationUnitsAsync( - await IdentityUserRepository.GetAsync(userId, true), + await IdentityUserRepository.GetAsync(userId, cancellationToken: CancellationToken), organizationUnitIds - ); + ); } public virtual async Task SetOrganizationUnitsAsync(IdentityUser user, params Guid[] organizationUnitIds) @@ -163,38 +158,36 @@ namespace Volo.Abp.Identity Check.NotNull(user, nameof(user)); Check.NotNull(organizationUnitIds, nameof(organizationUnitIds)); - await CheckMaxUserOrganizationUnitMembershipCountAsync(user.TenantId, organizationUnitIds.Length); + await CheckMaxUserOrganizationUnitMembershipCountAsync(organizationUnitIds.Length); - var currentOus = await IdentityUserRepository.GetOrganizationUnitsAsync(user.Id); + await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token); //Remove from removed OUs - foreach (var currentOu in currentOus) + foreach (var ouId in user.OrganizationUnits.Select(uou => uou.OrganizationUnitId).ToArray()) { - if (!organizationUnitIds.Contains(currentOu.Id)) + if (!organizationUnitIds.Contains(ouId)) { - await RemoveFromOrganizationUnitAsync(user.Id, currentOu.Id); + user.RemoveOrganizationUnit(ouId); } } //Add to added OUs foreach (var organizationUnitId in organizationUnitIds) { - if (currentOus.All(ou => ou.Id != organizationUnitId)) + if (!user.IsInOrganizationUnit(organizationUnitId)) { - await AddToOrganizationUnitAsync( - user, - await OrganizationUnitRepository.GetAsync(organizationUnitId) - ); + user.AddOrganizationUnit(organizationUnitId); } } } - private async Task CheckMaxUserOrganizationUnitMembershipCountAsync(Guid? tenantId, int requestedCount) + private async Task CheckMaxUserOrganizationUnitMembershipCountAsync(int requestedCount) { var maxCount = await SettingProvider.GetAsync(IdentitySettingNames.OrganizationUnit.MaxUserMembershipCount); if (requestedCount > maxCount) { - throw new AbpException(string.Format("Can not set more than {0} organization unit for a user!", maxCount)); + throw new BusinessException(IdentityErrorCodes.MaxAllowedOuMembership) + .WithData("MaxUserMembershipCount", maxCount); } } @@ -203,33 +196,33 @@ namespace Volo.Abp.Identity { await IdentityUserRepository.EnsureCollectionLoadedAsync(user, u => u.OrganizationUnits, CancellationTokenProvider.Token); - var ouOfUser = user.OrganizationUnits; - - return await OrganizationUnitRepository.GetListAsync(ouOfUser.Select(t => t.OrganizationUnitId)); + return await OrganizationUnitRepository.GetListAsync( + user.OrganizationUnits.Select(t => t.OrganizationUnitId), + CancellationToken + ); } [UnitOfWork] - public virtual async Task> GetUsersInOrganizationUnitAsync(OrganizationUnit organizationUnit, + public virtual async Task> GetUsersInOrganizationUnitAsync( + OrganizationUnit organizationUnit, bool includeChildren = false) { if (includeChildren) { return await IdentityUserRepository - .GetUsersInOrganizationUnitWithChildrenAsync(organizationUnit.Code) - ; + .GetUsersInOrganizationUnitWithChildrenAsync(organizationUnit.Code, CancellationToken); } else { return await IdentityUserRepository - .GetUsersInOrganizationUnitAsync(organizationUnit.Id) - ; + .GetUsersInOrganizationUnitAsync(organizationUnit.Id, CancellationToken); } } public virtual async Task AddDefaultRolesAsync([NotNull] IdentityUser user) { await UserRepository.EnsureCollectionLoadedAsync(user, u => u.Roles, CancellationToken); - + foreach (var role in await RoleRepository.GetDefaultOnesAsync(cancellationToken: CancellationToken)) { if (!user.IsInRole(role.Id)) @@ -237,9 +230,8 @@ namespace Volo.Abp.Identity user.AddRole(role.Id); } } - - return await UpdateUserAsync(user); + return await UpdateUserAsync(user); } } } diff --git a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/Organizations/IOrganizationUnitRepository.cs b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/Organizations/IOrganizationUnitRepository.cs index 4d90ba3a1d..f57425d84a 100644 --- a/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/Organizations/IOrganizationUnitRepository.cs +++ b/modules/identity/src/Volo.Abp.Identity.Domain/Volo/Abp/Identity/Organizations/IOrganizationUnitRepository.cs @@ -17,6 +17,5 @@ namespace Volo.Abp.Identity.Organizations Task> GetListAsync(bool includeDetails = true, CancellationToken cancellationToken = default); Task GetOrganizationUnitAsync(string displayName, bool includeDetails = false, CancellationToken cancellationToken = default); - } } diff --git a/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs index 67795268ff..9148888eac 100644 --- a/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs +++ b/modules/identity/test/Volo.Abp.Identity.Domain.Tests/Volo/Abp/Identity/OrganizationUnitManager_Tests.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Text; using System.Threading.Tasks; using Volo.Abp.Identity.Organizations; +using Volo.Abp.Uow; using Xunit; namespace Volo.Abp.Identity @@ -17,6 +18,7 @@ namespace Volo.Abp.Identity private readonly IdentityTestData _testData; private readonly IIdentityRoleRepository _identityRoleRepository; private readonly ILookupNormalizer _lookupNormalizer; + private readonly IUnitOfWorkManager _unitOfWorkManager; public OrganizationUnitManager_Tests() { _organizationUnitManager = GetRequiredService(); @@ -24,6 +26,7 @@ namespace Volo.Abp.Identity _identityRoleRepository = GetRequiredService(); _lookupNormalizer = GetRequiredService(); _testData = GetRequiredService(); + _unitOfWorkManager = GetRequiredService(); } [Fact] @@ -87,13 +90,21 @@ namespace Volo.Abp.Identity [Fact] public async Task AddRoleToOrganizationUnitAsync() { - var ou = await _organizationUnitRepository.GetOrganizationUnitAsync("OU1", true); - var adminRole = await _identityRoleRepository.FindByNormalizedNameAsync(_lookupNormalizer.NormalizeName("admin")); - await _organizationUnitManager.AddRoleToOrganizationUnitAsync(adminRole, ou); - - //TODO: This method has a bug: add role not work + OrganizationUnit ou = null; + IdentityRole adminRole = null; + + using (var uow = _unitOfWorkManager.Begin()) + { + ou = await _organizationUnitRepository.GetOrganizationUnitAsync("OU1", true); + adminRole = await _identityRoleRepository.FindByNormalizedNameAsync(_lookupNormalizer.NormalizeName("admin")); + await _organizationUnitManager.AddRoleToOrganizationUnitAsync(adminRole, ou); + await _organizationUnitRepository.UpdateAsync(ou); + + await uow.CompleteAsync(); + } + ou = await _organizationUnitRepository.GetOrganizationUnitAsync("OU1", includeDetails: true); - ou.Roles.FirstOrDefault().RoleId.ShouldBe(adminRole.Id); + ou.Roles.First().RoleId.ShouldBe(adminRole.Id); } [Fact]