refactor(uploadLambda): fix bugs, future-proof

- Fix bugs:
    1. Cloud9 findFile() branch wasn't tested.
    2. Cloud9 findFile() doesn't find `.application.json` at more than one level of nesting.
- Add `techdebt.test.ts` as a mechanism to enforce removal of temporary
  code once our dependencies or other factors make that possible.

Author: justinmk3

src/lambda/commands/uploadLambda.ts

@@ -12,7 +12,12 @@ import * as AdmZip from 'adm-zip'
 import * as fs from 'fs-extra'
 import * as path from 'path'
 import { showConfirmationMessage, showViewLogsMessage } from '../../shared/utilities/messages'
-import { fileExists, findFile, makeTemporaryToolkitFolder, tryRemoveFolder } from '../../shared/filesystemUtilities'
+import {
+    fileExists,
+    cloud9Findfile,
+    makeTemporaryToolkitFolder,
+    tryRemoveFolder,
+} from '../../shared/filesystemUtilities'
 import * as localizedText from '../../shared/localizedText'
 import { getLogger } from '../../shared/logger'
 import { SamCliBuildInvocation } from '../../shared/sam/cli/samCliBuild'
@@ -35,6 +40,41 @@ import { toArrayAsync } from '../../shared/utilities/collectionUtils'
 import { fromExtensionManifest } from '../../shared/settings'
 import { createRegionPrompter } from '../../shared/ui/common/region'
 
