diff --git a/extensions/sql-database-projects/src/common/constants.ts b/extensions/sql-database-projects/src/common/constants.ts index 61d1abdae5..c2753486d4 100644 --- a/extensions/sql-database-projects/src/common/constants.ts +++ b/extensions/sql-database-projects/src/common/constants.ts @@ -94,6 +94,7 @@ export const invalidDataSchemaProvider = localize('invalidDataSchemaProvider', " export const invalidDatabaseReference = localize('invalidDatabaseReference', "Invalid database reference in .sqlproj file"); export const databaseSelectionRequired = localize('databaseSelectionRequired', "Database selection is required to import a project"); export const unableToCreateDeploymentConnection = localize('unableToCreateDeploymentConnection', "Unable to construct connection"); +export const databaseReferenceAlreadyExists = localize('databaseReferenceAlreadyExists', "A reference to this database already exists in this project"); export function projectAlreadyOpened(path: string) { return localize('projectAlreadyOpened', "Project '{0}' is already opened.", path); } export function projectAlreadyExists(name: string, path: string) { return localize('projectAlreadyExists', "A project named {0} already exists in {1}.", name, path); } export function noFileExist(fileName: string) { return localize('noFileExist', "File {0} doesn't exist", fileName); } diff --git a/extensions/sql-database-projects/src/models/project.ts b/extensions/sql-database-projects/src/models/project.ts index 709604157a..c77d688922 100644 --- a/extensions/sql-database-projects/src/models/project.ts +++ b/extensions/sql-database-projects/src/models/project.ts @@ -23,7 +23,7 @@ export class Project { public files: ProjectEntry[] = []; public dataSources: DataSource[] = []; public importedTargets: string[] = []; - public databaseReferences: string[] = []; + public databaseReferences: DatabaseReferenceProjectEntry[] = []; public sqlCmdVariables: Record = {}; public get projectFolderPath() { @@ -67,15 +67,17 @@ export class Project { this.sqlCmdVariables = utils.readSqlCmdVariables(this.projFileXmlDoc); // find all database references to include - for (let r = 0; r < this.projFileXmlDoc.documentElement.getElementsByTagName(constants.ArtifactReference).length; r++) { - if (this.projFileXmlDoc.documentElement.getElementsByTagName(constants.ArtifactReference)[r].getAttribute(constants.Condition) !== constants.NotNetCoreCondition) { - const filepath = this.projFileXmlDoc.documentElement.getElementsByTagName(constants.ArtifactReference)[r].getAttribute(constants.Include); + const references = this.projFileXmlDoc.documentElement.getElementsByTagName(constants.ArtifactReference); + for (let r = 0; r < references.length; r++) { + if (references[r].getAttribute(constants.Condition) !== constants.NotNetCoreCondition) { + const filepath = references[r].getAttribute(constants.Include); if (!filepath) { throw new Error(constants.invalidDatabaseReference); } - const platformSafeFilePath = utils.getPlatformSafeFileEntryPath(filepath); - this.databaseReferences.push(path.parse(platformSafeFilePath).name); + let nameNodes = references[r].getElementsByTagName(constants.DatabaseVariableLiteralValue); + let name = nameNodes.length === 1 ? nameNodes[0].childNodes[0].nodeValue : undefined; + this.databaseReferences.push(new DatabaseReferenceProjectEntry(Uri.parse(filepath), name ? DatabaseReferenceLocation.differentDatabaseSameServer : DatabaseReferenceLocation.sameDatabase, name)); } } } @@ -296,6 +298,11 @@ export class Project { } private addDatabaseReferenceToProjFile(entry: DatabaseReferenceProjectEntry): void { + // check if reference to this database already exists + if (this.databaseReferenceExists(entry)) { + throw new Error(constants.databaseReferenceAlreadyExists); + } + let referenceNode = this.projFileXmlDoc.createElement(constants.ArtifactReference); const isSystemDatabaseProjectEntry = (entry).ssdtUri; @@ -307,7 +314,7 @@ export class Project { referenceNode.setAttribute(constants.Include, isSystemDatabaseProjectEntry ? entry.fsUri.fsPath.substring(1) : entry.fsUri.fsPath); // need to remove the leading slash for system database path for build to work on Windows this.addDatabaseReferenceChildren(referenceNode, entry.name); this.findOrCreateItemGroup(constants.ArtifactReference).appendChild(referenceNode); - this.databaseReferences.push(path.parse(entry.fsUri.fsPath.toString()).name); + this.databaseReferences.push(entry); // add a reference to the system dacpac in SSDT if it's a system db if (isSystemDatabaseProjectEntry) { @@ -319,6 +326,11 @@ export class Project { } } + private databaseReferenceExists(entry: DatabaseReferenceProjectEntry): boolean { + const found = this.databaseReferences.find(reference => reference.fsUri.fsPath === entry.fsUri.fsPath) !== undefined; + return found; + } + private addDatabaseReferenceChildren(referenceNode: any, name?: string): void { let suppressMissingDependenciesErrorNode = this.projFileXmlDoc.createElement(constants.SuppressMissingDependenciesErrors); let falseTextNode = this.projFileXmlDoc.createTextNode('False'); @@ -389,7 +401,7 @@ export class Project { } // remove from database references because it'll get added again later - this.databaseReferences.splice(this.databaseReferences.findIndex(n => n === (name === SystemDatabase.master ? constants.master : constants.msdb)), 1); + this.databaseReferences.splice(this.databaseReferences.findIndex(n => n.databaseName === (name === SystemDatabase.master ? constants.master : constants.msdb)), 1); await this.addSystemDatabaseReference(name); } @@ -472,6 +484,10 @@ class DatabaseReferenceProjectEntry extends ProjectEntry { constructor(uri: Uri, public databaseLocation: DatabaseReferenceLocation, public name?: string) { super(uri, '', EntryType.DatabaseReference); } + + public get databaseName(): string { + return path.parse(utils.getPlatformSafeFileEntryPath(this.fsUri.fsPath)).name; + } } class SystemDatabaseReferenceProjectEntry extends DatabaseReferenceProjectEntry { diff --git a/extensions/sql-database-projects/src/models/tree/databaseReferencesTreeItem.ts b/extensions/sql-database-projects/src/models/tree/databaseReferencesTreeItem.ts index 7038c1ecaa..a571274f49 100644 --- a/extensions/sql-database-projects/src/models/tree/databaseReferencesTreeItem.ts +++ b/extensions/sql-database-projects/src/models/tree/databaseReferencesTreeItem.ts @@ -23,7 +23,7 @@ export class DatabaseReferencesTreeItem extends BaseProjectTreeItem { private construct() { for (const reference of (this.parent as ProjectRootTreeItem).project.databaseReferences) { - this.references.push(new MessageTreeItem(reference)); + this.references.push(new MessageTreeItem(reference.databaseName)); } } diff --git a/extensions/sql-database-projects/src/test/project.test.ts b/extensions/sql-database-projects/src/test/project.test.ts index 5d008be85c..0bd7e81829 100644 --- a/extensions/sql-database-projects/src/test/project.test.ts +++ b/extensions/sql-database-projects/src/test/project.test.ts @@ -46,7 +46,7 @@ describe('Project: sqlproj content operations', function (): void { // Database references // should only have one database reference even though there are two master.dacpac references (1 for ADS and 1 for SSDT) should(project.databaseReferences.length).equal(1); - should(project.databaseReferences[0]).containEql(constants.master); + should(project.databaseReferences[0].databaseName).containEql(constants.master); }); it('Should add Folder and Build entries to sqlproj', async function (): Promise { @@ -158,24 +158,47 @@ describe('Project: sqlproj content operations', function (): void { const project = new Project(projFilePath); await project.readProjFile(); - should(project.databaseReferences.length).equal(0); + should(project.databaseReferences.length).equal(0, 'There should be no datbase references to start with'); await project.addSystemDatabaseReference(SystemDatabase.master); - should(project.databaseReferences.length).equal(1); - should(project.databaseReferences[0]).equal(constants.master); + 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'); // make sure reference to SSDT master dacpac was added let projFileText = (await fs.readFile(projFilePath)).toString(); should(projFileText).containEql(project.getSystemDacpacSsdtUri(constants.master).fsPath.substring(1)); await project.addSystemDatabaseReference(SystemDatabase.msdb); - should(project.databaseReferences.length).equal(2); - should(project.databaseReferences[1]).equal(constants.msdb); + should(project.databaseReferences.length).equal(2, 'There should be two database references after adding a reference to msdb'); + should(project.databaseReferences[1].databaseName).equal(constants.msdb, 'The database reference should be msdb'); // make sure reference to SSDT msdb dacpac was added projFileText = (await fs.readFile(projFilePath)).toString(); should(projFileText).containEql(project.getSystemDacpacSsdtUri(constants.msdb).fsPath.substring(1)); await project.addDatabaseReference(Uri.parse('test.dacpac'), DatabaseReferenceLocation.sameDatabase); - should(project.databaseReferences.length).equal(3); - should(project.databaseReferences[2]).equal('test'); + should(project.databaseReferences.length).equal(3, 'There should be three database references after adding a reference to test'); + should(project.databaseReferences[2].databaseName).equal('test', 'The database reference should be test'); + }); + + it('Should not allow adding duplicate database references', async function (): Promise { + projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline); + const project = new Project(projFilePath); + await project.readProjFile(); + + should(project.databaseReferences.length).equal(0, 'There should be no database references to start with'); + await project.addSystemDatabaseReference(SystemDatabase.master); + 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, 'project.databaseReferences[0].databaseName should be master'); + + // try to add reference to master again + await testUtils.shouldThrowSpecificError(async () => await project.addSystemDatabaseReference(SystemDatabase.master), constants.databaseReferenceAlreadyExists); + should(project.databaseReferences.length).equal(1, 'There should only be one database reference after trying to add a reference to master again'); + + await project.addDatabaseReference(Uri.parse('test.dacpac'), DatabaseReferenceLocation.sameDatabase); + should(project.databaseReferences.length).equal(2, 'There should be two database references after adding a reference to test.dacpac'); + should(project.databaseReferences[1].databaseName).equal('test', 'project.databaseReferences[1].databaseName should be test'); + + // try to add reference to test.dacpac again + await testUtils.shouldThrowSpecificError(async () => await project.addDatabaseReference(Uri.parse('test.dacpac'), DatabaseReferenceLocation.sameDatabase), constants.databaseReferenceAlreadyExists); + should(project.databaseReferences.length).equal(2, 'There should be two database references after trying to add a reference to test.dacpac again'); }); });