fix: Configure the spellchecking in the NLU section

BREAKING CHANGE: The spellchecking is now configured in the NLU section.
Botfuel · Jul 16, 2018 · c1f889e · c1f889e
1 parent 31df0bc
commit c1f889e
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 87 deletions.
diff --git a/packages/botfuel-dialog/src/bot.js b/packages/botfuel-dialog/src/bot.js
@@ -28,7 +28,6 @@ import type Brain from './brains/brain';
 import type Nlu from './nlus/nlu';
 
 const Logger = require('logtown');
-const { Spellchecking } = require('botfuel-nlp-sdk');
 const AdapterResolver = require('./adapter-resolver');
 const BrainResolver = require('./brain-resolver');
 const NluResolver = require('./nlu-resolver');
@@ -51,8 +50,7 @@ const logger = Logger.getLogger('Bot');
  * - a {@link Config},
  * - a {@link DialogManager},
  * - a {@link MiddlewareManager},
- * - a {@link Nlu} (Natural Language Understanding) module,
- * - an optional {@link Spellchecking} module.
+ * - a {@link Nlu} (Natural Language Understanding) module.
  */
 class Bot {
   adapter: Adapter;
@@ -61,14 +59,12 @@ class Bot {
   dm: DialogManager;
   middlewareManager: MiddlewareManager;
   nlu: Nlu;
-  spellchecking: ?Spellchecking;
 
   constructor(config: RawConfig) {
     this.config = getConfiguration(config);
     logger.debug('constructor', this.config);
     checkCredentials(this.config);
     this.brain = new BrainResolver(this).resolve(this.config.brain.name);
-    this.spellchecking = null;
     this.nlu = new NluResolver(this).resolve(this.config.nlu.name);
     this.dm = new DialogManager(this);
     this.adapter = new AdapterResolver(this).resolve(this.config.adapter.name);
@@ -84,18 +80,6 @@ class Bot {
     await this.brain.init();
     // NLU
     await this.nlu.init();
-    // Spellchecking
-    if (this.config.spellchecking) {
-      if (!process.env.BOTFUEL_APP_ID || !process.env.BOTFUEL_APP_KEY) {
-        logger.error(
-          'BOTFUEL_APP_ID and BOTFUEL_APP_KEY are required for using the spellchecking service!',
-        );
-      }
-      this.spellchecking = new Spellchecking({
-        appId: process.env.BOTFUEL_APP_ID,
-        appKey: process.env.BOTFUEL_APP_KEY,
-      });
-    }
   }
 
   /**
@@ -210,7 +194,6 @@ class Bot {
   async respondWhenText(userMessage: TextMessage): Promise<BotMessageJson[]> {
     logger.debug('respondWhenText', userMessage);
     let sentence = userMessage.payload.value;
-    sentence = await this.spellcheck(sentence);
 
     const { classificationResults, messageEntities } = await this.nlu.compute(
       sentence,
@@ -260,33 +243,6 @@ class Bot {
     const botMessages = await this.dm.executeDialog(userMessage, dialog);
     return botMessages;
   }
-
-  /**
-   * Spellchecks a sentence.
-   * @param sentence - a sentence
-   * @returns the spellchecked sentence
-   */
-  async spellcheck(sentence: string): Promise<string> {
-    const key = this.config.spellchecking;
-
-    logger.debug('spellcheck', sentence, key);
-
-    if (!key || !this.spellchecking) {
-      return sentence;
-    }
-
-    try {
-      const result = await this.spellchecking.compute({ sentence, key });
-      logger.debug('spellcheck: result', result);
-      return result.correctSentence;
-    } catch (error) {
-      logger.error('spellcheck: error');
-      if (error.statusCode === 403) {
-        throw new AuthenticationError();
-      }
-      throw error;
-    }
-  }
 }
 
 module.exports = Bot;
diff --git a/packages/botfuel-dialog/src/config.js b/packages/botfuel-dialog/src/config.js
@@ -35,9 +35,9 @@ export type Config = {|
   multiintent: boolean,
   nlu: {
     name: string,
+    spellchecking: string,
   },
   path: string,
-  spellchecking: boolean,
   custom: Object,
 |};
 
@@ -68,8 +68,6 @@ const defaultConfig = {
   multiIntent: false,
 };
 
-const whitelist = Object.keys(defaultConfig).concat(['spellchecking', 'custom']);
-
 /**
  * Returns the contents of the bot config file.
  * @param configFileName - the bot config file name/path
@@ -135,10 +133,7 @@ const getComponentRoots = function (config): string[] {
  */
 const getConfiguration = (botConfig: RawConfig = {}): Config => {
   // get the config by extending defaultConfig with botConfig
-  const config = _.merge(
-    defaultConfig,
-    _.omitBy(botConfig, (val, key: string): boolean => !whitelist.includes(key)),
-  );
+  const config = _.merge(defaultConfig, botConfig);
 
   Object.assign(defaultConfig);
 

diff --git a/packages/botfuel-dialog/src/nlus/botfuel-nlu.js b/packages/botfuel-dialog/src/nlus/botfuel-nlu.js
@@ -25,6 +25,7 @@ const CompositeExtractor = require('../extractors/composite-extractor');
 const SdkError = require('../errors/sdk-error');
 const ClassificationResult = require('./classification-result');
 const Nlu = require('./nlu');
+const { Spellchecking } = require('botfuel-nlp-sdk');
 
 /**
  * NLU using Botfuel Trainer API
@@ -54,6 +55,10 @@ class BotfuelNlu extends Nlu {
         this.classificationFilter = require(classificationFilterPath);
       }
     }
+    this.spellchecking = new Spellchecking({
+      appId: process.env.BOTFUEL_APP_ID,
+      appKey: process.env.BOTFUEL_APP_KEY,
+    });
   }
 
   /**
@@ -101,6 +106,8 @@ class BotfuelNlu extends Nlu {
   async compute(sentence, context) {
     logger.debug('compute', sentence); // Context is not loggable
 
+    sentence = await this.spellcheck(sentence);
+
     // compute entities
     const messageEntities = await this.computeEntities(sentence);
 
@@ -144,6 +151,22 @@ class BotfuelNlu extends Nlu {
     const entities = await this.extractor.compute(sentence);
     return entities;
   }
+
+  /**
+   * Spellchecks a sentence.
+   * @param sentence - a sentence
+   * @returns the spellchecked sentence
+   */
+  async spellcheck(sentence: string): Promise<string> {
+    if (!this.config.nlu || !this.config.nlu.spellchecking) {
+      return sentence;
+    }
+    const key = this.config.nlu.spellchecking;
+    logger.debug('spellcheck', sentence, key);
+    const result = await this.spellchecking.compute({ sentence, key });
+    logger.debug('spellcheck: result', result);
+    return result.correctSentence;
+  }
 }
 
 module.exports = BotfuelNlu;
diff --git a/packages/botfuel-dialog/src/utils/environment.js b/packages/botfuel-dialog/src/utils/environment.js
@@ -51,11 +51,6 @@ const checkCredentials = (config) => {
         'BOTFUEL_APP_ID and BOTFUEL_APP_KEY are required to use Botfuel NLU.',
       );
     }
-    if (config.spellchecking) {
-      throw new MissingCredentialsError(
-        'BOTFUEL_APP_ID and BOTFUEL_APP_KEY are required to use the spellchecking service.',
-      );
-    }
   }
 
   // Botfuel app id

diff --git a/packages/botfuel-dialog/tests/config/config.test.js b/packages/botfuel-dialog/tests/config/config.test.js
@@ -25,20 +25,14 @@ describe('Config', () => {
     expect(noConfig).toEqual(defaultConfig);
   });
 
-  test('should return a valid configuration when one provided', () => {
-    const config = getConfiguration({ adapter: { name: 'botfuel' }, outOfScope: true });
-    expect(config.adapter.name).toEqual('botfuel');
-    expect(config.outOfScope).toBe(undefined);
-  });
-
   test('should return a valid configuration when qna provided', () => {
     const config = getConfiguration({ nlu: { qna: { when: 'before' } } });
     expect(config.nlu.qna.when).toEqual('before');
   });
 
   test('should return a valid configuration when spellchecking provided', () => {
-    const config = getConfiguration({ spellchecking: 'EN_1' });
-    expect(config.spellchecking).toEqual('EN_1');
+    const config = getConfiguration({ nlu: { spellchecking: 'EN_1' } });
+    expect(config.nlu.spellchecking).toEqual('EN_1');
   });
 
   test('should return empty object when no config filename provided', () => {
@@ -49,10 +43,4 @@ describe('Config', () => {
   test('should throw an error when file not exists', () => {
     expect(() => resolveConfigFile('invalid')).toThrow();
   });
-
-  test('should merge correctly config', () => {
-    const config = getConfiguration({ nlu: { name: 'custom' } });
-    expect(config.adapter.name).toEqual('botfuel');
-    expect(config.nlu.intentThreshold).toBeDefined();
-  });
 });
diff --git a/packages/botfuel-dialog/tests/nlus/botfuel-nlu.test.js b/packages/botfuel-dialog/tests/nlus/botfuel-nlu.test.js
@@ -46,7 +46,7 @@ describe('Botfuel Nlu', () => {
   // IMPORTANT: for REPLAY record, use what ever test app that has greetings intent and qna
   describe('compute', () => {
     test('to correctly detect intents ', async () => {
-      const nlu = new BotfuelNlu();
+      const nlu = new BotfuelNlu( { nlu: {} });
       // fake extractor
       nlu.extractor = new CompositeExtractor({
         extractors: [],
@@ -58,7 +58,7 @@ describe('Botfuel Nlu', () => {
     });
 
     test('to correctly detect qnas', async () => {
-      const nlu = new BotfuelNlu();
+      const nlu = new BotfuelNlu( { nlu: {} });
       // fake extractor
       nlu.extractor = new CompositeExtractor({
         extractors: [],

diff --git a/packages/botfuel-dialog/tests/utils/environment.test.js b/packages/botfuel-dialog/tests/utils/environment.test.js
@@ -29,8 +29,8 @@ describe('Environment utils', () => {
   });
 
   describe('When BOTFUEL_APP_TOKEN, BOTFUEL_APP_KEY, BOTFUEL_APP_ID are defined', () => {
-    test('should not throw an error when using botfuel nlu or spellchecking', async () => {
-      const config = buildConfig({ nlu: { name: 'botfuel' }, spellchecking: true });
+    test('should not throw an error when using botfuel nlu', async () => {
+      const config = buildConfig({ nlu: { name: 'botfuel' } });
       expect(() => checkCredentials(config)).not.toThrowError(MissingCredentialsError);
     });
   });
@@ -63,11 +63,6 @@ describe('Environment utils', () => {
       expect(() => checkCredentials(config)).toThrowError(MissingCredentialsError);
     });
 
-    test('should throw an error when using spellchecking service', async () => {
-      const config = buildConfig({ spellchecking: true });
-      expect(() => checkCredentials(config)).toThrowError(MissingCredentialsError);
-    });
-
     test('should not throw an error when not using botfuel-nlu', async () => {
       const config = buildConfig({ nlu: { name: 'custom' } });
       expect(() => checkCredentials(config)).not.toThrowError(MissingCredentialsError);
@@ -85,12 +80,6 @@ describe('Environment utils', () => {
       expect(f).toThrowError(MissingCredentialsError);
     });
 
-    test('should throw an error when using spellchecking service', async () => {
-      const config = buildConfig({ spellchecking: true });
-      const f = () => checkCredentials(config);
-      expect(f).toThrowError(MissingCredentialsError);
-    });
-
     test('should not throw an error when not using botfuel-nlu', async () => {
       const config = buildConfig({ nlu: { name: 'custom' } });
       expect(() => checkCredentials(config)).not.toThrowError(MissingCredentialsError);

diff --git a/packages/test-ecommerce/test-config.js b/packages/test-ecommerce/test-config.js
@@ -18,7 +18,9 @@ module.exports = {
   adapter: {
     name: 'test',
   },
-  spellchecking: 'EN_1',
+  nlu: {
+    spellchecking: 'EN_1',
+  },
   logger: 'error',
   path: __dirname,
 };