Skip to content

Commit

Permalink
fix: CreateStateHandler incorrectly update function key
Browse files Browse the repository at this point in the history
This commit contains the follow changes:
- Fix a bug where CreateStateHandler, when invoked multiple times
will repetitively append state string

fix #438
  • Loading branch information
Zhang authored and tianrenz committed Jul 30, 2018
1 parent 3fb0167 commit dcec7e6
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 67 deletions.
35 changes: 30 additions & 5 deletions ask-sdk-v1adapter/lib/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class Adapter extends EventEmitter {

constructor(event : RequestEnvelope, context : any, callback? : (err : Error, result? : any) => void) {
super();

if (!event.session) {
event.session = {
new : undefined,
Expand All @@ -66,6 +67,8 @@ export class Adapter extends EventEmitter {
event.session.attributes = {};
}

this.setMaxListeners(Infinity);

this._event = event;
this._context = context;
this._callback = callback;
Expand All @@ -85,14 +88,21 @@ export class Adapter extends EventEmitter {

public registerHandlers(...v1Handlers : V1Handler[]) : void {
for ( const handler of v1Handlers) {
if (!isObject(handler)) {
if (!IsObject(handler)) {
throw createAskSdkError(this.constructor.name, `Argument #${handler.constructor.name} was not an Object`);
}
const eventNames = Object.keys(handler);
for (const eventName of eventNames) {
if (typeof(handler[eventName]) !== 'function') {
throw createAskSdkError(this.constructor.name, `Event handler for '${eventName}' was not a function`);
}

let targetEventName = eventName;

if (handler[StateString as any]) {
targetEventName += handler[StateString as any];
}

const handlerContext = {
on: this.on.bind(this),
emit: this.emit.bind(this),
Expand All @@ -107,11 +117,11 @@ export class Adapter extends EventEmitter {
attributes : this._event.session.attributes,
context: this._context,
callback : this._callback,
name: eventName,
isOverridden: IsOverridden.bind(this, eventName),
name: targetEventName,
isOverridden: IsOverridden.bind(this, targetEventName),
response: new ResponseBuilder(this),
};
this.on(eventName, handler[eventName].bind(handlerContext));
this.on(targetEventName, handler[eventName].bind(handlerContext));
}
}
}
Expand Down Expand Up @@ -140,6 +150,21 @@ export class Adapter extends EventEmitter {
}
}

export const StateString = Symbol('StateString');

export function CreateStateHandler(state : string, requestHandler : V1Handler) : V1Handler {
if (!requestHandler) {
requestHandler = {};
}

Object.defineProperty(requestHandler, StateString, {
value : state || '',
enumerable : false,
});

return requestHandler;
}

let dynamoDbPersistenceAdapter : DynamoDbPersistenceAdapter;

/* tslint:disable */
Expand Down Expand Up @@ -252,7 +277,7 @@ function IsOverridden(name : string) : boolean {
return this.listenerCount(name) > 1;
}

function isObject(obj : any) : boolean {
function IsObject(obj : any) : boolean {
return (!!obj) && (obj.constructor === Object);
}

Expand Down
63 changes: 18 additions & 45 deletions ask-sdk-v1adapter/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,18 @@ import { ImageUtils } from './utils/imageUtils';
import { TextUtils } from './utils/textUtils';
import { V1Handler } from './v1Handler';

export { Adapter } from './adapter';
export {
Adapter,
StateString,
CreateStateHandler,
} from './adapter';
export { Handler } from './handler';
export { V1Handler } from './v1Handler';
export { ResponseBuilder } from './responseBuilderShim';

export const services = {
DeviceAddressService,
DirectiveService,
ListManagementService,
};

export const directives = {
VoicePlayerSpeakDirective,
};
export function handler(event : RequestEnvelope, context : any, callback? : (err : Error, result? : any) => void) : Adapter {
return new Adapter(event, context, callback);
}

export const templateBuilders = {
BodyTemplate1Builder,
Expand All @@ -55,42 +53,17 @@ export const templateBuilders = {
ListTemplate2Builder,
};

export const services = {
DeviceAddressService,
DirectiveService,
ListManagementService,
};

export const directives = {
VoicePlayerSpeakDirective,
};

export const utils = {
ImageUtils,
TextUtils,
};

export function handler(event : RequestEnvelope, context : any, callback? : (err : Error, result? : any) => void) : Adapter {
if (!event.session) {
event.session = {
new : null,
user : null,
sessionId : null,
application : null,
attributes: {} };
} else if (!event.session.attributes) {
event.session.attributes = {};
}
const adapter = new Adapter(event, context, callback);
adapter.setMaxListeners(Infinity);

return adapter;
}

export function CreateStateHandler(state : string, requestHandler : V1Handler) : V1Handler {
if (!requestHandler) {
requestHandler = {};
}
for ( const eventName of Object.keys(requestHandler)) {
if (typeof(requestHandler[eventName]) !== 'function') {
throw new Error(`Event handler for '${eventName}' was not a function`);
}
if (state) {
const newEventName = eventName + state;
Object.defineProperty(requestHandler, newEventName, Object.getOwnPropertyDescriptor(requestHandler, eventName));
delete requestHandler[eventName];
}
}

return requestHandler;
}
75 changes: 58 additions & 17 deletions ask-sdk-v1adapter/tst/adapter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@
import { ResponseEnvelope } from 'ask-sdk-model';
import { expect } from 'chai';
import * as sinon from 'sinon';
import { Adapter } from '../lib/adapter';
import {
Adapter,
CreateStateHandler,
StateString,
} from '../lib/adapter';
import { V1Handler } from '../lib/v1Handler';
import { LaunchRequest, PlaybackControllerRequest } from './mock/mockSampleRequest';
import { mockV2Requesthandler } from './mock/mockV2RequestHandler';

Expand All @@ -28,9 +33,24 @@ const mockContext = {
// do something
},
};
/* tslint:disable */

describe('CreateStateHandler', () => {
it('should be able to create handler object with state tag', () => {
const mockHandler : V1Handler = {
LaunchRequest() : (...args : any[]) => void {
return;
},
};

const stateHandler = CreateStateHandler('TestState', mockHandler);

expect(stateHandler[StateString as any]).eq('TestState');
expect(Object.keys(mockHandler)).deep.equal(['LaunchRequest']);
});
});

describe('Adapter', () => {
it('should pass lambda inputs /to adapter by handler function', () => {
it('should pass lambda inputs to adapter by handler function', () => {
const adapter = new Adapter(LaunchRequest, mockContext);

expect(adapter._event).to.deep.equal(LaunchRequest);
Expand All @@ -46,22 +66,46 @@ describe('Adapter', () => {
new: undefined,
sessionId: undefined,
user: undefined,
})
});
});

it('should be able to register v1 style request handlers', () => {
const mockHandler : V1Handler = {
LaunchRequest() : (...args : any[]) => void {
return;
},
};
const mockStateHandler : V1Handler = CreateStateHandler('TestState', {
LaunchRequest() : (...args : any[]) => void {
return;
},
});

// tslint:disable-next-line
console.log(mockStateHandler);

const adapter = new Adapter(LaunchRequest, mockContext);

adapter.registerHandlers(mockHandler, mockStateHandler);

expect(adapter.listenerCount('LaunchRequest')).eq(1);
expect(adapter.listenerCount('LaunchRequestTestState')).eq(1);
});

it('should be able to add V2 style request handlers', () => {
it('should be able to register V2 style request handlers', () => {
const adapter = new Adapter(LaunchRequest, mockContext);

adapter.registerV2Handlers(mockV2Requesthandler);

// tslint:disable-next-line
expect(adapter['v2RequestHandlers'].length).equal(1);
expect(mockV2Requesthandler.canHandle()).to.equal(true);
});

it('should fail if the appId does not match the requestAppId', () => {
const mockHandler = {
LaunchRequest: function() {
// do something
const mockHandler : V1Handler = {
LaunchRequest() : (...args : any[]) => void {
return;
},
};
const adapter = new Adapter(LaunchRequest, mockContext);
Expand All @@ -75,26 +119,23 @@ describe('Adapter', () => {
});

it('should init i18n client if there exists resources', () => {
const mockHandler = {
LaunchRequest: function() {
// do something
const mockHandler : V1Handler = {
LaunchRequest() : (...args : any[]) => void {
return;
},
};
const adapter = new Adapter(LaunchRequest, mockContext);
adapter.resources = {
en : {
translation : {
test : 'mock test in en',
}
},
},
de : {
translation : {
test : 'mock test in de',
}
}
};
const handlerContext = {
handler: this,
},
},
};

adapter.registerHandlers(mockHandler);
Expand Down

0 comments on commit dcec7e6

Please sign in to comment.