Skip to content

Commit

Permalink
fix(aws-ssm): correctly emit Association.Name
Browse files Browse the repository at this point in the history
The rename used to happen in the cfnspec, which helped the model
generate correct names, but the code generator would lose the ability to
access the original name, and so it could not be emitted correctly.

Add a property renaming feature to the 'cfn2ts' code generator
(configured via a special entry in package.json).

Fixes #852.
  • Loading branch information
Rico Huijbers committed Oct 5, 2018
1 parent a67b2d9 commit d131223
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 50 deletions.
5 changes: 5 additions & 0 deletions packages/@aws-cdk/aws-ssm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@
"cdk-build": {
"cloudformation": "AWS::SSM"
},
"cfn2ts": {
"rename": {
"AWS::SSM::Association/Name": "DocumentName"
}
},
"keywords": [
"aws",
"cdk",
Expand Down
23 changes: 23 additions & 0 deletions packages/@aws-cdk/aws-ssm/test/test.ssm-document.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { expect, haveResource } from '@aws-cdk/assert';
import cdk = require('@aws-cdk/cdk');
import { Test } from 'nodeunit';
import ssm = require('../lib');

export = {
'association name is rendered properly in L1 construct'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new ssm.cloudformation.AssociationResource(stack, 'Assoc', {
documentName: 'document',
});

// THEN
expect(stack).to(haveResource('AWS::SSM::Association', {
Name: 'document',
}));

test.done();
}
};

This file was deleted.

This file was deleted.

