fix: check injected request option types (#95)

Author: ssube

src/BaseService.ts

@@ -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
@@ -24,6 +25,7 @@ export interface InjectedServiceOptions {
   logger: Logger;
   math: MathFactory;
   metrics: Registry;
+  request: RequestFactory;
   schema: Schema;
   services: ServiceModule;
 }

src/controller/SearchController.ts

@@ -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';
 
@@ -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);
   }
 
@@ -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,

src/controller/WeatherController.ts

@@ -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 {
@@ -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> {
@@ -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,

src/module/BotModule.ts

@@ -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';
@@ -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 {
@@ -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) {
@@ -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);
-  }
 }

src/utils/Math.ts

@@ -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

src/utils/Request.ts

@@ -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);
+      });
+    });
+  }
+}

test/controller/TestWeatherController.ts

@@ -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';
 
@@ -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>({