Skip to content

Commit

Permalink
fix: update execution behavior of interceptor
Browse files Browse the repository at this point in the history
This commit contains changes to the execution order of interceptors.
Now, the request interceptors registered from SkillBuilders will be executed
before request handler's canhandle is invoked. Response interceptors will be executed
immediately after request handler's handle returns.
  • Loading branch information
Zhang authored and tianrenz committed May 16, 2018
1 parent ea22d20 commit 352f638
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 109 deletions.
36 changes: 30 additions & 6 deletions ask-sdk-core/lib/dispatcher/DefaultRequestDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { ErrorMapper } from './error/ErrorMapper';
import { HandlerAdapter } from './request/handler/HandlerAdapter';
import { HandlerInput } from './request/handler/HandlerInput';
import { RequestHandlerChain } from './request/handler/RequestHandlerChain';
import { RequestInterceptor } from './request/interceptor/RequestInterceptor';
import { ResponseInterceptor } from './request/interceptor/ResponseInterceptor';
import { RequestMapper } from './request/mapper/RequestMapper';
import { RequestDispatcher } from './RequestDispatcher';

Expand All @@ -30,15 +32,21 @@ export class DefaultRequestDispatcher implements RequestDispatcher {
protected requestMappers : RequestMapper[];
protected errorMapper : ErrorMapper;
protected handlerAdapters : HandlerAdapter[];
protected requestInterceptors : RequestInterceptor[];
protected responseInterceptors : ResponseInterceptor[];

constructor(options : {
requestMappers : RequestMapper[],
handlerAdapters : HandlerAdapter[],
errorMapper? : ErrorMapper,
requestInterceptors? : RequestInterceptor[],
responseInterceptors? : ResponseInterceptor[],
}) {
this.requestMappers = options.requestMappers;
this.handlerAdapters = options.handlerAdapters;
this.errorMapper = options.errorMapper;
this.requestInterceptors = options.requestInterceptors;
this.responseInterceptors = options.responseInterceptors;
}

/**
Expand All @@ -50,7 +58,22 @@ export class DefaultRequestDispatcher implements RequestDispatcher {
public async dispatch(handlerInput : HandlerInput) : Promise<Response> {
let response : Response;
try {
// Execute global request interceptors
if (this.requestInterceptors) {
for (const requestInterceptor of this.requestInterceptors) {
await requestInterceptor.process(handlerInput);
}
}

// Dispatch request to handler chain
response = await this.dispatchRequest(handlerInput);

// Execute global response interceptors
if (this.responseInterceptors) {
for (const responseInterceptor of this.responseInterceptors) {
await responseInterceptor.process(handlerInput, response);
}
}
} catch (err) {
response = await this.dispatchError(handlerInput, err);
}
Expand All @@ -64,49 +87,50 @@ export class DefaultRequestDispatcher implements RequestDispatcher {
* @returns {Promise<Response>}
*/
protected async dispatchRequest(handlerInput : HandlerInput) : Promise<Response> {
// Get the request handler chain that can handle the request
let handlerChain : RequestHandlerChain;

for (const requestMapper of this.requestMappers) {
handlerChain = await requestMapper.getRequestHandlerChain(handlerInput);
if (handlerChain) {
break;
}
}

if (!handlerChain) {
throw createAskSdkError(
this.constructor.name,
`Could not find handler that can handle the request: ${JSON.stringify(handlerInput.requestEnvelope.request, null, 2)}`);
}

// Extract the handler and interceptors from the handler chain
const handler = handlerChain.getRequestHandler();
const requestInterceptors = handlerChain.getRequestInterceptors();
const responseInterceptors = handlerChain.getResponseInterceptors();

// Get the handler adapter that supports the request handler
let adapter : HandlerAdapter;
for (const handlerAdapter of this.handlerAdapters) {
if (handlerAdapter.supports(handler)) {
adapter = handlerAdapter;
break;
}
}

if (!adapter) {
throw createAskSdkError(
this.constructor.name,
'HandlerAdapter not found!');
`Could not find the handler adapter that supports the request handler.`);
}

// Pre-processing logic. Going in order
// Execute request interceptors that are local to the handler chain
if (requestInterceptors) {
for (const requestInterceptor of requestInterceptors) {
await requestInterceptor.process(handlerInput);
}
}

// Invoke the request handler
const response = await adapter.execute(handlerInput, handler);

// Post-processing logic. Going in order
// Execute response interceptors that are local to the handler chain
if (responseInterceptors) {
for (const responseInterceptor of responseInterceptors) {
await responseInterceptor.process(handlerInput, response);
Expand Down
2 changes: 2 additions & 0 deletions ask-sdk-core/lib/skill/Skill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ export class Skill {
requestMappers : skillConfiguration.requestMappers,
handlerAdapters : skillConfiguration.handlerAdapters,
errorMapper : skillConfiguration.errorMapper,
requestInterceptors : skillConfiguration.requestInterceptors,
responseInterceptors : skillConfiguration.responseInterceptors,
});
}

Expand Down
4 changes: 4 additions & 0 deletions ask-sdk-core/lib/skill/SkillConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { services } from 'ask-sdk-model';
import { PersistenceAdapter } from '../attributes/persistence/PersistenceAdapter';
import { ErrorMapper } from '../dispatcher/error/ErrorMapper';
import { HandlerAdapter } from '../dispatcher/request/handler/HandlerAdapter';
import { RequestInterceptor } from '../dispatcher/request/interceptor/RequestInterceptor';
import { ResponseInterceptor } from '../dispatcher/request/interceptor/ResponseInterceptor';
import { RequestMapper } from '../dispatcher/request/mapper/RequestMapper';

/**
Expand All @@ -26,6 +28,8 @@ export interface SkillConfiguration {
requestMappers : RequestMapper[];
handlerAdapters : HandlerAdapter[];
errorMapper? : ErrorMapper;
requestInterceptors? : RequestInterceptor[];
responseInterceptors? : ResponseInterceptor[];
persistenceAdapter? : PersistenceAdapter;
apiClient? : services.ApiClient;
customUserAgent? : string;
Expand Down
33 changes: 15 additions & 18 deletions ask-sdk-core/lib/skill/factory/BaseSkillFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export type LambdaHandler = (

export class BaseSkillFactory {
public static init() : BaseSkillBuilder {
const thisRequestHandlers : RequestHandler[] = [];
const thisRequestHandlerChains : DefaultRequestHandlerChain[] = [];
const thisRequestInterceptors : RequestInterceptor[] = [];
const thisResponseInterceptors : ResponseInterceptor[] = [];
const thisErrorHandlers : ErrorHandler[] = [];
Expand Down Expand Up @@ -78,16 +78,21 @@ export class BaseSkillFactory {
`Matcher must be of type string or function, got: ${typeof matcher}!`);
}
}

thisRequestHandlers.push({
canHandle,
handle : executor,
});
thisRequestHandlerChains.push(new DefaultRequestHandlerChain({
requestHandler : {
canHandle,
handle : executor,
},
}));

return this;
},
addRequestHandlers(...requestHandlers : RequestHandler[]) : BaseSkillBuilder {
thisRequestHandlers.push(...requestHandlers);
for ( const requestHandler of requestHandlers ) {
thisRequestHandlerChains.push(new DefaultRequestHandlerChain({
requestHandler,
}));
}

return this;
},
Expand Down Expand Up @@ -165,18 +170,8 @@ export class BaseSkillFactory {
return this;
},
getSkillConfiguration() : SkillConfiguration {
const requestHandlerChains : DefaultRequestHandlerChain[] = [];

for ( const requestHandler of thisRequestHandlers ) {
requestHandlerChains.push(new DefaultRequestHandlerChain({
requestHandler,
requestInterceptors : thisRequestInterceptors,
responseInterceptors : thisResponseInterceptors,
}));
}

const requestMapper = new DefaultRequestMapper({
requestHandlerChains,
requestHandlerChains : thisRequestHandlerChains,
});

const errorMapper = thisErrorHandlers.length
Expand All @@ -189,6 +184,8 @@ export class BaseSkillFactory {
requestMappers : [requestMapper],
handlerAdapters : [new DefaultHandlerAdapter()],
errorMapper,
requestInterceptors : thisRequestInterceptors,
responseInterceptors : thisResponseInterceptors,
customUserAgent : thisCustomUserAgent,
skillId : thisSkillId,
};
Expand Down
Loading

0 comments on commit 352f638

Please sign in to comment.