Don't allow adding duplicate database references to sql project (#11010)

* don't allow adding duplicate database references

* add test

* addressing comments

* remove XML checking from databaseReferenceExists()

* change to get
This commit is contained in:
Kim Santiago
2020-06-23 12:24:49 -07:00
committed by GitHub
parent a303144226
commit 74798f7cda
4 changed files with 57 additions and 17 deletions

View File

@@ -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); }

View File

@@ -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<string, string> = {};
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 = (<SystemDatabaseReferenceProjectEntry>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 {

View File

@@ -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));
}
}

View File

@@ -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<void> {
@@ -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<void> {
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');
});
});