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

feat(aws): add schema for alexaSkill #8290

Merged
merged 2 commits into from Sep 25, 2020
Merged

feat(aws): add schema for alexaSkill #8290

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6d6d54
feat(aws): add schema for `alexaSkill`
pgrzesik Sep 25, 2020
2e6f0e4
simplify the last else
pgrzesik Sep 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/deprecations.md
View file
Open in desktop
Expand Up @@ -6,6 +6,12 @@ layout: Doc

# Serverless Framework Deprecations

<a name="ALEXA_SKILL_EVENT_WITHOUT_APP_ID"><div>&nbsp;</div></a>

## Support for `alexaSkill` event without `appId` is to be removed

Starting with v3.0.0, support for `alexaSkill` event without `appId` provided will be removed.

<a name="RESOURCES_EXTENSIONS_REFERENCE_TO_NONEXISTENT_RESOURCE"><div>&nbsp;</div></a>

## Defining extensions to nonexistent resources in `resources.extensions`
Expand Down
42 changes: 17 additions & 25 deletions lib/plugins/aws/package/compile/events/alexaSkill/index.js
View file
Open in desktop
Expand Up @@ -11,9 +11,19 @@ class AwsCompileAlexaSkillEvents {
'package:compileEvents': this.compileAlexaSkillEvents.bind(this),
};

// TODO: Complete schema, see https://github.com/serverless/serverless/issues/8023
this.serverless.configSchemaHandler.defineFunctionEvent('aws', 'alexaSkill', {
anyOf: [{ type: 'string' }, { type: 'object' }],
anyOf: [
{ $ref: '#/definitions/awsAlexaEventToken' },
{
type: 'object',
properties: {
appId: { $ref: '#/definitions/awsAlexaEventToken' },
enabled: { type: 'boolean' },
},
required: ['appId'],
additionalProperties: false,
},
],
});
}

Expand All @@ -28,34 +38,16 @@ class AwsCompileAlexaSkillEvents {
let enabled = true;
let appId;
if (event === 'alexaSkill') {
const warningMessage = [
"Warning! You are using an old syntax for alexaSkill which doesn't",
' restrict the invocation solely to your skill.',
' Please refer to the documentation for additional information.',
].join('');
this.serverless.cli.log(warningMessage);
this.serverless._logDeprecation(
'ALEXA_SKILL_EVENT_WITHOUT_APP_ID',
'Starting with next major version, support for alexaSkill event without appId specified will be removed.'
);
} else if (typeof event.alexaSkill === 'string') {
appId = event.alexaSkill;
} else if (_.isPlainObject(event.alexaSkill)) {
if (typeof event.alexaSkill.appId !== 'string') {
const errorMessage = [
`Missing "appId" property for alexaSkill event in function ${functionName}`,
' The correct syntax is: appId: amzn1.ask.skill.xx-xx-xx-xx-xx',
' OR an object with "appId" property.',
' Please check the docs for more info.',
].join('');
throw new this.serverless.classes.Error(errorMessage);
}
} else if (typeof event.alexaSkill === 'object') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it's a last acceptable form, we may simplify this to } else { (it'll be nice also because typeof x === 'object' feels quirky (e.g. null passes such condition))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call, changed 👍

appId = event.alexaSkill.appId;
// Parameter `enabled` is optional, hence the explicit non-equal check for false.
enabled = event.alexaSkill.enabled !== false;
} else {
const errorMessage = [
`Alexa Skill event of function "${functionName}" is not an object or string.`,
' The correct syntax is: alexaSkill.',
' Please check the docs for more info.',
].join('');
throw new this.serverless.classes.Error(errorMessage);
}
alexaSkillNumberInFunction++;

Expand Down
71 changes: 0 additions & 71 deletions lib/plugins/aws/package/compile/events/alexaSkill/index.test.js
View file
Open in desktop
Expand Up @@ -10,19 +10,11 @@ const Serverless = require('../../../../../../Serverless');
describe('AwsCompileAlexaSkillEvents', () => {
let serverless;
let awsCompileAlexaSkillEvents;
let consolePrinted;

beforeEach(() => {
serverless = new Serverless();
serverless.service.provider.compiledCloudFormationTemplate = { Resources: {} };
serverless.setProvider('aws', new AwsProvider(serverless));
consolePrinted = '';
serverless.cli = {
// serverless.cli isn't available in tests, so we will mimic it.
log: txt => {
consolePrinted += `${txt}\r\n`;
},
};
awsCompileAlexaSkillEvents = new AwsCompileAlexaSkillEvents(serverless);
});

Expand All @@ -35,69 +27,6 @@ describe('AwsCompileAlexaSkillEvents', () => {
});

describe('#compileAlexaSkillEvents()', () => {
it('should show a warning if alexaSkill appId is not specified', () => {
awsCompileAlexaSkillEvents.serverless.service.functions = {
first: {
events: ['alexaSkill'],
},
};

awsCompileAlexaSkillEvents.compileAlexaSkillEvents();

expect(consolePrinted).to.contain.string('old syntax for alexaSkill');

expect(
awsCompileAlexaSkillEvents.serverless.service.provider.compiledCloudFormationTemplate
.Resources.FirstLambdaPermissionAlexaSkill1.Type
).to.equal('AWS::Lambda::Permission');
expect(
awsCompileAlexaSkillEvents.serverless.service.provider.compiledCloudFormationTemplate
.Resources.FirstLambdaPermissionAlexaSkill1.Properties.FunctionName
).to.deep.equal({ 'Fn::GetAtt': ['FirstLambdaFunction', 'Arn'] });
expect(
awsCompileAlexaSkillEvents.serverless.service.provider.compiledCloudFormationTemplate
.Resources.FirstLambdaPermissionAlexaSkill1.Properties.Action
).to.equal('lambda:InvokeFunction');
expect(
awsCompileAlexaSkillEvents.serverless.service.provider.compiledCloudFormationTemplate
.Resources.FirstLambdaPermissionAlexaSkill1.Properties.Principal
).to.equal('alexa-appkit.amazon.com');
expect(
awsCompileAlexaSkillEvents.serverless.service.provider.compiledCloudFormationTemplate
.Resources.FirstLambdaPermissionAlexaSkill1.Properties.EventSourceToken
).to.be.undefined;
});

it('should throw an error if alexaSkill event is not a string or an object', () => {
awsCompileAlexaSkillEvents.serverless.service.functions = {
first: {
events: [
{
alexaSkill: 42,
},
],
},
};

expect(() => awsCompileAlexaSkillEvents.compileAlexaSkillEvents()).to.throw(Error);
});

it('should throw an error if alexaSkill event appId is not a string', () => {
awsCompileAlexaSkillEvents.serverless.service.functions = {
first: {
events: [
{
alexaSkill: {
appId: 42,
},
},
],
},
};

expect(() => awsCompileAlexaSkillEvents.compileAlexaSkillEvents()).to.throw(Error);
});

it('should create corresponding resources when multiple alexaSkill events are provided', () => {
const skillId1 = 'amzn1.ask.skill.xx-xx-xx-xx';
const skillId2 = 'amzn1.ask.skill.yy-yy-yy-yy';
Expand Down