Skip to content

Commit

Permalink
fix: async persist env params on success (#11367)
Browse files Browse the repository at this point in the history
  • Loading branch information
edwardfoyle authored Nov 15, 2022
1 parent 082b6f6 commit 0433431
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 20 deletions.
1 change: 0 additions & 1 deletion packages/amplify-cli-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,3 @@ export * from './errors/amplify-error';
export * from './errors/amplify-fault';
export * from './errors/amplify-exception';
export * from './errors/project-not-initialized-error';
/* eslint-enable */
7 changes: 6 additions & 1 deletion packages/amplify-cli/src/__tests__/commands/status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('amplify status:', () => {
const { run } = require('../../commands/status');
const runStatusCmd = run;
const statusPluginInfo = `${process.cwd()}/../amplify-console-hosting`;
const mockPath = './';
const mockPath = './help';

afterAll(() => {
jest.clearAllMocks();
Expand All @@ -53,6 +53,7 @@ describe('amplify status:', () => {
showGlobalSandboxModeWarning: jest.fn(),
showHelpfulProviderLinks: jest.fn(),
getCategoryPluginInfo: jest.fn().mockReturnValue({ packageLocation: mockPath }),
pathManager: pathManagerMock,
},
parameters: {
input: {
Expand All @@ -75,6 +76,7 @@ describe('amplify status:', () => {
showGlobalSandboxModeWarning: jest.fn(),
showHelpfulProviderLinks: jest.fn(),
getCategoryPluginInfo: jest.fn().mockReturnValue({ packageLocation: statusPluginInfo }),
pathManager: pathManagerMock,
},
input: {
command: 'status',
Expand All @@ -94,6 +96,7 @@ describe('amplify status:', () => {
showGlobalSandboxModeWarning: jest.fn(),
showHelpfulProviderLinks: jest.fn(),
getCategoryPluginInfo: jest.fn().mockReturnValue({ packageLocation: statusPluginInfo }),
pathManager: pathManagerMock,
},
input: {
command: 'status',
Expand All @@ -116,6 +119,7 @@ describe('amplify status:', () => {
showGlobalSandboxModeWarning: jest.fn(),
showHelpfulProviderLinks: jest.fn(),
getCategoryPluginInfo: jest.fn().mockReturnValue({ packageLocation: statusPluginInfo }),
pathManager: pathManagerMock,
},
input: {
command: 'status',
Expand All @@ -134,6 +138,7 @@ describe('amplify status:', () => {
showGlobalSandboxModeWarning: jest.fn(),
showHelpfulProviderLinks: jest.fn(),
getCategoryPluginInfo: jest.fn().mockReturnValue({ packageLocation: statusPluginInfo }),
pathManager: pathManagerMock,
},
input: {
command: 'status',
Expand Down
3 changes: 3 additions & 0 deletions packages/amplify-cli/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { EventEmitter } from 'events';
import * as fs from 'fs-extra';
import * as path from 'path';
import { printer, prompter } from 'amplify-prompts';
import { saveAll as saveAllEnvParams } from '@aws-amplify/amplify-environment-parameters';
import { logInput } from './conditional-local-logging-init';
import { attachUsageData, constructContext } from './context-manager';
import { displayBannerMessages } from './display-banner-messages';
Expand Down Expand Up @@ -195,6 +196,8 @@ export const run = async (startTime: number): Promise<void> => {
// Checks for available update, defaults to a 1 day interval for notification
notify({ defer: true, isGlobal: true });
}

await saveAllEnvParams();
};

const ensureFilePermissions = (filePath: string): void => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable spellcheck/spell-checker */
import { pathManager, stateManager } from 'amplify-cli-core';
import { IEnvironmentParameterManager } from '../environment-parameter-manager';
import { IEnvironmentParameterManager, saveAll } from '../environment-parameter-manager';

jest.mock('amplify-cli-core');
const stateManagerMock = stateManager as jest.Mocked<typeof stateManager>;
Expand Down Expand Up @@ -34,20 +33,8 @@ beforeEach(() => {

describe('init', () => {
it('loads params and registers save on exit listener', async () => {
// eslint-disable-next-line @typescript-eslint/ban-types
const processEventListeners: Record<string | symbol, Function[]> = {};
jest.spyOn(process, 'on').mockImplementation((event, func) => {
if (Array.isArray(processEventListeners[event])) {
processEventListeners[event].push(func);
} else {
processEventListeners[event] = [func];
}
return process;
});
await ensureEnvParamManager();
// it's important that the save callback is registered on exit instead of beforeExit
// because if save fails, beforeExit will be called again
processEventListeners.exit.forEach(fn => fn(0));
await saveAll();
expect(stateManagerMock.setTeamProviderInfo).toHaveBeenCalledWith(undefined, stubTPI);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ export const getEnvParamManager = (envName: string = stateManager.getLocalEnvInf
});
};

/**
* Execute the save method of all currently initialized IEnvironmentParameterManager instances
*/
export const saveAll = async (): Promise<void> => {
for (const envParamManager of Object.values(envParamManagerMap)) {
// save methods must be executed in sequence to avoid race conditions writing to the tpi file
await envParamManager.save();
}
};

/**
* Class for interfacing with environment-specific parameters
*/
Expand All @@ -53,8 +63,6 @@ class EnvironmentParameterManager implements IEnvironmentParameterManager {
this.getResourceParamManager(category, resource).setAllParams(parameters);
});
});

process.on('exit', () => this.save());
}

removeResourceParamManager(category: string, resource: string): void {
Expand All @@ -78,7 +86,7 @@ class EnvironmentParameterManager implements IEnvironmentParameterManager {
return !!this.resourceParamManagers[getResourceKey(category, resource)];
}

save(): void {
async save(): Promise<void> {
if (!pathManager.findProjectRoot()) {
// assume that the project is deleted if we cannot find a project root
return;
Expand Down

0 comments on commit 0433431

Please sign in to comment.