fix DefaultValue getting loaded for publish profiles (#17526)

* fix DefaultValue getting loaded for publish profiles

* add comment

extensions/sql-database-projects/src/common/utils.ts

@@ -156,14 +156,18 @@ export function convertSlashesForSqlProj(filePath: string): string {
 /**
  * Read SQLCMD variables from xmlDoc and return them
  * @param xmlDoc xml doc to read SQLCMD variables from. Format must be the same that sqlproj and publish profiles use
+ * @param publishProfile true if reading from publish profile
  */
-export function readSqlCmdVariables(xmlDoc: any): Record<string, string> {
+export function readSqlCmdVariables(xmlDoc: any, publishProfile: boolean): Record<string, string> {
 	let sqlCmdVariables: Record<string, string> = {};
 	for (let i = 0; i < xmlDoc.documentElement.getElementsByTagName(constants.SqlCmdVariable)?.length; i++) {
 		const sqlCmdVar = xmlDoc.documentElement.getElementsByTagName(constants.SqlCmdVariable)[i];
 		const varName = sqlCmdVar.getAttribute(constants.Include);
 
-		if (sqlCmdVar.getElementsByTagName(constants.DefaultValue)[0] !== undefined) {
+		// Publish profiles only support Value, so don't use DefaultValue even if it's there
+		// SSDT uses the Value (like <Value>$(SqlCmdVar__1)</Value>) where there
+		// are local variable values you can set in VS in the properties. Since we don't support that in ADS, only DefaultValue is supported for sqlproj.
+		if (!publishProfile && sqlCmdVar.getElementsByTagName(constants.DefaultValue)[0] !== undefined) {
 			// project file path
 			sqlCmdVariables[varName] = sqlCmdVar.getElementsByTagName(constants.DefaultValue)[0].childNodes[0].nodeValue;
 		}

extensions/sql-database-projects/src/models/project.ts

@@ -175,7 +175,7 @@ export class Project implements ISqlProject {
 		}
 
 		// find all SQLCMD variables to include
-		this._sqlCmdVariables = utils.readSqlCmdVariables(this.projFileXmlDoc);
+		this._sqlCmdVariables = utils.readSqlCmdVariables(this.projFileXmlDoc, false);
 
 		// find all database references to include
 		const references = this.projFileXmlDoc.documentElement.getElementsByTagName(constants.ArtifactReference);

extensions/sql-database-projects/src/models/publishProfile/publishProfile.ts

@@ -53,7 +53,7 @@ export async function load(profileUri: vscode.Uri, dacfxService: utils.IDacFxSer
 	const optionsResult = await dacfxService.getOptionsFromProfile(profileUri.fsPath);
 
 	// get all SQLCMD variables to include from the profile
-	const sqlCmdVariables = utils.readSqlCmdVariables(profileXmlDoc);
+	const sqlCmdVariables = utils.readSqlCmdVariables(profileXmlDoc, true);
 
 	return {
 		databaseName: targetDbName,

extensions/sql-database-projects/src/test/baselines/baselines.ts

@@ -19,6 +19,7 @@ export let SSDTProjectBaselineWithBeforeBuildTarget: string;
 export let SSDTProjectBaselineWithBeforeBuildTargetAfterUpdate: string;
 export let publishProfileIntegratedSecurityBaseline: string;
 export let publishProfileSqlLoginBaseline: string;
+export let publishProfileDefaultValueBaseline: string;
 export let openProjectWithProjectReferencesBaseline: string;
 export let openSqlProjectWithPrePostDeploymentError: string;
 export let openSqlProjectWithAdditionalSqlCmdVariablesBaseline: string;
@@ -45,6 +46,7 @@ export async function loadBaselines() {
 	SSDTProjectBaselineWithBeforeBuildTargetAfterUpdate = await loadBaseline(baselineFolderPath, 'SSDTProjectBaselineWithBeforeBuildTargetAfterUpdate.xml');
 	publishProfileIntegratedSecurityBaseline = await loadBaseline(baselineFolderPath, 'publishProfileIntegratedSecurityBaseline.publish.xml');
 	publishProfileSqlLoginBaseline = await loadBaseline(baselineFolderPath, 'publishProfileSqlLoginBaseline.publish.xml');
+	publishProfileDefaultValueBaseline = await loadBaseline(baselineFolderPath, 'publishProfileDefaultValueBaseline.publish.xml');
 	openProjectWithProjectReferencesBaseline = await loadBaseline(baselineFolderPath, 'openSqlProjectWithProjectReferenceBaseline.xml');
 	openSqlProjectWithPrePostDeploymentError = await loadBaseline(baselineFolderPath, 'openSqlProjectWithPrePostDeploymentError.xml');
 	openSqlProjectWithAdditionalSqlCmdVariablesBaseline = await loadBaseline(baselineFolderPath, 'openSqlProjectWithAdditionalSqlCmdVariablesBaseline.xml');

extensions/sql-database-projects/src/test/baselines/publishProfileDefaultValueBaseline.publish.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>
+    <IncludeCompositeObjects>True</IncludeCompositeObjects>
+    <TargetDatabaseName>targetDb</TargetDatabaseName>
+    <DeployScriptFileName>DatabaseProject1.sql</DeployScriptFileName>
+    <ProfileVersionNumber>1</ProfileVersionNumber>
+  </PropertyGroup>
+  <ItemGroup>
+    <SqlCmdVariable Include="ProdDatabaseName">
+      <DefaultValue>defaultProdName</DefaultValue>
+      <Value>MyProdDatabase</Value>
+    </SqlCmdVariable>
+  </ItemGroup>
+</Project>

extensions/sql-database-projects/src/test/publishProfile.test.ts

@@ -31,7 +31,7 @@ describe('Publish profile tests', function (): void {
 
 	it('Should read database name, integrated security connection string, and SQLCMD variables from publish profile', async function (): Promise<void> {
 		await baselines.loadBaselines();
-		let profilePath = await testUtils.createTestFile(baselines.publishProfileIntegratedSecurityBaseline, 'publishProfile.publish.xml');
+		const profilePath = await testUtils.createTestFile(baselines.publishProfileIntegratedSecurityBaseline, 'publishProfile.publish.xml');
 		const connectionResult = {
 			connected: true,
 			connectionId: 'connId',
@@ -43,7 +43,7 @@ describe('Publish profile tests', function (): void {
 		});
 		sinon.stub(azdata.connection, 'connect').resolves(connectionResult);
 
-		let result = await load(vscode.Uri.file(profilePath), testContext.dacFxService.object);
+		const result = await load(vscode.Uri.file(profilePath), testContext.dacFxService.object);
 		should(result.databaseName).equal('targetDb');
 		should(Object.keys(result.sqlCmdVariables).length).equal(1);
 		should(result.sqlCmdVariables['ProdDatabaseName']).equal('MyProdDatabase');
@@ -54,7 +54,7 @@ describe('Publish profile tests', function (): void {
 
 	it('Should read database name, SQL login connection string, and SQLCMD variables from publish profile', async function (): Promise<void> {
 		await baselines.loadBaselines();
-		let profilePath = await testUtils.createTestFile(baselines.publishProfileSqlLoginBaseline, 'publishProfile.publish.xml');
+		const profilePath = await testUtils.createTestFile(baselines.publishProfileSqlLoginBaseline, 'publishProfile.publish.xml');
 		const connectionResult = {
 			providerName: 'MSSQL',
 			connectionId: 'connId',
@@ -68,7 +68,7 @@ describe('Publish profile tests', function (): void {
 		});
 		sinon.stub(azdata.connection, 'openConnectionDialog').resolves(connectionResult);
 
-		let result = await load(vscode.Uri.file(profilePath), testContext.dacFxService.object);
+		const result = await load(vscode.Uri.file(profilePath), testContext.dacFxService.object);
 		should(result.databaseName).equal('targetDb');
 		should(Object.keys(result.sqlCmdVariables).length).equal(1);
 		should(result.sqlCmdVariables['ProdDatabaseName']).equal('MyProdDatabase');
@@ -77,9 +77,23 @@ describe('Publish profile tests', function (): void {
 		should(result.options).equal(mockDacFxOptionsResult.deploymentOptions);
 	});
 
+	it('Should read SQLCMD variables correctly from publish profile even if DefaultValue is used', async function (): Promise<void> {
+		await baselines.loadBaselines();
+		const profilePath = await testUtils.createTestFile(baselines.publishProfileDefaultValueBaseline, 'publishProfile.publish.xml');
+		testContext.dacFxService.setup(x => x.getOptionsFromProfile(TypeMoq.It.isAny())).returns(async () => {
+			return Promise.resolve(mockDacFxOptionsResult);
+		});
+
+		const result = await load(vscode.Uri.file(profilePath), testContext.dacFxService.object);
+		should(Object.keys(result.sqlCmdVariables).length).equal(1);
+
+		// the profile has both Value and DefaultValue, but Value should be the one used
+		should(result.sqlCmdVariables['ProdDatabaseName']).equal('MyProdDatabase');
+	});
+
 	it('Should throw error when connecting does not work', async function (): Promise<void> {
 		await baselines.loadBaselines();
-		let profilePath = await testUtils.createTestFile(baselines.publishProfileIntegratedSecurityBaseline, 'publishProfile.publish.xml');
+		const profilePath = await testUtils.createTestFile(baselines.publishProfileIntegratedSecurityBaseline, 'publishProfile.publish.xml');
 
 		sinon.stub(azdata.connection, 'connect').throws(new Error('Could not connect'));