diff --git a/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts b/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts index 05bf2a275..646f78ab3 100644 --- a/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts +++ b/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts @@ -20,10 +20,10 @@ describe(__filename, async function () { await tmpdir.cleanup(); }); - function getWorkspaceFolderManagerMock() { + function getWorkspaceFolderManagerMock(projectDir?: string) { const mockWorkspaceFolderManager = mock(); const mockWorkspaceFolder = mock(); - const uri = Uri.file(tmpdir.path); + const uri = Uri.file(projectDir ?? tmpdir.path); when(mockWorkspaceFolder.uri).thenReturn(uri); when(mockWorkspaceFolderManager.activeWorkspaceFolder).thenReturn( instance(mockWorkspaceFolder) @@ -73,6 +73,115 @@ describe(__filename, async function () { expect(await bundleFileSet.getRootFile()).to.be.undefined; }); + describe("parent-directory includes", async () => { + it("getIncludedFiles should find files referenced via .. paths", async () => { + // Structure: tmpdir/shared/config.yml (included), tmpdir/project/sub/ (project root) + const sharedDir = path.join(tmpdir.path, "shared"); + const projectDir = path.join(tmpdir.path, "project", "sub"); + await fs.mkdir(sharedDir, {recursive: true}); + await fs.mkdir(projectDir, {recursive: true}); + + const sharedFile = path.join(sharedDir, "config.yml"); + const sharedFile2 = path.join(sharedDir, "config2.yml"); + await fs.writeFile(sharedFile, ""); + await fs.writeFile(sharedFile2, ""); + + const rootBundleData: BundleSchema = { + include: [ + "../../shared/config.yml", + "../../shared/config2.yml", + ], + }; + await fs.writeFile( + path.join(projectDir, "databricks.yml"), + yaml.stringify(rootBundleData) + ); + + const bundleFileSet = new BundleFileSet( + getWorkspaceFolderManagerMock(projectDir) + ); + + const files = await bundleFileSet.getIncludedFiles(); + expect(files).to.not.be.undefined; + expect(files!.map((f) => f.fsPath).sort()).to.deep.equal( + [ + Uri.file(sharedFile).fsPath, + Uri.file(sharedFile2).fsPath, + ].sort() + ); + }); + + it("isIncludedBundleFile should return true for files referenced via .. paths", async () => { + const sharedDir = path.join(tmpdir.path, "shared"); + const projectDir = path.join(tmpdir.path, "project", "sub"); + await fs.mkdir(sharedDir, {recursive: true}); + await fs.mkdir(projectDir, {recursive: true}); + + const sharedFile = path.join(sharedDir, "config.yml"); + await fs.writeFile(sharedFile, ""); + + const rootBundleData: BundleSchema = { + include: ["../../shared/config.yml", "local.yml"], + }; + await fs.writeFile( + path.join(projectDir, "databricks.yml"), + yaml.stringify(rootBundleData) + ); + + const bundleFileSet = new BundleFileSet( + getWorkspaceFolderManagerMock(projectDir) + ); + + expect( + await bundleFileSet.isIncludedBundleFile(Uri.file(sharedFile)) + ).to.be.true; + + expect( + await bundleFileSet.isIncludedBundleFile( + Uri.file(path.join(projectDir, "other.yml")) + ) + ).to.be.false; + }); + + it("getExternalIncludeWatchTargets should only return bases outside the project root", async () => { + const sharedDir = path.join(tmpdir.path, "shared"); + const projectDir = path.join(tmpdir.path, "project", "sub"); + await fs.mkdir(sharedDir, {recursive: true}); + await fs.mkdir(projectDir, {recursive: true}); + + const rootBundleData: BundleSchema = { + include: [ + "../../shared/config.yml", + "../../shared/*.yml", + "local.yml", + "includes/**/*.yml", + ], + }; + await fs.writeFile( + path.join(projectDir, "databricks.yml"), + yaml.stringify(rootBundleData) + ); + + const bundleFileSet = new BundleFileSet( + getWorkspaceFolderManagerMock(projectDir) + ); + + const targets = + await bundleFileSet.getExternalIncludeWatchTargets(); + + // Only the two ../../shared patterns escape the project root; the + // in-tree patterns (local.yml, includes/**) are covered by the + // default recursive watcher and must be excluded. + const summary = targets + .map((t) => `${t.baseUri.fsPath}|${t.pattern}`) + .sort(); + const sharedBase = Uri.file(sharedDir).fsPath; + expect(summary).to.deep.equal( + [`${sharedBase}|config.yml`, `${sharedBase}|*.yml`].sort() + ); + }); + }); + describe("file listing", async () => { beforeEach(async () => { const rootBundleData: BundleSchema = { @@ -96,30 +205,6 @@ describe(__filename, async function () { ); }); - it("should return correct included files", async () => { - const tmpdirUri = Uri.file(tmpdir.path); - const bundleFileSet = new BundleFileSet( - getWorkspaceFolderManagerMock() - ); - - expect(await bundleFileSet.getIncludedFilesGlob()).to.equal( - `{included.yaml,${path.join("includes", "**", "*.yaml")}}` - ); - - const actual = (await bundleFileSet.getIncludedFiles())?.map( - (v) => v.fsPath - ); - const expected = [ - Uri.file(path.join(tmpdirUri.fsPath, "included.yaml")), - Uri.file( - path.join(tmpdirUri.fsPath, "includes", "included.yaml") - ), - ].map((v) => v.fsPath); - expect(Array.from(new Set(actual).values()).sort()).to.deep.equal( - Array.from(new Set(expected).values()).sort() - ); - }); - it("should return all bundle files", async () => { const tmpdirUri = Uri.file(tmpdir.path); const bundleFileSet = new BundleFileSet( diff --git a/packages/databricks-vscode/src/bundle/BundleFileSet.ts b/packages/databricks-vscode/src/bundle/BundleFileSet.ts index f362e004e..366c001b6 100644 --- a/packages/databricks-vscode/src/bundle/BundleFileSet.ts +++ b/packages/databricks-vscode/src/bundle/BundleFileSet.ts @@ -55,6 +55,35 @@ function toGlobPath(path: string) { return path; } +const globMagicChars = /[*?{}[\]!+@()]/; + +/** + * Splits an absolute glob path into its static base directory (the leading + * segments that contain no glob magic characters) and the remaining glob + * pattern. e.g. "/a/b/sub/**\/*.yml" -> {base: "/a/b/sub", pattern: "**\/*.yml"}. + * A pattern without any magic characters is treated as a literal file: its + * directory becomes the base and its filename the pattern. + */ +function splitGlobBase(absolutePath: string): {base: string; pattern: string} { + const segments = absolutePath.split(path.sep); + const staticSegments: string[] = []; + let i = 0; + for (; i < segments.length; i++) { + if (globMagicChars.test(segments[i])) { + break; + } + staticSegments.push(segments[i]); + } + // If no segment contains magic chars, treat the path as a literal file and + // use its parent directory as the base. + if (i === segments.length) { + staticSegments.pop(); + } + const base = staticSegments.join(path.sep) || path.sep; + const pattern = segments.slice(staticSegments.length).join("/"); + return {base, pattern}; +} + export class BundleFileSet { public readonly bundleDataCache: CachedValue = new CachedValue(async () => { @@ -88,33 +117,71 @@ export class BundleFileSet { return Uri.file(rootFile[0]); } - async getIncludedFilesGlob() { + private async getIncludePatterns(): Promise { const rootFile = await this.getRootFile(); if (rootFile === undefined) { - return undefined; + return []; + } + const bundle = await parseBundleYaml(rootFile); + if (!bundle?.include?.length) { + return []; } - const bundle = await parseBundleYaml(Uri.file(rootFile.fsPath)); - if (bundle?.include === undefined || bundle?.include.length === 0) { + return bundle.include; + } + + async getIncludedFiles() { + const patterns = await this.getIncludePatterns(); + if (patterns.length === 0) { return undefined; } - if (bundle?.include.length === 1) { - return bundle.include[0]; + + const allFiles: string[] = []; + for (const pattern of patterns) { + const absolutePattern = toGlobPath( + path.resolve(this.projectRoot.fsPath, pattern) + ); + const files = await glob.glob(absolutePattern, { + nocase: process.platform === "win32", + }); + allFiles.push(...files); } - return `{${bundle.include.join(",")}}`; + + return [...new Set(allFiles)].map((f) => Uri.file(f)); } - async getIncludedFiles() { - const includedFilesGlob = await this.getIncludedFilesGlob(); - if (includedFilesGlob !== undefined) { - return ( - await glob.glob( - toGlobPath( - path.join(this.projectRoot.fsPath, includedFilesGlob) - ), - {nocase: process.platform === "win32"} - ) - ).map((i) => Uri.file(i)); + /** + * Returns watch targets for include patterns whose static base resolves + * outside the active project root (e.g. "../../shared/*.yml"). The default + * recursive workspace watcher only observes files under the project root, + * so these external bases need dedicated watchers. Each target is a base + * directory plus a relative glob suitable for a vscode RelativePattern. + */ + async getExternalIncludeWatchTargets(): Promise< + {baseUri: Uri; pattern: string}[] + > { + const patterns = await this.getIncludePatterns(); + const projectRoot = path.normalize(this.projectRoot.fsPath); + const targets = new Map(); + + for (const pattern of patterns) { + const resolved = path.resolve(projectRoot, pattern); + const {base, pattern: relativePattern} = splitGlobBase(resolved); + // Keep only bases that escape the project root. The default + // recursive watcher already covers everything under the root. + const relativeToRoot = path.relative(projectRoot, base); + if (!relativeToRoot.startsWith("..")) { + continue; + } + const key = `${base}\0${relativePattern}`; + if (!targets.has(key)) { + targets.set(key, { + baseUri: Uri.file(base), + pattern: relativePattern, + }); + } } + + return [...targets.values()]; } async allFiles() { @@ -152,15 +219,16 @@ export class BundleFileSet { } async isIncludedBundleFile(e: Uri) { - let includedFilesGlob = await this.getIncludedFilesGlob(); - if (includedFilesGlob === undefined) { - return false; + const patterns = await this.getIncludePatterns(); + for (const pattern of patterns) { + const absolutePattern = toGlobPath( + path.resolve(this.projectRoot.fsPath, pattern) + ); + if (minimatch(toGlobPath(e.fsPath), absolutePattern)) { + return true; + } } - includedFilesGlob = getAbsoluteGlobPath( - includedFilesGlob, - this.projectRoot - ); - return minimatch(e.fsPath, toGlobPath(includedFilesGlob)); + return false; } async isBundleFile(e: Uri) { diff --git a/packages/databricks-vscode/src/bundle/BundleWatcher.ts b/packages/databricks-vscode/src/bundle/BundleWatcher.ts index 3eca980eb..b107d9b2a 100644 --- a/packages/databricks-vscode/src/bundle/BundleWatcher.ts +++ b/packages/databricks-vscode/src/bundle/BundleWatcher.ts @@ -1,7 +1,14 @@ -import {Disposable, EventEmitter, Uri, workspace} from "vscode"; +import { + Disposable, + EventEmitter, + RelativePattern, + Uri, + workspace, +} from "vscode"; import {BundleFileSet, getAbsoluteGlobPath} from "./BundleFileSet"; import path from "path"; import {WorkspaceFolderManager} from "../vscode-objs/WorkspaceFolderManager"; +import {Mutex} from "../locking"; export class BundleWatcher implements Disposable { private disposables: Disposable[] = []; @@ -19,15 +26,24 @@ export class BundleWatcher implements Disposable { public readonly onDidDelete = this._onDidDelete.event; private initCleanup: Disposable; + // Watchers for include files that resolve outside the active project root. + // Tracked separately so they can be rebuilt when the include list changes + // without tearing down the main in-tree watcher. + private externalWatchersCleanup: Disposable | undefined; + // Serializes refreshExternalWatchers() so overlapping invocations (e.g. a + // burst of root-file changes) don't leak watchers or create duplicates. + private readonly externalWatchersMutex = new Mutex(); constructor( private readonly bundleFileSet: BundleFileSet, private readonly workspaceFolderManager: WorkspaceFolderManager ) { this.initCleanup = this.init(); + void this.refreshExternalWatchers(); this.disposables.push( this.workspaceFolderManager.onDidChangeActiveProjectFolder(() => { this.initCleanup.dispose(); this.initCleanup = this.init(); + void this.refreshExternalWatchers(); this.bundleFileSet.bundleDataCache.invalidate(); }) ); @@ -61,6 +77,51 @@ export class BundleWatcher implements Disposable { }; } + /** + * (Re)create watchers for include files that live outside the active + * project root. The default recursive watcher created in init() only + * observes files under the project root, so parent-folder includes + * (e.g. "../../shared/databricks-shared.yml") need dedicated watchers + * to keep the bundle cache and downstream models in sync. + */ + private async refreshExternalWatchers() { + return this.externalWatchersMutex.synchronise(async () => { + this.externalWatchersCleanup?.dispose(); + this.externalWatchersCleanup = undefined; + + const targets = + await this.bundleFileSet.getExternalIncludeWatchTargets(); + if (targets.length === 0) { + return; + } + + const disposables: Disposable[] = []; + for (const {baseUri, pattern} of targets) { + const watcher = workspace.createFileSystemWatcher( + new RelativePattern(baseUri, pattern) + ); + disposables.push( + watcher, + watcher.onDidCreate((e) => { + this.yamlFileChangeHandler(e, "CREATE"); + }), + watcher.onDidChange((e) => { + this.yamlFileChangeHandler(e, "CHANGE"); + }), + watcher.onDidDelete((e) => { + this.yamlFileChangeHandler(e, "DELETE"); + }) + ); + } + + this.externalWatchersCleanup = { + dispose: () => { + disposables.forEach((i) => i.dispose()); + }, + }; + }); + } + private async yamlFileChangeHandler( e: Uri, type: "CREATE" | "CHANGE" | "DELETE" @@ -74,6 +135,9 @@ export class BundleWatcher implements Disposable { // to provide additional granularity, we also fire an event when the root bundle file changes if (this.bundleFileSet.isRootBundleFile(e)) { this._onDidChangeRootFile.fire(); + // The include list lives in the root file, so its set of external + // include locations may have changed. Rebuild those watchers. + void this.refreshExternalWatchers(); } switch (type) { case "CREATE": @@ -88,5 +152,6 @@ export class BundleWatcher implements Disposable { dispose() { this.disposables.forEach((i) => i.dispose()); this.initCleanup.dispose(); + this.externalWatchersCleanup?.dispose(); } }