Skip to content

Commit

Permalink
fix: refactor amplify.json file handling (#5282)
Browse files Browse the repository at this point in the history
  • Loading branch information
Attila Hajdrik authored Sep 11, 2020
1 parent 5a324b2 commit a6269f3
Show file tree
Hide file tree
Showing 50 changed files with 650 additions and 372 deletions.
153 changes: 83 additions & 70 deletions .circleci/config.yml

Large diffs are not rendered by default.

155 changes: 85 additions & 70 deletions packages/amplify-cli-core/src/__tests__/featureFlags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,23 @@ import * as os from 'os';
import * as path from 'path';
import * as rimraf from 'rimraf';
import { v4 as uuid } from 'uuid';
import { EnvVarFormatError, FeatureFlags, CLIEnvironmentProvider, CLIContextEnvironmentProvider, JSONUtilities } from '..';
import { amplifyConfigFileName } from '../constants';
import {
EnvVarFormatError,
FeatureFlags,
CLIEnvironmentProvider,
CLIContextEnvironmentProvider,
JSONUtilities,
pathManager,
pathManager as realPathManager,
FeatureFlagRegistration,
} from '..';
import { FeatureFlagEnvironmentProvider } from '../feature-flags/featureFlagEnvironmentProvider';
import { FeatureFlagFileProvider } from '../feature-flags/featureFlagFileProvider';

// These constants are not exported, hence the redefinition for tests
const amplifyDirName = 'amplify';
const amplifyConfigFileName = 'cli.json';

describe('feature flags', () => {
describe('featureflag provider tests', () => {
const realProcessEnv: NodeJS.ProcessEnv = { ...process.env };
Expand All @@ -29,11 +41,16 @@ describe('feature flags', () => {
try {
await fs.mkdirs(tempProjectDir);

const projectConfigFileName = path.join(tempProjectDir, amplifyConfigFileName);
process.chdir(tempProjectDir);

const projectConfigFileName = pathManager.getCLIJSONFilePath(tempProjectDir);

await fs.mkdirs(path.dirname(projectConfigFileName));

await fs.copyFile(templateConfigFileName, projectConfigFileName);

await FeatureFlags.initialize(({ getCurrentEnvName: () => 'dev' } as unknown) as CLIEnvironmentProvider, tempProjectDir);
await FeatureFlags.initialize(({ getCurrentEnvName: () => 'dev' } as unknown) as CLIEnvironmentProvider, undefined, getTestFlags());

await FeatureFlags.ensureDefaultFeatureFlags(true);

const updatedConfig = JSONUtilities.readJson<any>(projectConfigFileName);
Expand All @@ -54,11 +71,15 @@ describe('feature flags', () => {
try {
await fs.mkdirs(tempProjectDir);

process.chdir(tempProjectDir);

const projectConfigFileName = path.join(tempProjectDir, amplifyConfigFileName);

await fs.mkdirs(path.dirname(projectConfigFileName));

await fs.copyFile(templateConfigFileName, projectConfigFileName);

await FeatureFlags.initialize(({ getCurrentEnvName: () => 'dev' } as unknown) as CLIEnvironmentProvider, tempProjectDir);
await FeatureFlags.initialize(({ getCurrentEnvName: () => 'dev' } as unknown) as CLIEnvironmentProvider, undefined, getTestFlags());
await FeatureFlags.ensureDefaultFeatureFlags(true);

const originalConfig = (await fs.readFile(templateConfigFileName)).toString();
Expand All @@ -77,9 +98,13 @@ describe('feature flags', () => {
try {
await fs.mkdirs(tempProjectDir);

const projectConfigFileName = path.join(tempProjectDir, amplifyConfigFileName);
process.chdir(tempProjectDir);

const projectConfigFileName = path.join(tempProjectDir, amplifyDirName, amplifyConfigFileName);

await FeatureFlags.initialize(({ getCurrentEnvName: () => 'dev' } as unknown) as CLIEnvironmentProvider, tempProjectDir);
await fs.mkdirs(path.dirname(projectConfigFileName));

await FeatureFlags.initialize(({ getCurrentEnvName: () => 'dev' } as unknown) as CLIEnvironmentProvider, undefined, getTestFlags());
await FeatureFlags.ensureDefaultFeatureFlags(true);

const createdConfig = (await fs.readFile(projectConfigFileName)).toString();
Expand All @@ -92,42 +117,10 @@ describe('feature flags', () => {

test('missing environmentProvider argument', async () => {
await expect(async () => {
await FeatureFlags.initialize((undefined as unknown) as CLIContextEnvironmentProvider, (undefined as unknown) as string);
await FeatureFlags.initialize((undefined as unknown) as CLIContextEnvironmentProvider, undefined, getTestFlags());
}).rejects.toThrowError(`'environmentProvider' argument is required`);
});

test('missing projectPath argument for initialize', async () => {
const context: any = {
getEnvInfo: (_: boolean): any => {
return {
envName: 'dev',
};
},
};

const envProvider: CLIEnvironmentProvider = new CLIContextEnvironmentProvider(context);

await expect(async () => {
await FeatureFlags.initialize(envProvider, (undefined as unknown) as string);
}).rejects.toThrowError(`'projectPath' argument is required`);
});

test('projectPath does not exist for initialize', async () => {
const context: any = {
getEnvInfo: (_: boolean): any => {
return {
envName: 'dev',
};
},
};

const envProvider: CLIEnvironmentProvider = new CLIContextEnvironmentProvider(context);

await expect(async () => {
await FeatureFlags.initialize(envProvider, '/foo/bar');
}).rejects.toThrowError(`Project path: '/foo/bar' does not exist.`);
});

test('initialize feature flag provider successfully', async () => {
const context: any = {
getEnvInfo: (_: boolean): any => {
Expand All @@ -143,7 +136,7 @@ describe('feature flags', () => {
// Set current cwd to projectPath for .env to work correctly
process.chdir(projectPath);

await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());

const transformerVersion = FeatureFlags.getNumber('graphQLTransformer.transformerVersion');
const isDefaultQueryEnabled = FeatureFlags.getBoolean('keyTransformer.defaultQuery');
Expand All @@ -152,6 +145,27 @@ describe('feature flags', () => {
expect(isDefaultQueryEnabled).toBe(true);
});

test('initialize feature flag provider successfully with no files and return new project defaults', async () => {
const osTempDir = await fs.realpath(os.tmpdir());
const tempProjectDir = path.join(osTempDir, `amp-${uuid()}`);

try {
await fs.mkdirs(tempProjectDir);

process.chdir(tempProjectDir);

await FeatureFlags.initialize(({ getCurrentEnvName: () => 'dev' } as unknown) as CLIEnvironmentProvider, true, getTestFlags());

const transformerVersion = FeatureFlags.getNumber('graphQLTransformer.transformerVersion');
const isDefaultQueryEnabled = FeatureFlags.getBoolean('keyTransformer.defaultQuery');

expect(transformerVersion).toBe(5);
expect(isDefaultQueryEnabled).toBe(true);
} finally {
rimraf.sync(tempProjectDir);
}
});

test('initialize feature flag provider fail with json error', async () => {
const context: any = {
getEnvInfo: (_: boolean): any => {
Expand All @@ -168,7 +182,7 @@ describe('feature flags', () => {
process.chdir(projectPath);

await expect(async () => {
await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
}).rejects.toThrowError(
`Found '}' where a key name was expected (check your syntax or use quotes if the key name includes {}[],: or whitespace) at line 1,0 >>> Not a json {\n ...`,
);
Expand All @@ -189,7 +203,7 @@ describe('feature flags', () => {
// Set current cwd to projectPath for .env to work correctly
process.chdir(projectPath);

await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
const effectiveFlags = FeatureFlags.getEffectiveFlags();

expect(effectiveFlags).toMatchObject({
Expand Down Expand Up @@ -217,7 +231,7 @@ describe('feature flags', () => {
// Set current cwd to projectPath for .env to work correctly
process.chdir(projectPath);

await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
const effectiveFlags = FeatureFlags.getEffectiveFlags();

expect(effectiveFlags).toMatchObject({
Expand Down Expand Up @@ -245,7 +259,7 @@ describe('feature flags', () => {
// Set current cwd to projectPath for .env to work correctly
process.chdir(projectPath);

await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
const effectiveFlags = FeatureFlags.getEffectiveFlags();

expect(effectiveFlags).toMatchObject({
Expand Down Expand Up @@ -273,7 +287,7 @@ describe('feature flags', () => {
// Set current cwd to projectPath for .env to work correctly
process.chdir(projectPath);

await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
const effectiveFlags = FeatureFlags.getEffectiveFlags();

expect(effectiveFlags).toMatchObject({
Expand Down Expand Up @@ -302,7 +316,7 @@ describe('feature flags', () => {
process.chdir(projectPath);

await expect(async () => {
await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
}).rejects.toThrowError(`Section 'foo' is not registered in feature provider`);
});

Expand All @@ -322,7 +336,7 @@ describe('feature flags', () => {
process.chdir(projectPath);

await expect(async () => {
await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
}).rejects.toThrowError(`Flag 'bar' within 'graphqltransformer' is not registered in feature provider`);
});

Expand All @@ -342,7 +356,7 @@ describe('feature flags', () => {
process.chdir(projectPath);

await expect(async () => {
await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
}).rejects.toThrowError(`Invalid boolean value: 'invalid' for 'defaultquery' in section 'keytransformer'`);
});

Expand All @@ -362,9 +376,30 @@ describe('feature flags', () => {
process.chdir(projectPath);

await expect(async () => {
await FeatureFlags.initialize(envProvider, projectPath);
await FeatureFlags.initialize(envProvider, undefined, getTestFlags());
}).rejects.toThrowError(`Invalid number value: 'invalid' for 'transformerversion' in section 'graphqltransformer'`);
});

const getTestFlags = (): Record<string, FeatureFlagRegistration[]> => {
return {
graphQLTransformer: [
{
name: 'transformerVersion',
type: 'number',
defaultValueForExistingProjects: 4,
defaultValueForNewProjects: 5,
},
],
keyTransformer: [
{
name: 'defaultQuery',
type: 'boolean',
defaultValueForExistingProjects: false,
defaultValueForNewProjects: true,
},
],
};
};
});

describe('environment provider tests', () => {
Expand Down Expand Up @@ -491,26 +526,6 @@ describe('feature flags', () => {
}).rejects.toThrowError(`'projectPath' option is missing`);
});

test('projectPath does not exist', async () => {
const context: any = {
getEnvInfo: (_: boolean): any => {
return {
envName: 'dev',
};
},
};

const envProvider: CLIEnvironmentProvider = new CLIContextEnvironmentProvider(context);

await expect(async () => {
const provider = new FeatureFlagFileProvider(envProvider, {
projectPath: '/foo/bar',
});

await provider.load();
}).rejects.toThrowError(`Project path: '/foo/bar' does not exist.`);
});

test('reads features when both files exists', async () => {
const context: any = {
getEnvInfo: (_: boolean): any => {
Expand Down
2 changes: 0 additions & 2 deletions packages/amplify-cli-core/src/constants.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import * as fs from 'fs-extra';
import * as _ from 'lodash';
import * as path from 'path';
import _ from 'lodash';
import { FeatureFlagConfiguration, FeatureFlagsEntry } from '.';
import { CLIEnvironmentProvider, JSONUtilities } from '..';
import { CLIEnvironmentProvider } from '..';
import { FeatureFlagValueProvider } from './featureFlagValueProvider';
import { amplifyConfigFileName, amplifyConfigEnvFileNameTemplate } from '../constants';
import { stateManager } from '../state-manager';

export type FeatureFlagFileProviderOptions = {
projectPath?: string;
Expand All @@ -18,28 +16,23 @@ export class FeatureFlagFileProvider implements FeatureFlagValueProvider {
throw new Error(`'projectPath' option is missing`);
}

if (!(await fs.pathExists(this.options.projectPath))) {
throw new Error(`Project path: '${this.options.projectPath}' does not exist.`);
}

const result: FeatureFlagConfiguration = {
project: {},
environments: {},
};

// Read project level file exists
const projectConfigFileName = path.join(this.options.projectPath, amplifyConfigFileName);
const projectFeatures = await this.loadConfig(projectConfigFileName);
// Read project level file if exists
const projectFeatures = await this.loadConfig(this.options.projectPath);

if (projectFeatures) {
result.project = projectFeatures;
}

// Read environment level file if we've a valid environment and file exists
// Read environment level file if we have a valid environment and the file exists
const envName = this.environmentProvider.getCurrentEnvName();

if (envName !== '') {
const envConfigFileName = path.join(this.options.projectPath, amplifyConfigEnvFileNameTemplate(envName));
const envFeatures = await this.loadConfig(envConfigFileName);
const envFeatures = await this.loadConfig(this.options.projectPath, envName);

if (envFeatures) {
result.environments[envName] = envFeatures;
Expand All @@ -49,8 +42,8 @@ export class FeatureFlagFileProvider implements FeatureFlagValueProvider {
return result;
};

private loadConfig = async (fileName: string): Promise<FeatureFlagsEntry | undefined> => {
const configFileData = JSONUtilities.readJson<{ features: FeatureFlagsEntry }>(fileName, {
private loadConfig = async (projectPath: string, env?: string): Promise<FeatureFlagsEntry | undefined> => {
const configFileData = <{ features: FeatureFlagsEntry }>stateManager.getCLIJSON(projectPath, env, {
throwIfNotExist: false,
});

Expand Down
Loading

0 comments on commit a6269f3

Please sign in to comment.