fix: hoist invariant queries outside per-message loop in visibility restrictions

https://sonarly.com/issue/23331?type=bug The `applyMessagesVisibilityRestrictions` post-query hook executes 3 redundant DB queries per message (workspaceMember, userWorkspace, connectedAccount) inside a loop, creating an N+1 pattern that adds ~200ms+ of unnecessary DB time per MCP `find_messages` call. Fix: Hoist workspaceMember, userWorkspace, and connectedAccount lookups outside the per-message loop. Execute a single batched connectedAccount query for all non-SHARE_EVERYTHING channel IDs. Build a Set<string> of owned channel IDs and use Set.has() membership check inside the loop. This reduces DB round-trips from O(3N) to O(3) constant, eliminating the N+1 pattern entirely.
2026-04-09 10:10:37 +00:00
2 changed files with 58 additions and 39 deletions
@@ -253,7 +253,9 @@ describe('ApplyMessagesVisibilityRestrictionsService', () => {
      userId: 'user-id',
    });

-    mockConnectedAccountRepository.find.mockResolvedValue([{ id: '1' }]);
+    mockConnectedAccountRepository.find.mockResolvedValue([
+      { id: '1', messageChannels: [{ id: 'messageChannelId' }] },
+    ]);

    const result = await service.applyMessagesVisibilityRestrictions(
      messages,
@@ -338,9 +340,7 @@ describe('ApplyMessagesVisibilityRestrictionsService', () => {
      userId: 'user-id',
    });

-    mockConnectedAccountRepository.find
-      .mockResolvedValueOnce([]) // request for message 3
-      .mockResolvedValueOnce([]); // request for message 2
+    mockConnectedAccountRepository.find.mockResolvedValue([]);

    const result = await service.applyMessagesVisibilityRestrictions(
      messages,
@@ -385,9 +385,7 @@ describe('ApplyMessagesVisibilityRestrictionsService', () => {
      id: 'workspace-member-id',
    });

-    mockConnectedAccountRepository.find
-      .mockResolvedValueOnce([]) // request for message 3
-      .mockResolvedValueOnce([]); // request for message 2
+    mockConnectedAccountRepository.find.mockResolvedValue([]);

    const result = await service.applyMessagesVisibilityRestrictions(
      messages,
@@ -71,11 +71,54 @@ export class ApplyMessagesVisibilityRestrictionsService {
          messageChannelsFromCore.map((ch) => [ch.id, ch]),
        );

-        const workspaceMemberRepository =
-          await this.globalWorkspaceOrmManager.getRepository<WorkspaceMemberWorkspaceEntity>(
-            workspaceId,
-            'workspaceMember',
-          );
+        // Resolve user ownership once for all message channels to avoid N+1 queries
+        let ownedMessageChannelIds: Set<string> | undefined;
+
+        if (isDefined(userId)) {
+          const workspaceMemberRepository =
+            await this.globalWorkspaceOrmManager.getRepository<WorkspaceMemberWorkspaceEntity>(
+              workspaceId,
+              'workspaceMember',
+            );
+
+          const workspaceMember =
+            await workspaceMemberRepository.findOneByOrFail({
+              userId,
+            });
+
+          const userWorkspace = await this.userWorkspaceRepository.findOne({
+            where: { userId: workspaceMember.userId, workspaceId },
+          });
+
+          if (userWorkspace) {
+            const nonShareEverythingChannelIds = messageChannelsFromCore
+              .filter(
+                (ch) =>
+                  ch.visibility !== MessageChannelVisibility.SHARE_EVERYTHING,
+              )
+              .map((ch) => ch.id);
+
+            if (nonShareEverythingChannelIds.length > 0) {
+              const connectedAccounts =
+                await this.connectedAccountRepository.find({
+                  where: {
+                    userWorkspaceId: userWorkspace.id,
+                    workspaceId,
+                    messageChannels: {
+                      id: In(nonShareEverythingChannelIds),
+                    },
+                  },
+                  relations: { messageChannels: true },
+                });
+
+              ownedMessageChannelIds = new Set(
+                connectedAccounts.flatMap((account) =>
+                  account.messageChannels.map((ch) => ch.id),
+                ),
+              );
+            }
+          }
+        }

        for (let i = messages.length - 1; i >= 0; i--) {
          const associations = messageChannelMessagesAssociations.filter(
@@ -105,33 +148,11 @@ export class ApplyMessagesVisibilityRestrictionsService {
            continue;
          }

-          if (isDefined(userId)) {
-            const workspaceMember =
-              await workspaceMemberRepository.findOneByOrFail({
-                userId,
-              });
-
-            const userWorkspace = await this.userWorkspaceRepository.findOne({
-              where: { userId: workspaceMember.userId, workspaceId },
-            });
-
-            if (userWorkspace) {
-              const connectedAccounts =
-                await this.connectedAccountRepository.find({
-                  where: {
-                    userWorkspaceId: userWorkspace.id,
-                    workspaceId,
-                    messageChannels: {
-                      id: In(messageChannels.map((channel) => channel.id)),
-                    },
-                  },
-                  relations: { messageChannels: true },
-                });
-
-              if (connectedAccounts.length > 0) {
-                continue;
-              }
-            }
+          if (
+            isDefined(ownedMessageChannelIds) &&
+            messageChannels.some((ch) => ownedMessageChannelIds!.has(ch.id))
+          ) {
+            continue;
          }

          if (