fix: set default filters/transforms in schema, fix conditionals

src/BaseService.ts

@@ -1,4 +1,4 @@
-import { kebabCase } from 'lodash';
+import { isNil, kebabCase } from 'lodash';
 import { Inject, MissingValueError } from 'noicejs';
 import { BaseOptions } from 'noicejs/Container';
 import { Logger } from 'noicejs/logger/Logger';
@@ -60,7 +60,7 @@ export abstract class BaseService<TData extends BaseServiceData> implements Serv
     this.services = options.services;
 
     // check this, because bunyan will throw if it is missing
-    if (!this.name) {
+    if (isNil(this.name) || this.name === '') {
       throw new MissingValueError('missing service name');
     }
 
@@ -84,7 +84,7 @@ export abstract class BaseService<TData extends BaseServiceData> implements Serv
   }
 
   public async start() {
-    const filters = this.data.filters || [];
+    const filters = this.data.filters;
     this.logger.info('setting up filters');
     for (const def of filters) {
       const filter = await this.services.createService<Filter, FilterData>(def);

src/controller/LearnController.ts

@@ -1,3 +1,4 @@
+import { isNil } from 'lodash';
 import { Inject } from 'noicejs';
 import { Connection, Repository } from 'typeorm';
 
@@ -58,7 +59,7 @@ export class LearnController extends BaseController<LearnControllerData> impleme
     const existing = await this.keywordRepository.findOne({
       key,
     });
-    if (existing) {
+    if (isNil(existing)) {
       return this.errorReply(ctx, ErrorReplyType.EntityExists, key);
     }
 
@@ -78,7 +79,7 @@ export class LearnController extends BaseController<LearnControllerData> impleme
     const keyword = await this.keywordRepository.findOne({
       key,
     });
-    if (!keyword) {
+    if (isNil(keyword)) {
       return this.errorReply(ctx, ErrorReplyType.EntityMissing, key);
     }
 
@@ -103,7 +104,7 @@ export class LearnController extends BaseController<LearnControllerData> impleme
       key,
     });
 
-    if (!keyword) {
+    if (isNil(keyword)) {
       return this.errorReply(ctx, ErrorReplyType.EntityMissing, key);
     }
 

src/controller/README.md

@@ -20,11 +20,11 @@ See [docs/controller](../../docs/controller) for example config for each control
 
 Most controllers should extend the `BaseController`, which implements noun and filter checks, as well as transforms.
 
-Controllers typically handle a few nouns. To keep the noun/verb logic simple, controllers use a set of decorators:
+Controllers typically handle a few nouns. To keep the noun/verb logic simple, controllers use a decorator for each
+handler method:
 
 ```typescript
-@HandleNoun(NOUN_FOO)
-@HandleVerb(CommandVerb.Create)
+@Handler(NOUN_FOO, CommandVerb.Create)
 public async createFoo(cmd: Command) {
   const foo = await this.fooRepository.create({ /* ... */ });
   return this.transformJSON(foo);
@@ -44,6 +44,10 @@ public async getFoo(cmd: Command) {
 }
 ```
 
+When a command is received, the first handler which matches both the noun and verb will be selected. If the handler
+also has the `@CheckRBAC` decorator, grants will be checked and may cause an error reply *instead of* invoking the
+handler.
+
 Errors thrown during a handler method, sync or async, will be caught by the base controller and the message used as a
 reply.
 

src/controller/index.ts

@@ -2,14 +2,12 @@ import { BaseController } from 'src/controller/BaseController';
 import { ControllerData } from 'src/controller/Controller';
 import { CommandVerb } from 'src/entity/Command';
 
-export const SYMBOL_NOUN = Symbol('handle-noun');
+export const SYMBOL_HANDLER = Symbol('handler');
 export const SYMBOL_RBAC = Symbol('check-rbac');
-export const SYMBOL_VERB = Symbol('handle-verb');
 
 export function Handler(noun: string, verb: CommandVerb) {
   return (target: BaseController<ControllerData>, key: string, desc: PropertyDescriptor) => {
-    Reflect.defineMetadata(SYMBOL_NOUN, noun, desc.value);
-    Reflect.defineMetadata(SYMBOL_VERB, verb, desc.value);
+    Reflect.defineMetadata(SYMBOL_HANDLER, [noun, verb], desc.value);
   };
 }
 
@@ -41,17 +39,12 @@ export interface HandlerOptions {
 }
 
 export function getHandlerOptions(target: Function): HandlerOptions | undefined {
-  if (!Reflect.hasMetadata(SYMBOL_NOUN, target)) {
+  if (!Reflect.hasMetadata(SYMBOL_HANDLER, target)) {
     return;
   }
 
-  if (!Reflect.hasMetadata(SYMBOL_VERB, target)) {
-    return;
-  }
-
-  const noun = Reflect.getMetadata(SYMBOL_NOUN, target);
+  const [noun, verb] = Reflect.getMetadata(SYMBOL_HANDLER, target) as [string, CommandVerb];
   const rbac = Reflect.getMetadata(SYMBOL_RBAC, target) as RBACOptions;
-  const verb = Reflect.getMetadata(SYMBOL_VERB, target) as CommandVerb;
   return {
     noun,
     rbac,

src/controller/kubernetes/CoreController.ts

@@ -5,7 +5,7 @@ import { CheckRBAC, Handler } from 'src/controller';
 import { BaseController } from 'src/controller/BaseController';
 import { Controller, ControllerData, ControllerOptions } from 'src/controller/Controller';
 import { Command, CommandVerb } from 'src/entity/Command';
-import { mustExist } from 'src/utils';
+import { doesExist, mustExist } from 'src/utils';
 
 export const NOUN_POD = 'kubernetes-pod';
 export const NOUN_SERVICE = 'kubernetes-service';
@@ -14,7 +14,7 @@ export interface KubernetesControllerData extends ControllerData {
   context: {
     cluster: boolean;
     default: boolean;
-    path: string;
+    path?: string;
   };
   default: {
     namespace: string;
@@ -70,7 +70,7 @@ export class KubernetesCoreController extends BaseController<KubernetesControlle
     if (this.data.context.cluster) {
       config.loadFromCluster();
     }
-    if (this.data.context.path) {
+    if (doesExist(this.data.context.path)) {
       config.loadFromFile(this.data.context.path);
     }
     return config;

src/entity/Context.ts

@@ -1,5 +1,5 @@
 import { GraphQLInputObjectType, GraphQLObjectType, GraphQLString } from 'graphql';
-import { flatten } from 'lodash';
+import { flatten, isNil } from 'lodash';
 import { MissingValueError } from 'noicejs';
 import { newTrie } from 'shiro-trie';
 import { Column, Entity, PrimaryGeneratedColumn } from 'typeorm';
@@ -73,7 +73,7 @@ export class Context extends BaseEntity implements ContextOptions {
     super(options);
 
     if (doesExist(options)) {
-      if (!options.name || !options.uid) {
+      if (isNil(options.name) || isNil(options.uid)) {
         throw new MissingValueError('name and uid must be specified in context options');
       }
 
@@ -104,12 +104,12 @@ export class Context extends BaseEntity implements ContextOptions {
    * grants on a user).
    */
   public checkGrants(checks: Array<string>): boolean {
-    if (this.token && !this.token.permit(checks)) {
+    if (doesExist(this.token) && !this.token.permit(checks)) {
       return false;
     }
 
     const grants = this.getGrants();
-    if (!grants.length) {
+    if (grants.length === 0) {
       return false;
     }
 
@@ -120,7 +120,7 @@ export class Context extends BaseEntity implements ContextOptions {
 
   public listGrants(checks: Array<string>): Array<string> {
     const grants = this.getGrants();
-    if (!grants.length) {
+    if (grants.length === 0) {
       return [];
     }
 
@@ -130,7 +130,7 @@ export class Context extends BaseEntity implements ContextOptions {
   }
 
   public getGrants(): Array<string> {
-    if (this.user) {
+    if (doesExist(this.user)) {
       return flatten(this.user.roles.map((r) => r.grants));
     } else {
       return [];
@@ -143,15 +143,15 @@ export class Context extends BaseEntity implements ContextOptions {
    * If this context does not have a logged in user, default to the listener-provided UID.
    */
   public getUserId(): string {
-    if (this.user) {
+    if (doesExist(this.user)) {
       return this.user.id;
     } else {
       return this.uid;
     }
   }
 
   public toJSON(): object {
-    const user = this.user ? this.user.toJSON() : {};
+    const user = doesExist(this.user) ? this.user.toJSON() : {};
     return {
       channel: this.channel,
       id: this.id,

src/entity/Message.ts

@@ -5,6 +5,7 @@ import { LabelEntity, LabelEntityOptions } from 'src/entity/base/LabelEntity';
 import { Context, GRAPH_OUTPUT_CONTEXT } from 'src/entity/Context';
 import { GRAPH_INPUT_NAME_MULTI_VALUE_PAIR, GRAPH_INPUT_NAME_VALUE_PAIR } from 'src/schema/graph/input/Pairs';
 import { GRAPH_OUTPUT_NAME_MULTI_VALUE_PAIR, GRAPH_OUTPUT_NAME_VALUE_PAIR } from 'src/schema/graph/output/Pairs';
+import { doesExist } from 'src/utils';
 import { TYPE_TEXT } from 'src/utils/Mime';
 
 export interface MessageEntityOptions extends LabelEntityOptions {
@@ -57,7 +58,7 @@ export class Message extends LabelEntity implements MessageEntityOptions {
   constructor(options: MessageEntityOptions) {
     super(options);
 
-    if (options) {
+    if (doesExist(options)) {
       this.body = options.body;
       this.context = options.context;
       this.reactions = options.reactions;
@@ -66,7 +67,7 @@ export class Message extends LabelEntity implements MessageEntityOptions {
   }
 
   public toJSON(): object {
-    const context = this.context ? this.context.toJSON() : {};
+    const context = doesExist(this.context) ? this.context.toJSON() : {};
     return {
       body: this.body,
       context,

src/interval/BaseInterval.ts

@@ -49,12 +49,12 @@ export abstract class BaseInterval<TData extends IntervalData> extends BotServic
   public abstract tick(context: Context, last: Tick): Promise<number>;
 
   protected async startInterval() {
-    if (this.data.frequency.cron) {
+    if (doesExist(this.data.frequency.cron)) {
       this.logger.debug({ cron: this.data.frequency.cron }, 'starting a cron interval');
       throw new NotImplementedError('cron frequency is not implemented');
     }
 
-    if (this.data.frequency.time) {
+    if (doesExist(this.data.frequency.time)) {
       const ms = this.math.unit(this.data.frequency.time).toNumber('millisecond');
       this.logger.debug({ ms }, 'starting a clock interval');
       this.interval = this.clock.setInterval(() => this.nextTick().catch((err) => {
@@ -64,7 +64,7 @@ export abstract class BaseInterval<TData extends IntervalData> extends BotServic
   }
 
   protected async stopInterval() {
-    if (this.data.frequency.time && doesExist(this.interval)) {
+    if (doesExist(this.data.frequency.time) && doesExist(this.interval)) {
       this.clock.clearInterval(this.interval);
     }
   }

src/listener/SlackListener.ts

@@ -58,7 +58,7 @@ export class SlackListener extends SessionListener<SlackListenerData> implements
     const client = mustExist(this.client);
     const ctx = mustExist(msg.context);
 
-    if (ctx.channel.id) {
+    if (doesExist(ctx.channel.id)) {
       const result = await client.sendMessage(escape(msg.body), ctx.channel.id);
       if (doesExist(result.error)) {
         const err = new BaseError(result.error.msg);
@@ -87,11 +87,15 @@ export class SlackListener extends SessionListener<SlackListenerData> implements
     this.webClient = new WebClient(this.data.token.web);
 
     this.client.on('message', (msg) => {
-      this.convertMessage(msg).then((it) => this.bot.receive(it)).catch((err) => this.logger.error(err, 'error receiving message'));
+      this.convertMessage(msg).then((it) => this.bot.receive(it)).catch((err) => {
+        this.logger.error(err, 'error receiving message');
+      });
     });
 
     this.client.on('reaction_added', (reaction) => {
-      this.convertReaction(reaction).then((msg) => this.bot.receive(msg)).catch((err) => this.logger.error(err, 'error adding reaction'));
+      this.convertReaction(reaction).then((msg) => this.bot.receive(msg)).catch((err) => {
+        this.logger.error(err, 'error adding reaction');
+      });
     });
 
     await this.client.start();
@@ -135,7 +139,7 @@ export class SlackListener extends SessionListener<SlackListenerData> implements
       uid,
     });
     const session = await this.getSession(uid);
-    if (session) {
+    if (doesExist(session)) {
       context.user = session.user;
     }
 

src/parser/ArgsParser.ts

@@ -61,7 +61,7 @@ export class ArgsParser extends BaseParser<ArgsParserData> implements Parser {
 
   protected createReply(context: Context, data: Map<string, Array<string>>): Promise<Command> {
     const missing = this.validate(data);
-    if (missing.length) {
+    if (missing.length > 0) {
       this.logger.debug({ missing }, 'missing required arguments, creating completion');
       return this.createCompletion(context, data, missing);
     } else {

src/parser/BaseParser.ts

@@ -66,7 +66,7 @@ export abstract class BaseParser<TData extends ParserData> extends BotService<TD
     noun: string;
     verb: CommandVerb;
   } {
-    if (this.data.preferData) {
+    if (this.data.preferData === true) {
       return {
         noun: getHeadOrDefault(data, 'noun', cmd.noun),
         verb: getHeadOrDefault(data, 'verb', cmd.verb) as CommandVerb,

src/schema/graph/index.ts

@@ -1,5 +1,6 @@
 import * as express from 'express';
 import { GraphQLList, GraphQLObjectType, GraphQLSchema, GraphQLString } from 'graphql';
+import { isNil } from 'lodash';
 import { Inject } from 'noicejs';
 import { Connection } from 'typeorm';
 
@@ -56,7 +57,7 @@ export class GraphSchema extends BotService<GraphSchemaData> {
     const context = req.user as Context | undefined;
     this.logger.debug({ args, context }, 'execute commands');
 
-    if (!context) {
+    if (isNil(context)) {
       throw new SessionRequiredError();
     }
 
@@ -79,7 +80,7 @@ export class GraphSchema extends BotService<GraphSchemaData> {
     const context = req.user as Context | undefined;
     this.logger.debug({ args, context }, 'send messages');
 
-    if (!context) {
+    if (isNil(context)) {
       throw new SessionRequiredError();
     }
 

src/schema/schema.yml

@@ -756,6 +756,7 @@ definitions:
     properties:
       filters:
         type: array
+        default: []
         items:
           $ref: "#/definitions/service-definition"
       strict: