Repository: Yoctol/bottender
Pull request #186 (2018-02-05 10:52:30)

show warning if setState or resetState after session is written


Patch:
diff --git a/examples/console-hello-world/index.js b/examples/console-hello-world/index.js
index 5d296ac3..8c5f968b 100644
--- a/examples/console-hello-world/index.js
+++ b/examples/console-hello-world/index.js
@@ -1,6 +1,6 @@
 const { ConsoleBot } = require('bottender');
 
-const bot = new ConsoleBot();
+const bot = new ConsoleBot({ fallbackMethods: true });
 
 bot.onEvent(async context => {
   await context.sendText('Hello World');
diff --git a/src/bot/Bot.js b/src/bot/Bot.js
index bd3e95d8..13194ead 100644
--- a/src/bot/Bot.js
+++ b/src/bot/Bot.js
@@ -174,6 +174,10 @@ export default class Bot {
           if (sessionId && session) {
             // $FlowFixMe: suppressing this error until we can refactor
             session.lastActivity = Date.now();
+            contexts.forEach(context => {
+              context.isSessionWritten = true;
+            });
+
             await this._sessions.write(sessionId, session);
           }
         } catch (err) {
@@ -188,6 +192,10 @@ export default class Bot {
           if (sessionId && session) {
             // $FlowFixMe: suppressing this error until we can refactor
             session.lastActivity = Date.now();
+            contexts.forEach(context => {
+              context.isSessionWritten = true;
+            });
+
             return this._sessions.write(sessionId, session);
           }
         })
diff --git a/src/bot/ConsoleBot.js b/src/bot/ConsoleBot.js
index c693b403..53ac320f 100644
--- a/src/bot/ConsoleBot.js
+++ b/src/bot/ConsoleBot.js
@@ -7,8 +7,11 @@ import Bot from './Bot';
 import ConsoleConnector from './ConsoleConnector';
 
 export default class ConsoleBot extends Bot {
-  constructor({ sessionStore }: { sessionStore: SessionStore } = {}) {
-    const connector = new ConsoleConnector();
+  constructor({
+    sessionStore,
+    fallbackMethods,
+  }: { sessionStore: SessionStore, fallbackMethods?: boolean } = {}) {
+    const connector = new ConsoleConnector({ fallbackMethods });
     super({ connector, sessionStore, sync: true });
   }
 
diff --git a/src/bot/ConsoleConnector.js b/src/bot/ConsoleConnector.js
index 89c2498b..31f2f286 100644
--- a/src/bot/ConsoleConnector.js
+++ b/src/bot/ConsoleConnector.js
@@ -14,17 +14,20 @@ export type ConsoleClient = {
 
 type ConstructorOptions = {|
   client?: ConsoleClient,
+  fallbackMethods?: boolean,
 |};
 
 export default class ConsoleConnector implements Connector<ConsoleRequestBody> {
   _client: ConsoleClient;
+  _fallbackMethods: boolean;
 
-  constructor({ client }: ConstructorOptions = {}) {
+  constructor({ client, fallbackMethods }: ConstructorOptions = {}) {
     this._client = client || {
       sendText: text => {
         process.stdout.write(`Bot > ${text}\n`);
       },
     };
+    this._fallbackMethods = fallbackMethods || false;
   }
 
   get platform(): string {
@@ -75,6 +78,7 @@ export default class ConsoleConnector implements Connector<ConsoleRequestBody> {
       event,
       session,
       initialState,
+      fallbackMethods: this._fallbackMethods,
     });
   }
 }
diff --git a/src/context/ConsoleContext.js b/src/context/ConsoleContext.js
index 44d7d07d..c170cdae 100644
--- a/src/context/ConsoleContext.js
+++ b/src/context/ConsoleContext.js
@@ -13,6 +13,7 @@ type Options = {|
   event: ConsoleEvent,
   session: ?Session,
   initialState: ?Object,
+  fallbackMethods: boolean,
 |};
 
 export default class ConsoleContext extends Context implements PlatformContext {
@@ -20,8 +21,29 @@ export default class ConsoleContext extends Context implements PlatformContext {
   _event: ConsoleEvent;
   _session: ?Session;
 
-  constructor({ client, event, session, initialState }: Options) {
+  constructor({
+    client,
+    event,
+    session,
+    initialState,
+    fallbackMethods,
+  }: Options) {
     super({ client, event, session, initialState });
+    if (fallbackMethods) {
+      // $FlowExpectedError
+      return new Proxy(this, {
+        get(target, key) {
+          if (target[key]) {
+            return target[key];
+          }
+          // should return undefined when check if it is a promise
+          if (key === 'then') return;
+          return async (...args) => {
+            await target._methodMissing(key, args);
+          };
+        },
+      });
+    }
   }
 
   /**
@@ -50,4 +72,9 @@ export default class ConsoleContext extends Context implements PlatformContext {
     this._isHandled = true;
     this._client.sendText(text);
   }
+
+  async _methodMissing(method: string, args: Array<any>): Promise<void> {
+    this._isHandled = true;
+    this._client.sendText(`${method}: ${JSON.stringify(args)}`);
+  }
 }
diff --git a/src/context/Context.js b/src/context/Context.js
index ca1f1e94..dc72d8b7 100644
--- a/src/context/Context.js
+++ b/src/context/Context.js
@@ -18,6 +18,7 @@ type Response = {
 
 export default class Context {
   _isHandled = false;
+  _isSessionWritten = false;
 
   _client: Object;
   _event: Object;
@@ -72,6 +73,13 @@ export default class Context {
     return this._isHandled;
   }
 
+  get isSessionWritten(): boolean {
+    return this._isSessionWritten;
+  }
+  set isSessionWritten(bool: boolean): void {
+    this._isSessionWritten = bool;
+  }
+
   /**
    * The state in the conversation context.
    *
@@ -93,8 +101,14 @@ export default class Context {
    */
   setState(state: Object): void {
     if (this._session) {
-      this._session._state = {
-        ...this._session._state,
+      const sess = this._session;
+
+      warning(
+        !this._isSessionWritten,
+        'Calling `context.setState` after session has been written. Some changes to state will not be saved.\nDid you forget to await any async function?'
+      );
+      sess._state = {
+        ...sess._state,
         ...state,
       };
     } else {
@@ -112,6 +126,11 @@ export default class Context {
   resetState(): void {
     if (this._session) {
       const sess = this._session;
+
+      warning(
+        !this._isSessionWritten,
+        'Calling `context.resetState` after session has been written. Some changes to state will not be saved.\nDid you forget to await any async function?'
+      );
       sess._state = cloneDeep(this._initialState);
     } else {
       warning(


Test patch:
diff --git a/src/context/__tests__/ConsoleContext.spec.js b/src/context/__tests__/ConsoleContext.spec.js
index b9a17c41..e2d88386 100644
--- a/src/context/__tests__/ConsoleContext.spec.js
+++ b/src/context/__tests__/ConsoleContext.spec.js
@@ -28,7 +28,8 @@ const userSession = {
     name: 'you',
   },
 };
-const setup = ({ session } = { session: userSession }) => {
+
+const setup = ({ session, fallbackMethods } = { session: userSession }) => {
   const client = {
     sendText: jest.fn(),
   };
@@ -36,6 +37,7 @@ const setup = ({ session } = { session: userSession }) => {
     client,
     event: new ConsoleEvent(rawEvent),
     session,
+    fallbackMethods: fallbackMethods || false,
   });
   return {
     client,
@@ -87,6 +89,24 @@ describe('#sendText', () => {
   });
 });
 
+describe('method missing', () => {
+  it('should write text to stdout', async () => {
+    const { context, client } = setup({ fallbackMethods: true });
+
+    await context.sendABC('hello', { json: true });
+
+    expect(client.sendText).toBeCalledWith('sendABC: ["hello",{"json":true}]');
+  });
+
+  it('should mark context as handled', async () => {
+    const { context } = setup({ fallbackMethods: true });
+
+    await context.sendABC('hello');
+
+    expect(context.isHandled).toBe(true);
+  });
+});
+
 describe('#typing', () => {
   it('avoid delay 0', async () => {
     const { context } = setup();
diff --git a/src/context/__tests__/Context.spec.js b/src/context/__tests__/Context.spec.js
index 58105c23..0a4b5417 100644
--- a/src/context/__tests__/Context.spec.js
+++ b/src/context/__tests__/Context.spec.js
@@ -86,6 +86,19 @@ describe('state', () => {
 
       expect(warning).toBeCalled();
     });
+
+    it('should call warning if session has been written', async () => {
+      const state = { a: 1 };
+      const { context } = setup({ session: { _state: state } });
+
+      context.setState({ b: 2 });
+      expect(warning.mock.calls[0][0]).toBe(true);
+
+      context.isSessionWritten = true;
+
+      context.setState({ c: 3 });
+      expect(warning.mock.calls[1][0]).toBe(false);
+    });
   });
 
   describe('#resetState', () => {
@@ -129,5 +142,18 @@ describe('state', () => {
 
       expect(warning).toBeCalled();
     });
+
+    it('should call warning if session has been written', async () => {
+      const state = { a: 1 };
+      const { context } = setup({ session: { _state: state } });
+
+      context.resetState();
+      expect(warning.mock.calls[0][0]).toBe(true);
+
+      context.isSessionWritten = true;
+
+      context.resetState();
+      expect(warning.mock.calls[1][0]).toBe(false);
+    });
   });
 });