Merge together template registry and template registry manager (#1391)

- Merge together template registry and template registry manager
- Move template registry to ext as it's global like telemetry, etc where we register one instance at start then use it until deactivation
  - This is a replacement for the existing singleton

Author: hunterwerlla

src/integrationTest/cloudformation/templateRegistryManager.test.ts renamed to src/integrationTest/cloudformation/templateRegistry.test.ts

@@ -7,17 +7,15 @@ import * as path from 'path'
 import * as fs from 'fs-extra'
 
 import { CloudFormationTemplateRegistry } from '../../shared/cloudformation/templateRegistry'
-import { CloudFormationTemplateRegistryManager } from '../../shared/cloudformation/templateRegistryManager'
 import { makeSampleSamTemplateYaml, strToYamlFile } from '../../test/shared/cloudformation/cloudformationTestUtils'
 import { getTestWorkspaceFolder } from '../integrationTestsUtilities'
 
 /**
  * Note: these tests are pretty shallow right now. They do not test the following:
  * * Adding/removing workspace folders
  */
-describe('CloudFormation Template Registry Manager', async () => {
+describe('CloudFormation Template Registry', async () => {
     let registry: CloudFormationTemplateRegistry
-    let manager: CloudFormationTemplateRegistryManager
     let workspaceDir: string
     let testDir: string
     let testDirNested: string
@@ -32,11 +30,10 @@ describe('CloudFormation Template Registry Manager', async () => {
         testDirNested = path.join(testDir, 'nested')
         await fs.mkdirp(testDirNested)
         registry = new CloudFormationTemplateRegistry()
-        manager = new CloudFormationTemplateRegistryManager(registry)
     })
 
     afterEach(async () => {
-        manager.dispose()
+        registry.dispose()
         await fs.remove(testDir)
         dir++
     })
@@ -45,13 +42,13 @@ describe('CloudFormation Template Registry Manager', async () => {
         await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDir, 'test.yaml'))
         await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDirNested, 'test.yml'))
 
-        await manager.addTemplateGlob('**/test.{yaml,yml}')
+        await registry.addTemplateGlob('**/test.{yaml,yml}')
 
         await registryHasTargetNumberOfFiles(registry, 2)
     })
 
     it('adds dynamically-added template files with yaml and yml extensions at various nesting levels', async () => {
-        await manager.addTemplateGlob('**/test.{yaml,yml}')
+        await registry.addTemplateGlob('**/test.{yaml,yml}')
 
         await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDir, 'test.yml'))
         await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDirNested, 'test.yaml'))