+interface SavedLambdas {
+    [profile: string]: { [region: string]: string }
+}
+
+class LambdaSettings extends fromExtensionManifest('aws.lambda', { recentlyUploaded: Object }) {
+    static #instance: LambdaSettings
+
+    public getRecentLambdas(): SavedLambdas | undefined {
+        try {
+            return this.get('recentlyUploaded')
+        } catch (error) {
+            this.delete('recentlyUploaded')
+        }
+    }
+
+    /**
+     * Adds a new "recently used Lambda" to user settings for the given profile
+     * and region (limit of one item per profile+region).
+     */
+    public setRecentLambda(profile: string, region: string, lambdaName: string): Promise<boolean> {
+        const oldLambdas = this.getRecentLambdas()
+        return this.update('recentlyUploaded', {
+            ...oldLambdas,
+            [profile]: {
+                ...(oldLambdas?.[profile] ?? {}),
+                [region]: lambdaName,
+            },
+        })
+    }
+
+    public static get instance() {
+        return (this.#instance ??= new this())
+    }
+}
+
 interface LambdaFunction {
     readonly name: string
     readonly region: string
@@ -84,7 +124,7 @@ export async function uploadLambdaCommand(lambdaArg?: LambdaFunction, path?: vsc
         if (result === 'Succeeded') {
             const profile = globals.awsContext.getCredentialProfileName()
             if (profile && lambda) {
-                updateSavedLambda(profile, lambda.region, lambda.name)
+                LambdaSettings.instance.setRecentLambda(profile, lambda.region, lambda.name)
             }
         }
     }
@@ -442,27 +482,34 @@ async function uploadZipBuffer(
     )
 }
 
-export async function findApplicationJsonFile(startPath: vscode.Uri): Promise<vscode.Uri | undefined> {
-    const isTemplateFile = /template.(yaml|yml|json)$/.test(startPath.fsPath)
-    const parentDir = isTemplateFile ? vscode.Uri.joinPath(startPath, '..') : startPath
-
-    if (isCloud9()) {
-        const found = await findFile(parentDir.fsPath, '.application.json')
-        if (!found) {
-            getLogger().debug(
-                'lambda: .application.json file not found in parent directory or sub directories of %s',
-                parentDir.fsPath
-            )
-        }
-        return found ? vscode.Uri.file(found) : undefined
-    } else {
-        const found = await vscode.workspace.findFiles(
-            new vscode.RelativePattern(parentDir.path, '**/*.application.json'),
-            undefined,
-            1
+export async function findApplicationJsonFile(
+    startPath: vscode.Uri,
+    cloud9 = isCloud9()
+): Promise<vscode.Uri | undefined> {
+    if (!(await fileExists(startPath.fsPath))) {
+        getLogger().error(
+            'findApplicationJsonFile() invalid path (not accessible or does not exist): "%s"',
+            startPath.fsPath
         )
-        return found[0]
+        return undefined
     }
+    const isdir = fs.statSync(startPath.fsPath).isDirectory()
+    const parentDir = isdir ? startPath.fsPath : path.dirname(startPath.fsPath)
+    const found = cloud9
+        ? await cloud9Findfile(parentDir, '.application.json')
+        : await vscode.workspace.findFiles(
+              new vscode.RelativePattern(parentDir, '**/.application.json'),
+              // exclude:
+              // - null      = NO excludes apply
+              // - undefined = default excludes apply (e.g. the `files.exclude` setting but not `search.exclude`).
+              // eslint-disable-next-line no-null/no-null
+              null,
+              1
+          )
+    if (!found || found.length === 0) {
+        getLogger().debug('uploadLambda: .application.json not found in: "%s"', parentDir)
+    }
+    return found[0]
 }
 
 export function getFunctionNames(file: vscode.Uri, region: string): string[] | undefined {
@@ -489,45 +536,56 @@ export function getFunctionNames(file: vscode.Uri, region: string): string[] | u
 
 async function listAllLambdaNames(region: string, path?: vscode.Uri) {
     const lambdaFunctionNames: DataQuickPickItem<string>[] = []
+
+    // Get Lambda functions from .application.json #2588
     if (path) {
-        const namesFromAppFile = await getLambdaNamesFromApplicationFile(region, path)
-        if (namesFromAppFile) {
+        const appFile = await findApplicationJsonFile(path)
+        const namesFromAppFile = appFile ? getFunctionNames(appFile, region) : undefined
+        if (!appFile) {
+            getLogger().debug('lambda: .application.json not found')
+        } else if (!namesFromAppFile) {
+            getLogger().debug('lambda: no functions in .application.json for region: %s', region)
+        } else {
             lambdaFunctionNames.push(
                 ...namesFromAppFile.map(n => {
                     return {
                         label: n,
-                        description: localize('AWS.lambda.upload.fromC9AppFile', 'from appliication.json'),
+                        description: localize('AWS.lambda.upload.fromAppJson', 'from .application.json'),
                         data: n,
                     }
                 })
             )
         }
     }
+
+    // Get Lambda functions from user AWS account.
     const lambdaClient = globals.toolkitClientBuilder.createLambdaClient(region)
     try {
         const foundLambdas = await toArrayAsync(listLambdaFunctions(lambdaClient))
         for (const l of foundLambdas) {
             lambdaFunctionNames.push({ label: l.FunctionName!, data: l.FunctionName })
         }
     } catch (error) {
-        getLogger().error('upload lambda: Error listing lambdas: %s', (error as Error).message)
+        getLogger().error('lambda: failed to list Lambda functions: %s', (error as Error).message)
     }
-    const previousLambdas = getSavedLambdas()
+
+    // Get "recently used" Lambda functions.
+    const recent = LambdaSettings.instance.getRecentLambdas()
     const profile = globals.awsContext.getCredentialProfileName()
-    if (previousLambdas && profile && previousLambdas[profile] && previousLambdas[profile][region]) {
+    if (profile && recent?.[profile]?.[region]) {
         let isInList = false
         for (const l of lambdaFunctionNames) {
-            if (l.label === previousLambdas[profile][region]) {
+            if (l.label === recent[profile][region]) {
                 l.description = localizedText.recentlyUsed
                 l.recentlyUsed = true
                 isInList = true
             }
         }
         if (!isInList) {
             lambdaFunctionNames.splice(0, 0, {
-                label: previousLambdas[profile][region],
+                label: recent[profile][region],
                 recentlyUsed: true,
-                data: previousLambdas[profile][region],
+                data: recent[profile][region],
                 description: localizedText.recentlyUsed,
             })
         }
@@ -558,39 +616,3 @@ function createFunctionNamePrompter(region: string, path?: vscode.Uri) {
     })
     return prompter
 }
-
-interface SavedLambdas {
-    [profile: string]: { [region: string]: string }
-}
-
-function getSavedLambdas(): SavedLambdas | undefined {
-    const settings = new LambdaSettings()
-    try {
-        return settings.get('recentlyUploaded')
-    } catch (error) {
-        getLogger().error('settings: Error retreiving saved lambdas from settings: %s', (error as Error).message)
-    }
-}
-
-async function updateSavedLambda(profile: string, region: string, lambdaName: string): Promise<boolean> {
-    const settings = new LambdaSettings()
-    const oldLambdas = getSavedLambdas()
-    return settings.update('recentlyUploaded', {
-        ...oldLambdas,
-        [profile]: {
-            ...(oldLambdas?.[profile] ?? {}),
-            [region]: lambdaName,
-        },
-    })
-}
-
-async function getLambdaNamesFromApplicationFile(region: string, path: vscode.Uri) {
-    const appFile = await findApplicationJsonFile(path)
-    if (appFile) {
-        return getFunctionNames(appFile, region)
-    } else {
-        getLogger().debug('lambda: No .application.json files found')
-    }
-}
-
-class LambdaSettings extends fromExtensionManifest('aws.lambda', { recentlyUploaded: Object }) {}

src/shared/filesystemUtilities.ts

@@ -29,9 +29,14 @@ export function downloadsDir(): string {
     }
 }
 
-export async function fileExists(filePath: string): Promise<boolean> {
+/**
+ * Checks if file or directory `p` exists.
+ *
+ * TODO: optionally check read/write permissions and return a granular status.
+ */
+export async function fileExists(p: string): Promise<boolean> {
     try {
-        await access(filePath)
+        await access(p)
     } catch (err) {
         return false
     }
@@ -164,24 +169,29 @@ export async function hasFileWithSuffix(dir: string, suffix: string, exclude?: v
 }
 
 /**
- * Searches directory and all sub-directories for a filename.
+ * TEMPORARY SHIM for vscode.workspace.findFiles() on Cloud9.
+ *
  * @param dir Directory to search
  * @param fileName Name of file to locate
- * @returns  The absolute path if found.
+ * @returns  List of one or zero Uris (for compat with vscode.workspace.findFiles())
  */
-export async function findFile(dir: string, fileName: string): Promise<string | undefined> {
+export async function cloud9Findfile(dir: string, fileName: string): Promise<vscode.Uri[]> {
     const files = await readdir(dir)
-    const subDirs = []
+    const subDirs: vscode.Uri[] = []
     for (const file of files) {
         const filePath = path.join(dir, file)
         if (filePath === path.join(dir, fileName)) {
-            return filePath
+            return [vscode.Uri.file(filePath)]
         }
         if ((await stat(filePath)).isDirectory()) {
-            subDirs.push(filePath)
+            subDirs.push(vscode.Uri.file(filePath))
         }
     }
-    for (const dir of subDirs) {
-        findFile(dir, fileName)
+    for (const d of subDirs) {
+        const found = await cloud9Findfile(d.fsPath, fileName)
+        if (found.length > 0) {
+            return found
+        }
     }
+    return []
 }

src/shared/vscode/env.ts

@@ -79,5 +79,12 @@ export { extensionVersion }
  * by the `engines` field in `package.json`
  */
 export function isMinimumVersion(): boolean {
-    return vscode.version.startsWith(packageJson.engines.vscode.replace(/\^\~/, ''))
+    return vscode.version.startsWith(getMinVscodeVersion())
+}
+
+/**
+ * Returns the minimum vscode "engine" version declared in `package.json`.
+ */
+export function getMinVscodeVersion(): string {
+    return packageJson.engines.vscode.replace(/[^~]/, '')
 }

src/test/lambda/commands/uploadLambda.test.ts

@@ -0,0 +1,90 @@
+/*!
+ * Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import * as vscode from 'vscode'
+import * as assert from 'assert'
+import * as fs from 'fs-extra'
+import * as path from 'path'
+import { makeTemporaryToolkitFolder } from '../../../shared/filesystemUtilities'
+import { findApplicationJsonFile, getFunctionNames } from '../../../lambda/commands/uploadLambda'
+import { assertEqualPaths, toFile } from '../../testUtil'
+
+describe('uploadLambda', async function () {
+    let tempFolder: string
+    let folderUri: vscode.Uri
+    const dotApplicationJsonData = `{
+        "DeploymentMethod": "lambda",
+        "Functions": {
+            "sampleFunction": {
+                "PhysicalId": {
+                    "us-west-2": "sampleFunction-w2",
+                    "us-east-1": "sampleFunction-e1"
+                }
+            },
+            "differentFunction": {
+                "PhysicalId": {
+                    "us-east-1": "differentFunction-e1"
+                }
+            }
+        }
+    }`
+    beforeEach(async function () {
+        tempFolder = await makeTemporaryToolkitFolder()
+        folderUri = vscode.Uri.file(tempFolder)
+    })
+    afterEach(async function () {
+        await fs.remove(tempFolder)
+    })
+
+    it('finds application.json file from dir path - flat', async function () {
+        toFile('top secret data', path.join(tempFolder, '.application.json'))
+        assertEqualPaths(
+            (await findApplicationJsonFile(folderUri))?.fsPath ?? '',
+            path.join(tempFolder, '.application.json')
+        )
+        // Also test Cloud9 temporary workaround.
+        assertEqualPaths(
+            (await findApplicationJsonFile(folderUri, true))?.fsPath ?? '',
+            path.join(tempFolder, '.application.json')
+        )
+    })
+
+    it('finds application.json file from dir path - nested', async function () {
+        const subfolder = path.join(tempFolder, 'one', 'two')
+        const appjsonPath = path.join(subfolder, '.application.json')
+        toFile('top secret data', appjsonPath)
+
+        assertEqualPaths((await findApplicationJsonFile(folderUri))?.fsPath ?? '', appjsonPath)
+        // Also test Cloud9 temporary workaround.
+        assertEqualPaths((await findApplicationJsonFile(folderUri, true))?.fsPath ?? '', appjsonPath)
+    })
+
+    it('finds application.json file from template file path', async function () {
+        const templateUri = vscode.Uri.file(path.join(tempFolder, 'template.yaml'))
+        const appjsonPath = path.join(tempFolder, '.application.json')
+        toFile('SAM stuff...', templateUri.fsPath)
+        toFile('top secret data', appjsonPath)
+
+        assertEqualPaths((await findApplicationJsonFile(templateUri))?.fsPath ?? '', appjsonPath)
+        // Also test Cloud9 temporary workaround.
+        assertEqualPaths((await findApplicationJsonFile(templateUri, true))?.fsPath ?? '', appjsonPath)
+    })
+
+    it('lists functions from .application.json', async function () {
+        const filePath = path.join(tempFolder, '.application.json')
+        toFile(dotApplicationJsonData, filePath)
+        const foundFunctions1 = getFunctionNames(vscode.Uri.file(filePath), 'us-west-2')
+        const foundFunctions2 = getFunctionNames(vscode.Uri.file(filePath), 'us-east-1')
+        assert.deepStrictEqual(foundFunctions1, ['sampleFunction-w2'])
+        assert.deepStrictEqual(foundFunctions2, ['sampleFunction-e1', 'differentFunction-e1'])
+    })
+
+    it('invalid .application.json', async function () {
+        const filePath = path.join(tempFolder, '.application.json')
+        const invalidJson = '{ "DeploymentMethod": "lambda", "Functions": { ?? } }'
+        toFile(invalidJson, filePath)
+        assert.deepStrictEqual(getFunctionNames(vscode.Uri.file(filePath), 'us-west-2'), undefined)
+        assert.deepStrictEqual(getFunctionNames(vscode.Uri.file(filePath), 'us-east-1'), undefined)
+    })
+})