Skip to content
This repository has been archived by the owner on Apr 16, 2022. It is now read-only.

Fixes invalid parameterized type resolution for properties containing nested aggregate types. #208

Merged
merged 5 commits into from
Nov 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ and this project adheres to [Semantic
Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Fixed
- Merge PR #208, fixing invalid parameterized type resolution for properties containing nested aggregate types

## [1.9.1] - 2018-10-30
### Changed
Expand Down
2 changes: 1 addition & 1 deletion data/sam_20161031_cfn.json

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions data/sam_20161031_custom_specification.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
{
"PropertyTypes": {
"AWS::Serverless::Function.IAMPolicyDocument": {
"Documentation": "",
"Properties": {
"Version": {
"Required": false,
"PrimitiveType": "String",
"UpdateType": "Mutable"
}
}
},
"AWS::Serverless::Function.EventSource": {
"Properties": {
"Properties": {
Expand Down
2 changes: 1 addition & 1 deletion src/awsData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export const awsExtraDocs = require('../data/aws_extra_docs.json') as AWSExtraDo

export const awsPrimitiveTypes = ['String', 'Integer', 'Boolean', 'Json', 'Double', 'Long', 'Timestamp'];

export const awsComplexTypes = ['Map', 'List'];
export const awsAggregateTypes = ['Map', 'List'];

export type AWSPrimitiveType = 'String' | 'Integer' | 'Boolean' | 'Json' | 'Double' | 'Long' | 'Timestamp';

Expand Down
8 changes: 4 additions & 4 deletions src/resourcesSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ export function rebaseTypeFormat(baseType: string, type: string): string {
return parameterizeTypeFormat(typeName, typeArgument);
}

if (isPrimitiveType(type) || isComplexType(type)) {
if (isPrimitiveType(type) || isAggregateType(type)) {
return type;
}

Expand All @@ -301,11 +301,11 @@ export function isPrimitiveType(type: string) {
return false;
}

export function isComplexType(type: string) {
export function isAggregateType(type: string) {
if (isParameterizedTypeFormat(type)) {
type = getParameterizedTypeName(type);
}
if (!!~awsData.awsComplexTypes.indexOf(type)) {
if (!!~awsData.awsAggregateTypes.indexOf(type)) {
return true;
}
return false;
Expand Down Expand Up @@ -445,7 +445,7 @@ export function getItemType(baseType: string, propType: string, key: string) {
const property = getProperty(propType, key);
if (!property.ItemType) {
return undefined;
} else if (isComplexType(property.ItemType)) {
} else if (isAggregateType(property.ItemType)) {
return property.ItemType;
} else {
return baseType + '.' + property.ItemType;
Expand Down
168 changes: 168 additions & 0 deletions src/test/validatorTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {awsResources} from '../awsData';
import util = require('util');
import { isObject } from 'util';

const clone = require('clone');

describe('validator', () => {

before(() => {
Expand Down Expand Up @@ -915,6 +917,13 @@ describe('validator', () => {
expect(result).to.have.deep.property('templateValid', true);
expect(result['errors']['crit']).to.have.lengthOf(0);
});

it('a template with properties containing nested aggregate types should be validated successfully', () => {
const input = 'testData/valid/yaml/sam_20161031_issue_207.yaml';
let result = validator.validateFile(input);
expect(result).to.have.deep.property('templateValid', true);
expect(result['errors']['crit']).to.have.lengthOf(0);
});
});

describe('parameters-validation', () => {
Expand Down Expand Up @@ -1472,6 +1481,30 @@ describe('validator', () => {
expect(result).to.equal('Timestamp');
});

it('should be nullipotent upon input data - typeof number', () => {
let input = 1;
let result = validator.__TESTING__.inferPrimitiveValueType(input);
expect(input).to.equal(1);
});

it('should be nullipotent upon input data - typeof string', () => {
let input = 'someString';
let result = validator.__TESTING__.inferPrimitiveValueType(input);
expect(input).to.equal('someString');
});

it('should be nullipotent upon input data - typeof boolean', () => {
let input = true;
let result = validator.__TESTING__.inferPrimitiveValueType(input);
expect(input).to.equal(true);
});

it('should be nullipotent upon input data - typeof object', () => {
let input = {someKey: 'someValue'};
let result = validator.__TESTING__.inferPrimitiveValueType(input);
expect(input).to.deep.equal({someKey: 'someValue'});
});

});

describe('inferStructureValueType', () => {
Expand Down Expand Up @@ -1514,6 +1547,26 @@ describe('validator', () => {
expect(result).to.equal(null);
});

it('should be nullipotent upon input data', () => {
let input = {
'Type': 'AWS::Serverless::Function',
'Properties': {
'Variables': {
'someKey': 'someValue'
},
}
};
let result = validator.__TESTING__.inferPrimitiveValueType(input);
expect(input).to.deep.equal({
'Type': 'AWS::Serverless::Function',
'Properties': {
'Variables': {
'someKey': 'someValue'
},
}
});
});

});

describe('inferAggregateValueType', () => {
Expand All @@ -1527,6 +1580,15 @@ describe('validator', () => {
expect(result).to.equal('List<String>');
});

it('should be able to infer a List of Json aggregate value type', () => {
let input = [
{ 1: 'somethingBeautiful' }, { 2: 'somethingAwesome' }, { 3: 'somethingCool' }
];
let candidateTypes: string[] = [];
let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
expect(result).to.equal('List<Json>');
});

it('should be able to infer a Map of Strings aggregate value type', () => {
let input = {
1: 'somethingBeautiful',
Expand All @@ -1538,13 +1600,76 @@ describe('validator', () => {
expect(result).to.equal('Map<String>');
});

it('should be able to infer a Map of Json aggregate value type', () => {
let input = {
1: { 1: 'somethingBeautiful' },
2: { 2: 'somethingAwesome' },
3: { 3: 'somethingCool' }
};
let candidateTypes: string[] = [];
let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
expect(result).to.equal('Map<Json>');
});

it('should not be able to infer a non-aggregate value type', () => {
let input = {};
let candidateTypes: string[] = [];
let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
expect(result).to.equal(null);
});

it('should be nullipotent upon input data - List of Strings aggregate value type', () => {
let input = [
'somethingBeautiful', 'somethingAwesome', 'somethingCool'
];
let candidateTypes: string[] = [];
let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
expect(input).to.deep.equal([
'somethingBeautiful', 'somethingAwesome', 'somethingCool'
]);
});

it('should be nullipotent upon input data - List of Json aggregate value type', () => {
let input = [
{ 1: 'somethingBeautiful' }, { 2: 'somethingAwesome' }, { 3: 'somethingCool' }
];
let candidateTypes: string[] = [];
let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
expect(input).to.deep.equal([
{ 1: 'somethingBeautiful' }, { 2: 'somethingAwesome' }, { 3: 'somethingCool' }
]);
});

it('should be nullipotent upon input data - Map of Strings aggregate value type', () => {
let input = {
1: 'somethingBeautiful',
2: 'somethingAwesome',
3: 'somethingCool'
};
let candidateTypes: string[] = [];
let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
expect(input).to.deep.equal({
1: 'somethingBeautiful',
2: 'somethingAwesome',
3: 'somethingCool'
});
});

it('should be nullipotent upon input data - Map of Json aggregate value type', () => {
let input = {
1: { 1: 'somethingBeautiful' },
2: { 2: 'somethingAwesome' },
3: { 3: 'somethingCool' }
};
let candidateTypes: string[] = [];
let result = validator.__TESTING__.inferAggregateValueType(input, candidateTypes);
expect(input).to.deep.equal({
1: { 1: 'somethingBeautiful' },
2: { 2: 'somethingAwesome' },
3: { 3: 'somethingCool' }
});
});

});

describe('inferValueType', () => {
Expand Down Expand Up @@ -1579,6 +1704,49 @@ describe('validator', () => {

});

describe('templateHasUnresolvedIntrinsics', () => {

it('should be able to detect if a given template has top-level unresolved intrinsics', () => {
const input = {
'Ref': 'AWS::Region'
};
const result = validator.__TESTING__.templateHasUnresolvedIntrinsics(clone(input));
expect(result).to.equal(true);
expect(input).to.deep.equal(input);
});

it('should be able to detect if a given template has nested unresolved intrinsics', () => {
const input = {
'Resources': {
'MyBucket': {
'Type': 'AWS::S3::Bucket',
'BucketName': {
'Ref': 'AWS::Region'
}
}
}
};
const result = validator.__TESTING__.templateHasUnresolvedIntrinsics(clone(input));
expect(result).to.equal(true);
expect(input).to.deep.equal(input);
});

it('should be able to detect if a given template does not contain unresolved intrinsics', () => {
const input = {
'Resources': {
'MyBucket': {
'Type': 'AWS::S3::Bucket',
'BucketName': 'somethingCool'
}
}
};
const result = validator.__TESTING__.templateHasUnresolvedIntrinsics(clone(input));
expect(result).to.equal(false);
expect(input).to.deep.equal(input);
});

});

describe('SAM-20161031', function() {

this.timeout(5000);
Expand Down
Loading