@@ -60,8 +57,8 @@ describe('CloudFormation Template Registry Manager', async () => {
     })
 
     it('Ignores templates matching excluded patterns', async () => {
-        await manager.addTemplateGlob('**/test.{yaml,yml}')
-        await manager.addExcludedPattern(/.*nested.*/)
+        await registry.addTemplateGlob('**/test.{yaml,yml}')
+        await registry.addExcludedPattern(/.*nested.*/)
 
         await strToYamlFile(makeSampleSamTemplateYaml(false), path.join(testDir, 'test.yml'))
         await strToYamlFile(makeSampleSamTemplateYaml(true), path.join(testDirNested, 'test.yaml'))
@@ -73,7 +70,7 @@ describe('CloudFormation Template Registry Manager', async () => {
         const filepath = path.join(testDir, 'changeMe.yml')
         await strToYamlFile(makeSampleSamTemplateYaml(false), filepath)
 
-        await manager.addTemplateGlob('**/changeMe.yml')
+        await registry.addTemplateGlob('**/changeMe.yml')
 
         await registryHasTargetNumberOfFiles(registry, 1)
 
@@ -85,7 +82,7 @@ describe('CloudFormation Template Registry Manager', async () => {
     })
 
     it('can handle deleted files', async () => {
-        await manager.addTemplateGlob('**/deleteMe.yml')
+        await registry.addTemplateGlob('**/deleteMe.yml')
 
         // Specifically creating the file after the watcher is added
         // Otherwise, it seems the file is deleted before the file watcher realizes the file exists

src/lambda/local/debugConfiguration.ts

@@ -6,7 +6,6 @@
 import * as path from 'path'
 import * as vscode from 'vscode'
 import { CloudFormation } from '../../shared/cloudformation/cloudformation'
-import { CloudFormationTemplateRegistry } from '../../shared/cloudformation/templateRegistry'
 import {
     AwsSamDebuggerConfiguration,
     AWS_SAM_DEBUG_TARGET_TYPES,
@@ -18,6 +17,7 @@ import { localize } from '../../shared/utilities/vsCodeUtils'
 import { tryGetAbsolutePath } from '../../shared/utilities/workspaceUtils'
 import { RuntimeFamily } from '../models/samLambdaRuntime'
 import { SamLaunchRequestArgs } from '../../shared/sam/debugger/awsSamDebugger'
+import { ext } from '../../shared/extensionGlobals'
 
 export const DOTNET_CORE_DEBUGGER_PATH = '/tmp/lambci_debug_files/vsdbg'
 
@@ -155,9 +155,8 @@ export function getTemplate(
         return undefined
     }
     const templateInvoke = config.invokeTarget as TemplateTargetProperties
-    const cftRegistry = CloudFormationTemplateRegistry.getRegistry()
     const fullPath = tryGetAbsolutePath(folder, templateInvoke.templatePath)
-    const cfnTemplate = cftRegistry.getRegisteredTemplate(fullPath)?.template
+    const cfnTemplate = ext.templateRegistry.getRegisteredTemplate(fullPath)?.template
     return cfnTemplate
 }
 

src/lambda/wizards/samDeployWizard.ts

@@ -29,7 +29,6 @@ import {
 } from '../../shared/wizards/multiStepWizard'
 import { configureParameterOverrides } from '../config/configureParameterOverrides'
 import { getOverriddenParameters, getParameters } from '../utilities/parameterUtils'
-import { CloudFormationTemplateRegistry } from '../../shared/cloudformation/templateRegistry'
 import { ext } from '../../shared/extensionGlobals'
 
 export interface SamDeployWizardResponse {
@@ -620,8 +619,7 @@ function validateStackName(value: string): string | undefined {
 }
 
 async function getTemplateChoices(...workspaceFolders: vscode.Uri[]): Promise<SamTemplateQuickPickItem[]> {
-    const cfnRegistry = CloudFormationTemplateRegistry.getRegistry()
-    const templateUris = cfnRegistry.registeredTemplates.map(o => vscode.Uri.file(o.path))
+    const templateUris = ext.templateRegistry.registeredTemplates.map(o => vscode.Uri.file(o.path))
     const uriToLabel: Map<vscode.Uri, string> = new Map<vscode.Uri, string>()
     const labelCounts: Map<string, number> = new Map()
 

src/shared/cloudformation/activation.ts

@@ -8,7 +8,7 @@ import { getLogger } from '../logger'
 import { localize } from '../utilities/vsCodeUtils'
 
 import { CloudFormationTemplateRegistry } from './templateRegistry'
-import { CloudFormationTemplateRegistryManager } from './templateRegistryManager'
+import { ext } from '../extensionGlobals'
 
 export const TEMPLATE_FILE_GLOB_PATTERN = '**/template.{yaml,yml}'
 
@@ -28,11 +28,11 @@ export const TEMPLATE_FILE_EXCLUDE_PATTERN = /.*[/\\]\.aws-sam([/\\].*|$)/
  */
 export async function activate(extensionContext: vscode.ExtensionContext): Promise<void> {
     try {
-        const registry = CloudFormationTemplateRegistry.getRegistry()
-        const manager = new CloudFormationTemplateRegistryManager(registry)
-        await manager.addExcludedPattern(TEMPLATE_FILE_EXCLUDE_PATTERN)
-        await manager.addTemplateGlob(TEMPLATE_FILE_GLOB_PATTERN)
-        extensionContext.subscriptions.push(manager)
+        const registry = new CloudFormationTemplateRegistry()
+        await registry.addExcludedPattern(TEMPLATE_FILE_EXCLUDE_PATTERN)
+        await registry.addTemplateGlob(TEMPLATE_FILE_GLOB_PATTERN)
+        extensionContext.subscriptions.push(registry)
+        ext.templateRegistry = registry
     } catch (e) {
         vscode.window.showErrorMessage(
             localize(

src/shared/cloudformation/templateRegistry.ts

@@ -4,38 +4,80 @@
  */
 
 import * as vscode from 'vscode'
-import * as path_ from 'path'
 import { getLogger } from '../logger/logger'
 import { CloudFormation } from './cloudformation'
 import * as pathutils from '../utilities/pathUtils'
+import * as path from 'path'
 import { isInDirectory } from '../filesystemUtilities'
 import { dotNetRuntimes } from '../../lambda/models/samLambdaRuntime'
 import { getLambdaDetails } from '../../lambda/utils'
+import { ext } from '../extensionGlobals'
 
 export interface TemplateDatum {
     path: string
     template: CloudFormation.Template
 }
 
-export class CloudFormationTemplateRegistry {
-    private static INSTANCE: CloudFormationTemplateRegistry | undefined
-    private readonly templateRegistryData: Map<string, CloudFormation.Template>
+export class CloudFormationTemplateRegistry implements vscode.Disposable {
+    private readonly disposables: vscode.Disposable[] = []
+    private _isDisposed: boolean = false
+    private readonly globs: vscode.GlobPattern[] = []
+    private readonly excludedFilePatterns: RegExp[] = []
+    private readonly templateRegistryData: Map<string, CloudFormation.Template> = new Map<
+        string,
+        CloudFormation.Template
+    >()
 
     public constructor() {
-        this.templateRegistryData = new Map<string, CloudFormation.Template>()
+        this.disposables.push(
+            vscode.workspace.onDidChangeWorkspaceFolders(async () => {
+                await this.rebuildRegistry()
+            })
+        )
     }
 
-    private assertAbsolute(path: string) {
-        if (!path_.isAbsolute(path)) {
-            throw Error(`CloudFormationTemplateRegistry: path is relative: ${path}`)
+    /**
+     * Adds a glob pattern to use for lookups and resets the registry to use it.
+     * Added templates cannot be removed without restarting the extension.
+     * Throws an error if this manager has already been disposed.
+     * @param glob vscode.GlobPattern to be used for lookups
+     */
+    public async addTemplateGlob(glob: vscode.GlobPattern): Promise<void> {
+        if (this._isDisposed) {
+            throw new Error('Manager has already been disposed!')
+        }
+        this.globs.push(glob)
+
+        const watcher = vscode.workspace.createFileSystemWatcher(glob)
+        this.addWatcher(watcher)
+
+        await this.rebuildRegistry()
+    }
+
+    /**
+     * Adds a regex pattern to ignore paths containing the pattern
+     */
+    public async addExcludedPattern(pattern: RegExp): Promise<void> {
+        if (this._isDisposed) {
+            throw new Error('Manager has already been disposed!')
         }
+        this.excludedFilePatterns.push(pattern)
+
+        await this.rebuildRegistry()
     }
 
     /**
      * Adds template to registry. Wipes any existing template in its place with newly-parsed copy of the data.
      * @param templateUri vscode.Uri containing the template to load in
      */
     public async addTemplateToRegistry(templateUri: vscode.Uri, quiet?: boolean): Promise<void> {
+        const excluded = this.excludedFilePatterns.find(pattern => templateUri.fsPath.match(pattern))
+        if (excluded) {
+            getLogger().verbose(
+                `Manager did not add template ${templateUri.fsPath} matching excluded pattern ${excluded}`
+            )
+            return
+        }
         const pathAsString = pathutils.normalize(templateUri.fsPath)
         this.assertAbsolute(pathAsString)
         try {
@@ -91,6 +133,35 @@ export class CloudFormationTemplateRegistry {
         this.templateRegistryData.delete(pathAsString)
     }
 
+    /**
+     * Disposes CloudFormationTemplateRegistryManager and marks as disposed.
+     */
+    public dispose(): void {
+        if (!this._isDisposed) {
+            while (this.disposables.length > 0) {
+                const disposable = this.disposables.pop()
+                if (disposable) {
+                    disposable.dispose()
+                }
+            }
+            this._isDisposed = true
+        }
+    }
+
+    /**
+     * Rebuilds registry using current glob and exclusion patterns.
+     * All functionality is currently internal to class, but can be made public if we want a manual "refresh" button
+     */
+    private async rebuildRegistry(): Promise<void> {
+        this.reset()
+        for (const glob of this.globs) {
+            const templateUris = await vscode.workspace.findFiles(glob)
+            for (const template of templateUris) {
+                await this.addTemplateToRegistry(template, true)
+            }
+        }
+    }
+
     /**
      * Removes all templates from the registry.
      */
@@ -99,15 +170,31 @@ export class CloudFormationTemplateRegistry {
     }
 
     /**
-     * Returns the CloudFormationTemplateRegistry singleton.
-     * If the singleton doesn't exist, creates it.
+     * Sets watcher functionality and adds to this.disposables
+     * @param watcher vscode.FileSystemWatcher
      */
-    public static getRegistry(): CloudFormationTemplateRegistry {
-        if (!CloudFormationTemplateRegistry.INSTANCE) {
-            CloudFormationTemplateRegistry.INSTANCE = new CloudFormationTemplateRegistry()
-        }
+    private addWatcher(watcher: vscode.FileSystemWatcher): void {
+        this.disposables.push(
+            watcher,
+            watcher.onDidChange(async uri => {
+                getLogger().verbose(`Manager detected a change to template file: ${uri.fsPath}`)
+                await this.addTemplateToRegistry(uri)
+            }),
+            watcher.onDidCreate(async uri => {
+                getLogger().verbose(`Manager detected a new template file: ${uri.fsPath}`)
+                await this.addTemplateToRegistry(uri)
+            }),
+            watcher.onDidDelete(async uri => {
+                getLogger().verbose(`Manager detected a deleted template file: ${uri.fsPath}`)
+                this.removeTemplateFromRegistry(uri)
+            })
+        )
+    }
 
-        return CloudFormationTemplateRegistry.INSTANCE
+    private assertAbsolute(p: string) {
+        if (!path.isAbsolute(p)) {
+            throw Error(`CloudFormationTemplateRegistry: path is relative: ${p}`)
+        }
     }
 }
 
@@ -121,7 +208,7 @@ export class CloudFormationTemplateRegistry {
 export function getResourcesForHandler(
     filepath: string,
     handler: string,
-    unfilteredTemplates: TemplateDatum[] = CloudFormationTemplateRegistry.getRegistry().registeredTemplates
+    unfilteredTemplates: TemplateDatum[] = ext.templateRegistry.registeredTemplates
 ): { templateDatum: TemplateDatum; name: string; resourceData: CloudFormation.Resource }[] {
     // TODO: Array.flat and Array.flatMap not introduced until >= Node11.x -- migrate when VS Code updates Node ver
     const o = unfilteredTemplates.map(templateDatum => {
@@ -150,9 +237,9 @@ export function getResourcesForHandlerFromTemplateDatum(
     templateDatum: TemplateDatum
 ): { name: string; resourceData: CloudFormation.Resource }[] {
     const matchingResources: { name: string; resourceData: CloudFormation.Resource }[] = []
-    const templateDirname = path_.dirname(templateDatum.path)
+    const templateDirname = path.dirname(templateDatum.path)
     // template isn't a parent or sibling of file
-    if (!isInDirectory(templateDirname, path_.dirname(filepath))) {
+    if (!isInDirectory(templateDirname, path.dirname(filepath))) {
         return []
     }
 
@@ -191,7 +278,7 @@ export function getResourcesForHandlerFromTemplateDatum(
                     if (
                         handler === registeredHandler &&
                         isInDirectory(
-                            pathutils.normalize(path_.join(templateDirname, registeredCodeUri)),
+                            pathutils.normalize(path.join(templateDirname, registeredCodeUri)),
                             pathutils.normalize(filepath)
                         )
                     ) {
@@ -210,7 +297,7 @@ export function getResourcesForHandlerFromTemplateDatum(
                         if (
                             pathutils.normalize(filepath) ===
                                 pathutils.normalize(
-                                    path_.join(templateDirname, registeredCodeUri, parsedLambda.fileName)
+                                    path.join(templateDirname, registeredCodeUri, parsedLambda.fileName)
                                 ) &&
                             functionName === parsedLambda.functionName
                         ) {