Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: refactor amplify.json file handling #5282

Merged
merged 1 commit into from
Sep 11, 2020
Merged

fix: refactor amplify.json file handling #5282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 83 additions & 70 deletions .circleci/config.yml
View file
Open in desktop

Large diffs are not rendered by default.

155 changes: 85 additions & 70 deletions packages/amplify-cli-core/src/__tests__/featureFlags.test.ts
View file
Open in desktop
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
0 ...tFiles/project-with-features/amplify.json → .../testFiles/project-with-features/cli.json
View file
Open in desktop
File renamed without changes.
0 ...les/project-with-no-features/amplify.json → ...stFiles/project-with-no-features/cli.json
View file
Open in desktop
File renamed without changes.
0 ...s/testProject-both-files/amplify.dev.json → ...stProject-both-files/amplify/cli.dev.json
View file
Open in desktop
File renamed without changes.
0 ...Files/testProject-both-files/amplify.json → ...s/testProject-both-files/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 ...les/testProject-initialize-1/amplify.json → ...testProject-initialize-1/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 ...estProject-initialize-1/amplify.prod.json → ...roject-initialize-1/amplify/cli.prod.json
View file
Open in desktop
File renamed without changes.
0 ...testProject-initialize-2/amplify.dev.json → ...Project-initialize-2/amplify/cli.dev.json
View file
Open in desktop
File renamed without changes.
0 ...les/testProject-initialize-2/amplify.json → ...testProject-initialize-2/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 ...estProject-initialize-2/amplify.prod.json → ...roject-initialize-2/amplify/cli.prod.json
View file
Open in desktop
File renamed without changes.
0 ...testProject-initialize-3/amplify.dev.json → ...Project-initialize-3/amplify/cli.dev.json
View file
Open in desktop
File renamed without changes.
0 ...les/testProject-initialize-3/amplify.json → ...testProject-initialize-3/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 ...estProject-initialize-3/amplify.prod.json → ...roject-initialize-3/amplify/cli.prod.json
View file
Open in desktop
File renamed without changes.
0 ...testProject-initialize-4/amplify.dev.json → ...Project-initialize-4/amplify/cli.dev.json
View file
Open in desktop
File renamed without changes.
0 ...les/testProject-initialize-4/amplify.json → ...testProject-initialize-4/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 ...estProject-initialize-4/amplify.prod.json → ...roject-initialize-4/amplify/cli.prod.json
View file
Open in desktop
File renamed without changes.
0 ...roject-initialize-json-error/amplify.json → ...ct-initialize-json-error/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 ...s/testProject-initialize/amplify.dev.json → ...stProject-initialize/amplify/cli.dev.json
View file
Open in desktop
File renamed without changes.
0 ...Files/testProject-initialize/amplify.json → ...s/testProject-initialize/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 .../testProject-initialize/amplify.prod.json → ...tProject-initialize/amplify/cli.prod.json
View file
Open in desktop
File renamed without changes.
0 ...testFiles/testProject-no-env/amplify.json → ...Files/testProject-no-env/amplify/cli.json
View file
Open in desktop
File renamed without changes.
0 ...s/testProject-no-project/amplify.dev.json → ...stProject-no-project/amplify/cli.dev.json
View file
Open in desktop
File renamed without changes.
2 changes: 0 additions & 2 deletions packages/amplify-cli-core/src/constants.ts
View file
Open in desktop

This file was deleted.

27 changes: 10 additions & 17 deletions packages/amplify-cli-core/src/feature-flags/featureFlagFileProvider.ts
View file
Open in desktop
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