ckaczor/azuredatastudio
1
0
Fork 0
mirror of https://github.com/ckaczor/azuredatastudio.git synced 2026-01-15 17:22:25 -05:00
Code Issues Packages Projects Releases Wiki Activity

Allow to ignore duplicate entires when adding files to SQL project. (#15332)

Browse Source 
This change introduces new boolean flag for `addToProject`, `addScriptItem` and `addFolderItem` APIs that allows to skip files/folders if they are already included in the project file. Original behavior was to throw an error if duplicate entry is encountered, so I kept that as a default (new flag is not set).

I started by fixing the original behavior, as it was not always working as expected. In our extension that relies on `addToProject` API we've hit an issue where duplicate items were added and no errors were thrown. There was a test for this scenario, but unfortunately the test had few bugs in it as well, so I addressed that first. First issue with the test code was missing `await` on the `testUtils.shouldThrowSpecificError` calls, so test was not actually testing anything. After adding missing keywords, I hit an issue where exception was not thrown, but this turned out to be different issue, compared to what we were hitting. In the test code, it was using the very first folder from the generate list of test entires. This folder wass actually the root of the project (where sqlproj file is located), and `addToProject` API had a special case to ignore the root. This means neither first call nor second call was actually adding anything to the project and no errors were produced. I fixed this problem by using next available folder in the generated files list.

After addressing the test code I could not reproduce the issue that we were seeing with duplicate entires being added, everything was working as expected and errors were thrown. I started adding more tests that better resemble our production scenario - add files in subfolders, add files to existing project, rather than a new one. Finally I was able to reproduce the problem in tests when adding a file in a subfolder to an existing project. After investigation this turned out to be an issue with mismatch in how `relativePath` is maintained within the `FileProjectEntry`. When loading an existing project, `relativePath` is populated based on the value of the `Include` attribute of the `Build` item. This attribute is normalized to Windows-style path, using `\`, so for nested file you will have `folder\file.sql`. When adding new item to the project, one could pass either Windows or Unix-style path (`folder/file.sql`), so the path comparison between loaded Windows-style path and newly added Unix-style path was failing, resulting in them being treated as different items. In fact, `addToProject` API that we were using was relying on `Uri` helpers to extract relative path, thus the path was forced to be Unix-style and path was never the same as the loaded one. After this discovery I added a dedicated test to validate the round-trip of the `relativePath` for serialized and desirialized project file.

In order to address this problem, I updated the factory method `createFileProjectEntry` to always run `utils.convertSlashesForSqlProj` on the relative path to ensure we have Windows-style path stored in there. I also optimized the helper code slightly to not do split/join, if there are no split points in the input string, which should eliminate unnecessary array instantiation. It is worth mentioning that I had to normalize the input relative paths in the `addScriptItem` and `addFolderItem` APIs, because there is no guarantee that they will be Windows-style when we try to compare them to `relativePath` of the existing project items.

Finally I was able to add a simply flag and update the condition to return existing record, if duplicates were allowed. I also updated typings file for the extension and added tests to cover this scenario.
This commit is contained in:
Alexander Ivanov
2021-05-10 13:59:24 -07:00
committed by GitHub
parent 7dca953660
commit 676296fbbb
4 changed files with 195 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
extensions/sql-database-projects/src/common/utils.ts
View File

@@ -110,18 +110,24 @@ function getQuotedNonWindowsPath(filePath: string): string {
* Get safe relative path for Windows and non-Windows Platform
* This is needed to read sqlproj entried created on SSDT and opened in MAC
* '/' in tree is recognized all platforms but "\\" only by windows
*
* @param filePath Path to the file or folder.
*/
export function getPlatformSafeFileEntryPath(filePath: string): string {
const parts = filePath.split('\\');
return parts.join('/');
return filePath.includes('\\')
? filePath.split('\\').join('/')
: filePath;
}
/**
* Standardizes slashes to be "\\" for consistency between platforms and compatibility with SSDT
*
* @param filePath Path to the file of folder.
*/
export function convertSlashesForSqlProj(filePath: string): string {
const parts = filePath.split('/');
return parts.join('\\');
return filePath.includes('/')
? filePath.split('/').join('\\')
: filePath;
}
/**

51
extensions/sql-database-projects/src/models/project.ts
View File

@@ -307,22 +307,31 @@ export class Project implements ISqlProject {
/**
* Adds a folder to the project, and saves the project file
* @param relativeFolderPath Relative path of the folder
* @param doNotThrowOnDuplicate
* Flag that indicates whether duplicate entries should be ignored or throw an error. If flag is set to `true` and
* item already exists in the project file, then existing entry will be returned.
*/
public async addFolderItem(relativeFolderPath: string): Promise<FileProjectEntry> {
public async addFolderItem(relativeFolderPath: string, doNotThrowOnDuplicate?: boolean): Promise<FileProjectEntry> {
const absoluteFolderPath = path.join(this.projectFolderPath, relativeFolderPath);
const normalizedRelativeFolderPath = utils.convertSlashesForSqlProj(relativeFolderPath);
// check if folder already has been added to sqlproj
if (this.files.find(f => f.relativePath.toLowerCase() === relativeFolderPath.toLowerCase())) {
throw new Error(constants.folderAlreadyAddedToProject((relativeFolderPath)));
const existingEntry = this.files.find(f => f.relativePath.toUpperCase() === normalizedRelativeFolderPath.toUpperCase());
if (existingEntry) {
if (!doNotThrowOnDuplicate) {
throw new Error(constants.folderAlreadyAddedToProject((relativeFolderPath)));
}
return existingEntry;
}
//If folder doesn't exist, create it
// If folder doesn't exist, create it
let exists = await utils.exists(absoluteFolderPath);
if (!exists) {
await fs.mkdir(absoluteFolderPath, { recursive: true });
}
const folderEntry = this.createFileProjectEntry(relativeFolderPath, EntryType.Folder);
const folderEntry = this.createFileProjectEntry(normalizedRelativeFolderPath, EntryType.Folder);
this._files.push(folderEntry);
await this.addToProjFile(folderEntry);
@@ -333,8 +342,12 @@ export class Project implements ISqlProject {
* Writes a file to disk if contents are provided, adds that file to the project, and writes it to disk
* @param relativeFilePath Relative path of the file
* @param contents Contents to be written to the new file
* @param itemType Type of the project entry to add. This maps to the build action for the item.
* @param doNotThrowOnDuplicate
* Flag that indicates whether duplicate entries should be ignored or throw an error. If flag is set to `true` and
* item already exists in the project file, then existing entry will be returned.
*/
public async addScriptItem(relativeFilePath: string, contents?: string, itemType?: string): Promise<FileProjectEntry> {
public async addScriptItem(relativeFilePath: string, contents?: string, itemType?: string, doNotThrowOnDuplicate?: boolean): Promise<FileProjectEntry> {
const absoluteFilePath = path.join(this.projectFolderPath, relativeFilePath);
// check if file already exists if content was passed to write to a new file
@@ -342,9 +355,16 @@ export class Project implements ISqlProject {
throw new Error(constants.fileAlreadyExists(path.parse(absoluteFilePath).name));
}
const normalizedRelativeFilePath = utils.convertSlashesForSqlProj(relativeFilePath);
// check if file already has been added to sqlproj
if (this.files.find(f => f.relativePath.toLowerCase() === relativeFilePath.toLowerCase())) {
throw new Error(constants.fileAlreadyAddedToProject((relativeFilePath)));
const existingEntry = this.files.find(f => f.relativePath.toUpperCase() === normalizedRelativeFilePath.toUpperCase());
if (existingEntry) {
if (!doNotThrowOnDuplicate) {
throw new Error(constants.fileAlreadyAddedToProject((relativeFilePath)));
}
return existingEntry;
}
// create the file if contents were passed in
@@ -360,7 +380,7 @@ export class Project implements ISqlProject {
}
// update sqlproj XML
const fileEntry = this.createFileProjectEntry(relativeFilePath, EntryType.File);
const fileEntry = this.createFileProjectEntry(normalizedRelativeFilePath, EntryType.File);
let xmlTag;
switch (itemType) {
@@ -553,7 +573,11 @@ export class Project implements ISqlProject {
public createFileProjectEntry(relativePath: string, entryType: EntryType, sqlObjectType?: string): FileProjectEntry {
let platformSafeRelativePath = utils.getPlatformSafeFileEntryPath(relativePath);
return new FileProjectEntry(Uri.file(path.join(this.projectFolderPath, platformSafeRelativePath)), relativePath, entryType, sqlObjectType);
return new FileProjectEntry(
Uri.file(path.join(this.projectFolderPath, platformSafeRelativePath)),
utils.convertSlashesForSqlProj(relativePath),
entryType,
sqlObjectType);
}
private findOrCreateItemGroup(containedTag?: string, prePostScriptExist?: { scriptExist: boolean; }): any {
@@ -981,8 +1005,9 @@ export class Project implements ISqlProject {
/**
* Adds the list of sql files and directories to the project, and saves the project file
* @param list list of files and folder Uris. Files and folders must already exist. No files or folders will be added if any do not exist.
* @param doNotThrowOnDuplicate Flag that indicates whether duplicate entries should be ignored or throw an error.
*/
public async addToProject(list: Uri[]): Promise<void> {
public async addToProject(list: Uri[], doNotThrowOnDuplicate?: boolean): Promise<void> {
// verify all files/folders exist. If not all exist, none will be added
for (let file of list) {
const exists = await utils.exists(file.fsPath);
@@ -999,9 +1024,9 @@ export class Project implements ISqlProject {
const fileStat = await fs.stat(file.fsPath);
if (fileStat.isFile() && file.fsPath.toLowerCase().endsWith(constants.sqlFileExtension)) {
await this.addScriptItem(relativePath);
await this.addScriptItem(relativePath, undefined, undefined, doNotThrowOnDuplicate);
} else if (fileStat.isDirectory()) {
await this.addFolderItem(relativePath);
await this.addFolderItem(relativePath, doNotThrowOnDuplicate);
}
}
}

14
extensions/sql-database-projects/src/sqldbproj.d.ts vendored
View File

@@ -37,21 +37,29 @@ declare module 'sqldbproj' {
/**
* Adds the list of sql files and directories to the project, and saves the project file
* @param list list of files and folder Uris. Files and folders must already exist. No files or folders will be added if any do not exist.
* @param doNotThrowOnDuplicate Flag that indicates whether duplicate entries should be ignored or throw an error.
*/
addToProject(list: vscode.Uri[]): Promise<void>;
addToProject(list: vscode.Uri[], doNotThrowOnDuplicate?: boolean): Promise<void>;
/**
* Adds a folder to the project, and saves the project file
* @param relativeFolderPath Relative path of the folder
* @param doNotThrowOnDuplicate
* Flag that indicates whether duplicate entries should be ignored or throw an error. If flag is set to `true` and
* item already exists in the project file, then existing entry will be returned.
*/
addFolderItem(relativeFolderPath: string): Promise<IFileProjectEntry>;
addFolderItem(relativeFolderPath: string, doNotThrowOnDuplicate?: boolean): Promise<IFileProjectEntry>;
/**
* Writes a file to disk if contents are provided, adds that file to the project, and writes it to disk
* @param relativeFilePath Relative path of the file
* @param contents Contents to be written to the new file
* @param itemType Type of the project entry to add. This maps to the build action for the item.
* @param doNotThrowOnDuplicate
* Flag that indicates whether duplicate entries should be ignored or throw an error. If flag is set to `true` and
* item already exists in the project file, then existing entry will be returned.
*/
addScriptItem(relativeFilePath: string, contents?: string, itemType?: string): Promise<IFileProjectEntry>;
addScriptItem(relativeFilePath: string, contents?: string, itemType?: string, doNotThrowOnDuplicate?: boolean): Promise<IFileProjectEntry>;
/**
* Adds a SQLCMD variable to the project

146
extensions/sql-database-projects/src/test/project.test.ts
View File

@@ -570,25 +570,151 @@ describe('Project: sqlproj content operations', function (): void {
});
it('Should not allow adding duplicate file/folder entries in sqlproj', async function (): Promise<void> {
it('Should not allow adding duplicate file/folder entries in new sqlproj by default', async function (): Promise<void> {
projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline);
const project: Project = await Project.openProject(projFilePath);
const fileList = await testUtils.createListOfFiles(path.dirname(projFilePath));
// verify first entry in list is a folder
const existingFolderUri = fileList[0];
// 1. Add a folder to the project
const existingFolderUri = fileList[2];
const folderStats = await fs.stat(existingFolderUri.fsPath);
should(folderStats.isDirectory()).equal(true, 'First entry in fileList should be a folder');
should(folderStats.isDirectory()).equal(true, 'Third entry in fileList should be a subfolder');
await project.addToProject([existingFolderUri]);
const folderRelativePath = trimChars(trimUri(Uri.file(projFilePath), existingFolderUri), '');
testUtils.shouldThrowSpecificError(async () => await project.addToProject([existingFolderUri]), constants.folderAlreadyAddedToProject(folderRelativePath));
// verify duplicate file can't be added
const existingFileUri = fileList[1];
const fileStats = await fs.stat(existingFileUri.fsPath);
// Try adding the folder to the project again
const folderRelativePath = trimChars(trimUri(Uri.file(projFilePath), existingFolderUri), '');
await testUtils.shouldThrowSpecificError(async () => await project.addToProject([existingFolderUri]), constants.folderAlreadyAddedToProject(folderRelativePath));
// 2. Add a file to the project
let existingFileUri = fileList[1];
let fileStats = await fs.stat(existingFileUri.fsPath);
should(fileStats.isFile()).equal(true, 'Second entry in fileList should be a file');
await project.addToProject([existingFileUri]);
// Try adding the file to the project again
let fileRelativePath = trimChars(trimUri(Uri.file(projFilePath), existingFileUri), '/');
await testUtils.shouldThrowSpecificError(async () => await project.addToProject([existingFileUri]), constants.fileAlreadyAddedToProject(fileRelativePath));
// 3. Add a file from subfolder to the project
existingFileUri = fileList[3];
fileStats = await fs.stat(existingFileUri.fsPath);
should(fileStats.isFile()).equal(true, 'Fourth entry in fileList should be a file');
await project.addToProject([existingFileUri]);
// Try adding the file from subfolder to the project again
fileRelativePath = trimChars(trimUri(Uri.file(projFilePath), existingFileUri), '/');
await testUtils.shouldThrowSpecificError(async () => await project.addToProject([existingFileUri]), constants.fileAlreadyAddedToProject(fileRelativePath));
});
it('Should ignore duplicate file/folder entries in new sqlproj if requested', async function (): Promise<void> {
projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline);
const project: Project = await Project.openProject(projFilePath);
const fileList = await testUtils.createListOfFiles(path.dirname(projFilePath));
// 1. Add a folder to the project
const existingFolderUri = fileList[2];
const folderStats = await fs.stat(existingFolderUri.fsPath);
should(folderStats.isDirectory()).equal(true, 'Third entry in fileList should be a subfolder');
const folderEntry = await project.addToProject([existingFolderUri]);
should(project.files.length).equal(1, 'New folder entry should be added to the project');
// Add the folder to the project again
should(await project.addToProject([existingFolderUri], true))
.equal(folderEntry, 'Original folder entry should be returned when adding same folder for a second time');
should(project.files.length).equal(1, 'No new entries should be added to the project when adding same folder for a second time');
// 2. Add a file to the project
let existingFileUri = fileList[1];
let fileStats = await fs.stat(existingFileUri.fsPath);
should(fileStats.isFile()).equal(true, 'Second entry in fileList should be a file');
let fileEntry = await project.addToProject([existingFileUri]);
should(project.files.length).equal(2, 'New file entry should be added to the project');
// Add the file to the project again
should(await project.addToProject([existingFileUri], true))
.equal(fileEntry, 'Original file entry should be returned when adding same file for a second time');
should(project.files.length).equal(2, 'No new entries should be added to the project when adding same file for a second time');
// 3. Add a file from subfolder to the project
existingFileUri = fileList[3];
fileStats = await fs.stat(existingFileUri.fsPath);
should(fileStats.isFile()).equal(true, 'Fourth entry in fileList should be a file');
fileEntry = await project.addToProject([existingFileUri]);
should(project.files.length).equal(3, 'New file entry should be added to the project');
// Add the file from subfolder to the project again
should(await project.addToProject([existingFileUri], true))
.equal(fileEntry, 'Original file entry should be returned when adding same file for a second time');
should(project.files.length).equal(3, 'No new entries should be added to the project when adding same file for a second time');
});
it('Should not allow adding duplicate file entries in existing sqlproj by default', async function (): Promise<void> {
// Create new sqlproj
projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline);
const fileList = await testUtils.createListOfFiles(path.dirname(projFilePath));
let project: Project = await Project.openProject(projFilePath);
// Add a file to the project
let existingFileUri = fileList[3];
let fileStats = await fs.stat(existingFileUri.fsPath);
should(fileStats.isFile()).equal(true, 'Fourth entry in fileList should be a file');
await project.addToProject([existingFileUri]);
// Reopen existing project
project = await Project.openProject(projFilePath);
// Try adding the same file to the project again
const fileRelativePath = trimChars(trimUri(Uri.file(projFilePath), existingFileUri), '/');
testUtils.shouldThrowSpecificError(async () => await project.addToProject([existingFileUri]), constants.fileAlreadyAddedToProject(fileRelativePath));
await testUtils.shouldThrowSpecificError(async () => await project.addToProject([existingFileUri]), constants.fileAlreadyAddedToProject(fileRelativePath));
});
it('Should ignore duplicate file entries in existing sqlproj if requested', async function (): Promise<void> {
// Create new sqlproj
projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline);
const fileList = await testUtils.createListOfFiles(path.dirname(projFilePath));
let project: Project = await Project.openProject(projFilePath);
// Add a file to the project
let existingFileUri = fileList[3];
let fileStats = await fs.stat(existingFileUri.fsPath);
should(fileStats.isFile()).equal(true, 'Fourth entry in fileList should be a file');
await project.addToProject([existingFileUri]);
// Reopen existing project
project = await Project.openProject(projFilePath);
// Try adding the same file to the project again
await project.addToProject([existingFileUri], true);
});
it('Project entry relative path should not change after round-trip', async function (): Promise<void> {
// Create new sqlproj
projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline);
const fileList = await testUtils.createListOfFiles(path.dirname(projFilePath));
let project: Project = await Project.openProject(projFilePath);
// Add a file to the project
let existingFileUri = fileList[3];
let fileStats = await fs.stat(existingFileUri.fsPath);
should(fileStats.isFile()).equal(true, 'Fourth entry in fileList should be a file');
await project.addToProject([existingFileUri]);
// Store the original `relativePath` of the project entry
should(project.files.length).equal(1, 'An entry should be created in the project');
const originalRelativePath = project.files[0].relativePath;
// Reopen existing project
project = await Project.openProject(projFilePath);
// Try adding the same file to the project again
should(project.files.length).equal(1, 'Single entry is expected in the loaded project');
should(project.files[0].relativePath).equal(originalRelativePath, 'Relative path should match after a round-trip');
});
});