Skip to content

Commit

Permalink
fix: check injected request option types (#95)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssube committed Dec 31, 2018
1 parent 73028a1 commit ac10229
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/BaseService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { Clock } from 'src/utils/Clock';
import { JsonPath } from 'src/utils/JsonPath';
import { dictToMap } from 'src/utils/Map';
import { MathFactory } from 'src/utils/Math';
import { RequestFactory } from 'src/utils/Request';

/**
* TODO: these should be optional and must be included in the decorator to be available
Expand All @@ -24,6 +25,7 @@ export interface InjectedServiceOptions {
logger: Logger;
math: MathFactory;
metrics: Registry;
request: RequestFactory;
schema: Schema;
services: ServiceModule;
}
Expand Down
16 changes: 9 additions & 7 deletions src/controller/SearchController.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Container, Inject } from 'noicejs';
import { Inject } from 'noicejs';

import { BaseController } from 'src/controller/BaseController';
import { Controller, ControllerData, ControllerOptions } from 'src/controller/Controller';
import { Command } from 'src/entity/Command';
import { RequestFactory } from 'src/utils/Request';
import { Template } from 'src/utils/Template';
import { TemplateCompiler } from 'src/utils/TemplateCompiler';

Expand All @@ -19,15 +20,16 @@ export interface SearchControllerOptions extends ControllerOptions<SearchControl
compiler: TemplateCompiler;
}

@Inject('compiler')
export const NOUN_SEARCH = 'search';

@Inject('compiler', 'request')
export class SearchController extends BaseController<SearchControllerData> implements Controller {
protected container: Container;
protected url: Template;
protected readonly request: RequestFactory;
protected readonly url: Template;

constructor(options: SearchControllerOptions) {
super(options, 'isolex#/definitions/service-controller-search');
super(options, 'isolex#/definitions/service-controller-search', [NOUN_SEARCH]);

this.container = options.container;
this.url = options.compiler.compile(options.data.request.url);
}

Expand All @@ -40,7 +42,7 @@ export class SearchController extends BaseController<SearchControllerData> imple
const requestUrl = this.url.render({ data });
this.logger.debug({ requestUrl }, 'searching at url');

const response = await this.container.create<any, any>('request', {
const response = await this.request.create({
json: true,
method: this.data.request.method,
uri: requestUrl,
Expand Down
14 changes: 7 additions & 7 deletions src/controller/WeatherController.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Container, Inject } from 'noicejs';
import { BaseOptions } from 'noicejs/Container';
import { CoreOptions, RequiredUriUrl } from 'request';
import { Inject } from 'noicejs';

import { BaseController } from 'src/controller/BaseController';
import { Controller, ControllerData, ControllerOptions } from 'src/controller/Controller';
import { Command } from 'src/entity/Command';
import { RequestFactory } from 'src/utils/Request';
import { TemplateCompiler } from 'src/utils/TemplateCompiler';

export interface WeatherControllerData extends ControllerData {
Expand All @@ -20,13 +19,14 @@ export interface WeatherControllerOptions extends ControllerOptions<WeatherContr

export const NOUN_WEATHER = 'weather';

@Inject('compiler')
@Inject('compiler', 'request')
export class WeatherController extends BaseController<WeatherControllerData> implements Controller {
protected readonly container: Container;
protected readonly request: RequestFactory;

constructor(options: WeatherControllerOptions) {
super(options, 'isolex#/definitions/service-controller-weather', [NOUN_WEATHER]);
this.container = options.container;

this.request = options.request;
}

public async handle(cmd: Command): Promise<void> {
Expand All @@ -50,7 +50,7 @@ export class WeatherController extends BaseController<WeatherControllerData> imp
this.logger.debug({ location, query, root: this.data.api.root }, 'requesting weather data from API');

try {
return this.container.create<WeatherReply, BaseOptions & CoreOptions & RequiredUriUrl>('request', {
return this.request.create({
json: true,
method: 'GET',
qs: query,
Expand Down
8 changes: 2 additions & 6 deletions src/module/BotModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { kebabCase } from 'lodash';
import { Container, Logger, Module, Provides } from 'noicejs';
import { ModuleOptions } from 'noicejs/Module';
import { Registry } from 'prom-client';
import * as request from 'request-promise';
import { Connection } from 'typeorm';

import { Bot } from 'src/Bot';
Expand All @@ -11,6 +10,7 @@ import { GraphSchema } from 'src/schema/graph';
import { Clock } from 'src/utils/Clock';
import { JsonPath } from 'src/utils/JsonPath';
import { MathFactory } from 'src/utils/Math';
import { RequestFactory } from 'src/utils/Request';
import { TemplateCompiler } from 'src/utils/TemplateCompiler';

export interface BotModuleOptions {
Expand Down Expand Up @@ -41,6 +41,7 @@ export class BotModule extends Module {
this.bind(kebabCase(GraphSchema.name)).toConstructor(GraphSchema);
this.bind('jsonpath').toConstructor(JsonPath);
this.bind('math').toConstructor(MathFactory);
this.bind('request').toConstructor(RequestFactory);
}

public setBot(bot: Bot) {
Expand Down Expand Up @@ -71,9 +72,4 @@ export class BotModule extends Module {
public async getStorage(): Promise<Connection> {
return this.bot.getStorage();
}

@Provides('request')
public async createRequest(options: any): Promise<Request> {
return request(options);
}
}
4 changes: 4 additions & 0 deletions src/utils/Math.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ export function clamp(v: number, min: number, max: number) {
return Math.max(Math.min(v, min), max);
}

/**
* Since each user needs to create a math environment with typed options, the container needs to inject an instance of
* something (this).
*/
export class MathFactory {
public create(options: mathjs.ConfigOptions): mathjs.MathJsStatic {
return ((mathjs as unknown) as MathCreate).create(options); // thanks mathjs types
Expand Down
19 changes: 19 additions & 0 deletions src/utils/Request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as request from 'request';

export type RequestOptions = request.CoreOptions & request.UriOptions;

/**
* Work around for the lack of existing create method (default export is the function).
*
* Since each user needs to create requests with typed options, the container needs to inject an instance of
* something (this).
*/
export class RequestFactory {
public create(options: RequestOptions): Promise<any> {
return new Promise((res, rej) => {
request(options, (error: Error, response: request.Response, body: unknown) => {
res(body);
});
});
}
}
5 changes: 4 additions & 1 deletion test/controller/TestWeatherController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Message } from 'src/entity/Message';
import { ServiceModule } from 'src/module/ServiceModule';
import { TransformModule } from 'src/module/TransformModule';
import { Transform } from 'src/transform/Transform';
import { RequestFactory } from 'src/utils/Request';
import { Template } from 'src/utils/Template';
import { TemplateCompiler } from 'src/utils/TemplateCompiler';

Expand All @@ -21,7 +22,9 @@ describeAsync('weather controller', async () => {
const { container, module } = await createContainer(...modules);

const data = { test: 'test' };
module.bind('request').toFactory(async () => data);
module.bind('request').toInstance(ineeda<RequestFactory>({
create: async () => data,
}));

const sent: Array<Message> = [];
const bot = ineeda<Bot>({
Expand Down

0 comments on commit ac10229

Please sign in to comment.