From 0f598dd30bb1cccb3bed0dcae69f01a2dfec8ad2 Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Mon, 18 Apr 2022 11:38:10 -0700 Subject: [PATCH] Add sqlproj property to trace the origin of the project. (#18670) * Add sqlproj property to trace the origin of the project. As part of the database migration process (schema conversion, in particular) we want to be able to tell when converted schemas are being built/deployed to the actual database server. Given that we rely on the SQL Database Projects ADS extension for the compilation/deployment, we don't have too many options other than updating the said extension. The suggested approach is to make the following changes: 1) Add new property to the sqlproj file (called `DatabaseSource`), which will maintain the origin(s) of the project. The property can contain multiple values (separated by semicolon), in case same project contains objects produced by multiple sources (extract schema, convert from another database, etc.). 2) During build and deploy actions, send the well-known values from the newly added property to the telemetry. We don't want to send any random value of the property, as it may raise some privacy concerns. Instead we define a list of the well-known values that we know do not carry any personal information and send those, if they are specified. This change adds all necessary APIs to the SQl Database projects extension which will be consumed by our migration extensions to populate new `DatabaseSource` property. * Use `undefined` instead of `null` Co-authored-by: Kim Santiago --- .../src/common/constants.ts | 12 +- .../sql-database-projects/src/common/utils.ts | 30 +++ .../src/controllers/projectController.ts | 3 + .../src/models/project.ts | 202 +++++++++++++++++- .../sql-database-projects/src/sqldbproj.d.ts | 16 ++ .../src/test/project.test.ts | 114 +++++++++- 6 files changed, 374 insertions(+), 3 deletions(-) diff --git a/extensions/sql-database-projects/src/common/constants.ts b/extensions/sql-database-projects/src/common/constants.ts index 79ac226339..26821d5c24 100644 --- a/extensions/sql-database-projects/src/common/constants.ts +++ b/extensions/sql-database-projects/src/common/constants.ts @@ -332,7 +332,8 @@ export function fileAlreadyExists(filename: string) { return localize('fileAlrea export function folderAlreadyExists(filename: string) { return localize('folderAlreadyExists', "A folder with the name '{0}' already exists on disk at this location. Please choose another name.", filename); } export function folderAlreadyExistsChooseNewLocation(filename: string) { return localize('folderAlreadyExistsChooseNewLocation', "A folder with the name '{0}' already exists on disk at this location. Please choose another location.", filename); } 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 invalidProjectPropertyValueInSqlProj(propertyName: string) { return localize('invalidPropertyValueInSqlProj', "Invalid value specified for the property '{0}' in .sqlproj file", propertyName); } +export function invalidProjectPropertyValueProvided(propertyName: string) { return localize('invalidPropertyValueProvided', "Project property value '{0} is invalid", 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 errorFindingBuildFilesLocation(err: any) { return localize('errorFindingBuildFilesLocation', "Error finding build files location: {0}", utils.getErrorMessage(err)); } @@ -423,6 +424,7 @@ export const PropertyGroup = 'PropertyGroup'; export const Type = 'Type'; export const ExternalStreamingJob: string = 'ExternalStreamingJob'; export const Sdk: string = 'Sdk'; +export const DatabaseSource = 'DatabaseSource'; export const BuildElements = localize('buildElements', "Build Elements"); export const FolderElements = localize('folderElements', "Folder Elements"); @@ -440,6 +442,14 @@ 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'; +/** + * Well-known database source values that are allowed to be sent in telemetry. + * + * 'dsct-oracle-to-ms-sql' is the name of an extension which allows users to migrate from Oracle to Microsoft SQL platform. + * When looking at telemetry, we would like to know if a built or deployed database originated from the DSCT extension. + */ +export const WellKnownDatabaseSources = ['dsct-oracle-to-ms-sql']; + // 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/common/utils.ts b/extensions/sql-database-projects/src/common/utils.ts index ebcdae481e..cf694a029f 100644 --- a/extensions/sql-database-projects/src/common/utils.ts +++ b/extensions/sql-database-projects/src/common/utils.ts @@ -599,3 +599,33 @@ export function getFoldersAlongPath(startFolder: string, endFolder: string): str return folders; } + +/** + * Determines whether provided value is a well-known database source and therefore is allowed to be sent in telemetry. + * + * @param value Value to check if it is a well-known database source + * @returns Normalized database source value if it is well-known, otherwise returns undefined + */ +export function getWellKnownDatabaseSource(value: string): string | undefined { + const upperCaseValue = value.toUpperCase(); + return constants.WellKnownDatabaseSources + .find(wellKnownSource => wellKnownSource.toUpperCase() === upperCaseValue); +} + +/** + * Filters an array of specified database project sources to only those that are well-known. + * + * @param databaseSourceValues Array of database source values to filter + * @returns Array of well-known database sources + */ +export function getWellKnownDatabaseSources(databaseSourceValues: string[]): string[] { + const databaseSourceSet = new Set(); + for (let databaseSourceValue of databaseSourceValues) { + const wellKnownDatabaseSourceValue = getWellKnownDatabaseSource(databaseSourceValue); + if (wellKnownDatabaseSourceValue) { + databaseSourceSet.add(wellKnownDatabaseSourceValue); + } + } + + return Array.from(databaseSourceSet); +} diff --git a/extensions/sql-database-projects/src/controllers/projectController.ts b/extensions/sql-database-projects/src/controllers/projectController.ts index 7e0658890d..7bdaa92243 100644 --- a/extensions/sql-database-projects/src/controllers/projectController.ts +++ b/extensions/sql-database-projects/src/controllers/projectController.ts @@ -260,6 +260,7 @@ export class ProjectsController { TelemetryReporter.createActionEvent(TelemetryViews.ProjectController, TelemetryActions.build) .withAdditionalMeasurements({ duration: timeToBuild }) + .withAdditionalProperties({ databaseSource: utils.getWellKnownDatabaseSources(project.getDatabaseSourceValues()).join(';') }) .send(); return project.dacpacOutputPath; @@ -272,6 +273,7 @@ export class ProjectsController { TelemetryReporter.createErrorEvent(TelemetryViews.ProjectController, TelemetryActions.build) .withAdditionalMeasurements({ duration: timeToFailureBuild }) + .withAdditionalProperties({ databaseSource: utils.getWellKnownDatabaseSources(project.getDatabaseSourceValues()).join(';') }) .send(); const message = utils.getErrorMessage(err); @@ -401,6 +403,7 @@ export class ProjectsController { const buildEndTime = new Date().getTime(); telemetryMeasures.buildDuration = buildEndTime - buildStartTime; telemetryProps.buildSucceeded = (dacpacPath !== '').toString(); + telemetryProps.databaseSource = utils.getWellKnownDatabaseSources(project.getDatabaseSourceValues()).join(';'); if (!dacpacPath) { TelemetryReporter.createErrorEvent(TelemetryViews.ProjectController, TelemetryActions.publishProject) diff --git a/extensions/sql-database-projects/src/models/project.ts b/extensions/sql-database-projects/src/models/project.ts index 704e88ffe6..4b09e128f9 100644 --- a/extensions/sql-database-projects/src/models/project.ts +++ b/extensions/sql-database-projects/src/models/project.ts @@ -1016,6 +1016,35 @@ export class Project implements ISqlProject { await this.addToProjFile(sqlCmdVariableEntry); } + /** + * Appends given database source to the DatabaseSource property element. + * If property element does not exist, then new one will be created. + * + * @param databaseSource Source of the database to add + */ + public addDatabaseSource(databaseSource: string): Promise { + return this.addValueToCollectionProjectProperty(constants.DatabaseSource, databaseSource); + } + + /** + * Removes database source from the DatabaseSource property element. + * If no sources remain, then property element will be removed from the project file. + * + * @param databaseSource Source of the database to remove + */ + public removeDatabaseSource(databaseSource: string): Promise { + return this.removeValueFromCollectionProjectProperty(constants.DatabaseSource, databaseSource); + } + + /** + * Gets an array of all database sources specified in the project. + * + * @returns Array of all database sources + */ + public getDatabaseSourceValues(): string[] { + return this.getCollectionProjectPropertyValue(constants.DatabaseSource); + } + public createFileProjectEntry(relativePath: string, entryType: EntryType, sqlObjectType?: string): FileProjectEntry { let platformSafeRelativePath = utils.getPlatformSafeFileEntryPath(relativePath); return new FileProjectEntry( @@ -1625,6 +1654,73 @@ export class Project implements ISqlProject { } } + /** + * Adds a value to the project property, where multiple values are separated by semicolon. + * If property does not exist, the new one will be added. Otherwise a value will be appended + * to the existing property. + * + * @param propertyName Name of the project property + * @param valueToAdd Value to add to the project property. Values containing semicolon are not supported + * @param caseSensitive Flag that indicates whether to use case-sensitive comparison when determining, if value is already present + */ + private async addValueToCollectionProjectProperty(propertyName: string, valueToAdd: string, caseSensitive: boolean = false): Promise { + if (valueToAdd.includes(';')) { + throw new Error(constants.invalidProjectPropertyValueProvided(valueToAdd)); + } + + let collectionValues = this.getCollectionProjectPropertyValue(propertyName); + + // Respect case-sensitivity flag + const normalizedValueToAdd = caseSensitive ? valueToAdd : valueToAdd.toUpperCase(); + + // Only add value if it is not present yet + if (collectionValues.findIndex(value => (caseSensitive ? value : value.toUpperCase()) === normalizedValueToAdd) < 0) { + collectionValues.push(valueToAdd); + await this.setProjectPropertyValue(propertyName, collectionValues.join(';')); + } + } + + /** + * Removes a value from the project property, where multiple values are separated by semicolon. + * If property becomes empty after the removal of the value, then it will be completely removed + * from the project file. + * If value appears in the collection multiple times, only the first occurance will be removed. + * + * @param propertyName Name of the project property + * @param valueToRemove Value to remove from the project property. Values containing semicolon are not supported + * @param caseSensitive Flag that indicates whether to use case-sensitive comparison when removing the value + */ + protected async removeValueFromCollectionProjectProperty(propertyName: string, valueToRemove: string, caseSensitive: boolean = false): Promise { + if (this.projFileXmlDoc === undefined) { + return; + } + + if (valueToRemove.includes(';')) { + throw new Error(constants.invalidProjectPropertyValueProvided(valueToRemove)); + } + + let collectionValues = this.getCollectionProjectPropertyValue(propertyName); + + // Respect case-sensitivity flag + const normalizedValueToRemove = caseSensitive ? valueToRemove : valueToRemove.toUpperCase(); + + const indexToRemove = + collectionValues.findIndex(value => (caseSensitive ? value : value.toUpperCase()) === normalizedValueToRemove); + + if (indexToRemove >= 0) { + collectionValues.splice(indexToRemove, 1); + + if (collectionValues.length === 0) { + // No elements left in the collection - remove the property entirely + this.removeProjectPropertyTag(propertyName); + await this.serializeToProjFile(this.projFileXmlDoc); + } else { + // Update property value with modified collection + await this.setProjectPropertyValue(propertyName, collectionValues.join(';')); + } + } + } + /** * Evaluates the value of the property item in the loaded project. * @@ -1665,12 +1761,116 @@ export class Project implements ISqlProject { const firstPropertyElement = propertyElements[0]; if (firstPropertyElement.childNodes.length !== 1) { // Property items are expected to have simple string content - throw new Error(constants.invalidProjectPropertyValue(propertyName)); + throw new Error(constants.invalidProjectPropertyValueInSqlProj(propertyName)); } return firstPropertyElement.childNodes[0].nodeValue!; } + /** + * Retrieves all semicolon-separated values specified in the project property. + * + * @param propertyName Name of the project property + * @returns Array of semicolon-separated values specified in the property + */ + private getCollectionProjectPropertyValue(propertyName: string): string[] { + const propertyValue = this.evaluateProjectPropertyValue(propertyName); + if (propertyValue === undefined) { + return []; + } + + return propertyValue.split(';') + .filter(value => value.length > 0); + } + + /** + * Sets the value of the project property. + * + * @param propertyName Name of the project property + * @param propertyValue New value of the project property + */ + private async setProjectPropertyValue(propertyName: string, propertyValue: string): Promise { + if (this.projFileXmlDoc === undefined) { + return; + } + + let propertyElement: Element | undefined; + + // Try to find an existing property element with the requested name. + // There could be multiple elements in different property groups or even within the + // same property group (different `Condition` attribute, for example). As of now, + // we always choose the first one and update it. + const propertyGroups = this.projFileXmlDoc.getElementsByTagName(constants.PropertyGroup); + for (let propertyGroupIndex = 0; propertyGroupIndex < propertyGroups.length; ++propertyGroupIndex) { + const propertyElements = propertyGroups[propertyGroupIndex].getElementsByTagName(propertyName); + + if (propertyElements.length > 0) { + propertyElement = propertyElements[0]; + break; + } + } + + if (propertyElement === undefined) { + // If existing property element was not found, then we add a new one + propertyElement = this.addProjectPropertyTag(propertyName); + } + + // Ensure property element was found or successfully added + if (propertyElement) { + if (propertyElement.childNodes.length > 0) { + propertyElement.replaceChild(this.projFileXmlDoc.createTextNode(propertyValue), propertyElement.childNodes[0]); + } else { + propertyElement.appendChild(this.projFileXmlDoc.createTextNode(propertyValue)); + } + + await this.serializeToProjFile(this.projFileXmlDoc); + } + } + + /** + * Adds an empty project property tag. + * + * @param propertyTag Tag to add + * @returns Added HTMLElement tag + */ + private addProjectPropertyTag(propertyTag: string): HTMLElement | undefined { + if (this.projFileXmlDoc === undefined) { + return; + } + + const propertyGroups = this.projFileXmlDoc.getElementsByTagName(constants.PropertyGroup); + let propertyGroup = propertyGroups.length > 0 ? propertyGroups[0] : undefined; + if (propertyGroup === undefined) { + propertyGroup = this.projFileXmlDoc.createElement(constants.PropertyGroup); + this.projFileXmlDoc.documentElement?.appendChild(propertyGroup); + } + + const propertyElement = this.projFileXmlDoc.createElement(propertyTag); + propertyGroup.appendChild(propertyElement); + return propertyElement; + } + + /** + * Removes first occurrence of the project property. + * + * @param propertyTag Tag to remove + */ + private removeProjectPropertyTag(propertyTag: string) { + if (this.projFileXmlDoc === undefined) { + return; + } + + const propertyGroups = this.projFileXmlDoc.getElementsByTagName(constants.PropertyGroup); + + for (let propertyGroupIndex in propertyGroups) { + let propertiesWithTagName = propertyGroups[propertyGroupIndex].getElementsByTagName(propertyTag); + if (propertiesWithTagName.length > 0) { + propertiesWithTagName[0].parentNode?.removeChild(propertiesWithTagName[0]); + return; + } + } + } + /** * Adds all folders in the path to the project and saves the project file, if provided path is under the project folder. * If path is outside the project folder, then no action is taken. diff --git a/extensions/sql-database-projects/src/sqldbproj.d.ts b/extensions/sql-database-projects/src/sqldbproj.d.ts index 40e0b91a9a..18002ac1ca 100644 --- a/extensions/sql-database-projects/src/sqldbproj.d.ts +++ b/extensions/sql-database-projects/src/sqldbproj.d.ts @@ -106,6 +106,22 @@ declare module 'sqldbproj' { */ addSqlCmdVariable(name: string, defaultValue: string): Promise; + /** + * Appends given database source to the DatabaseSource property element. + * If property element does not exist, then new one will be created. + * + * @param databaseSource Source of the database to add + */ + addDatabaseSource(databaseSource: string): Promise; + + /** + * Removes database source from the DatabaseSource property element. + * If no sources remain, then property element will be removed from the project file. + * + * @param databaseSource Source of the database to remove + */ + removeDatabaseSource(databaseSource: string): Promise; + /** * Excludes entry from project by removing it from the project file * @param entry diff --git a/extensions/sql-database-projects/src/test/project.test.ts b/extensions/sql-database-projects/src/test/project.test.ts index bc49c13c1c..1b1925afb5 100644 --- a/extensions/sql-database-projects/src/test/project.test.ts +++ b/extensions/sql-database-projects/src/test/project.test.ts @@ -14,7 +14,7 @@ import * as constants from '../common/constants'; import { promises as fs } from 'fs'; import { Project } from '../models/project'; -import { exists, convertSlashesForSqlProj } from '../common/utils'; +import { exists, convertSlashesForSqlProj, getWellKnownDatabaseSources } from '../common/utils'; import { Uri, window } from 'vscode'; import { IDacpacReferenceSettings, IProjectReferenceSettings, ISystemDatabaseReferenceSettings } from '../models/IDatabaseReferenceSettings'; import { SqlTargetPlatform } from 'sqldbproj'; @@ -1558,6 +1558,118 @@ describe('Project: properties', function (): void { should(() => project.getDatabaseDefaultCollation()) .throw('Invalid value specified for the property \'DefaultCollation\' in .sqlproj file'); }); + + it('Should add database source to project property', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectInvalidCollationBaseline); + const project = await Project.openProject(projFilePath); + + // Should add a single database source + await project.addDatabaseSource('test1'); + let databaseSourceItems: string[] = project.getDatabaseSourceValues(); + should(databaseSourceItems.length).equal(1); + should(databaseSourceItems[0]).equal('test1'); + + // Should add multiple database sources + await project.addDatabaseSource('test2'); + await project.addDatabaseSource('test3'); + databaseSourceItems = project.getDatabaseSourceValues(); + should(databaseSourceItems.length).equal(3); + should(databaseSourceItems[0]).equal('test1'); + should(databaseSourceItems[1]).equal('test2'); + should(databaseSourceItems[2]).equal('test3'); + + // Should not add duplicate database sources + await project.addDatabaseSource('test1'); + await project.addDatabaseSource('test2'); + await project.addDatabaseSource('test3'); + should(databaseSourceItems.length).equal(3); + should(databaseSourceItems[0]).equal('test1'); + should(databaseSourceItems[1]).equal('test2'); + should(databaseSourceItems[2]).equal('test3'); + }); + + it('Should remove database source from project property', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectInvalidCollationBaseline); + const project = await Project.openProject(projFilePath); + + await project.addDatabaseSource('test1'); + await project.addDatabaseSource('test2'); + await project.addDatabaseSource('test3'); + await project.addDatabaseSource('test4'); + + let databaseSourceItems: string[] = project.getDatabaseSourceValues(); + should(databaseSourceItems.length).equal(4); + + // Should remove database sources + await project.removeDatabaseSource('test2'); + await project.removeDatabaseSource('test1'); + await project.removeDatabaseSource('test4'); + + databaseSourceItems = project.getDatabaseSourceValues(); + should(databaseSourceItems.length).equal(1); + should(databaseSourceItems[0]).equal('test3'); + + // Should remove database source tag when last database source is removed + await project.removeDatabaseSource('test3'); + databaseSourceItems = project.getDatabaseSourceValues(); + + should(databaseSourceItems.length).equal(0); + }); + + it('Should add and remove values from project properties according to specified case sensitivity', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectInvalidCollationBaseline); + const project = await Project.openProject(projFilePath); + const propertyName = 'TestProperty'; + + // Should add value to collection + await project['addValueToCollectionProjectProperty'](propertyName, 'test'); + should(project['evaluateProjectPropertyValue'](propertyName)).equal('test'); + + // Should not allow duplicates of different cases when comparing case insitively + await project['addValueToCollectionProjectProperty'](propertyName, 'TEST'); + should(project['evaluateProjectPropertyValue'](propertyName)).equal('test'); + + // Should allow duplicates of differnt cases when comparing case sensitively + await project['addValueToCollectionProjectProperty'](propertyName, 'TEST', true); + should(project['evaluateProjectPropertyValue'](propertyName)).equal('test;TEST'); + + // Should remove values case insesitively + await project['removeValueFromCollectionProjectProperty'](propertyName, 'Test'); + should(project['evaluateProjectPropertyValue'](propertyName)).equal('TEST'); + + // Should remove values case sensitively + await project['removeValueFromCollectionProjectProperty'](propertyName, 'Test', true); + should(project['evaluateProjectPropertyValue'](propertyName)).equal('TEST'); + await project['removeValueFromCollectionProjectProperty'](propertyName, 'TEST', true); + should(project['evaluateProjectPropertyValue'](propertyName)).equal(undefined); + }); + + it('Should only return well known database strings when getWellKnownDatabaseSourceString function is called', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectInvalidCollationBaseline); + const project = await Project.openProject(projFilePath); + + await project.addDatabaseSource('test1'); + await project.addDatabaseSource('test2'); + await project.addDatabaseSource('test3'); + await project.addDatabaseSource(constants.WellKnownDatabaseSources[0]); + + should(getWellKnownDatabaseSources(project.getDatabaseSourceValues()).length).equal(1); + should(getWellKnownDatabaseSources(project.getDatabaseSourceValues())[0]).equal(constants.WellKnownDatabaseSources[0]); + }); + + it('Should throw error when adding or removing database source that contains semicolon', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.sqlProjectInvalidCollationBaseline); + const project = await Project.openProject(projFilePath); + const semicolon = ';'; + + await testUtils.shouldThrowSpecificError( + async () => await project.addDatabaseSource(semicolon), + constants.invalidProjectPropertyValueProvided(semicolon)); + + await testUtils.shouldThrowSpecificError( + async () => await project.removeDatabaseSource(semicolon), + constants.invalidProjectPropertyValueProvided(semicolon)); + }); }); describe('Project: round trip updates', function (): void {