From fc814ff352437151e29d59090605c080225b7dbd Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 16 Aug 2023 13:42:09 +1000 Subject: [PATCH] [AC-1597] Revert GetByUserIdWithPolicyDetailsAsync changes to unblock SQL CPU (#3203) * Revert "[PM-3007] Caching user policies on PolicyService variable (#3117)" This reverts commit 78588d0246d21cfbd8bb01b36d6cec380647c9d2. * Don't delete old migration script * Add migration to revert sproc --- .../IOrganizationUserRepository.cs | 2 +- .../Services/Implementations/PolicyService.cs | 17 +++------ .../OrganizationUserRepository.cs | 4 +-- .../OrganizationUserRepository.cs | 5 +-- ...tionUser_ReadByUserIdWithPolicyDetails.sql | 30 +++++++++------- test/Core.Test/Services/PolicyServiceTests.cs | 10 ++++-- .../OrganizationUserRepositoryTests.cs | 2 +- ...ationUserReadByUserIdWithPolicyDetails.sql | 35 +++++++++++++++++++ 8 files changed, 71 insertions(+), 34 deletions(-) create mode 100644 util/Migrator/DbScripts/2023-08-16_00_HotfixRevertOrganizationUserReadByUserIdWithPolicyDetails.sql diff --git a/src/Core/Repositories/IOrganizationUserRepository.cs b/src/Core/Repositories/IOrganizationUserRepository.cs index fb012c623..f9dfa12c2 100644 --- a/src/Core/Repositories/IOrganizationUserRepository.cs +++ b/src/Core/Repositories/IOrganizationUserRepository.cs @@ -39,6 +39,6 @@ public interface IOrganizationUserRepository : IRepository> GetManyByMinimumRoleAsync(Guid organizationId, OrganizationUserType minRole); Task RevokeAsync(Guid id); Task RestoreAsync(Guid id, OrganizationUserStatusType status); - Task> GetByUserIdWithPolicyDetailsAsync(Guid userId); + Task> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType); Task GetOccupiedSmSeatCountByOrganizationIdAsync(Guid organizationId); } diff --git a/src/Core/Services/Implementations/PolicyService.cs b/src/Core/Services/Implementations/PolicyService.cs index 595a798e7..6b1009093 100644 --- a/src/Core/Services/Implementations/PolicyService.cs +++ b/src/Core/Services/Implementations/PolicyService.cs @@ -20,8 +20,6 @@ public class PolicyService : IPolicyService private readonly IMailService _mailService; private readonly GlobalSettings _globalSettings; - private IEnumerable _cachedOrganizationUserPolicyDetails; - public PolicyService( IEventService eventService, IOrganizationRepository organizationRepository, @@ -196,25 +194,18 @@ public class PolicyService : IPolicyService return result.Any(); } - private async Task> QueryOrganizationUserPolicyDetailsAsync(Guid userId, PolicyType? policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted) + private async Task> QueryOrganizationUserPolicyDetailsAsync(Guid userId, PolicyType policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted) { - // Check if the cached policies are available - if (_cachedOrganizationUserPolicyDetails == null) - { - // Cached policies not available, retrieve from the repository - _cachedOrganizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId); - } - + var organizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId, policyType); var excludedUserTypes = GetUserTypesExcludedFromPolicy(policyType); - return _cachedOrganizationUserPolicyDetails.Where(o => - (policyType == null || o.PolicyType == policyType) && + return organizationUserPolicyDetails.Where(o => o.PolicyEnabled && !excludedUserTypes.Contains(o.OrganizationUserType) && o.OrganizationUserStatus >= minStatus && !o.IsProvider); } - private OrganizationUserType[] GetUserTypesExcludedFromPolicy(PolicyType? policyType) + private OrganizationUserType[] GetUserTypesExcludedFromPolicy(PolicyType policyType) { switch (policyType) { diff --git a/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs index 27410fb3f..008242c26 100644 --- a/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs @@ -505,13 +505,13 @@ public class OrganizationUserRepository : Repository, IO } } - public async Task> GetByUserIdWithPolicyDetailsAsync(Guid userId) + public async Task> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType) { using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.QueryAsync( $"[{Schema}].[{Table}_ReadByUserIdWithPolicyDetails]", - new { UserId = userId }, + new { UserId = userId, PolicyType = policyType }, commandType: CommandType.StoredProcedure); return results.ToList(); diff --git a/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs index f5ba2b9c1..8256696d9 100644 --- a/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs @@ -588,7 +588,7 @@ public class OrganizationUserRepository : Repository> GetByUserIdWithPolicyDetailsAsync(Guid userId) + public async Task> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType) { using (var scope = ServiceScopeFactory.CreateScope()) { @@ -604,7 +604,8 @@ public class OrganizationUserRepository : Repository sutProvider) { sutProvider.GetDependency() - .GetByUserIdWithPolicyDetailsAsync(userId) + .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.RequireSso) .Returns(new List { new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = false, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false}, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = true }, + new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = true } + }); + + sutProvider.GetDependency() + .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.DisableSend) + .Returns(new List + { new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = false }, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = true } }); diff --git a/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs b/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs index b1ee1cbc9..4f3b912b6 100644 --- a/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs +++ b/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs @@ -274,7 +274,7 @@ public class OrganizationUserRepositoryTests } // Act - var result = await orgUserRepos[i].GetByUserIdWithPolicyDetailsAsync(savedUser.Id); + var result = await orgUserRepos[i].GetByUserIdWithPolicyDetailsAsync(savedUser.Id, policy.Type); results.Add(result.FirstOrDefault()); } diff --git a/util/Migrator/DbScripts/2023-08-16_00_HotfixRevertOrganizationUserReadByUserIdWithPolicyDetails.sql b/util/Migrator/DbScripts/2023-08-16_00_HotfixRevertOrganizationUserReadByUserIdWithPolicyDetails.sql new file mode 100644 index 000000000..7da87f4ae --- /dev/null +++ b/util/Migrator/DbScripts/2023-08-16_00_HotfixRevertOrganizationUserReadByUserIdWithPolicyDetails.sql @@ -0,0 +1,35 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ReadByUserIdWithPolicyDetails] + @UserId UNIQUEIDENTIFIER, + @PolicyType TINYINT +AS +BEGIN + SET NOCOUNT ON +SELECT + OU.[Id] AS OrganizationUserId, + P.[OrganizationId], + P.[Type] AS PolicyType, + P.[Enabled] AS PolicyEnabled, + P.[Data] AS PolicyData, + OU.[Type] AS OrganizationUserType, + OU.[Status] AS OrganizationUserStatus, + OU.[Permissions] AS OrganizationUserPermissionsData, + CASE WHEN EXISTS ( + SELECT 1 + FROM [dbo].[ProviderUserView] PU + INNER JOIN [dbo].[ProviderOrganizationView] PO ON PO.[ProviderId] = PU.[ProviderId] + WHERE PU.[UserId] = OU.[UserId] AND PO.[OrganizationId] = P.[OrganizationId] + ) THEN 1 ELSE 0 END AS IsProvider +FROM [dbo].[PolicyView] P +INNER JOIN [dbo].[OrganizationUserView] OU + ON P.[OrganizationId] = OU.[OrganizationId] +WHERE P.[Type] = @PolicyType AND + ( + (OU.[Status] != 0 AND OU.[UserId] = @UserId) -- OrgUsers who have accepted their invite and are linked to a UserId + OR EXISTS ( + SELECT 1 + FROM [dbo].[UserView] U + WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email + ) + ) +END +GO \ No newline at end of file