79 changes: 55 additions & 24 deletions tools/cfn2ts/lib/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class CodeGenerator {
* @param moduleName the name of the module (used to determine the file name).
* @param spec CloudFormation resource specification
*/
constructor(moduleName: string, private readonly spec: schema.Specification) {
constructor(moduleName: string, private readonly spec: schema.Specification, private readonly renames: {[key: string]: string}) {
this.outputFile = `${moduleName}.generated.ts`;
this.code.openFile(this.outputFile);

Expand Down Expand Up @@ -117,23 +117,26 @@ export default class CodeGenerator {
this.code.closeBlock();
}

private emitPropsType(resourceName: genspec.CodeName, spec: schema.ResourceType): genspec.CodeName | undefined {
if (!spec.Properties || Object.keys(spec.Properties).length === 0) { return; }
const name = genspec.CodeName.forResourceProperties(resourceName);
/**
* Emit the XxxProps interface for a resource
*/
private emitPropsType(resourceName: genspec.CodeName, spec: schema.ResourceType): PropsInterfaceType {
if (!spec.Properties || Object.keys(spec.Properties).length === 0) { return { attributesTable: {}}; }
const codeName = genspec.CodeName.forResourceProperties(resourceName);

this.docLink(spec.Documentation);
this.code.openBlock(`export interface ${name.className}`);
this.code.openBlock(`export interface ${codeName.className}`);

const conversionTable = this.emitPropsTypeProperties(resourceName.specName!, spec.Properties);
const attributesTable = this.emitPropsTypeProperties(resourceName.specName!, spec.Properties);

this.code.closeBlock();

this.code.line();
this.emitValidator(name, spec.Properties, conversionTable);
this.emitValidator(codeName, spec.Properties, attributesTable);
this.code.line();
this.emitCloudFormationMapper(name, spec.Properties, conversionTable);
this.emitCloudFormationMapper(codeName, spec.Properties, attributesTable);

return name;
return { codeName, attributesTable };
}

/**
Expand All @@ -143,28 +146,37 @@ export default class CodeGenerator {
*/
private emitPropsTypeProperties(resourceName: SpecName, propertiesSpec: { [name: string]: schema.Property }): Dictionary<string> {
const propertyMap: Dictionary<string> = {};

// Sanity check that our renamed "Name" is not going to conflict with a real property
const renamedNameProperty = resourceNameProperty(resourceName);
const lowerNames = Object.keys(propertiesSpec).map(s => s.toLowerCase());
if (lowerNames.indexOf('name') !== -1 && lowerNames.indexOf(renamedNameProperty.toLowerCase()) !== -1) {
// tslint:disable-next-line:max-line-length
throw new Error(`Oh gosh, we want to rename ${resourceName.fqn}'s 'Name' property to '${renamedNameProperty}', but that property already exists! We need to find a solution to this problem.`);
}
const finalNames = new Set<string>();

Object.keys(propertiesSpec).sort(propertyComparator).forEach(propName => {
const originalName = propName;
const propSpec = propertiesSpec[propName];
const additionalDocs = resourceName.relativeName(propName).fqn;

// Apply property renames
const renameKey = `${resourceName.fqn}/${propName}`;
if (renameKey in this.renames) {
// User-defined rename
propName = this.renames[renameKey];
}

if (propName.toLocaleLowerCase() === 'name') {
propName = renamedNameProperty;
// We like to include the resource name in the "name" property
propName = resourceNameProperty(resourceName);
}

if (originalName !== propName) {
// tslint:disable-next-line:no-console
console.error(`Renamed property 'Name' of ${resourceName.fqn} to '${renamedNameProperty}'`);
console.error(`[${resourceName.fqn}] Renamed property '${originalName}' to '${propName}'`);
}
if (finalNames.has(propName)) {
throw new Error(`Oh gosh, there was already a property named ${propName}!`);
}
finalNames.add(propName);

const resourceCodeName = genspec.CodeName.forResource(resourceName);
const newName = this.emitProperty(resourceCodeName, propName, propSpec, quoteCode(additionalDocs));

propertyMap[originalName] = newName;
});
return propertyMap;
Expand Down Expand Up @@ -265,16 +277,16 @@ export default class CodeGenerator {
this.code.line(` * @param properties the properties of this ${quoteCode(resourceName.className)}`);
this.code.line(' */');
const optionalProps = spec.Properties && !Object.values(spec.Properties).some(p => p.Required);
const propsArgument = propsType ? `, properties${optionalProps ? '?' : ''}: ${propsType.className}` : '';
const propsArgument = propsType.codeName ? `, properties${optionalProps ? '?' : ''}: ${propsType.codeName.className}` : '';
this.code.openBlock(`constructor(parent: ${CONSTRUCT_CLASS}, name: string${propsArgument})`);
this.code.line(`super(parent, name, { type: ${resourceName.className}.resourceTypeName${propsType ? ', properties' : ''} });`);
// verify all required properties
if (spec.Properties) {
for (const pname of Object.keys(spec.Properties)) {
const prop = spec.Properties[pname];
if (prop.Required) {
const propName = pname.toLocaleLowerCase() === 'name' ? resourceNameProperty(resourceName.specName!) : pname;
this.code.line(`${CORE}.requireProperty(properties, '${genspec.cloudFormationToScriptName(propName)}', this);`);
const attributeName = propsType.attributesTable[pname];
this.code.line(`${CORE}.requireProperty(properties, '${genspec.cloudFormationToScriptName(attributeName)}', this);`);
}
}
}
Expand Down Expand Up @@ -304,9 +316,9 @@ export default class CodeGenerator {

this.code.closeBlock();

if (propsType) {
if (propsType.codeName) {
this.code.line();
this.emitCloudFormationPropertiesOverride(propsType);
this.emitCloudFormationPropertiesOverride(propsType.codeName);
}

this.closeClass(resourceName);
Expand Down Expand Up @@ -619,6 +631,25 @@ export default class CodeGenerator {
}
}

/**
* Represents a PropsType that has been generated
*/
interface PropsInterfaceType {
/**
* Name in the code of the XxxProps type
*
* Missing if no type got generated because there were no properties.
*/
codeName?: genspec.CodeName;

/**
* Name conversion table for properties on this props type
*
* Maps cfn name -> attribute name.
*/
attributesTable: Dictionary<string>;
}

/**
* Return a comma-separated list of validator functions for the given types
*/
Expand Down
18 changes: 17 additions & 1 deletion tools/cfn2ts/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,23 @@ export default async function(scope: string, outPath: string, force: boolean) {
}
const name = packageName(scope);

const generator = new CodeGenerator(name, spec);
// Read package.json in the current directory to determine renames
const renames: {[key: string]: string} = {};
let packageJson;
try {
packageJson = require(path.resolve(process.cwd(), 'package.json'));
} catch (e) {
// Nothing
}
if (packageJson && packageJson.cfn2ts && packageJson.cfn2ts.rename) {
// tslint:disable-next-line:no-console
console.error('Reading renames from package.json');
for (const [key, value] of Object.entries(packageJson.cfn2ts.rename)) {
renames[key] = value as any;
}
}

const generator = new CodeGenerator(name, spec, renames);

if (!force && await generator.upToDate(outPath)) {
// tslint:disable-next-line:no-console
Expand Down

0 comments on commit d131223

Please sign in to comment.