-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[PM-21031] Optimize GET Members endpoint performance #5907
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking โSign up for GitHubโ, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
92f7a77
08688d6
3fa2797
5fdad60
b40e813
0579ec4
516509a
b9e2f5f
79b0041
baf42f7
c1534d1
fbfd101
72b41c8
26c288b
1e0c685
0f70876
2485331
d6c0e8d
df9a198
9ecf758
2728467
405d8b2
cfa0cb1
8323e53
38214b4
0d34e87
046794c
2a559ad
e2d489f
a69064f
8f8d3f2
234c1b6
bcf4eb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
๏ปฟCREATE PROCEDURE [dbo].[OrganizationUserUserDetails_ReadByOrganizationId_V2] | ||
@OrganizationId UNIQUEIDENTIFIER, | ||
@IncludeGroups BIT = 0, | ||
@IncludeCollections BIT = 0 | ||
AS | ||
BEGIN | ||
SET NOCOUNT ON | ||
|
||
-- Result Set 1: User Details (always returned) | ||
SELECT * | ||
FROM [dbo].[OrganizationUserUserDetailsView] | ||
WHERE OrganizationId = @OrganizationId | ||
|
||
-- Result Set 2: Group associations (if requested) | ||
IF @IncludeGroups = 1 | ||
BEGIN | ||
SELECT gu.* | ||
FROM [dbo].[GroupUser] gu | ||
INNER JOIN [dbo].[OrganizationUser] ou ON gu.OrganizationUserId = ou.Id | ||
WHERE ou.OrganizationId = @OrganizationId | ||
END | ||
|
||
-- Result Set 3: Collection associations (if requested) | ||
IF @IncludeCollections = 1 | ||
BEGIN | ||
SELECT cu.* | ||
FROM [dbo].[CollectionUser] cu | ||
INNER JOIN [dbo].[OrganizationUser] ou ON cu.OrganizationUserId = ou.Id | ||
WHERE ou.OrganizationId = @OrganizationId | ||
END | ||
END |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
๏ปฟCREATE PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2] | ||
@OrganizationId UNIQUEIDENTIFIER | ||
AS | ||
BEGIN | ||
SET NOCOUNT ON; | ||
|
||
WITH OrgUsers AS ( | ||
SELECT * | ||
FROM [dbo].[OrganizationUserView] | ||
WHERE [OrganizationId] = @OrganizationId | ||
), | ||
UserDomains AS ( | ||
SELECT U.[Id], U.[EmailDomain] | ||
FROM [dbo].[UserEmailDomainView] U | ||
WHERE EXISTS ( | ||
SELECT 1 | ||
FROM [dbo].[OrganizationDomainView] OD | ||
WHERE OD.[OrganizationId] = @OrganizationId | ||
AND OD.[VerifiedDate] IS NOT NULL | ||
AND OD.[DomainName] = U.[EmailDomain] | ||
) | ||
) | ||
SELECT OU.* | ||
FROM OrgUsers OU | ||
JOIN UserDomains UD ON OU.[UserId] = UD.[Id] | ||
OPTION (RECOMPILE); | ||
END |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
CREATE VIEW [dbo].[UserEmailDomainView] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ๐ญ Should this use the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This represents columns from just the |
||
AS | ||
SELECT | ||
Id, | ||
Email, | ||
SUBSTRING(Email, CHARINDEX('@', Email) + 1, LEN(Email)) AS EmailDomain | ||
FROM dbo.[User] | ||
WHERE Email IS NOT NULL | ||
AND CHARINDEX('@', Email) > 0 | ||
GO |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
CREATE OR ALTER VIEW [dbo].[UserEmailDomainView] | ||
AS | ||
SELECT | ||
Id, | ||
Email, | ||
SUBSTRING(Email, CHARINDEX('@', Email) + 1, LEN(Email)) AS EmailDomain | ||
FROM dbo.[User] | ||
WHERE Email IS NOT NULL | ||
AND CHARINDEX('@', Email) > 0 | ||
GO | ||
|
||
-- Index on OrganizationUser for efficient filtering | ||
IF NOT EXISTS(SELECT name FROM sys.indexes WHERE name = 'IX_OrganizationUser_OrganizationId_UserId') | ||
BEGIN | ||
CREATE NONCLUSTERED INDEX [IX_OrganizationUser_OrganizationId_UserId] | ||
ON [dbo].[OrganizationUser] ([OrganizationId], [UserId]) | ||
INCLUDE ([Email], [Status], [Type], [ExternalId], [CreationDate], | ||
[RevisionDate], [Permissions], [ResetPasswordKey], [AccessSecretsManager]) | ||
END | ||
GO | ||
|
||
IF NOT EXISTS(SELECT name FROM sys.indexes WHERE name = 'IX_User_Id_EmailDomain') | ||
BEGIN | ||
CREATE NONCLUSTERED INDEX [IX_User_Id_EmailDomain] | ||
ON [dbo].[User] ([Id], [Email]) | ||
END | ||
GO | ||
|
||
IF NOT EXISTS(SELECT name FROM sys.indexes WHERE name = 'IX_OrganizationDomain_OrganizationId_VerifiedDate') | ||
BEGIN | ||
CREATE NONCLUSTERED INDEX [IX_OrganizationDomain_OrganizationId_VerifiedDate] | ||
ON [dbo].[OrganizationDomain] ([OrganizationId], [VerifiedDate]) | ||
INCLUDE ([DomainName]) | ||
WHERE [VerifiedDate] IS NOT NULL | ||
END | ||
GO | ||
|
||
CREATE OR ALTER PROCEDURE [dbo].[OrganizationUserUserDetails_ReadByOrganizationId_V2] | ||
@OrganizationId UNIQUEIDENTIFIER, | ||
@IncludeGroups BIT = 0, | ||
@IncludeCollections BIT = 0 | ||
AS | ||
BEGIN | ||
SET NOCOUNT ON | ||
|
||
-- Result Set 1: User Details (always returned) | ||
SELECT * | ||
FROM [dbo].[OrganizationUserUserDetailsView] | ||
WHERE OrganizationId = @OrganizationId | ||
|
||
-- Result Set 2: Group associations (if requested) | ||
IF @IncludeGroups = 1 | ||
BEGIN | ||
SELECT gu.* | ||
FROM [dbo].[GroupUser] gu | ||
INNER JOIN [dbo].[OrganizationUser] ou ON gu.OrganizationUserId = ou.Id | ||
WHERE ou.OrganizationId = @OrganizationId | ||
END | ||
|
||
-- Result Set 3: Collection associations (if requested) | ||
IF @IncludeCollections = 1 | ||
BEGIN | ||
SELECT cu.* | ||
FROM [dbo].[CollectionUser] cu | ||
INNER JOIN [dbo].[OrganizationUser] ou ON cu.OrganizationUserId = ou.Id | ||
WHERE ou.OrganizationId = @OrganizationId | ||
END | ||
END | ||
GO | ||
|
||
CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationIdWithClaimedDomains_V2] | ||
@OrganizationId UNIQUEIDENTIFIER | ||
AS | ||
BEGIN | ||
SET NOCOUNT ON; | ||
|
||
WITH OrgUsers AS ( | ||
SELECT * | ||
FROM [dbo].[OrganizationUserView] | ||
WHERE [OrganizationId] = @OrganizationId | ||
), | ||
UserDomains AS ( | ||
SELECT U.[Id], U.[EmailDomain] | ||
FROM [dbo].[UserEmailDomainView] U | ||
WHERE EXISTS ( | ||
SELECT 1 | ||
FROM [dbo].[OrganizationDomainView] OD | ||
WHERE OD.[OrganizationId] = @OrganizationId | ||
AND OD.[VerifiedDate] IS NOT NULL | ||
AND OD.[DomainName] = U.[EmailDomain] | ||
) | ||
) | ||
SELECT OU.* | ||
FROM OrgUsers OU | ||
JOIN UserDomains UD ON OU.[UserId] = UD.[Id] | ||
OPTION (RECOMPILE); | ||
END | ||
GO |
Unchanged files with check annotations Beta
} | ||
[HttpGet("{id}")] | ||
public async Task<IActionResult> Get(Guid organizationId, Guid id) | ||
{ | ||
var orgUser = await _organizationUserRepository.GetDetailsByIdAsync(id); | ||
if (orgUser == null || orgUser.OrganizationId != organizationId) | ||
} | ||
[HttpGet("")] | ||
public async Task<IActionResult> Get( | ||
Guid organizationId, | ||
[FromQuery] GetUsersQueryParamModel model) | ||
{ | ||
} | ||
[HttpPost("")] | ||
public async Task<IActionResult> Post(Guid organizationId, [FromBody] ScimUserRequestModel model) | ||
{ | ||
var orgUser = await _postUserCommand.PostUserAsync(organizationId, model); | ||
var scimUserResponseModel = new ScimUserResponseModel(orgUser); | ||
} | ||
[HttpPut("{id}")] | ||
public async Task<IActionResult> Put(Guid organizationId, Guid id, [FromBody] ScimUserRequestModel model) | ||
{ | ||
var orgUser = await _organizationUserRepository.GetByIdAsync(id); | ||
if (orgUser == null || orgUser.OrganizationId != organizationId) | ||
} | ||
[HttpPatch("{id}")] | ||
public async Task<IActionResult> Patch(Guid organizationId, Guid id, [FromBody] ScimPatchModel model) | ||
{ | ||
await _patchUserCommand.PatchUserAsync(organizationId, id, model); | ||
return new NoContentResult(); |
} | ||
[HttpPost("webhook")] | ||
public async Task<IActionResult> PostWebhook([FromQuery, Required] string key, | ||
[FromBody, Required] FreshdeskWebhookModel model) | ||
{ | ||
if (string.IsNullOrWhiteSpace(key) || !CoreHelpers.FixedTimeEquals(key, _billingSettings.FreshDesk.WebhookKey)) | ||
} | ||
[HttpPost("webhook-onyx-ai")] | ||
public async Task<IActionResult> PostWebhookOnyxAi([FromQuery, Required] string key, | ||
[FromBody, Required] FreshdeskOnyxAiWebhookModel model) | ||
{ | ||
// ensure that the key is from Freshdesk |
[HttpPost("webhook")] | ||
public async Task<IActionResult> PostWebhook([FromHeader(Name = "Authorization")] string key, | ||
[FromBody] CustomWebhookRequestModel request, | ||
CancellationToken cancellationToken) | ||
{ |
} | ||
[HttpPost("ipn")] | ||
public async Task<IActionResult> PostIpn([FromBody] BitPayEventModel model, [FromQuery] string key) | ||
{ | ||
if (!CoreHelpers.FixedTimeEquals(key, _billingSettings.BitPayWebhookKey)) | ||
{ | ||
return CoreHelpers.FromEpocMilliseconds(invoice.CurrentTime); | ||
} | ||
public Tuple<Guid?, Guid?, Guid?> GetIdsFromPosData(BitPayLight.Models.Invoice.Invoice invoice) | ||
{ | ||
Guid? orgId = null; | ||
Guid? userId = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐ฑ Run this in parallel and wait for them before L38?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it but the
organizationAbility
value comes from the cache, so it's quick to access. Ifif (organizationAbility is { Enabled: true, UseOrganizationDomains: true })
returnsfalse
there's no need to query the database.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cache may have issues for that lookup https://bitwarden.atlassian.net/wiki/x/iYDibg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats an issue on the cache mechanism, right? If so, that seems to be out of scope here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://bitwarden.atlassian.net/browse/PM-23845