fix(mongodb-compass): Do not overfetch connectionInfo and do not update state too often COMPASS-5327

packages/collection-model/lib/model.js

@@ -83,6 +83,15 @@ function propagateCollectionEvents(namespace) {
   };
 }
 
+function getParentByType(model, type) {
+  const parent = getParent(model);
+  return parent
+    ? parent.modelType === type
+      ? parent
+      : getParentByType(parent, type)
+    : null;
+}
+
 function pickCollectionInfo({
   type,
   readonly,
@@ -235,18 +244,26 @@ const CollectionCollection = AmpersandCollection.extend(
      * @returns {Promise<void>}
      */
     async fetch({ dataService, fetchInfo = true }) {
-      const databaseName = this.parent && this.parent.getId();
+      const databaseName = getParentByType(this, 'Database')?.getId();
 
       if (!databaseName) {
         throw new Error(
-          "Trying to fetch MongoDBCollectionCollection that doesn't have the parent model"
+          `Trying to fetch ${this.modelType} that doesn't have the Database parent model`
+        );
+      }
+
+      const instanceModel = getParentByType(this, 'Instance');
+
+      if (!instanceModel) {
+        throw new Error(
+          `Trying to fetch ${this.modelType} that doesn't have the Instance parent model`
         );
       }
 
       const collections = await dataService.listCollections(
         databaseName,
         {},
-        { nameOnly: !fetchInfo }
+        { nameOnly: !fetchInfo, privileges: instanceModel.auth.privileges }
       );
 
       this.set(

packages/compass-sidebar/src/stores/store.js

@@ -43,26 +43,33 @@ store.onActivated = (appRegistry) => {
     onInstanceChange(instance);
     onDatabasesChange(instance.databases);
 
-    instance.on('change:isRefreshing', () => {
-      onInstanceChange(instance);
-    });
-
-    instance.on('change:status', () => {
-      onInstanceChange(instance);
-    });
-
-    instance.on('change:databasesStatus', () => {
-      onInstanceChange(instance);
-      onDatabasesChange(instance.databases);
-    });
-
-    instance.on('change:databases.collectionsLength', () => {
-      onInstanceChange(instance);
-    });
-
-    instance.on('change:databases.collectionsStatus', () => {
-      onDatabasesChange(instance.databases);
-    });
+    if (process.env.COMPASS_NO_GLOBAL_OVERLAY !== 'true') {
+      instance.on('change:isRefreshing', () => {
+        onInstanceChange(instance);
+        onDatabasesChange(instance.databases);
+      });
+    } else {
+      instance.on('change:isRefreshing', () => {
+        onInstanceChange(instance);
+      });
+
+      instance.on('change:status', () => {
+        onInstanceChange(instance);
+      });
+
+      instance.on('change:databases.collectionsLength', () => {
+        onInstanceChange(instance);
+      });
+
+      instance.on('change:databasesStatus', () => {
+        onInstanceChange(instance);
+        onDatabasesChange(instance.databases);
+      });
+
+      instance.on('change:databases.collectionsStatus', () => {
+        onDatabasesChange(instance.databases);
+      });
+    }
 
     function onIsGenuineChange(isGenuine) {
       store.dispatch(toggleIsGenuineMongoDB(!!isGenuine));

packages/data-service/src/data-service.spec.ts

@@ -1363,6 +1363,7 @@ describe('DataService', function () {
             listDatabases: {
               databases: [{ name: 'foo' }, { name: 'bar' }],
             },
+            connectionStatus: { authInfo: { authenticatedUserPrivileges: [] } },
           },
         });
         const dbs = (await dataService.listDatabases()).map((db) => db.name);
@@ -1463,6 +1464,9 @@ describe('DataService', function () {
     describe('#listCollections', function () {
       it('returns collections for a database', async function () {
         const dataService = createDataServiceWithMockedClient({
+          commands: {
+            connectionStatus: { authInfo: { authenticatedUserPrivileges: [] } },
+          },
           collections: {
             buz: ['foo', 'bar'],
           },

packages/data-service/src/data-service.ts

@@ -55,7 +55,7 @@ import { ConnectionOptions } from './connection-options';
 import {
   adaptCollectionInfo,
   adaptDatabaseInfo,
-  getDatabasesAndCollectionsFromPrivileges,
+  getPrivilegesByDatabaseAndCollection,
   getInstance,
   InstanceDetails,
 } from './instance-detail-helper';
@@ -69,7 +69,7 @@ import {
 } from './types';
 
 import getPort from 'get-port';
-import { runCommand } from './run-command';
+import { ConnectionStatusWithPrivileges, runCommand } from './run-command';
 
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const { fetch: getIndexes } = require('mongodb-index-model');
@@ -289,58 +289,108 @@ class DataService extends EventEmitter {
     return this._isMongos;
   }
 
+  async connectionStatus(): Promise<ConnectionStatusWithPrivileges> {
+    const logop = this._startLogOp(
+      mongoLogId(1_001_000_100),
+      'Running connectionStatus'
+    );
+    try {
+      const adminDb = this._initializedClient.db('admin');
+      const result = await runCommand(adminDb, {
+        connectionStatus: 1,
+        showPrivileges: true,
+      });
+      logop(null);
+      return result;
+    } catch (e) {
+      logop(e);
+      throw e;
+    }
+  }
+
+  private async _getPrivilegesOrFallback(
+    privileges:
+      | ConnectionStatusWithPrivileges['authInfo']['authenticatedUserPrivileges']
+      | null = null
+  ) {
+    if (privileges) {
+      return privileges;
+    }
+    const {
+      authInfo: { authenticatedUserPrivileges },
+    } = await this.connectionStatus();
+    return authenticatedUserPrivileges;
+  }
+
   /**
    * List all collections for a database.
    */
   async listCollections(
     databaseName: string,
     filter: Document = {},
-    { nameOnly }: { nameOnly?: true } = {}
+    {
+      nameOnly,
+      privileges = null,
+    }: {
+      nameOnly?: true;
+      privileges?:
+        | ConnectionStatusWithPrivileges['authInfo']['authenticatedUserPrivileges']
+        | null;
+    } = {}
   ): Promise<ReturnType<typeof adaptCollectionInfo>[]> {
-    const db = this._initializedClient.db(databaseName);
     const logop = this._startLogOp(
       mongoLogId(1_001_000_032),
       'Running listCollections',
       { db: databaseName, nameOnly: nameOnly ?? false }
     );
+
+    const db = this._initializedClient.db(databaseName);
+
+    const listCollections = async () => {
+      try {
+        return db.listCollections(filter, { nameOnly }).toArray();
+      } catch (err) {
+        // Currently Compass should not fail if listCollections failed for
+        // any possible reason to preserve current behavior. We probably
+        // want this to check at least that what we got back is a server
+        // error and not a weird runtime issue on our side that can be
+        // swallowed in this case, ideally we know exactly what server
+        // errors we want to handle here and only avoid throwing in these
+        // cases
+        //
+        // TODO: https://jira.mongodb.org/browse/COMPASS-5275
+        log.warn(
+          mongoLogId(1_001_000_099),
+          this._logCtx(),
+          'Failed to run listCollections',
+          { message: (err as Error).message }
+        );
+        return [] as { name: string }[];
+      }
+    };
+
+    const getCollectionsFromPrivileges = async () => {
+      const databases = getPrivilegesByDatabaseAndCollection(
+        await this._getPrivilegesOrFallback(privileges),
+        ['find']
+      );
+      return Object.keys(
+        // Privileges might not have a database we are looking for
+        databases[databaseName] || {}
+      )
+        .filter(
+          // Privileges can have collection name '' that indicates
+          // privileges on all collections in the database, we don't want
+          // those registered as "real" collection names
+          Boolean
+        )
+        .map((name) => ({ name }));
+    };
+
     try {
       const [listedCollections, collectionsFromPrivileges] = await Promise.all([
-        db
-          .listCollections(filter, { nameOnly })
-          .toArray()
-          .catch((err) => {
-            // Currently Compass should not fail if listCollections failed for
-            // any possible reason to preserve current behavior. We probably
-            // want this to check at least that what we got back is a server
-            // error and not a weird runtime issue on our side that can be
-            // swallowed in this case, ideally we know exactly what server
-            // errors we want to handle here and only avoid throwing in these
-            // cases
-            //
-            // TODO: https://jira.mongodb.org/browse/COMPASS-5275
-            log.warn(
-              mongoLogId(1_001_000_099),
-              this._logCtx(),
-              'Failed to run listCollections',
-              { message: err.message }
-            );
-            return [] as { name: string }[];
-          }),
-        getDatabasesAndCollectionsFromPrivileges(this._initializedClient, [
-          'find',
-        ]).then((databases) => {
-          return Object.keys(
-            // Privileges might not have a database we are looking for
-            databases[databaseName] || {}
-          )
-            .filter(
-              // Privileges can have collection name '' that indicates
-              // privileges on all collections in the database, we don't want
-              // those registered as "real" collection names
-              Boolean
-            )
-            .map((name) => ({ name }));
-        }),
+        listCollections(),
+        getCollectionsFromPrivileges(),
       ]);
 
       const collections = uniqueBy(
@@ -364,8 +414,12 @@ class DataService extends EventEmitter {
    */
   async listDatabases({
     nameOnly,
+    privileges = null,
   }: {
     nameOnly?: true;
+    privileges?:
+      | ConnectionStatusWithPrivileges['authInfo']['authenticatedUserPrivileges']
+      | null;
   } = {}): Promise<ReturnType<typeof adaptDatabaseInfo>[]> {
     const logop = this._startLogOp(
       mongoLogId(1_001_000_033),
@@ -375,52 +429,60 @@ class DataService extends EventEmitter {
 
     const adminDb = this._initializedClient.db('admin');
 
+    const listDatabases = async () => {
+      try {
+        const { databases } = await runCommand(adminDb, {
+          listDatabases: 1,
+          nameOnly,
+        } as {
+          listDatabases: 1;
+        });
+        return databases;
+      } catch (err) {
+        // Currently Compass should not fail if listDatabase failed for any
+        // possible reason to preserve current behavior. We probably want this
+        // to check at least that what we got back is a server error and not a
+        // weird runtime issue on our side that can be swallowed in this case,
+        // ideally we know exactly what server errors we want to handle here
+        // and only avoid throwing in these cases
+        //
+        // TODO: https://jira.mongodb.org/browse/COMPASS-5275
+        log.warn(
+          mongoLogId(1_001_000_098),
+          this._logCtx(),
+          'Failed to run listDatabases',
+          { message: (err as Error).message }
+        );
+        return [];
+      }
+    };
+
+    const getDatabasesFromPrivileges = async () => {
+      const databases = getPrivilegesByDatabaseAndCollection(
+        await this._getPrivilegesOrFallback(privileges),
+        ['find']
+      );
+      return Object.keys(databases)
+        .filter(
+          // For the roles created in admin database, the database name
+          // can be '' meaning that it applies to all databases. We can't
+          // meaningfully handle this in the UI so we are filtering these
+          // out
+          Boolean
+        )
+        .map((name) => ({ name }));
+    };
+
     try {
       const [listedDatabases, databasesFromPrivileges] = await Promise.all([
-        runCommand(adminDb, { listDatabases: 1, nameOnly } as {
-          listDatabases: 1;
-        }).catch((err) => {
-          // Currently Compass should not fail if listDatabase failed for any
-          // possible reason to preserve current behavior. We probably want this
-          // to check at least that what we got back is a server error and not a
-          // weird runtime issue on our side that can be swallowed in this case,
-          // ideally we know exactly what server errors we want to handle here
-          // and only avoid throwing in these cases
-          //
-          // TODO: https://jira.mongodb.org/browse/COMPASS-5275
-          log.warn(
-            mongoLogId(1_001_000_098),
-            this._logCtx(),
-            'Failed to run listDatabases',
-            { message: err.message }
-          );
-          return { databases: [] };
-        }),
-        // If we somehow failed to get user privileges to get a fallback for the
-        // databases/collections, we do want to hard fail, there is no good
-        // reason this command will ever fail, unless server is in a bad shape
-        // or we messed something up
-        getDatabasesAndCollectionsFromPrivileges(this._initializedClient, [
-          'find',
-        ]).then((databases) => {
-          return {
-            databases: Object.keys(databases)
-              .filter(
-                // For the roles created in admin database, the database name
-                // can be '' meaning that it applies to all databases. We can't
-                // meaningfully handle this in the UI so we are filtering these
-                // out
-                Boolean
-              )
-              .map((name) => ({ name })),
-          };
-        }),
+        listDatabases(),
+        getDatabasesFromPrivileges(),
       ]);
 
       const databases = uniqueBy(
         // NB: Order is important, we want listed collections to take precedence
         // if they were fetched successfully
-        [...databasesFromPrivileges.databases, ...listedDatabases.databases],
+        [...databasesFromPrivileges, ...listedDatabases],
         'name'
       ).map((db) => adaptDatabaseInfo({ db: db.name, ...db }));