From c6fec978190c55842c0cea6c36efde6faa2082a1 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Fri, 28 May 2021 12:42:21 -0700 Subject: [PATCH] Expose default database collation through 'sql-database-projects' extension API (#15538) * Expose default database collation through 'sql-database-projects' extension API. For the purpose of schema conversion we would need to know whether target database is configured as CI or CS. This will be used to produce a warning, if we detect a case-sensitive identifier, but database is configured as CI. In order to support this scenario we need to access `` property of the project. This change adds new method to the `ISqlProject` interface that allows to read the value of the project property. There already was similar method for the SQL version/platform (`` property) and while working on the change, I took an opportunity to refactor the way project properties are extracted from the XML. Original code was hardcoded in the `getProjectTargetVersion` and I extracted it into separate `evaluateProjectPropertyValue` helper, that can be used in the future by any getter or access method that is supposed to return a value of the single property. This also allows us to improve the way properties are retrieved from the XML. Today the logic is very rudimentary - we read the first matching XML element with the required name. This is not correct as it does not verify the parent to be ``, neither it evaluates the `Condition` attributes nor property value itself. I did not invest in this, but added a TODO there explaining that the method may not perform as expected. After the helper method was added, I updated the existing `getProjectTargetVersion` method to leverage it. The only complication here was the error throwing logic, as it was using custom error message. I preserved that, as there were tests verifying it already. For the new accessor method I did not introduce a special error message and rely on generic one I defined for use within the helper method. Additionally, for the collation we return default value of `SQL_Latin1_General_CP1_CI_AS`, if project does not have the property defined. This is what SSDT for Visual Studio shows in the UI when property is missing and I decided to align with that. Finally, I added tests for both - original `getProjectTargetVersion` and new collation extraction method to make sure they work as expected. While working on the tests, I've noticed complaints that some rejected promises were not awaited. I added missing `await`s. --- .../src/common/constants.ts | 7 ++ .../src/models/project.ts | 80 +++++++++++++++++-- .../sql-database-projects/src/sqldbproj.d.ts | 7 ++ .../src/test/baselines/baselines.ts | 8 ++ .../sqlProjectCustomCollationBaseline.xml | 68 ++++++++++++++++ .../sqlProjectInvalidCollationBaseline.xml | 68 ++++++++++++++++ .../sqlProjectInvalidVersionBaseline.xml | 67 ++++++++++++++++ .../sqlProjectMissingVersionBaseline.xml | 66 +++++++++++++++ .../src/test/project.test.ts | 53 +++++++++++- 9 files changed, 414 insertions(+), 10 deletions(-) create mode 100644 extensions/sql-database-projects/src/test/baselines/sqlProjectCustomCollationBaseline.xml create mode 100644 extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidCollationBaseline.xml create mode 100644 extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidVersionBaseline.xml create mode 100644 extensions/sql-database-projects/src/test/baselines/sqlProjectMissingVersionBaseline.xml diff --git a/extensions/sql-database-projects/src/common/constants.ts b/extensions/sql-database-projects/src/common/constants.ts index 773d85eacc..a4f75a738c 100644 --- a/extensions/sql-database-projects/src/common/constants.ts +++ b/extensions/sql-database-projects/src/common/constants.ts @@ -203,6 +203,7 @@ export function folderAlreadyExists(filename: string) { return localize('folderA export function fileAlreadyAddedToProject(filepath: string) { return localize('fileAlreadyAddedToProject', "A file with the path '{0}' has already been added to the project", filepath); } export function folderAlreadyAddedToProject(folderpath: string) { return localize('folderAlreadyAddedToProject', "A folder with the path '{0}' has already been added to the project", folderpath); } export function invalidInput(input: string) { return localize('invalidInput', "Invalid input: {0}", input); } +export function invalidProjectPropertyValue(propertyName: string) { return localize('invalidPropertyValue', "Invalid value specified for the property '{0}' in .sqlproj file", propertyName); } export function unableToCreatePublishConnection(input: string) { return localize('unableToCreatePublishConnection', "Unable to construct connection: {0}", input); } export function circularProjectReference(project1: string, project2: string) { return localize('cicularProjectReference', "Circular reference from project {0} to project {1}", project1, project2); } export function mssqlNotFound(mssqlConfigDir: string) { return localize('mssqlNotFound', "Could not get mssql extension's install location at {0}", mssqlConfigDir); } @@ -277,6 +278,12 @@ export const ProjectGuid = 'ProjectGuid'; export const Type = 'Type'; export const ExternalStreamingJob: string = 'ExternalStreamingJob'; +/** Name of the property item in the project file that defines default database collation. */ +export const DefaultCollationProperty = 'DefaultCollation'; + +/** Default database collation to use when none is specified in the project */ +export const DefaultCollation = 'SQL_Latin1_General_CP1_CI_AS'; + // SqlProj File targets export const NetCoreTargets = '$(NETCoreTargetsPath)\\Microsoft.Data.Tools.Schema.SqlTasks.targets'; export const SqlDbTargets = '$(SQLDBExtensionsRefPath)\\Microsoft.Data.Tools.Schema.SqlTasks.targets'; diff --git a/extensions/sql-database-projects/src/models/project.ts b/extensions/sql-database-projects/src/models/project.ts index 9d920c2ff0..db42a1ec44 100644 --- a/extensions/sql-database-projects/src/models/project.ts +++ b/extensions/sql-database-projects/src/models/project.ts @@ -516,18 +516,27 @@ export class Project implements ISqlProject { } public getProjectTargetVersion(): string { - // check for invalid DSP - if (this.projFileXmlDoc.getElementsByTagName(constants.DSP).length !== 1 || this.projFileXmlDoc.getElementsByTagName(constants.DSP)[0].childNodes.length !== 1) { + let dsp: string | undefined; + + try { + dsp = this.evaluateProjectPropertyValue(constants.DSP); + } + catch { + // We will throw specialized error instead + } + + // Check if DSP is missing or invalid + if (!dsp) { throw new Error(constants.invalidDataSchemaProvider); } - let dsp: string = this.projFileXmlDoc.getElementsByTagName(constants.DSP)[0].childNodes[0].data; - // get version from dsp, which is a string like Microsoft.Data.Tools.Schema.Sql.Sql130DatabaseSchemaProvider - // remove part before the number - let version: any = dsp.substring(constants.MicrosoftDatatoolsSchemaSqlSql.length); - // remove DatabaseSchemaProvider - version = version.substring(0, version.length - constants.databaseSchemaProvider.length); + // Remove prefix and suffix to only get the actual version number/name. For the example above the result + // should be just '130'. + const version = + dsp.substring( + constants.MicrosoftDatatoolsSchemaSqlSql.length, + dsp.length - constants.databaseSchemaProvider.length); // make sure version is valid if (!Array.from(constants.targetPlatformToVersion.values()).includes(version)) { @@ -537,6 +546,15 @@ export class Project implements ISqlProject { return version; } + /** + * Gets the default database collation set in the project. + * + * @returns Default collation for the database set in the project. + */ + public getDatabaseDefaultCollation(): string { + return this.evaluateProjectPropertyValue(constants.DefaultCollationProperty, constants.DefaultCollation); + } + /** * Adds reference to a dacpac to the project * @param uri Uri of the dacpac @@ -1039,6 +1057,52 @@ export class Project implements ISqlProject { } } } + + /** + * Evaluates the value of the property item in the loaded project. + * + * @param propertyName Name of the property item to evaluate. + * @returns Value of the property or `undefined`, if property is missing. + */ + private evaluateProjectPropertyValue(propertyName: string): string | undefined; + + /** + * Evaluates the value of the property item in the loaded project. + * + * @param propertyName Name of the property item to evaluate. + * @param defaultValue Default value to return, if property is not set. + * @returns Value of the property or `defaultValue`, if property is missing. + */ + private evaluateProjectPropertyValue(propertyName: string, defaultValue: string): string; + + /** + * Evaluates the value of the property item in the loaded project. + * + * @param propertyName Name of the property item to evaluate. + * @param defaultValue Default value to return, if property is not set. + * @returns Value of the property or `defaultValue`, if property is missing. + */ + private evaluateProjectPropertyValue(propertyName: string, defaultValue?: string): string | undefined { + // TODO: Currently we simply read the value of the first matching element. The code should be updated to: + // 1) Narrow it down to items under only + // 2) Respect the `Condition` attribute on group and property itself + // 3) Evaluate any expressions within the property value + + // Check if property is set in the project + const propertyElements = this.projFileXmlDoc.getElementsByTagName(propertyName); + if (propertyElements.length === 0) { + return defaultValue; + } + + // Try to extract the value from the first matching element + const firstPropertyElement = propertyElements[0]; + if (firstPropertyElement.childNodes.length !== 1) { + // Property items are expected to have simple string content + throw new Error(constants.invalidProjectPropertyValue(propertyName)); + } + + return firstPropertyElement.childNodes[0].data; + } } /** diff --git a/extensions/sql-database-projects/src/sqldbproj.d.ts b/extensions/sql-database-projects/src/sqldbproj.d.ts index daea839fd9..595d35466f 100644 --- a/extensions/sql-database-projects/src/sqldbproj.d.ts +++ b/extensions/sql-database-projects/src/sqldbproj.d.ts @@ -85,6 +85,13 @@ declare module 'sqldbproj' { */ getProjectTargetVersion(): string; + /** + * Gets the default database collation set in the project. + * + * @returns Default collation for the database set in the project. + */ + getDatabaseDefaultCollation(): string; + /** * Path where dacpac is output to after a successful build */ diff --git a/extensions/sql-database-projects/src/test/baselines/baselines.ts b/extensions/sql-database-projects/src/test/baselines/baselines.ts index f60ed300f1..191303ecf3 100644 --- a/extensions/sql-database-projects/src/test/baselines/baselines.ts +++ b/extensions/sql-database-projects/src/test/baselines/baselines.ts @@ -22,6 +22,10 @@ export let publishProfileSqlLoginBaseline: string; export let openProjectWithProjectReferencesBaseline: string; export let openSqlProjectWithPrePostDeploymentError: string; export let openSqlProjectWithAdditionalSqlCmdVariablesBaseline: string; +export let sqlProjectMissingVersionBaseline: string; +export let sqlProjectInvalidVersionBaseline: string; +export let sqlProjectCustomCollationBaseline: string; +export let sqlProjectInvalidCollationBaseline: string; const baselineFolderPath = __dirname; @@ -41,6 +45,10 @@ export async function loadBaselines() { openProjectWithProjectReferencesBaseline = await loadBaseline(baselineFolderPath, 'openSqlProjectWithProjectReferenceBaseline.xml'); openSqlProjectWithPrePostDeploymentError = await loadBaseline(baselineFolderPath, 'openSqlProjectWithPrePostDeploymentError.xml'); openSqlProjectWithAdditionalSqlCmdVariablesBaseline = await loadBaseline(baselineFolderPath, 'openSqlProjectWithAdditionalSqlCmdVariablesBaseline.xml'); + sqlProjectMissingVersionBaseline = await loadBaseline(baselineFolderPath, 'sqlProjectMissingVersionBaseline.xml'); + sqlProjectInvalidVersionBaseline = await loadBaseline(baselineFolderPath, 'sqlProjectInvalidVersionBaseline.xml'); + sqlProjectCustomCollationBaseline = await loadBaseline(baselineFolderPath, 'sqlProjectCustomCollationBaseline.xml'); + sqlProjectInvalidCollationBaseline = await loadBaseline(baselineFolderPath, 'sqlProjectInvalidCollationBaseline.xml'); } async function loadBaseline(baselineFolderPath: string, fileName: string): Promise { diff --git a/extensions/sql-database-projects/src/test/baselines/sqlProjectCustomCollationBaseline.xml b/extensions/sql-database-projects/src/test/baselines/sqlProjectCustomCollationBaseline.xml new file mode 100644 index 0000000000..c639cca9e3 --- /dev/null +++ b/extensions/sql-database-projects/src/test/baselines/sqlProjectCustomCollationBaseline.xml @@ -0,0 +1,68 @@ + + + + Debug + AnyCPU + TestProjectName + 2.0 + 4.1 + {BA5EBA11-C0DE-5EA7-ACED-BABB1E70A575} + Microsoft.Data.Tools.Schema.Sql.Sql150DatabaseSchemaProvider + Database + + + TestProjectName + TestProjectName + 1033, CI + BySchemaAndSchemaType + True + v4.5 + CS + Properties + False + True + True + SQL_Latin1_General_CP1255_CS_AS + + + bin\Release\ + $(MSBuildProjectName).sql + False + pdbonly + true + false + true + prompt + 4 + + + bin\Debug\ + $(MSBuildProjectName).sql + false + true + full + false + true + true + prompt + 4 + + + 11.0 + + True + 11.0 + + + + + + + + + + + + + + diff --git a/extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidCollationBaseline.xml b/extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidCollationBaseline.xml new file mode 100644 index 0000000000..83b209f76b --- /dev/null +++ b/extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidCollationBaseline.xml @@ -0,0 +1,68 @@ + + + + Debug + AnyCPU + TestProjectName + 2.0 + 4.1 + {BA5EBA11-C0DE-5EA7-ACED-BABB1E70A575} + Microsoft.Data.Tools.Schema.Sql.Sql150DatabaseSchemaProvider + Database + + + TestProjectName + TestProjectName + 1033, CI + BySchemaAndSchemaType + True + v4.5 + CS + Properties + False + True + True + + + + bin\Release\ + $(MSBuildProjectName).sql + False + pdbonly + true + false + true + prompt + 4 + + + bin\Debug\ + $(MSBuildProjectName).sql + false + true + full + false + true + true + prompt + 4 + + + 11.0 + + True + 11.0 + + + + + + + + + + + + + + diff --git a/extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidVersionBaseline.xml b/extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidVersionBaseline.xml new file mode 100644 index 0000000000..469d908f65 --- /dev/null +++ b/extensions/sql-database-projects/src/test/baselines/sqlProjectInvalidVersionBaseline.xml @@ -0,0 +1,67 @@ + + + + Debug + AnyCPU + TestProjectName + 2.0 + 4.1 + {BA5EBA11-C0DE-5EA7-ACED-BABB1E70A575} + + Database + + + TestProjectName + TestProjectName + 1033, CI + BySchemaAndSchemaType + True + v4.5 + CS + Properties + False + True + True + + + bin\Release\ + $(MSBuildProjectName).sql + False + pdbonly + true + false + true + prompt + 4 + + + bin\Debug\ + $(MSBuildProjectName).sql + false + true + full + false + true + true + prompt + 4 + + + 11.0 + + True + 11.0 + + + + + + + + + + + + + + diff --git a/extensions/sql-database-projects/src/test/baselines/sqlProjectMissingVersionBaseline.xml b/extensions/sql-database-projects/src/test/baselines/sqlProjectMissingVersionBaseline.xml new file mode 100644 index 0000000000..1588d931a7 --- /dev/null +++ b/extensions/sql-database-projects/src/test/baselines/sqlProjectMissingVersionBaseline.xml @@ -0,0 +1,66 @@ + + + + Debug + AnyCPU + TestProjectName + 2.0 + 4.1 + {BA5EBA11-C0DE-5EA7-ACED-BABB1E70A575} + Database + + + TestProjectName + TestProjectName + 1033, CI + BySchemaAndSchemaType + True + v4.5 + CS + Properties + False + True + True + + + bin\Release\ + $(MSBuildProjectName).sql + False + pdbonly + true + false + true + prompt + 4 + + + bin\Debug\ + $(MSBuildProjectName).sql + false + true + full + false + true + true + prompt + 4 + + + 11.0 + + True + 11.0 + + + + + + + + + + + + + + diff --git a/extensions/sql-database-projects/src/test/project.test.ts b/extensions/sql-database-projects/src/test/project.test.ts index de83aa1304..be799ed325 100644 --- a/extensions/sql-database-projects/src/test/project.test.ts +++ b/extensions/sql-database-projects/src/test/project.test.ts @@ -233,11 +233,11 @@ describe('Project: sqlproj content operations', function (): void { projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline); const project = await Project.openProject(projFilePath); - should(project.databaseReferences.length).equal(0, 'There should be no datbase references to start with'); + should(project.databaseReferences.length).equal(0, 'There should be no database references to start with'); await project.addSystemDatabaseReference({ databaseName: 'master', systemDb: SystemDatabase.master, suppressMissingDependenciesErrors: false }); should(project.databaseReferences.length).equal(1, 'There should be one database reference after adding a reference to master'); should(project.databaseReferences[0].databaseName).equal(constants.master, 'The database reference should be master'); - should(project.databaseReferences[0].suppressMissingDependenciesErrors).equal(false, 'project.databaseReferences[1].suppressMissingDependenciesErrors should be false'); + should(project.databaseReferences[0].suppressMissingDependenciesErrors).equal(false, 'project.databaseReferences[0].suppressMissingDependenciesErrors should be false'); // make sure reference to ADS master dacpac and SSDT master dacpac was added let projFileText = (await fs.readFile(projFilePath)).toString(); should(projFileText).containEql(convertSlashesForSqlProj(project.getSystemDacpacUri(constants.master).fsPath.substring(1))); @@ -754,6 +754,55 @@ describe('Project: add SQLCMD Variables', function (): void { }); }); +describe('Project: properties', function (): void { + before(async function (): Promise { + await baselines.loadBaselines(); + }); + + it('Should read target database version', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.openProjectFileBaseline); + const project = await Project.openProject(projFilePath); + + should(project.getProjectTargetVersion()).equal('150'); + }); + + it('Should throw on missing target database version', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectMissingVersionBaseline); + const project = await Project.openProject(projFilePath); + + should(() => project.getProjectTargetVersion()).throw("Invalid DSP in .sqlproj file"); + }); + + it('Should throw on invalid target database version', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectInvalidVersionBaseline); + const project = await Project.openProject(projFilePath); + + should(() => project.getProjectTargetVersion()).throw("Invalid DSP in .sqlproj file"); + }); + + it('Should read default database collation', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectCustomCollationBaseline); + const project = await Project.openProject(projFilePath); + + should(project.getDatabaseDefaultCollation()).equal('SQL_Latin1_General_CP1255_CS_AS'); + }); + + it('Should return default value when database collation is not specified', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline); + const project = await Project.openProject(projFilePath); + + should(project.getDatabaseDefaultCollation()).equal('SQL_Latin1_General_CP1_CI_AS'); + }); + + it('Should throw on invalid default database collation', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectInvalidCollationBaseline); + const project = await Project.openProject(projFilePath); + + should(() => project.getDatabaseDefaultCollation()) + .throw("Invalid value specified for the property 'DefaultCollation' in .sqlproj file"); + }); +}); + describe('Project: round trip updates', function (): void { before(async function (): Promise { await baselines.loadBaselines();