diff --git a/extensions/mssql/src/constants/constants.ts b/extensions/mssql/src/constants/constants.ts index b0421e2dc0..73a87f1ae8 100644 --- a/extensions/mssql/src/constants/constants.ts +++ b/extensions/mssql/src/constants/constants.ts @@ -161,6 +161,7 @@ export const defaultPortNumber = 1433; export const integratedauth = "Integrated"; export const sqlAuthentication = "SqlLogin"; export const defaultDatabase = "master"; +export const systemDatabases = ["master", "tempdb", "model", "msdb"]; export const defaultSchema = "dbo"; export const errorPasswordExpired = 18487; export const errorPasswordNeedsReset = 18488; diff --git a/extensions/mssql/src/profiler/profilerController.ts b/extensions/mssql/src/profiler/profilerController.ts index dcd99010af..23e1bba631 100644 --- a/extensions/mssql/src/profiler/profilerController.ts +++ b/extensions/mssql/src/profiler/profilerController.ts @@ -24,12 +24,10 @@ import { ObjectExplorerUtils } from "../objectExplorer/objectExplorerUtils"; import { IConnectionProfile } from "../models/interfaces"; import { getServerTypes, isAzureSqlDbCompatible } from "../models/connectionInfo"; import { getErrorMessage, uuid } from "../utils/utils"; +import { isSystemDatabase } from "../utils/databaseUtils"; import { sendActionEvent, sendErrorEvent } from "../telemetry/telemetry"; import { TelemetryViews, TelemetryActions } from "../sharedInterfaces/telemetry"; -/** System databases that cannot be used for Azure SQL profiling */ -const SYSTEM_DATABASES = ["master", "tempdb", "model", "msdb"]; - /** * Controller for the profiler feature. * Handles command registration, connection management, and launching the profiler UI. @@ -116,7 +114,7 @@ export class ProfilerController { // connects at server level; otherwise the STS may look for the session // in the database scope and fail with "session not found". // Preserve the database as a client-side filter if one isn't already set. - if (profileToUse.database && !this.isSystemDatabase(profileToUse.database)) { + if (profileToUse.database && !isSystemDatabase(profileToUse.database)) { if (!databaseScopeFilter) { databaseScopeFilter = profileToUse.database; } @@ -173,18 +171,6 @@ export class ProfilerController { // Private Methods // ============================================================ - /** - * Checks if a database is a system database. - * @param databaseName - The name of the database to check - * @returns true if the database is a system database - */ - private isSystemDatabase(databaseName: string | undefined): boolean { - if (!databaseName) { - return true; // No database selected is treated as system database for this purpose - } - return SYSTEM_DATABASES.includes(databaseName.toLowerCase()); - } - /** * Ensures a user database is selected for Azure SQL connections. * If no database or a system database is selected, prompts the user to select one. @@ -195,7 +181,7 @@ export class ProfilerController { connectionProfile: IConnectionProfile, ): Promise { // Check if a user database is already selected - if (!this.isSystemDatabase(connectionProfile.database)) { + if (!isSystemDatabase(connectionProfile.database)) { this._logger.verbose(`User database already selected: ${connectionProfile.database}`); return connectionProfile; } @@ -218,7 +204,7 @@ export class ProfilerController { const databases = await this._connectionManager.listDatabases(tempUri); // Filter out system databases - const userDatabases = databases.filter((db) => !this.isSystemDatabase(db)); + const userDatabases = databases.filter((db) => !isSystemDatabase(db)); if (userDatabases.length === 0) { this._logger.verbose("No user databases found"); diff --git a/extensions/mssql/src/utils/databaseUtils.ts b/extensions/mssql/src/utils/databaseUtils.ts new file mode 100644 index 0000000000..df359db9d8 --- /dev/null +++ b/extensions/mssql/src/utils/databaseUtils.ts @@ -0,0 +1,20 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { systemDatabases } from "../constants/constants"; + +/** + * Checks whether a database name refers to a system database (or is empty/undefined). + * The check is case-insensitive. + * + * @param databaseName - The database name to check + * @returns `true` if the name is undefined, empty, or matches a system database + */ +export function isSystemDatabase(databaseName: string | undefined): boolean { + if (!databaseName) { + return true; + } + return systemDatabases.includes(databaseName.toLowerCase()); +} diff --git a/extensions/mssql/test/unit/profiler/profilerController.test.ts b/extensions/mssql/test/unit/profiler/profilerController.test.ts index ee6c2ed18f..6c923d867d 100644 --- a/extensions/mssql/test/unit/profiler/profilerController.test.ts +++ b/extensions/mssql/test/unit/profiler/profilerController.test.ts @@ -701,6 +701,7 @@ suite("ProfilerController Server Type Tests", () => { }, }; + connectionManager.listDatabases.resolves(["master", "tempdb", "UserDB1"]); showQuickPickStub.resolves("UserDB1"); createController(); @@ -720,6 +721,7 @@ suite("ProfilerController Server Type Tests", () => { }, }; + connectionManager.listDatabases.resolves(["master", "tempdb", "UserDB1"]); showQuickPickStub.resolves("UserDB1"); createController(); @@ -811,6 +813,7 @@ suite("ProfilerController Server Type Tests", () => { }, }; + // User cancels database selection via quick pick showQuickPickStub.resolves(undefined); // User cancelled createController(); @@ -831,7 +834,7 @@ suite("ProfilerController Server Type Tests", () => { }, }; - // First connect call (for temp connection to get database list) fails + // The temp connection for listing databases fails connectionManager.connect.resolves(false); const showErrorMessageStub = vscode.window.showErrorMessage as sinon.SinonStub; diff --git a/extensions/mssql/test/unit/utils/databaseUtils.test.ts b/extensions/mssql/test/unit/utils/databaseUtils.test.ts new file mode 100644 index 0000000000..f4d28bc5bc --- /dev/null +++ b/extensions/mssql/test/unit/utils/databaseUtils.test.ts @@ -0,0 +1,48 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { expect } from "chai"; +import * as chai from "chai"; +import sinonChai from "sinon-chai"; +import { isSystemDatabase } from "../../../src/utils/databaseUtils"; +import { systemDatabases } from "../../../src/constants/constants"; + +chai.use(sinonChai); + +suite("databaseUtils", () => { + suite("systemDatabases", () => { + test("should contain the four well-known system databases", () => { + expect(systemDatabases).to.include.members(["master", "tempdb", "model", "msdb"]); + expect(systemDatabases).to.have.lengthOf(4); + }); + }); + + suite("isSystemDatabase", () => { + test("should return true for undefined", () => { + expect(isSystemDatabase(undefined)).to.be.true; + }); + + test("should return true for empty string", () => { + expect(isSystemDatabase("")).to.be.true; + }); + + test("should return true for each system database", () => { + for (const db of systemDatabases) { + expect(isSystemDatabase(db)).to.be.true; + } + }); + + test("should be case-insensitive", () => { + expect(isSystemDatabase("MASTER")).to.be.true; + expect(isSystemDatabase("Master")).to.be.true; + expect(isSystemDatabase("TempDb")).to.be.true; + }); + + test("should return false for user databases", () => { + expect(isSystemDatabase("MyAppDb")).to.be.false; + expect(isSystemDatabase("AdventureWorks")).to.be.false; + }); + }); +});