Skip to content

Commit d539e9e

Browse files
justinmk3justinmk3
authored
"Create SAM app": default to a more intuitive name #1467
Problem: Before this commit the "AWS: Create new SAM application" command defaults the application folder name based on the parent folder: https://github.com/aws/aws-toolkit-vscode/blob/44dad848fbed20e3b081dfa4cad6f41e00e05a0c/src/lambda/wizards/samInitWizard.ts#L445 In cloud9 this always is `environment`, which is not helpful. Solution: Default to a name like "lambda-python3.8", and append a number if such a name already exists in the workspace.
1 parent 0a2a05a commit d539e9e

File tree

5 files changed

+105
-32
lines changed

5 files changed

+105
-32
lines changed

‎.changes/next-release/Feature-298a87ca-c8dc-4522-a990-529eb66b7f55.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Feature",
3+
"description": "\"Create new SAM Application\" command suggests a more-intuitive name"
4+
}

‎src/lambda/wizards/samInitWizard.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
} from '../models/samTemplates'
3838
import * as semver from 'semver'
3939
import { MINIMUM_SAM_CLI_VERSION_INCLUSIVE_FOR_IMAGE_SUPPORT } from '../../shared/sam/cli/samCliValidator'
40+
import * as fsutil from '../../shared/filesystemUtilities'
4041

4142
const localize = nls.loadMessageBundle()
4243

@@ -456,9 +457,9 @@ export class CreateNewSamAppWizard extends MultiStepWizard<CreateNewSamAppWizard
456457
}
457458

458459
private readonly NAME: WizardStep = async () => {
459-
this.name = await this.context.promptUserForName(
460-
this.name ?? (this.location ? path.basename(this.location.path) : '')
461-
)
460+
// Default to a name like "lambda-python3.8-1".
461+
const defaultName = fsutil.getNonexistentFilename(this.location!.fsPath, `lambda-${this.runtime}`, '', 99)
462+
this.name = await this.context.promptUserForName(this.name ?? defaultName)
462463

463464
return this.name ? WIZARD_TERMINATE : WIZARD_GOBACK
464465
}

‎src/shared/filesystemUtilities.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*/
55

66
import { access, mkdtemp, mkdirp, readFile, remove, existsSync } from 'fs-extra'
7+
import * as crypto from 'crypto'
8+
import * as fs from 'fs'
79
import * as os from 'os'
810
import * as path from 'path'
911
import { getLogger } from './logger'
@@ -115,3 +117,33 @@ export function isInDirectory(d: string, p: string): boolean {
115117
: value === containedPathPieces[index]
116118
})
117119
}
120+
121+
/**
122+
* Returns `name.suffix` if it does not already exist in directory `dir`, else appends
123+
* a number ("foo-1.txt", "foo-2.txt", etc.).
124+
*
125+
* To avoid excessive filesystem activity, if all filenames up to `max` exist,
126+
* the function instead appends a random string.
127+
*
128+
* @param dir Path to a directory
129+
* @param name Filename without extension
130+
* @param suffix Filename suffix, typically an extension (".txt"), may be empty
131+
* @param max Stop searching if all permutations up to this number exist
132+
*/
133+
export function getNonexistentFilename(dir: string, name: string, suffix: string, max: number = 99): string {
134+
if (!name) {
135+
throw new Error(`name is empty`)
136+
}
137+
if (!fs.existsSync(dir)) {
138+
throw new Error(`directory does not exist: ${dir}`)
139+
}
140+
for (let i = 0; true; i++) {
141+
const filename = i == 0
142+
? `${name}${suffix}`
143+
: `${name}-${i < max ? i : crypto.randomBytes(4).toString('hex')}${suffix}`
144+
const fullpath = path.join(dir, filename)
145+
if (!fs.existsSync(fullpath) || i >= max + 99) {
146+
return filename
147+
}
148+
}
149+
}

