fix(appbuilder): Prevent parallel build processes on the same template (aws#6172)

## Context
We currently allow multiple build processes to run at the same time for
the same template.

## Problem

When running a build, it’s possible to start a second build for the same
project. This results in both builds failings

## Solution
Prevent parallel builds on the same template

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

Author: mbfreder

packages/core/src/awsService/appBuilder/explorer/openTemplate.ts

@@ -5,8 +5,7 @@
 
 import { samSyncUrl } from '../../../shared/constants'
 import { CloudFormationTemplateRegistry } from '../../../shared/fs/templateRegistry'
-import { syncMementoRootKey } from '../../../shared/sam/sync'
-
+import { syncMementoRootKey } from '../../../shared/sam/constants'
 import { createExitPrompter } from '../../../shared/ui/common/exitPrompter'
 import { createTemplatePrompter, TemplateItem } from '../../../shared/ui/sam/templatePrompter'
 import { Wizard } from '../../../shared/wizards/wizard'

packages/core/src/shared/sam/build.ts

@@ -22,16 +22,17 @@ import { getSpawnEnv } from '../env/resolveEnv'
 import {
     getErrorCode,
     getProjectRoot,
+    getRecentResponse,
     getSamCliPathAndVersion,
     getTerminalFromError,
     isDotnetRuntime,
     updateRecentResponse,
 } from './utils'
 import { getConfigFileUri, validateSamBuildConfig } from './config'
 import { runInTerminal } from './processTerminal'
+import { buildMementoRootKey, buildProcessMementoRootKey, globalIdentifier } from './constants'
 import { SemVer } from 'semver'
 
-const buildMementoRootKey = 'samcli.build.params'
 export interface BuildParams {
     readonly template: TemplateItem
     readonly projectRoot: vscode.Uri
@@ -208,6 +209,9 @@ export async function runBuild(arg?: TreeNode): Promise<SamBuildResult> {
         throw new CancellationError('user')
     }
 
+    throwIfTemplateIsBeingBuilt(params.template.uri.path)
+    await registerTemplateBuild(params.template.uri.path)
+
     const projectRoot = params.projectRoot
 
     const defaultFlags: string[] = ['--cached', '--parallel', '--save-params', '--use-container']
@@ -233,16 +237,30 @@ export async function runBuild(arg?: TreeNode): Promise<SamBuildResult> {
     await updateRecentResponse(buildMementoRootKey, 'global', 'templatePath', templatePath)
 
     try {
-        // Create a child process to run the SAM build command
-        const buildProcess = new ChildProcess(samCliPath, ['build', ...buildFlags], {
-            spawnOptions: await addTelemetryEnvVar({
-                cwd: params.projectRoot.fsPath,
-                env: await getSpawnEnv(process.env),
-            }),
-        })
+        await vscode.window.withProgress(
+            {
+                cancellable: true,
+                location: vscode.ProgressLocation.Notification,
+            },
+            async (progress, token) => {
+                token.onCancellationRequested(async () => {
+                    throw new CancellationError('user')
+                })
 
-        // Run SAM build in Terminal
-        await runInTerminal(buildProcess, 'build')
+                progress.report({ message: `Building SAM template at ${params.template.uri.path}` })
+
+                // Create a child process to run the SAM build command
+                const buildProcess = new ChildProcess(samCliPath, ['build', ...buildFlags], {
+                    spawnOptions: await addTelemetryEnvVar({
+                        cwd: params.projectRoot.fsPath,
+                        env: await getSpawnEnv(process.env),
+                    }),
+                })
+
+                // Run SAM build in Terminal
+                await runInTerminal(buildProcess, 'build')
+            }
+        )
 
         return {
             isSuccess: true,
@@ -252,6 +270,8 @@ export async function runBuild(arg?: TreeNode): Promise<SamBuildResult> {
             details: { terminal: getTerminalFromError(error), ...resolveBuildArgConflict(buildFlags) },
             code: getErrorCode(error),
         })
+    } finally {
+        await unregisterTemplateBuild(params.template.uri.path)
     }
 }
 
@@ -293,3 +313,34 @@ export async function resolveBuildFlags(buildFlags: string[], samCliVersion: Sem
     }
     return buildFlags
 }
+
+/**
+ * Returns true if there's an ongoing build process for the provided template, false otherwise
+ * @Param templatePath The path to the template.yaml file
+ */
+function isBuildInProgress(templatePath: string): boolean {
+    const expirationDate = getRecentResponse(buildProcessMementoRootKey, globalIdentifier, templatePath)
+    if (expirationDate) {
+        return Date.now() < parseInt(expirationDate)
+    }
+    return false
+}
+
+/**
+ * Throws an error if there's a build in progress for the provided template
+ * @Param templatePath The path to the template.yaml file
+ */
+export function throwIfTemplateIsBeingBuilt(templatePath: string) {
+    if (isBuildInProgress(templatePath)) {
+        throw new ToolkitError('This template is already being built', { code: 'BuildInProgress' })
+    }
+}
+
+export async function registerTemplateBuild(templatePath: string) {
+    const expirationDate = Date.now() + 5 * 60 * 1000 // five minutes
+    await updateRecentResponse(buildProcessMementoRootKey, globalIdentifier, templatePath, expirationDate.toString())
+}
+
+export async function unregisterTemplateBuild(templatePath: string) {
+    await updateRecentResponse(buildProcessMementoRootKey, globalIdentifier, templatePath, undefined)
+}

packages/core/src/shared/sam/constants.ts

@@ -0,0 +1,10 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+export const buildProcessMementoRootKey = 'samcli.build.processes'
+export const globalIdentifier = 'global'
+export const buildMementoRootKey = 'samcli.build.params'
+export const deployMementoRootKey = 'samcli.deploy.params'
+export const syncMementoRootKey = 'samcli.sync.params'

packages/core/src/shared/sam/deploy.ts

@@ -26,6 +26,7 @@ import { TemplateItem, createTemplatePrompter } from '../ui/sam/templatePrompter
 import { createDeployParamsSourcePrompter, ParamsSource } from '../ui/sam/paramsSourcePrompter'
 import { getErrorCode, getProjectRoot, getSamCliPathAndVersion, getSource, updateRecentResponse } from './utils'
 import { runInTerminal } from './processTerminal'
+import { deployMementoRootKey } from './constants'
 import {
     TemplateParametersForm,
     TemplateParametersWizard,
@@ -64,7 +65,6 @@ function getDeployEntryPoint(arg: vscode.Uri | AWSTreeNodeBase | TreeNode | unde
         return SamDeployEntryPoints.CommandPalette
     }
 }
-const deployMementoRootKey = 'samcli.deploy.params'
 
 type DeployResult = {
     isSuccess: boolean

packages/core/src/shared/sam/sync.ts

@@ -51,6 +51,7 @@ import { ParamsSource, createSyncParamsSourcePrompter } from '../ui/sam/paramsSo
 import { createEcrPrompter } from '../ui/sam/ecrPrompter'
 import { BucketSource, createBucketNamePrompter, createBucketSourcePrompter } from '../ui/sam/bucketPrompter'
 import { runInTerminal } from './processTerminal'
+import { syncMementoRootKey } from './constants'
 import {
     TemplateParametersForm,
     TemplateParametersWizard,
@@ -73,8 +74,6 @@ export interface SyncParams {
     readonly syncFlags?: string
 }
 
-export const syncMementoRootKey = 'samcli.sync.params'
-
 // TODO: hook this up so it prompts the user when more than 1 environment is present in `samconfig.toml`
 export function createEnvironmentPrompter(config: SamConfig, environments = config.listEnvironments()) {
     const recentEnvironmentName = getRecentResponse(syncMementoRootKey, config.location.fsPath, 'environmentName')

packages/core/src/shared/sam/utils.ts

@@ -108,6 +108,7 @@ export async function updateRecentResponse(
         getLogger().warn(`sam: unable to save response at key "${key}": %s`, err)
     }
 }
+
 export function getSamCliErrorMessage(stderr: string): string {
     // Split the stderr string by newline, filter out empty lines, and get the last line
     const lines = stderr

packages/core/src/test/shared/sam/build.test.ts

@@ -18,6 +18,7 @@ import {
     ParamsSource,
     resolveBuildFlags,
     runBuild,
+    SamBuildResult,
 } from '../../../shared/sam/build'
 import { TreeNode } from '../../../shared/treeview/resourceTreeDataProvider'
 import { createWizardTester } from '../wizards/wizardTestUtils'
@@ -34,6 +35,7 @@ import { samconfigCompleteData, validTemplateData } from './samTestUtils'
 import { CloudFormationTemplateRegistry } from '../../../shared/fs/templateRegistry'
 import { getTestWindow } from '../vscode/window'
 import { CancellationError } from '../../../shared/utilities/timeoutUtils'
+import { SamAppLocation } from '../../../awsService/appBuilder/explorer/samProject'
 import { SemVer } from 'semver'
 
 describe('SAM BuildWizard', async function () {
@@ -423,22 +425,8 @@ describe('SAM runBuild', () => {
             verifyCorrectDependencyCall()
         })
 
-        it('[entry: appbuilder node] with default flags should instantiate correct process in terminal', async () => {
-            const prompterTester = PrompterTester.init()
-                .handleQuickPick('Specify parameter source for build', async (quickPick) => {
-                    // Need sometime to wait for the template to search for template file
-                    await quickPick.untilReady()
-
-                    assert.strictEqual(quickPick.items.length, 2)
-                    const items = quickPick.items
-                    assert.strictEqual(quickPick.items.length, 2)
-                    assert.deepStrictEqual(items[0], { data: ParamsSource.Specify, label: 'Specify build flags' })
-                    assert.deepStrictEqual(items[1].label, 'Use default values')
-                    quickPick.acceptItem(quickPick.items[1])
-                })
-                .build()
-
-            // Invoke sync command from command palette
+        it('[entry: appbuilder node] with default flags should instantiate correct process in terminal and show progress notification', async () => {
+            const prompterTester = getPrompterTester()
             const expectedSamAppLocation = {
                 workspaceFolder: workspaceFolder,
                 samTemplateUri: templateFile,
@@ -447,6 +435,10 @@ describe('SAM runBuild', () => {
 
             await runBuild(new AppNode(expectedSamAppLocation))
 
+            getTestWindow()
+                .getFirstMessage()
+                .assertProgress(`Building SAM template at ${expectedSamAppLocation.samTemplateUri.path}`)
+
             assert.deepEqual(mockChildProcessClass.getCall(0).args, [
                 'sam-cli-path',
                 [
@@ -472,6 +464,27 @@ describe('SAM runBuild', () => {
             prompterTester.assertCallAll()
         })
 
+        it('[entry: appbuilder node] should throw an error when running two build processes in parallel for the same template', async () => {
+            const prompterTester = getPrompterTester()
+            const expectedSamAppLocation = {
+                workspaceFolder: workspaceFolder,
+                samTemplateUri: templateFile,
+                projectRoot: projectRoot,
+            }
+            await assert.rejects(
+                async () => {
+                    await runInParallel(expectedSamAppLocation)
+                },
+                (e: any) => {
+                    assert.strictEqual(e instanceof ToolkitError, true)
+                    assert.strictEqual(e.message, 'This template is already being built')
+                    assert.strictEqual(e.code, 'BuildInProgress')
+                    return true
+                }
+            )
+            prompterTester.assertCallAll(undefined, 2)
+        })
+
         it('[entry: command palette] use samconfig should instantiate correct process in terminal', async () => {
             const samconfigFile = vscode.Uri.file(await testFolder.write('samconfig.toml', samconfigCompleteData))
 
@@ -587,6 +600,38 @@ describe('SAM runBuild', () => {
     })
 })
 
+async function runInParallel(samLocation: SamAppLocation): Promise<[SamBuildResult, SamBuildResult]> {
+    return Promise.all([runBuild(new AppNode(samLocation)), delayedRunBuild(samLocation)])
+}
+
+// We add a small delay to avoid the unlikely but possible race condition.
+async function delayedRunBuild(samLocation: SamAppLocation): Promise<SamBuildResult> {
+    return new Promise(async (resolve, reject) => {
+        // Add a small delay before returning the build promise
+        setTimeout(() => {
+            // Do nothing, just let the delay pass
+        }, 20)
+
+        const buildPromise = runBuild(new AppNode(samLocation))
+        buildPromise.then(resolve).catch(reject)
+    })
+}
+
+function getPrompterTester() {
+    return PrompterTester.init()
+        .handleQuickPick('Specify parameter source for build', async (quickPick) => {
+            // Need sometime to wait for the template to search for template file
+            await quickPick.untilReady()
+
+            assert.strictEqual(quickPick.items.length, 2)
+            const items = quickPick.items
+            assert.strictEqual(quickPick.items.length, 2)
+            assert.deepStrictEqual(items[0], { data: ParamsSource.Specify, label: 'Specify build flags' })
+            assert.deepStrictEqual(items[1].label, 'Use default values')
+            quickPick.acceptItem(quickPick.items[1])
+        })
+        .build()
+}
 function testResolveBuildFlags(
     sandbox: sinon.SinonSandbox,
     parsedVersion: SemVer,

packages/core/src/testInteg/sam.test.ts

@@ -551,7 +551,7 @@ describe('SAM Integration Tests', async function () {
     describe('SAM Build', async function () {
         let workspaceFolder: vscode.WorkspaceFolder
         // We're testing only one case (python 3.10 (ZIP)) on the integration tests. More niche cases are handled as unit tests.
-        const scenarioIndex = 2
+        const scenarioIndex = 3
         const scenario = scenarios[scenarioIndex]
         let testRoot: string
         let sandbox: sinon.SinonSandbox

packages/toolkit/.changes/next-release/Bug Fix-db0d3605-5cb7-4b78-b4ae-29be86f51f99.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "SAM build: prevent running multiple build processes for the same template"
+}