-
Notifications
You must be signed in to change notification settings - Fork 822
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.
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
fix: post-process cfn files for minification, consolidating concern of minification a bit #12072
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
import { | ||
amplifyPush, | ||
deleteProject, | ||
initJSProjectWithProfile, | ||
createNewProjectDir, | ||
deleteProjectDir, | ||
generateRandomShortId, | ||
getProjectMetaPath, | ||
addApiWithBlankSchema, | ||
updateApiSchema, | ||
amplifyPushUpdate, | ||
} from '@aws-amplify/amplify-e2e-core'; | ||
import fs from 'fs'; | ||
import path from 'path'; | ||
|
||
describe('minify behavior', () => { | ||
let projRoot: string; | ||
let projFolderName: string; | ||
|
||
beforeEach(async () => { | ||
projFolderName = `minify${generateRandomShortId()}`; | ||
projRoot = await createNewProjectDir(projFolderName); | ||
}); | ||
|
||
afterEach(async () => { | ||
if (fs.existsSync(getProjectMetaPath(projRoot))) { | ||
await deleteProject(projRoot); | ||
} | ||
deleteProjectDir(projRoot); | ||
}); | ||
|
||
it('reduces file size when minify flag is provided', async () => { | ||
const envName = 'devtest'; | ||
const projName = `minify${generateRandomShortId()}`; | ||
|
||
// Configure and push app without minification | ||
await initJSProjectWithProfile(projRoot, { name: projName, envName }); | ||
await addApiWithBlankSchema(projRoot); | ||
updateApiSchema(projRoot, projName, 'simple_model.graphql', false); | ||
await amplifyPush(projRoot, true, { minify: false }); | ||
|
||
// Read Cfn file sizes for both nested API stacks and top-level stacks | ||
const currentCloudBackendPath = path.join(projRoot, 'amplify', '#current-cloud-backend'); | ||
|
||
const nestedApiStackPath = path.join(currentCloudBackendPath, 'api', projName, 'build', 'stacks', 'Todo.json'); | ||
const rootApiStackPath = path.join( | ||
currentCloudBackendPath, | ||
'awscloudformation', | ||
'build', | ||
'api', | ||
projName, | ||
'build', | ||
'cloudformation-template.json', | ||
); | ||
const rootStackPath = path.join(currentCloudBackendPath, 'awscloudformation', 'build', 'root-cloudformation-stack.json'); | ||
|
||
const unminifiedNestedApiStackDefinition = fs.readFileSync(nestedApiStackPath, 'utf-8'); | ||
const unminifiedRootApiStackDefinition = fs.readFileSync(rootApiStackPath, 'utf-8'); | ||
const unminifiedRootStackDefinition = fs.readFileSync(rootStackPath, 'utf-8'); | ||
|
||
// Tweak schema file and push with minification | ||
updateApiSchema(projRoot, projName, 'simple_model.graphql', true); | ||
const pushParams = { | ||
projRoot, | ||
waitForText: undefined, | ||
useLatestCodebase: true, | ||
destructivePush: false, | ||
overrideTimeout: 0, | ||
minify: true, | ||
}; | ||
await amplifyPushUpdate( | ||
pushParams.projRoot, | ||
pushParams.waitForText, | ||
pushParams.waitForText, | ||
pushParams.destructivePush, | ||
pushParams.overrideTimeout, | ||
pushParams.minify, | ||
); | ||
|
||
// Read Cfn file sizes for both nested API stacks and top-level stacks, verify files are smaller than initial push. | ||
const minifiedNestedApiStackDefinition = fs.readFileSync(nestedApiStackPath, 'utf-8'); | ||
const minifiedRootApiStackDefinition = fs.readFileSync(rootApiStackPath, 'utf-8'); | ||
const minifiedRootStackDefinition = fs.readFileSync(rootStackPath, 'utf-8'); | ||
|
||
expect(minifiedNestedApiStackDefinition.length).toBeLessThan(unminifiedNestedApiStackDefinition.length); | ||
expect(minifiedRootApiStackDefinition.length).toBeLessThan(unminifiedRootApiStackDefinition.length); | ||
expect(minifiedRootStackDefinition.length).toBeLessThan(unminifiedRootStackDefinition.length); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import { minifyJSONFile, minifyAllJSONInFolderRecursively } from '../../utils/minify-json'; | ||
import path from 'path'; | ||
import fs from 'fs-extra'; | ||
import os from 'os'; | ||
|
||
const JSON_STRING = `{ | ||
"city": "Seattle", | ||
"state": "WA" | ||
}`; | ||
|
||
const NON_JSON_STRING = `CITY = Seattle | ||
STATE = WA | ||
`; | ||
|
||
describe('minifyJSONFile', () => { | ||
const writeFileMinifyAndReturnReadContents = (filePath: string, contents: string): string => { | ||
const tmpFilePath = path.join(os.tmpdir(), filePath); | ||
fs.writeFileSync(tmpFilePath, contents); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can this warning be fixed? |
||
minifyJSONFile(tmpFilePath); | ||
return fs.readFileSync(tmpFilePath, 'utf-8'); | ||
}; | ||
|
||
it('minifies JSON files', () => { | ||
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.json', JSON_STRING); | ||
expect(minifiedContents.length).toBeLessThan(JSON_STRING.length); | ||
}); | ||
|
||
it('does not change a non-json file with json contents', () => { | ||
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.dat', JSON_STRING); | ||
expect(minifiedContents.length).toEqual(JSON_STRING.length); | ||
}); | ||
|
||
it('does not change a non-json file with non-json contents', () => { | ||
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.env', NON_JSON_STRING); | ||
expect(minifiedContents.length).toEqual(NON_JSON_STRING.length); | ||
}); | ||
}); | ||
|
||
describe('minifyAllJSONInFolderRecursively', () => { | ||
it('will minify json files, but leave non-json files alone', () => { | ||
// This test will set up a directory structure as follows | ||
// . | ||
// ├── file.json | ||
// ├── config.env | ||
// └── nested_stacks | ||
// ├── nested.env | ||
// ├── nested_1.json | ||
// ├── nested_2.json | ||
// └── sub_nested_stacks | ||
// ├── sub_nested.env | ||
// └── sub_nested.json | ||
// We will then ensure that after a single invocation file.json, nested_1.json, nested_2.json, and sub_nested.json are all minified | ||
// and that config.env is the same length. | ||
const testDirectory = path.join(os.tmpdir(), 'minifyTestDir'); | ||
const nestedStacksDirectory = path.join(testDirectory, 'nested_stacks'); | ||
const subNestedStacksDirectory = path.join(nestedStacksDirectory, 'sub_nested_stacks'); | ||
|
||
// Clean and setup directory structure | ||
fs.removeSync(testDirectory); | ||
fs.mkdirSync(testDirectory); | ||
fs.mkdirSync(nestedStacksDirectory); | ||
fs.mkdirSync(subNestedStacksDirectory); | ||
|
||
// Write files | ||
fs.writeFileSync(path.join(testDirectory, 'file.json'), JSON_STRING); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
|
||
fs.writeFileSync(path.join(testDirectory, 'config.env'), NON_JSON_STRING); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
|
||
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested_1.json'), JSON_STRING); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
|
||
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested_2.json'), JSON_STRING); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
|
||
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested.env'), NON_JSON_STRING); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
|
||
fs.writeFileSync(path.join(subNestedStacksDirectory, 'sub_nested.json'), JSON_STRING); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
|
||
fs.writeFileSync(path.join(subNestedStacksDirectory, 'sub_nested.env'), NON_JSON_STRING); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
|
||
|
||
// Apply recursive minification | ||
minifyAllJSONInFolderRecursively(testDirectory); | ||
|
||
// Verify all `.json` files are minified, and `.env` files are not. | ||
expect(fs.readFileSync(path.join(testDirectory, 'file.json')).length).toBeLessThan(JSON_STRING.length); | ||
expect(fs.readFileSync(path.join(testDirectory, 'config.env')).length).toEqual(NON_JSON_STRING.length); | ||
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested_1.json')).length).toBeLessThan(JSON_STRING.length); | ||
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested_2.json')).length).toBeLessThan(JSON_STRING.length); | ||
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested.env')).length).toEqual(NON_JSON_STRING.length); | ||
expect(fs.readFileSync(path.join(subNestedStacksDirectory, 'sub_nested.json')).length).toBeLessThan(JSON_STRING.length); | ||
expect(fs.readFileSync(path.join(subNestedStacksDirectory, 'sub_nested.env')).length).toEqual(NON_JSON_STRING.length); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import * as fs from 'fs-extra'; | ||
import * as path from 'path'; | ||
|
||
/** | ||
* Given a path to a json file, minify that file by reading in as JSON | ||
* and writing back out without any whitespace. | ||
* @param jsonFilePath the path to the JSON file we are going to minify. | ||
* @returns when the file has been minified and written back to disk. | ||
*/ | ||
export const minifyJSONFile = (jsonFilePath: string): void => { | ||
if (!jsonFilePath.includes('.json')) return; // Don't convert files not ending in `.json` | ||
const originalJSON = fs.readFileSync(jsonFilePath, 'utf-8'); | ||
const minifiedJSON = JSON.stringify(JSON.parse(originalJSON)); | ||
fs.writeFileSync(jsonFilePath, minifiedJSON); | ||
Check failure Code scanning / CodeQL Insecure temporary file
Insecure creation of file in [the os temp dir](1).
Insecure creation of file in [the os temp dir](2).
|
||
}; | ||
|
||
/** | ||
* Recursively walk a folder, and minify (print without whitespace) all the json files you discover | ||
* @param rootPath the top of the tree to walk | ||
*/ | ||
export const minifyAllJSONInFolderRecursively = (rootPath: string): void => { | ||
fs.readdirSync(rootPath).forEach((childHandle) => { | ||
const childPath = path.join(rootPath, childHandle); | ||
if (fs.lstatSync(childPath).isDirectory()) minifyAllJSONInFolderRecursively(childPath); | ||
if (fs.lstatSync(childPath).isFile() && childPath.includes('.json')) minifyJSONFile(childPath); | ||
}); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PushOptions
would be much better.