‎src/test/lambda/wizards/samInitWizard.test.ts‎

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as assert from 'assert'
77
import { Runtime } from 'aws-sdk/clients/lambda'
88
import { Set } from 'immutable'
99
import * as path from 'path'
10+
import * as fs from 'fs'
1011
import * as vscode from 'vscode'
1112
import {
1213
eventBridgeHelloWorldTemplate,
@@ -20,6 +21,8 @@ import {
2021
CreateNewSamAppWizardResponse,
2122
} from '../../../lambda/wizards/samInitWizard'
2223
import { RuntimePackageType } from '../../../lambda/models/samLambdaRuntime'
24+
import { makeTemporaryToolkitFolder } from '../../../shared/filesystemUtilities'
25+
import { assertEqualPaths } from '../../testUtil'
2326

2427
function isMultiDimensionalArray(array: any[] | any[][] | undefined): boolean {
2528
if (!array) {
@@ -203,13 +206,22 @@ class MockCreateNewSamAppWizardContext implements CreateNewSamAppWizardContext {
203206
}
204207

205208
describe('CreateNewSamAppWizard', async () => {
209+
let dir: string
210+
let dir2: string
211+
before(async () => {
212+
dir = await makeTemporaryToolkitFolder()
213+
dir2 = await makeTemporaryToolkitFolder()
214+
})
215+
after(async () => {
216+
fs.rmdirSync(dir)
217+
})
206218
describe('runtime', async () => {
207219
it('uses user response as runtime', async () => {
208220
const context: CreateNewSamAppWizardContext = new MockCreateNewSamAppWizardContext(
209221
[],
210222
Set<Runtime>(['nodejs8.10']),
211223
'myName',
212-
[vscode.Uri.file(path.join('my', 'workspace', 'folder'))],
224+
[vscode.Uri.file(dir)],
213225
Set<SamTemplate>([helloWorldTemplate]),
214226
[],
215227
[],
@@ -227,7 +239,7 @@ describe('CreateNewSamAppWizard', async () => {
227239
[],
228240
Set<Runtime>(),
229241
'myName',
230-
[vscode.Uri.file(path.join('my', 'workspace', 'folder'))],
242+
[vscode.Uri.file(dir)],
231243
[],
232244
[],
233245
[],
@@ -242,12 +254,11 @@ describe('CreateNewSamAppWizard', async () => {
242254

243255
describe('template', async () => {
244256
it('uses user response as template', async () => {
245-
const locationPath = path.join('my', 'quick', 'pick', 'result')
246257
const context: CreateNewSamAppWizardContext = new MockCreateNewSamAppWizardContext(
247258
[],
248259
Set<Runtime>(['nodejs8.10']),
249260
'myName',
250-
[vscode.Uri.file(locationPath)],
261+
[vscode.Uri.file(dir)],
251262
Set<SamTemplate>([helloWorldTemplate]),
252263
[],
253264
[],
@@ -261,12 +272,11 @@ describe('CreateNewSamAppWizard', async () => {
261272
})
262273

263274
it('backtracks when cancelled', async () => {
264-
const locationPath = path.join('my', 'quick', 'pick', 'result')
265275
const context: CreateNewSamAppWizardContext = new MockCreateNewSamAppWizardContext(
266276
[],
267277
[Set<Runtime>(['python3.6']), Set<Runtime>(['nodejs8.10'])],
268278
'myName',
269-
[vscode.Uri.file(locationPath)],
279+
[vscode.Uri.file(dir)],
270280
[undefined, Set<SamTemplate>([helloWorldTemplate])],
271281
[],
272282
[],
@@ -281,7 +291,13 @@ describe('CreateNewSamAppWizard', async () => {
281291
})
282292

283293
describe('eventBridge-schema-app template', async () => {
284-
const locationPath = path.join('my', 'quick', 'pick', 'result')
294+
let locationPath: string
295+
before(async () => {
296+
locationPath = await makeTemporaryToolkitFolder()
297+
})
298+
after(async () => {
299+
fs.rmdirSync(locationPath)
300+
})
285301
let context: CreateNewSamAppWizardContext
286302
let wizard: CreateNewSamAppWizard
287303
let args: CreateNewSamAppWizardResponse | undefined
@@ -382,7 +398,7 @@ describe('CreateNewSamAppWizard', async () => {
382398
describe('location', async () => {
383399
it('uses user response as schema', async () => {
384400
assert.ok(args)
385-
assert.strictEqual(args!.location.fsPath, `${path.sep}${locationPath}`)
401+
assertEqualPaths(args!.location.fsPath, locationPath)
386402
})
387403

388404
it('backtracks when cancelled', async () => {
@@ -401,20 +417,19 @@ describe('CreateNewSamAppWizard', async () => {
401417

402418
assert.ok(args)
403419
assert.strictEqual(args!.schemaName, 'AWSBatchJobStateChange')
404-
assert.strictEqual(args!.location.fsPath, `${path.sep}${locationPath}`)
420+
assertEqualPaths(args!.location.fsPath, locationPath)
405421
})
406422
})
407423
})
408424
})
409425

410426
describe('location', async () => {
411427
it('uses user response as location', async () => {
412-
const locationPath = path.join('my', 'quick', 'pick', 'result')
413428
const context: CreateNewSamAppWizardContext = new MockCreateNewSamAppWizardContext(
414429
[],
415430
Set<Runtime>(['nodejs8.10']),
416431
'myName',
417-
[vscode.Uri.file(locationPath)],
432+
[vscode.Uri.file(dir)],
418433
Set<SamTemplate>([helloWorldTemplate]),
419434
[],
420435
[],
@@ -424,16 +439,15 @@ describe('CreateNewSamAppWizard', async () => {
424439
const args = await wizard.run()
425440

426441
assert.ok(args)
427-
assert.strictEqual(args!.location.fsPath, `${path.sep}${locationPath}`)
442+
assertEqualPaths(args!.location.fsPath, dir)
428443
})
429444

430445
it('backtracks when cancelled', async () => {
431-
const locationPath = path.join('my', 'quick', 'pick', 'result')
432446
const context: CreateNewSamAppWizardContext = new MockCreateNewSamAppWizardContext(
433447
[],
434448
[Set<Runtime>(['python3.6']), Set<Runtime>(['nodejs8.10'])],
435449
'myName',
436-
[undefined, [vscode.Uri.file(locationPath)]],
450+
[undefined, [vscode.Uri.file(dir)]],
437451
[Set<SamTemplate>([helloWorldTemplate]), Set<SamTemplate>([eventBridgeHelloWorldTemplate])],
438452
[],
439453
[],
@@ -444,18 +458,16 @@ describe('CreateNewSamAppWizard', async () => {
444458

445459
assert.ok(args)
446460
assert.strictEqual(args!.template, eventBridgeHelloWorldTemplate)
447-
assert.strictEqual(args!.location.fsPath, `${path.sep}${locationPath}`)
461+
assertEqualPaths(args!.location.fsPath, dir)
448462
})
449463

450464
it("contains a 'browse' option", async () => {
451465
const name = 'myInputBoxResult'
452-
const locationPath = path.join('my', 'quick', 'pick', 'result')
453-
454466
const context: CreateNewSamAppWizardContext = new MockCreateNewSamAppWizardContext(
455467
[],
456468
Set<Runtime>(['nodejs8.10']),
457469
name,
458-
[vscode.Uri.file(locationPath)],
470+
[vscode.Uri.file(dir)],
459471
Set<SamTemplate>([helloWorldTemplate]),
460472
[],
461473
[],
@@ -465,11 +477,11 @@ describe('CreateNewSamAppWizard', async () => {
465477
const args = await wizard.run()
466478

467479
assert.ok(args)
468-
assert.strictEqual(args!.location.fsPath, `${path.sep}${locationPath}`)
480+
assertEqualPaths(args!.location.fsPath, dir)
469481
})
470482

471483
it('contains an option for each workspace folder', async () => {
472-
const workspaceFolderPaths = [path.join('workspace', 'folder', '1'), path.join('workspace', 'folder', '2')]
484+
const workspaceFolderPaths = [dir, dir2]
473485

474486
let index = 0
475487
const context: CreateNewSamAppWizardContext = new MockCreateNewSamAppWizardContext(
@@ -490,7 +502,7 @@ describe('CreateNewSamAppWizard', async () => {
490502
const args = await wizard.run()
491503

492504
assert.ok(args)
493-
assert.strictEqual(args!.location.fsPath, `${path.sep}${workspaceFolderPaths[0]}`)
505+
assertEqualPaths(args!.location.fsPath, workspaceFolderPaths[0])
494506
})
495507
})
496508

@@ -500,7 +512,7 @@ describe('CreateNewSamAppWizard', async () => {
500512
[],
501513
Set<Runtime>(['nodejs8.10']),
502514
'myName',
503-
[vscode.Uri.file(path.join('my', 'quick', 'pick', 'result'))],
515+
[vscode.Uri.file(dir)],
504516
Set<SamTemplate>([helloWorldTemplate]),
505517
[],
506518
[],
@@ -518,10 +530,7 @@ describe('CreateNewSamAppWizard', async () => {
518530
[],
519531
Set<Runtime>(['nodejs8.10']),
520532
['', 'myName'],
521-
[
522-
[vscode.Uri.file(path.join('my', 'quick', 'pick', 'result', '1'))],
523-
[vscode.Uri.file(path.join('my', 'quick', 'pick', 'result', '2'))],
524-
],
533+
[[vscode.Uri.file(dir)], [vscode.Uri.file(dir2)]],
525534
Set<SamTemplate>([helloWorldTemplate]),
526535
[],
527536
[],
@@ -531,7 +540,7 @@ describe('CreateNewSamAppWizard', async () => {
531540
const args = await wizard.run()
532541

533542
assert.ok(args)
534-
assert.strictEqual(args!.location.fsPath, `${path.sep}${path.join('my', 'quick', 'pick', 'result', '2')}`)
543+
assertEqualPaths(args!.location.fsPath, dir2)
535544
assert.strictEqual(args!.name, 'myName')
536545
})
537546
})

‎src/test/shared/filesystemUtilities.test.ts‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as assert from 'assert'
77
import * as os from 'os'
88
import { writeFile, remove } from 'fs-extra'
99
import * as path from 'path'
10-
import { fileExists, isInDirectory, makeTemporaryToolkitFolder, tempDirPath } from '../../shared/filesystemUtilities'
10+
import { fileExists, getNonexistentFilename, isInDirectory, makeTemporaryToolkitFolder, tempDirPath } from '../../shared/filesystemUtilities'
1111

1212
describe('filesystemUtilities', () => {
1313
const targetFilename = 'findThisFile12345.txt'
@@ -30,8 +30,35 @@ describe('filesystemUtilities', () => {
3030
await remove(folder)
3131
}
3232
})
33+
34+
describe('getNonexistentFilename()', () => {
35+
it('failure modes', async () => {
36+
assert.throws(() => {
37+
getNonexistentFilename('/bogus/directory/', 'foo', '.txt', 99)
38+
})
39+
assert.throws(() => {
40+
getNonexistentFilename('', 'foo', '.txt', 99)
41+
})
42+
})
43+
it('returns a filename that does not exist in the directory', async () => {
44+
const dir = tempFolder
45+
await writeFile(path.join(dir, 'foo.txt'), '', 'utf8')
46+
await writeFile(path.join(dir, 'foo-0.txt'), '', 'utf8')
47+
await writeFile(path.join(dir, 'foo-1.txt'), '', 'utf8')
48+
await writeFile(path.join(dir, 'foo-2.txt'), '', 'utf8')
49+
assert.strictEqual(getNonexistentFilename(dir, 'foo', '.txt', 99), 'foo-3.txt')
50+
assert.strictEqual(getNonexistentFilename(dir, 'foo', '', 99), 'foo')
51+
})
52+
it('returns "foo-RANDOM.txt" if max is reached', async () => {
53+
const dir = tempFolder
54+
await writeFile(path.join(dir, 'foo.txt'), '', 'utf8')
55+
await writeFile(path.join(dir, 'foo-1.txt'), '', 'utf8')
56+
// Looks like "foo-75446d5d.txt".
57+
assert.ok(/^foo-[a-fA-F0-9]{8}.txt$/.test(getNonexistentFilename(dir, 'foo', '.txt', 1)))
58+
})
59+
})
3360

34-
describe('makeTemporaryToolkitFolder', () => {
61+
describe('makeTemporaryToolkitFolder()', () => {
3562
it(`makes temp dirs as children to filesystemUtilities.tempDirPath ('${tempDirPath}')`, async () => {
3663
const parentFolder = path.dirname(tempFolder)
3764

0 commit comments

Comments
 (0)